netdev - Re: [xdp-hints] [RFC bpf-next 01/23] ice: make RX hash reading code more reusable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e2b1bf53-b216-c90d-869e-24008943e41b@intel.com>
Date: Wed, 6 Sep 2023 14:23:05 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
CC: Larysa Zaremba <larysa.zaremba@...el.com>, <bpf@...r.kernel.org>,
	<ast@...nel.org>, <daniel@...earbox.net>, <andrii@...nel.org>,
	<martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>,
	<john.fastabend@...il.com>, <kpsingh@...nel.org>, <sdf@...gle.com>,
	<haoluo@...gle.com>, <jolsa@...nel.org>, David Ahern <dsahern@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>,
	Jesper Dangaard Brouer <brouer@...hat.com>, Anatoly Burakov
	<anatoly.burakov@...el.com>, Alexander Lobakin <alexandr.lobakin@...el.com>,
	Magnus Karlsson <magnus.karlsson@...il.com>, Maryam Tahhan
	<mtahhan@...hat.com>, <xdp-hints@...-project.net>, <netdev@...r.kernel.org>,
	Willem de Bruijn <willemdebruijn.kernel@...il.com>, Alexei Starovoitov
	<alexei.starovoitov@...il.com>, Simon Horman <simon.horman@...igine.com>,
	Tariq Toukan <tariqt@...lanox.com>, Saeed Mahameed <saeedm@...lanox.com>
Subject: Re: [xdp-hints] [RFC bpf-next 01/23] ice: make RX hash reading code
 more reusable

From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
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