netdev - Re: [PATCH bpf-next V2] igc: enable and fix RX hash usage by netstack

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] [day] [month] [year] [list]

Message-ID: <3bf858ee-d1d1-0ae1-7673-9f9e11e7ca9e@redhat.com>
Date:   Sat, 18 Feb 2023 20:51:40 +0100
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Daniel Borkmann <daniel@...earbox.net>, bpf@...r.kernel.org
Cc:     brouer@...hat.com, netdev@...r.kernel.org,
        Stanislav Fomichev <sdf@...gle.com>, martin.lau@...nel.org,
        ast@...nel.org, alexandr.lobakin@...el.com,
        yoong.siang.song@...el.com, anthony.l.nguyen@...el.com,
        intel-wired-lan@...ts.osuosl.org, Jakub Kicinski <kuba@...nel.org>,
        pmenzel@...gen.mpg.de, Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH bpf-next V2] igc: enable and fix RX hash usage by netstack


On 17/02/2023 21.59, Daniel Borkmann wrote:
> On 2/16/23 5:52 PM, Jesper Dangaard Brouer wrote:
>> When function igc_rx_hash() was introduced in v4.20 via commit 0507ef8a0372
>> ("igc: Add transmit and receive fastpath and interrupt handlers"), the
>> hardware wasn't configured to provide RSS hash, thus it made sense to not
>> enable net_device NETIF_F_RXHASH feature bit.
>>
>> The NIC hardware was configured to enable RSS hash info in v5.2 via commit
>> 2121c2712f82 ("igc: Add multiple receive queues control supporting"), but
>> forgot to set the NETIF_F_RXHASH feature bit.
>>
>> The original implementation of igc_rx_hash() didn't extract the associated
>> pkt_hash_type, but statically set PKT_HASH_TYPE_L3. The largest portions of
>> this patch are about extracting the RSS Type from the hardware and mapping
>> this to enum pkt_hash_types. This was based on Foxville i225 software user
>> manual rev-1.3.1 and tested on Intel Ethernet Controller I225-LM (rev 03).
>>
>> For UDP it's worth noting that RSS (type) hashing have been disabled both for
>> IPv4 and IPv6 (see IGC_MRQC_RSS_FIELD_IPV4_UDP + IGC_MRQC_RSS_FIELD_IPV6_UDP)
>> because hardware RSS doesn't handle fragmented pkts well when enabled (can
>> cause out-of-order). This results in PKT_HASH_TYPE_L3 for UDP packets, and
>> hash value doesn't include UDP port numbers. Not being PKT_HASH_TYPE_L4, have
>> the effect that netstack will do a software based hash calc calling into
>> flow_dissect, but only when code calls skb_get_hash(), which doesn't
>> necessary happen for local delivery.
>>
>> Fixes: 2121c2712f82 ("igc: Add multiple receive queues control supporting")
>> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
> 
> I presume this should go via net-next, not bpf-next? (Didn't find specific
> dependencies, so moved to patchwork netdev bucket..)

Thanks for letting me/us know.

I posted against bpf-next, because I have patches (for kfunc XDP-hints)
that depend on this patch.  As we are at rc8, netdev maintainers feel
free to simply drop the patch as the patch isn't critical.  I will just
include this patch as part of my kfunc XDP-hints series later.

--Jesper