On Sun, Jul 5, 2020 at 10:26 AM Ariel Levkovich <lar...@mellanox.com> wrote: > However I believe that from a concept point of view, using it is wrong. > > In my honest opinion, the concept here is to perform some calculation on > the packet itself and its headers while the skb->hash field > > is the storage location of the calculation result (in SW).
With skbedit, you don't have to pass a value either, whatever you pass to your act_hash, you can pass it to skbedit too. In your case, it seems to be an algorithm name. You can take a look at SKBEDIT_F_INHERITDSFIELD, it calculates skb->priority from headers, not passed from user-space. > > Furthermore, looking forward to HW offload support, the HW devices will > be offloading the hash calculation and > > not rewriting skb metadata fields. Therefore the action should be the > hash, not skbedit. Not sure if this makes sense, whatever your code under case TCA_HASH_ALG_L4 can be just moved to skbedit. I don't see how making it standalone could be different for HW offloading. > > Another thing that I can mention, which is kind of related to what I > wrote above, is that for all existing skbedit supported fields, > > user typically provides a desired value of his choosing to set to a skb > metadata field. Again, no one forces this rule. Please feel free to adjust it for your needs. Thanks.