pna icon indicating copy to clipboard operation
pna copied to clipboard

Should Hash extern for PNA add Toeplitz hash option?

Open jafingerhut opened this issue 3 years ago • 5 comments

This is sometimes specified as a required hash function for implementing Receive Side Scaling by some NIC customers/driver developers.

jafingerhut avatar May 15 '21 05:05 jafingerhut

@thantry This is an issue you and I discussed recently, that there is currently nothing said about it in the PNA spec. No details here yet, just a reminder for future discussions.

jfingerh avatar Nov 01 '22 17:11 jfingerh

@thantry This would be a good issue to add links to details about Toeplitz hash added to p4lang/behavioral-model that you mentioned in 2022-Nov-14 meeting.

jfingerh avatar Nov 15 '22 01:11 jfingerh

Here's the implementation @qobilidop added to the BMv2 backend.

https://github.com/p4lang/behavioral-model/pull/1132 https://github.com/p4lang/behavioral-model/pull/1134

thantry avatar Nov 15 '22 18:11 thantry

I can provide some additional context for your reference.

My implementation for the Toeplitz hash in https://github.com/p4lang/behavioral-model/pull/1134 was basically following the implementation for crc16_custom and crc32_custom hashes declared in v1model.p4, which also require internal states to be set. I dug into the history a little bit, and found this comment:

https://github.com/p4lang/behavioral-model/issues/982#issuecomment-786353701

bmv2 supports runtime-configurable CRCs, when selecting crc32_custom or crc16_custom as the compile-time algorithm. But these are only configurable through the runtime CLI at the moment, not P4Runtime.

So I was wrong when I previously said there is a P4Runtime interface to configure Toeplitz RSS key with bmv2. It's only configurable through the bmv2-specific P4 runtime CLI.

I wonder if we could provide a P4Runtime interface to configure the RSS key for the Toeplitz hash if we declare it as an extern object like the following (adapting from pna.p4):

extern ToeplitzHash<O> {
  Hash();
  void set_rss_key<K>(in K key);
  O get_hash<D>(in D data);
  O get_hash<T, D>(in T base, in D data, in T max);
}

Then there are probably standard ways to invoke the set_rss_key() method (maybe as a table action) through P4Runtime.

qobilidop avatar Nov 15 '22 20:11 qobilidop

@qobilidop FYI if you define a new extern, there is no off-the-shelf way in P4 source code or P4Runtime API to define its control plane API. They must be defined by hand, and then added to P4Runtime API spec somehow.

One of the reasons for the TernaryMap and ExactMap externs is that the proposal is that their control plane APIs will be exactly the same in P4Runtime API as for P4 tables, so they should inherit all of those APIs without requiring any new definitions other than to say "they are like P4 tables".

There is a little bit of an omission in the short description above, in that there is no action name for TernaryMap and ExactMap externs. The plan will be one of these:

(a) Pick an action name, e.g. "lookup_value", and always use that as the action name for all entries in the P4Runtime API. This would require no changes to the P4Runtime API spec. (b) define a slight variation in the P4Runtime API table APIs that do not have an action name, and use that for TernaryMap and ExactMap. This approach WOULD require modifying the P4Runtime API definition.

I personally think (a) is a good approach, because it does not require extending or modifying the P4Runtime API definition, but I do not think the final decision on that has been made yet.

jfingerh avatar Nov 15 '22 20:11 jfingerh