* [PATCH v2 0/3] igb, ixgbe, i40e UDP segmentation offload support
@ 2019-10-11 0:24 Josh Hunt
2019-10-11 0:25 ` [PATCH v2 1/3] igb: Add " Josh Hunt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-11 0:24 UTC (permalink / raw)
To: netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown, alexander.h.duyck, Josh Hunt
Alexander Duyck posted a series in 2018 proposing adding UDP segmentation
offload support to ixgbe and ixgbevf, but those patches were never
accepted:
https://lore.kernel.org/netdev/20180504003556.4769.11407.stgit@localhost.localdomain/
This series is a repost of his ixgbe patch along with a similar
change to the igb and i40e drivers. Testing using the udpgso_bench_tx
benchmark shows a noticeable performance improvement with these changes
applied.
I've shared the benchmark script I've used to generate the #s in this mail:
https://lore.kernel.org/netdev/0e0e706c-4ce9-c27a-af55-339b4eb6d524@akamai.com/T/#mfe4c35c57a3860cda5306b63f61068f837242ee5
v2 changes:
* Added Alex's suggested-by tag to ixgbe patch
* Checking for gso_type, suggested by Sridhar
* Fixed bug in ixgbe patch where I accidentally left the old type_tucmd
set to TCP
* Updated perf #s below
* includes changes above
* fixed bad config on my igb machine
* added chip used for testing next to driver name
All #s below were run with:
udpgso_bench_tx -C 1 -4 -D 172.25.43.133 -z -l 30 -S 0 -u -s $pkt_size
igb (i210)::
SW GSO (ethtool -K eth0 tx-udp-segmentation off):
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 120167.13 114 81278 81278 16.24 7.01
2944 120166.83 114 40641 40641 12.82 8.89
5888 120166.42 114 20320 20320 10.14 11.24
11776 120166.43 114 10160 10160 8.55 13.33
23552 120167.24 114 5080 5080 7.76 14.69
47104 120166.83 114 2540 2540 7.07 16.12
61824 120167.08 114 1935 1935 7.07 16.12
HW GSO offload (ethtool -K eth0 tx-udp-segmentation on):
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 120167.05 114 81276 81276 16.13 7.06
2944 120166.94 114 40640 40640 8.66 13.16
5888 120166.84 114 20320 20320 5.34 21.34
11776 120166.84 114 10160 10160 3.34 34.13
23552 120167.25 114 5080 5080 2.55 44.70
47104 120149.30 113 2539 2539 2.14 52.80
61824 120165.41 114 1935 1935 2.04 55.88
ixgbe (82599ES)::
SW GSO:
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 1043972.83 990 706122 706122 100 9.90
2944 1201623.17 1140 406367 406367 95.97 11.87
5888 1201629.46 1140 203184 203184 56 20.35
11776 1201631.38 1140 101590 101590 42.72 26.68
23552 1201633.94 1140 50796 50796 36.31 31.39
47104 1201631.05 1140 25397 25397 33.91 33.61
61824 1201629.96 1140 19350 19350 33.38 34.15
HW GSO Offload:
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 1053393.41 999 712679 712679 100 9.99
2944 1201631.19 1140 406357 406357 57.92 19.68
5888 1201622.51 1140 203178 203178 30.66 37.18
11776 1201626.60 1140 101590 101590 16.89 67.49
23552 1201617.19 1140 50795 50795 10.11 112.75
47104 1200218.72 1138 25368 25368 6.86 165.88
61824 1201619.66 1140 19350 19350 5.38 211.89
i40e (x710)::
SW GSO:
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 642718.15 609 434585 434585 100 6.09
2944 957988.80 909 324029 324029 100 9.09
5888 1199207.69 1138 202845 202845 81.51 13.96
11776 1200767.60 1139 101517 101517 63.93 17.81
23552 1201197.22 1140 50794 50794 59.14 19.27
47104 1201410.91 1139 25393 25393 57.15 19.93
61824 1201609.48 1140 19350 19350 55.12 20.68
HW GSO offload:
$pkt_size kB/s(sar) MB/s Calls/s Msg/s CPU MB2CPU
========================================================================
1472 666626.15 632 450920 450920 100 6.32
2944 1200831.63 1139 406093 406093 89.68 12.70
5888 1201603.35 1140 203178 203178 56.38 20.21
11776 1201597.29 1140 101588 101588 36.9 30.89
23552 1201596.98 1140 50794 50794 24.23 47.04
47104 1201491.67 1139 25394 25394 17.14 66.45
61824 1201598.88 1140 19350 19350 15.11 75.44
Josh Hunt (3):
igb: Add UDP segmentation offload support
ixgbe: Add UDP segmentation offload support
i40e: Add UDP segmentation offload support
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 12 +++++++++---
drivers/net/ethernet/intel/igb/e1000_82575.h | 1 +
drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++++++++------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 24 ++++++++++++++++++------
5 files changed, 46 insertions(+), 15 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] igb: Add UDP segmentation offload support
2019-10-11 0:24 [PATCH v2 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
@ 2019-10-11 0:25 ` Josh Hunt
2019-10-11 15:29 ` Alexander Duyck
2019-10-11 0:25 ` [PATCH v2 2/3] ixgbe: " Josh Hunt
2019-10-11 0:25 ` [PATCH v2 3/3] i40e: " Josh Hunt
2 siblings, 1 reply; 6+ messages in thread
From: Josh Hunt @ 2019-10-11 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown, alexander.h.duyck,
Josh Hunt, Alexander Duyck
Based on a series from Alexander Duyck this change adds UDP segmentation
offload support to the igb driver.
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Willem de Bruijn <willemb@google.com>
Signed-off-by: Josh Hunt <johunt@akamai.com>
---
drivers/net/ethernet/intel/igb/e1000_82575.h | 1 +
drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++++++++------
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
index 6ad775b1a4c5..63ec253ac788 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.h
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
@@ -127,6 +127,7 @@ struct e1000_adv_tx_context_desc {
};
#define E1000_ADVTXD_MACLEN_SHIFT 9 /* Adv ctxt desc mac len shift */
+#define E1000_ADVTXD_TUCMD_L4T_UDP 0x00000000 /* L4 Packet TYPE of UDP */
#define E1000_ADVTXD_TUCMD_IPV4 0x00000400 /* IP Packet Type: 1=IPv4 */
#define E1000_ADVTXD_TUCMD_L4T_TCP 0x00000800 /* L4 Packet TYPE of TCP */
#define E1000_ADVTXD_TUCMD_L4T_SCTP 0x00001000 /* L4 packet TYPE of SCTP */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 105b0624081a..4131bc8b079e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2516,6 +2516,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
if (unlikely(mac_hdr_len > IGB_MAX_MAC_HDR_LEN))
return features & ~(NETIF_F_HW_CSUM |
NETIF_F_SCTP_CRC |
+ NETIF_F_GSO_UDP_L4 |
NETIF_F_HW_VLAN_CTAG_TX |
NETIF_F_TSO |
NETIF_F_TSO6);
@@ -2524,6 +2525,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
if (unlikely(network_hdr_len > IGB_MAX_NETWORK_HDR_LEN))
return features & ~(NETIF_F_HW_CSUM |
NETIF_F_SCTP_CRC |
+ NETIF_F_GSO_UDP_L4 |
NETIF_F_TSO |
NETIF_F_TSO6);
@@ -3120,7 +3122,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_HW_CSUM;
if (hw->mac.type >= e1000_82576)
- netdev->features |= NETIF_F_SCTP_CRC;
+ netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
if (hw->mac.type >= e1000_i350)
netdev->features |= NETIF_F_HW_TC;
@@ -5694,6 +5696,7 @@ static int igb_tso(struct igb_ring *tx_ring,
} ip;
union {
struct tcphdr *tcp;
+ struct udphdr *udp;
unsigned char *hdr;
} l4;
u32 paylen, l4_offset;
@@ -5713,7 +5716,8 @@ static int igb_tso(struct igb_ring *tx_ring,
l4.hdr = skb_checksum_start(skb);
/* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
- type_tucmd = E1000_ADVTXD_TUCMD_L4T_TCP;
+ type_tucmd = (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)) ?
+ E1000_ADVTXD_TUCMD_L4T_TCP : E1000_ADVTXD_TUCMD_L4T_UDP;
/* initialize outer IP header fields */
if (ip.v4->version == 4) {
@@ -5741,12 +5745,19 @@ static int igb_tso(struct igb_ring *tx_ring,
/* determine offset of inner transport header */
l4_offset = l4.hdr - skb->data;
- /* compute length of segmentation header */
- *hdr_len = (l4.tcp->doff * 4) + l4_offset;
-
/* remove payload length from inner checksum */
paylen = skb->len - l4_offset;
- csum_replace_by_diff(&l4.tcp->check, htonl(paylen));
+ if (type_tucmd & E1000_ADVTXD_TUCMD_L4T_TCP) {
+ /* compute length of segmentation header */
+ *hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ csum_replace_by_diff(&l4.tcp->check,
+ (__force __wsum)htonl(paylen));
+ } else {
+ /* compute length of segmentation header */
+ *hdr_len = sizeof(*l4.udp) + l4_offset;
+ csum_replace_by_diff(&l4.udp->check,
+ (__force __wsum)htonl(paylen));
+ }
/* update gso size and bytecount with header size */
first->gso_segs = skb_shinfo(skb)->gso_segs;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] ixgbe: Add UDP segmentation offload support
2019-10-11 0:24 [PATCH v2 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 0:25 ` [PATCH v2 1/3] igb: Add " Josh Hunt
@ 2019-10-11 0:25 ` Josh Hunt
2019-10-11 0:25 ` [PATCH v2 3/3] i40e: " Josh Hunt
2 siblings, 0 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-11 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown, alexander.h.duyck,
Josh Hunt, Alexander Duyck
Repost from a series by Alexander Duyck to add UDP segmentation offload
support to the igb driver:
https://lore.kernel.org/netdev/20180504003916.4769.66271.stgit@localhost.localdomain/
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Willem de Bruijn <willemb@google.com>
Suggested-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Josh Hunt <johunt@akamai.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1ce2397306b9..7d50c1a4a3be 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7946,6 +7946,7 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
} ip;
union {
struct tcphdr *tcp;
+ struct udphdr *udp;
unsigned char *hdr;
} l4;
u32 paylen, l4_offset;
@@ -7969,7 +7970,8 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
l4.hdr = skb_checksum_start(skb);
/* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
- type_tucmd = IXGBE_ADVTXD_TUCMD_L4T_TCP;
+ type_tucmd = (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)) ?
+ IXGBE_ADVTXD_TUCMD_L4T_TCP : IXGBE_ADVTXD_TUCMD_L4T_UDP;
/* initialize outer IP header fields */
if (ip.v4->version == 4) {
@@ -7999,12 +8001,20 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
/* determine offset of inner transport header */
l4_offset = l4.hdr - skb->data;
- /* compute length of segmentation header */
- *hdr_len = (l4.tcp->doff * 4) + l4_offset;
-
/* remove payload length from inner checksum */
paylen = skb->len - l4_offset;
- csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
+
+ if (type_tucmd & IXGBE_ADVTXD_TUCMD_L4T_TCP) {
+ /* compute length of segmentation header */
+ *hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ csum_replace_by_diff(&l4.tcp->check,
+ (__force __wsum)htonl(paylen));
+ } else {
+ /* compute length of segmentation header */
+ *hdr_len = sizeof(*l4.udp) + l4_offset;
+ csum_replace_by_diff(&l4.udp->check,
+ (__force __wsum)htonl(paylen));
+ }
/* update gso size and bytecount with header size */
first->gso_segs = skb_shinfo(skb)->gso_segs;
@@ -10190,6 +10200,7 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev,
if (unlikely(mac_hdr_len > IXGBE_MAX_MAC_HDR_LEN))
return features & ~(NETIF_F_HW_CSUM |
NETIF_F_SCTP_CRC |
+ NETIF_F_GSO_UDP_L4 |
NETIF_F_HW_VLAN_CTAG_TX |
NETIF_F_TSO |
NETIF_F_TSO6);
@@ -10198,6 +10209,7 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev,
if (unlikely(network_hdr_len > IXGBE_MAX_NETWORK_HDR_LEN))
return features & ~(NETIF_F_HW_CSUM |
NETIF_F_SCTP_CRC |
+ NETIF_F_GSO_UDP_L4 |
NETIF_F_TSO |
NETIF_F_TSO6);
@@ -10907,7 +10919,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
IXGBE_GSO_PARTIAL_FEATURES;
if (hw->mac.type >= ixgbe_mac_82599EB)
- netdev->features |= NETIF_F_SCTP_CRC;
+ netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
#ifdef CONFIG_IXGBE_IPSEC
#define IXGBE_ESP_FEATURES (NETIF_F_HW_ESP | \
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] i40e: Add UDP segmentation offload support
2019-10-11 0:24 [PATCH v2 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 0:25 ` [PATCH v2 1/3] igb: Add " Josh Hunt
2019-10-11 0:25 ` [PATCH v2 2/3] ixgbe: " Josh Hunt
@ 2019-10-11 0:25 ` Josh Hunt
2 siblings, 0 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-11 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown, alexander.h.duyck,
Josh Hunt, Alexander Duyck
Based on a series from Alexander Duyck this change adds UDP segmentation
offload support to the i40e driver.
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Willem de Bruijn <willemb@google.com>
Signed-off-by: Josh Hunt <johunt@akamai.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 12 +++++++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6031223eafab..56f8c52cbba1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12911,6 +12911,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
NETIF_F_GSO_IPXIP6 |
NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_UDP_L4 |
NETIF_F_SCTP_CRC |
NETIF_F_RXHASH |
NETIF_F_RXCSUM |
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index e3f29dc8b290..2250284e27fd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2960,10 +2960,16 @@ static int i40e_tso(struct i40e_tx_buffer *first, u8 *hdr_len,
/* remove payload length from inner checksum */
paylen = skb->len - l4_offset;
- csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
- /* compute length of segmentation header */
- *hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)) {
+ csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
+ /* compute length of segmentation header */
+ *hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ } else {
+ csum_replace_by_diff(&l4.udp->check, (__force __wsum)htonl(paylen));
+ /* compute length of segmentation header */
+ *hdr_len = sizeof(*l4.udp) + l4_offset;
+ }
/* pull values out of skb_shinfo */
gso_size = skb_shinfo(skb)->gso_size;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] igb: Add UDP segmentation offload support
2019-10-11 0:25 ` [PATCH v2 1/3] igb: Add " Josh Hunt
@ 2019-10-11 15:29 ` Alexander Duyck
2019-10-11 16:18 ` Josh Hunt
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Duyck @ 2019-10-11 15:29 UTC (permalink / raw)
To: Josh Hunt, netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown
On Thu, 2019-10-10 at 20:25 -0400, Josh Hunt wrote:
> Based on a series from Alexander Duyck this change adds UDP segmentation
> offload support to the igb driver.
>
> CC: Alexander Duyck <alexander.h.duyck@intel.com>
> CC: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> ---
> drivers/net/ethernet/intel/igb/e1000_82575.h | 1 +
> drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++++++++------
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
> index 6ad775b1a4c5..63ec253ac788 100644
> --- a/drivers/net/ethernet/intel/igb/e1000_82575.h
> +++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
> @@ -127,6 +127,7 @@ struct e1000_adv_tx_context_desc {
> };
>
> #define E1000_ADVTXD_MACLEN_SHIFT 9 /* Adv ctxt desc mac len shift */
> +#define E1000_ADVTXD_TUCMD_L4T_UDP 0x00000000 /* L4 Packet TYPE of UDP */
> #define E1000_ADVTXD_TUCMD_IPV4 0x00000400 /* IP Packet Type: 1=IPv4 */
> #define E1000_ADVTXD_TUCMD_L4T_TCP 0x00000800 /* L4 Packet TYPE of TCP */
> #define E1000_ADVTXD_TUCMD_L4T_SCTP 0x00001000 /* L4 packet TYPE of SCTP */
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 105b0624081a..4131bc8b079e 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2516,6 +2516,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
> if (unlikely(mac_hdr_len > IGB_MAX_MAC_HDR_LEN))
> return features & ~(NETIF_F_HW_CSUM |
> NETIF_F_SCTP_CRC |
> + NETIF_F_GSO_UDP_L4 |
> NETIF_F_HW_VLAN_CTAG_TX |
> NETIF_F_TSO |
> NETIF_F_TSO6);
> @@ -2524,6 +2525,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
> if (unlikely(network_hdr_len > IGB_MAX_NETWORK_HDR_LEN))
> return features & ~(NETIF_F_HW_CSUM |
> NETIF_F_SCTP_CRC |
> + NETIF_F_GSO_UDP_L4 |
> NETIF_F_TSO |
> NETIF_F_TSO6);
>
> @@ -3120,7 +3122,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> NETIF_F_HW_CSUM;
>
> if (hw->mac.type >= e1000_82576)
> - netdev->features |= NETIF_F_SCTP_CRC;
> + netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
>
> if (hw->mac.type >= e1000_i350)
> netdev->features |= NETIF_F_HW_TC;
> @@ -5694,6 +5696,7 @@ static int igb_tso(struct igb_ring *tx_ring,
> } ip;
> union {
> struct tcphdr *tcp;
> + struct udphdr *udp;
> unsigned char *hdr;
> } l4;
> u32 paylen, l4_offset;
> @@ -5713,7 +5716,8 @@ static int igb_tso(struct igb_ring *tx_ring,
> l4.hdr = skb_checksum_start(skb);
>
> /* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
> - type_tucmd = E1000_ADVTXD_TUCMD_L4T_TCP;
> + type_tucmd = (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)) ?
> + E1000_ADVTXD_TUCMD_L4T_TCP : E1000_ADVTXD_TUCMD_L4T_UDP;
The logic here seems a bit convoluted. Instead of testing for
'!SKB_GSO_UDP_L4' why not just make L4T_UDP the first option and drop the
'!'? That will make the TCP offload the default case rather than the UDP
offload.
The same applies to the other 2 patches.
> /* initialize outer IP header fields */
> if (ip.v4->version == 4) {
> @@ -5741,12 +5745,19 @@ static int igb_tso(struct igb_ring *tx_ring,
> /* determine offset of inner transport header */
> l4_offset = l4.hdr - skb->data;
>
> - /* compute length of segmentation header */
> - *hdr_len = (l4.tcp->doff * 4) + l4_offset;
> -
> /* remove payload length from inner checksum */
> paylen = skb->len - l4_offset;
> - csum_replace_by_diff(&l4.tcp->check, htonl(paylen));
> + if (type_tucmd & E1000_ADVTXD_TUCMD_L4T_TCP) {
> + /* compute length of segmentation header */
> + *hdr_len = (l4.tcp->doff * 4) + l4_offset;
> + csum_replace_by_diff(&l4.tcp->check,
> + (__force __wsum)htonl(paylen));
> + } else {
> + /* compute length of segmentation header */
> + *hdr_len = sizeof(*l4.udp) + l4_offset;
> + csum_replace_by_diff(&l4.udp->check,
> + (__force __wsum)htonl(paylen));
> + }
>
> /* update gso size and bytecount with header size */
> first->gso_segs = skb_shinfo(skb)->gso_segs;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] igb: Add UDP segmentation offload support
2019-10-11 15:29 ` Alexander Duyck
@ 2019-10-11 16:18 ` Josh Hunt
0 siblings, 0 replies; 6+ messages in thread
From: Josh Hunt @ 2019-10-11 16:18 UTC (permalink / raw)
To: Alexander Duyck, netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, sridhar.samudrala, aaron.f.brown
On 10/11/19 8:29 AM, Alexander Duyck wrote:
> On Thu, 2019-10-10 at 20:25 -0400, Josh Hunt wrote:
>> Based on a series from Alexander Duyck this change adds UDP segmentation
>> offload support to the igb driver.
>>
>> CC: Alexander Duyck <alexander.h.duyck@intel.com>
>> CC: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: Josh Hunt <johunt@akamai.com>
>> ---
>> drivers/net/ethernet/intel/igb/e1000_82575.h | 1 +
>> drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++++++++------
>> 2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
>> index 6ad775b1a4c5..63ec253ac788 100644
>> --- a/drivers/net/ethernet/intel/igb/e1000_82575.h
>> +++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
>> @@ -127,6 +127,7 @@ struct e1000_adv_tx_context_desc {
>> };
>>
>> #define E1000_ADVTXD_MACLEN_SHIFT 9 /* Adv ctxt desc mac len shift */
>> +#define E1000_ADVTXD_TUCMD_L4T_UDP 0x00000000 /* L4 Packet TYPE of UDP */
>> #define E1000_ADVTXD_TUCMD_IPV4 0x00000400 /* IP Packet Type: 1=IPv4 */
>> #define E1000_ADVTXD_TUCMD_L4T_TCP 0x00000800 /* L4 Packet TYPE of TCP */
>> #define E1000_ADVTXD_TUCMD_L4T_SCTP 0x00001000 /* L4 packet TYPE of SCTP */
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index 105b0624081a..4131bc8b079e 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -2516,6 +2516,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
>> if (unlikely(mac_hdr_len > IGB_MAX_MAC_HDR_LEN))
>> return features & ~(NETIF_F_HW_CSUM |
>> NETIF_F_SCTP_CRC |
>> + NETIF_F_GSO_UDP_L4 |
>> NETIF_F_HW_VLAN_CTAG_TX |
>> NETIF_F_TSO |
>> NETIF_F_TSO6);
>> @@ -2524,6 +2525,7 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
>> if (unlikely(network_hdr_len > IGB_MAX_NETWORK_HDR_LEN))
>> return features & ~(NETIF_F_HW_CSUM |
>> NETIF_F_SCTP_CRC |
>> + NETIF_F_GSO_UDP_L4 |
>> NETIF_F_TSO |
>> NETIF_F_TSO6);
>>
>> @@ -3120,7 +3122,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> NETIF_F_HW_CSUM;
>>
>> if (hw->mac.type >= e1000_82576)
>> - netdev->features |= NETIF_F_SCTP_CRC;
>> + netdev->features |= NETIF_F_SCTP_CRC | NETIF_F_GSO_UDP_L4;
>>
>> if (hw->mac.type >= e1000_i350)
>> netdev->features |= NETIF_F_HW_TC;
>> @@ -5694,6 +5696,7 @@ static int igb_tso(struct igb_ring *tx_ring,
>> } ip;
>> union {
>> struct tcphdr *tcp;
>> + struct udphdr *udp;
>> unsigned char *hdr;
>> } l4;
>> u32 paylen, l4_offset;
>> @@ -5713,7 +5716,8 @@ static int igb_tso(struct igb_ring *tx_ring,
>> l4.hdr = skb_checksum_start(skb);
>>
>> /* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
>> - type_tucmd = E1000_ADVTXD_TUCMD_L4T_TCP;
>> + type_tucmd = (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)) ?
>> + E1000_ADVTXD_TUCMD_L4T_TCP : E1000_ADVTXD_TUCMD_L4T_UDP;
>
> The logic here seems a bit convoluted. Instead of testing for
> '!SKB_GSO_UDP_L4' why not just make L4T_UDP the first option and drop the
> '!'? That will make the TCP offload the default case rather than the UDP
> offload.
>
> The same applies to the other 2 patches.
OK sure. I think I was trying to keep the TCP case first for some
reason, but agree that I should probably just reverse the order. Will
send a v3.
Josh
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-11 16:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 0:24 [PATCH v2 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 0:25 ` [PATCH v2 1/3] igb: Add " Josh Hunt
2019-10-11 15:29 ` Alexander Duyck
2019-10-11 16:18 ` Josh Hunt
2019-10-11 0:25 ` [PATCH v2 2/3] ixgbe: " Josh Hunt
2019-10-11 0:25 ` [PATCH v2 3/3] i40e: " Josh Hunt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).