On 10/02/2023 16.07, 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.
Sending this fix against bpf-next, as I found this issue while playing
with implementing XDP-hints kfunc for xmo_rx_hash. I will hopefully send
kfunc patches next week, on top of this.IMHO this fix isn't very
critical and I hope it can simply go though the
bpf-next tree as it would ease followup kfunc patches.
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 were 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 result 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.
Intel QA tester wanting to verify this patch can use the small bpftrace
tool I wrote and placed here:
https://github.com/xdp-project/xdp-project/blob/master/areas/hints/monitor_skb_hash_on_dev.bt
Failure scenarios:
$ sudo ./monitor_skb_hash_on_dev.bt igc1
Attaching 2 probes...
Monitor net_device: igc1
Hit Ctrl-C to end.
IFNAME HASH Hash-type:L4 Software-hash
igc1 00000000 0 0
igc1 00000000 0 0
igc1 00000000 0 0
^C
Example output with patch:
$ sudo ./monitor_skb_hash_on_dev.bt igc1
Attaching 2 probes...
Monitor net_device: igc1
Hit Ctrl-C to end.
IFNAME HASH Hash-type:L4 Software-hash
igc1 FEF98EFE 0 0
igc1 00000000 0 0
igc1 00000000 0 0
igc1 FEF98EFE 0 0
igc1 FEF98EFE 0 0
igc1 FEF98EFE 0 0
igc1 310AF9EA 1 0
igc1 A229FA51 1 0
The repeating hash FEF98EFE is UDP packets that as desc note doesn't
have Hash-type:L4. The UDP has is repeating as port numbers aren't part
of the hash, and I was sending to same IP. The hash values with L4=1
were TCP packets.
Hope this eases QA work.
Fixes: 2121c2712f82 ("igc: Add multiple receive queues control supporting")
Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx>
---
drivers/net/ethernet/intel/igc/igc.h | 52 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/igc/igc_main.c | 35 +++++++++++++++++---
2 files changed, 83 insertions(+), 4 deletions(-)
--Jesper
