From: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
Date: Mon, 4 Sep 2023 16:37:45 +0200
> On Thu, Aug 24, 2023 at 09:26:40PM +0200, Larysa Zaremba wrote:
>> Previously, we only needed RX hash in skb path,
>> hence all related code was written with skb in mind.
>> But with the addition of XDP hints via kfuncs to the ice driver,
>> the same logic will be needed in .xmo_() callbacks.
[...]
>> - nic_mdid = (struct ice_32b_rx_flex_desc_nic *)rx_desc;
>> - hash = le32_to_cpu(nic_mdid->rss_hash);
>> - skb_set_hash(skb, hash, ice_ptype_to_htype(rx_ptype));
>> + hash = ice_get_rx_hash(rx_desc);
>> + if (likely(hash))
>> + skb_set_hash(skb, hash, ice_ptype_to_htype(rx_ptype));
>
> Looks like a behavior change as you wouldn't be setting l4_hash and
> sw_hash from skb in case !hash ? When can we get hash == 0 ?
I do the same in libie. hash == 0 makes no sense at all no matter if you
set sw or l4, esp. for GRO and other stack pieces.
BTW, sw_hash is never set by drivers, it's meant to be set only from the
core networking hashing functions (when it's hashed by CPU with SIPhash
with the help of Flow Dissector). So we only do care about l4_hash.
Valid hash == 0 for valid L4 frame has 0.0(0)1% probability even for
XOR, not speaking of Toeplitz / CRC (have you even seen MD5 == 0? :D).
if the frame is not L4, the kernel doesn't treat your hash as something
meaningful and falls back to SIP. But the prob of having hash == 0 for
L3- is not higher :D
>
>> }
>>
>> /**
>> @@ -186,7 +201,7 @@ ice_process_skb_fields(struct ice_rx_ring *rx_ring,
>> union ice_32b_rx_flex_desc *rx_desc,
>> struct sk_buff *skb, u16 ptype)
>> {
>> - ice_rx_hash(rx_ring, rx_desc, skb, ptype);
>> + ice_rx_hash_to_skb(rx_ring, rx_desc, skb, ptype);
>>
>> /* modifies the skb - consumes the enet header */
>> skb->protocol = eth_type_trans(skb, rx_ring->netdev);
>> --
>> 2.41.0
>>
Thanks,
Olek
