On Fri, Jun 19, 2020 at 7:42 PM Ariel Levkovich <lar...@mellanox.com> wrote: > I'll try to address both of your comments regarding existing > alternatives to this new action > > here so that we can have a single thread about it. > > Act_bpf indeed can provide a similar functionality. Furthermore, there > are already existing BPF helpers > > to allow user to change skb->hash within the BPF program, so there's no > need to perform act_skbedit > > after act_bpf.
What is "perform act_skbedit after act_bpf"? You mentioned avoiding act_bpf is one of your goals here. > > > However, since we are trying to offer the user multiple methods to > calculate the hash, and not only > > using a BPF program, act_bpf on its own is not enough. If this is the reason for you to add a new action, why not just extend act_skbedit? > > If we are looking at HW offload (as Daniel mentioned), like I mentioned > in the cover letter, > > it is important that SW will be able to get the same hash result as in > HW for a certain packet. > > When certain HW is not able to forward TC the hash result, using a BPF > program that mimics the > > HW hash function is useful to maintain consistency but there are cases > where the HW can and > > does forward the hash value via the received packet's metadata and the > vendor driver already > > fills in the skb->hash with this value. In such cases BPF program usage > can be avoided. > > So to sum it up, this api is offering user both ways to calculate the hash: > > 1. Use the value that is already there (If the vendor driver already set > it. If not, calculate using Linux jhash). Can be done by extending act_skbedit? > > 2. Use a given BPF program to calculate the hash and to set skb->hash > with it. Seems you agreed this can be done via act_bpf. > > > It's true, you can cover both cases with BPF - meaning, always use BPF > even if HW/driver can provide hash > > to TC in other means but we thought about giving an option to avoid > writing and using BPF when > > it's not necessary. How about extending act_skbedit? Adding TCA_SKBEDIT_HASH? Thanks.
