On Mon, Aug 31, 2020 at 2:33 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 8/31/20 9:25 PM, Harshitha Ramamurthy wrote: > > This patch adds a helper function called bpf_get_xdp_hash to calculate > > the hash for a packet at the XDP layer. In the helper function, we call > > the kernel flow dissector in non-skb mode by passing the net pointer > > to calculate the hash. > > So this commit msg says 'what' the patch does, but says nothing about 'why' it is > needed especially given there's the 1 mio insn limit in place where it should be > easy to write that up in BPF anyway. The commit msg needs to have a clear rationale > which describes the motivation behind this helper.. why it cannot be done in BPF > itself? > Daniel, We already have a fully functional, well tested, and very complete parser supporting many protocols and packet hash functions in the kernel in skb_flow_dissect and friends. I suppose we could replicate all that code in eBPF but I don't think it's fair to say it's easy to get to the same level in eBPF. eBPF does solve the problem of extensibility of kernel code, however IMO if someone is looking for an easy way to get a packet hash then a simple helper function makes sense and results in a much simpler XDP program. There's some nice potential follow on work also. If the hardware provides a quality L4 hash that could be passed into the XDP program to avoid having to call the helper. If we do invoke the helper it would make sense to return the hash to the driver so that it can set in the skbuff to avoid having the stack call skb_flow_dissect again. We can also have an flow_diessct helper that invokes skb_flow_dissect and returns the meta data based on input keys. This would be useful for filtering in XDP etc. Tom > > Changes since RFC: > > - accounted for vlans(David Ahern) > > - return the correct hash by not using skb_get_hash(David Ahern) > > - call __skb_flow_dissect in non-skb mode > >