On Wed, Aug 11, 2021 at 4:05 PM Jonathan Toppins [off-list ref] wrote:

On 8/11/21 4:22 AM, Jussi Maki wrote:

quoted

Hi Jonathan,

Thanks for catching this. You're right, this will NULL deref if XDP
bonding is used with the VLAN_SRCMAC xmit policy. I think what
happened was that a very early version restricted the xmit policies
that were applicable, but it got dropped when this was refactored.
I'll look into this today and will add in support (or refuse) the
VLAN_SRCMAC xmit policy and extend the tests to cover this.

In support of some customer requests and to stop adding more and more
hashing policies I was looking at adding a custom policy that exposes a
bitfield so userspace can select which header items should be included
in the hash. I was looking at a flow dissector implementation to parse
the packet and then generate the hash from the flow data pulled. It
looks like the outer hashing functions as they exist now,
bond_xmit_hash() and bond_xmit_hash_xdp(), could make the correctly
formatted call to __skb_flow_dissect(). We would then pass around the
resultant struct flow_keys, or bonding specific one to add MAC header
parsing support, and it appears we could avoid making the actual hashing
functions know if they need to hash an sk_buff vs xdp_buff. What do you
think?

That sounds great! I wasn't particularly happy about how it works with
skb being optional as that was just waiting to break (as it did). The
team driver does the hashing using a user-space provided bpf program
and I'm looking to figure out how to support XDP with it. I wonder if
we could have a single approach that would work for both bonding and
team (e.g. use bpf to hash). CC'ing Jiri as he wrote the team driver.