| Message ID | 167950087738.2796265.17812597177704199765.stgit@firesoul (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | XDP-hints kfuncs for Intel driver igc | expand |
On Thursday, March 23, 2023 12:01 AM, Jesper Dangaard Brouer <brouer@redhat.com> 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@redhat.com> >--- > drivers/net/ethernet/intel/igc/igc.h | 28 ++++++++++++++++++++++++++ > drivers/net/ethernet/intel/igc/igc_main.c | 31 +++++++++++++++++++++++++--- >- > 2 files changed, 55 insertions(+), 4 deletions(-) > >diff --git a/drivers/net/ethernet/intel/igc/igc.h >b/drivers/net/ethernet/intel/igc/igc.h >index df3e26c0cf01..f83cbc4a1afa 100644 >--- a/drivers/net/ethernet/intel/igc/igc.h >+++ b/drivers/net/ethernet/intel/igc/igc.h >@@ -13,6 +13,7 @@ > #include <linux/ptp_clock_kernel.h> > #include <linux/timecounter.h> > #include <linux/net_tstamp.h> >+#include <linux/bitfield.h> > > #include "igc_hw.h" > >@@ -311,6 +312,33 @@ extern char igc_driver_name[]; > #define IGC_MRQC_RSS_FIELD_IPV4_UDP 0x00400000 > #define IGC_MRQC_RSS_FIELD_IPV6_UDP 0x00800000 > >+/* RX-desc Write-Back format RSS Type's */ enum igc_rss_type_num { >+ IGC_RSS_TYPE_NO_HASH = 0, >+ IGC_RSS_TYPE_HASH_TCP_IPV4 = 1, >+ IGC_RSS_TYPE_HASH_IPV4 = 2, >+ IGC_RSS_TYPE_HASH_TCP_IPV6 = 3, >+ IGC_RSS_TYPE_HASH_IPV6_EX = 4, >+ IGC_RSS_TYPE_HASH_IPV6 = 5, >+ IGC_RSS_TYPE_HASH_TCP_IPV6_EX = 6, >+ IGC_RSS_TYPE_HASH_UDP_IPV4 = 7, >+ IGC_RSS_TYPE_HASH_UDP_IPV6 = 8, >+ IGC_RSS_TYPE_HASH_UDP_IPV6_EX = 9, >+ IGC_RSS_TYPE_MAX = 10, >+}; >+#define IGC_RSS_TYPE_MAX_TABLE 16 >+#define IGC_RSS_TYPE_MASK GENMASK(3,0) /* 4-bits (3:0) = mask >0x0F */ >+ >+/* igc_rss_type - Rx descriptor RSS type field */ static inline u32 >+igc_rss_type(const union igc_adv_rx_desc *rx_desc) { >+ /* RSS Type 4-bits (3:0) number: 0-9 (above 9 is reserved) >+ * Accessing the same bits via u16 (wb.lower.lo_dword.hs_rss.pkt_info) >+ * is slightly slower than via u32 (wb.lower.lo_dword.data) >+ */ >+ return le32_get_bits(rx_desc->wb.lower.lo_dword.data, >+IGC_RSS_TYPE_MASK); } >+ > /* Interrupt defines */ > #define IGC_START_ITR 648 /* ~6000 ints/sec */ > #define IGC_4K_ITR 980 >diff --git a/drivers/net/ethernet/intel/igc/igc_main.c >b/drivers/net/ethernet/intel/igc/igc_main.c >index 2928a6c73692..f6a54feec011 100644 >--- a/drivers/net/ethernet/intel/igc/igc_main.c >+++ b/drivers/net/ethernet/intel/igc/igc_main.c >@@ -1677,14 +1677,36 @@ static void igc_rx_checksum(struct igc_ring *ring, > le32_to_cpu(rx_desc->wb.upper.status_error)); > } > >+/* Mapping HW RSS Type to enum pkt_hash_types */ >+enum pkt_hash_types igc_rss_type_table[IGC_RSS_TYPE_MAX_TABLE] = { Hi Jesper, Since igc_rss_type_table is used on igc_main.c only, we can make it static to avoid following build warning: drivers/net/ethernet/intel/igc/igc_main.c:1681:21: warning: symbol 'igc_rss_type_table' was not declared. Should it be static? Thanks & Regards Siang >+ [IGC_RSS_TYPE_NO_HASH] = PKT_HASH_TYPE_L2, >+ [IGC_RSS_TYPE_HASH_TCP_IPV4] = PKT_HASH_TYPE_L4, >+ [IGC_RSS_TYPE_HASH_IPV4] = PKT_HASH_TYPE_L3, >+ [IGC_RSS_TYPE_HASH_TCP_IPV6] = PKT_HASH_TYPE_L4, >+ [IGC_RSS_TYPE_HASH_IPV6_EX] = PKT_HASH_TYPE_L3, >+ [IGC_RSS_TYPE_HASH_IPV6] = PKT_HASH_TYPE_L3, >+ [IGC_RSS_TYPE_HASH_TCP_IPV6_EX] = PKT_HASH_TYPE_L4, >+ [IGC_RSS_TYPE_HASH_UDP_IPV4] = PKT_HASH_TYPE_L4, >+ [IGC_RSS_TYPE_HASH_UDP_IPV6] = PKT_HASH_TYPE_L4, >+ [IGC_RSS_TYPE_HASH_UDP_IPV6_EX] = PKT_HASH_TYPE_L4, >+ [10] = PKT_HASH_TYPE_NONE, /* RSS Type above 9 "Reserved" by HW >*/ >+ [11] = PKT_HASH_TYPE_NONE, /* keep array sized for SW bit-mask */ >+ [12] = PKT_HASH_TYPE_NONE, /* to handle future HW revisons */ >+ [13] = PKT_HASH_TYPE_NONE, >+ [14] = PKT_HASH_TYPE_NONE, >+ [15] = PKT_HASH_TYPE_NONE, >+}; >+ > static inline void igc_rx_hash(struct igc_ring *ring, > union igc_adv_rx_desc *rx_desc, > struct sk_buff *skb) > { >- if (ring->netdev->features & NETIF_F_RXHASH) >- skb_set_hash(skb, >- le32_to_cpu(rx_desc->wb.lower.hi_dword.rss), >- PKT_HASH_TYPE_L3); >+ if (ring->netdev->features & NETIF_F_RXHASH) { >+ u32 rss_hash = le32_to_cpu(rx_desc->wb.lower.hi_dword.rss); >+ u32 rss_type = igc_rss_type(rx_desc); >+ >+ skb_set_hash(skb, rss_hash, igc_rss_type_table[rss_type]); >+ } > } > > static void igc_rx_vlan(struct igc_ring *rx_ring, @@ -6543,6 +6565,7 @@ static >int igc_probe(struct pci_dev *pdev, > netdev->features |= NETIF_F_TSO; > netdev->features |= NETIF_F_TSO6; > netdev->features |= NETIF_F_TSO_ECN; >+ netdev->features |= NETIF_F_RXHASH; > netdev->features |= NETIF_F_RXCSUM; > netdev->features |= NETIF_F_HW_CSUM; > netdev->features |= NETIF_F_SCTP_CRC; >
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h index df3e26c0cf01..f83cbc4a1afa 100644 --- a/drivers/net/ethernet/intel/igc/igc.h +++ b/drivers/net/ethernet/intel/igc/igc.h @@ -13,6 +13,7 @@ #include <linux/ptp_clock_kernel.h> #include <linux/timecounter.h> #include <linux/net_tstamp.h> +#include <linux/bitfield.h> #include "igc_hw.h" @@ -311,6 +312,33 @@ extern char igc_driver_name[]; #define IGC_MRQC_RSS_FIELD_IPV4_UDP 0x00400000 #define IGC_MRQC_RSS_FIELD_IPV6_UDP 0x00800000 +/* RX-desc Write-Back format RSS Type's */ +enum igc_rss_type_num { + IGC_RSS_TYPE_NO_HASH = 0, + IGC_RSS_TYPE_HASH_TCP_IPV4 = 1, + IGC_RSS_TYPE_HASH_IPV4 = 2, + IGC_RSS_TYPE_HASH_TCP_IPV6 = 3, + IGC_RSS_TYPE_HASH_IPV6_EX = 4, + IGC_RSS_TYPE_HASH_IPV6 = 5, + IGC_RSS_TYPE_HASH_TCP_IPV6_EX = 6, + IGC_RSS_TYPE_HASH_UDP_IPV4 = 7, + IGC_RSS_TYPE_HASH_UDP_IPV6 = 8, + IGC_RSS_TYPE_HASH_UDP_IPV6_EX = 9, + IGC_RSS_TYPE_MAX = 10, +}; +#define IGC_RSS_TYPE_MAX_TABLE 16 +#define IGC_RSS_TYPE_MASK GENMASK(3,0) /* 4-bits (3:0) = mask 0x0F */ + +/* igc_rss_type - Rx descriptor RSS type field */ +static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc) +{ + /* RSS Type 4-bits (3:0) number: 0-9 (above 9 is reserved) + * Accessing the same bits via u16 (wb.lower.lo_dword.hs_rss.pkt_info) + * is slightly slower than via u32 (wb.lower.lo_dword.data) + */ + return le32_get_bits(rx_desc->wb.lower.lo_dword.data, IGC_RSS_TYPE_MASK); +} + /* Interrupt defines */ #define IGC_START_ITR 648 /* ~6000 ints/sec */ #define IGC_4K_ITR 980 diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c index 2928a6c73692..f6a54feec011 100644 --- a/drivers/net/ethernet/intel/igc/igc_main.c +++ b/drivers/net/ethernet/intel/igc/igc_main.c @@ -1677,14 +1677,36 @@ static void igc_rx_checksum(struct igc_ring *ring, le32_to_cpu(rx_desc->wb.upper.status_error)); } +/* Mapping HW RSS Type to enum pkt_hash_types */ +enum pkt_hash_types igc_rss_type_table[IGC_RSS_TYPE_MAX_TABLE] = { + [IGC_RSS_TYPE_NO_HASH] = PKT_HASH_TYPE_L2, + [IGC_RSS_TYPE_HASH_TCP_IPV4] = PKT_HASH_TYPE_L4, + [IGC_RSS_TYPE_HASH_IPV4] = PKT_HASH_TYPE_L3, + [IGC_RSS_TYPE_HASH_TCP_IPV6] = PKT_HASH_TYPE_L4, + [IGC_RSS_TYPE_HASH_IPV6_EX] = PKT_HASH_TYPE_L3, + [IGC_RSS_TYPE_HASH_IPV6] = PKT_HASH_TYPE_L3, + [IGC_RSS_TYPE_HASH_TCP_IPV6_EX] = PKT_HASH_TYPE_L4, + [IGC_RSS_TYPE_HASH_UDP_IPV4] = PKT_HASH_TYPE_L4, + [IGC_RSS_TYPE_HASH_UDP_IPV6] = PKT_HASH_TYPE_L4, + [IGC_RSS_TYPE_HASH_UDP_IPV6_EX] = PKT_HASH_TYPE_L4, + [10] = PKT_HASH_TYPE_NONE, /* RSS Type above 9 "Reserved" by HW */ + [11] = PKT_HASH_TYPE_NONE, /* keep array sized for SW bit-mask */ + [12] = PKT_HASH_TYPE_NONE, /* to handle future HW revisons */ + [13] = PKT_HASH_TYPE_NONE, + [14] = PKT_HASH_TYPE_NONE, + [15] = PKT_HASH_TYPE_NONE, +}; + static inline void igc_rx_hash(struct igc_ring *ring, union igc_adv_rx_desc *rx_desc, struct sk_buff *skb) { - if (ring->netdev->features & NETIF_F_RXHASH) - skb_set_hash(skb, - le32_to_cpu(rx_desc->wb.lower.hi_dword.rss), - PKT_HASH_TYPE_L3); + if (ring->netdev->features & NETIF_F_RXHASH) { + u32 rss_hash = le32_to_cpu(rx_desc->wb.lower.hi_dword.rss); + u32 rss_type = igc_rss_type(rx_desc); + + skb_set_hash(skb, rss_hash, igc_rss_type_table[rss_type]); + } } static void igc_rx_vlan(struct igc_ring *rx_ring, @@ -6543,6 +6565,7 @@ static int igc_probe(struct pci_dev *pdev, netdev->features |= NETIF_F_TSO; netdev->features |= NETIF_F_TSO6; netdev->features |= NETIF_F_TSO_ECN; + netdev->features |= NETIF_F_RXHASH; netdev->features |= NETIF_F_RXCSUM; netdev->features |= NETIF_F_HW_CSUM; netdev->features |= NETIF_F_SCTP_CRC;
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@redhat.com> --- drivers/net/ethernet/intel/igc/igc.h | 28 ++++++++++++++++++++++++++ drivers/net/ethernet/intel/igc/igc_main.c | 31 +++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 4 deletions(-)