Felix Fietkau <nbd@xxxxxxxx> writes: > On 2020-12-17 14:01, Toke Høiland-Jørgensen wrote: >> Felix Fietkau <nbd@xxxxxxxx> writes: >> >>> On 2020-12-17 12:54, Toke Høiland-Jørgensen wrote: >>>> Felix Fietkau <nbd@xxxxxxxx> writes: >>>> >>>>> Depending on the source, a hardware calculated hash may not provide the >>>>> same level of collision resistance. >>>> >>>> This seems like it would have performance implications? >>>> >>>> Also, this can potentially discard information from tunnels that >>>> preserve the hash before encapsulation (we added support for this to >>>> Wireguard which had some nice effects on queueing of encapsulated >>>> traffic). >>> If the hash was calculated in software using the flow dissector, it will >>> be preserved, even if it went through a few virtual interfaces. >>> The only hashes discarded are hardware generated ones. >> >> Yeah, but I was thinking something like: >> >> Packet comes in with HW hash -> gets encapsulated (preserving the hash) >> -> gets to mac80211 which discards the HW hash. So now you're replacing >> a (possibly bad-quality) HW hash with a software hash of the *outer* >> encapsulation header... > If this becomes a problem, I think we should add a similar patch to > wireguard, which already calls skb_get_hash before encapsulating. > Other regular tunnels should already get a proper hash, since the flow > dissector will take care of it. But then we'd need to go around adding this to all the places that uses the hash just to work around a particular piece of broken(ish) hardware. And we're hard-coding a behaviour in mac80211 that means we'll *always* recompute the hash, even for hardware that's not similarly broken. > The reason I did this patch is because I have a patch to set the hw flow > hash in the skb on mtk_eth_soc, which does help GRO, but leads to > collisions on mac80211 fq. So wouldn't the right thing to do here be to put a flag into the RX device that makes the stack clear the hash after using it for GRO? -Toke
