* [PATCH v3 0/3] igb, ixgbe, i40e UDP segmentation offload support
@ 2019-10-11 16:53 Josh Hunt
2019-10-11 16:53 ` [PATCH v3 1/3] igb: Add " Josh Hunt
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Josh Hunt @ 2019-10-11 16:53 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
v3 change:
* Change gso_type order check, suggested by Alex
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] 11+ messages in thread
* [PATCH v3 1/3] igb: Add UDP segmentation offload support
2019-10-11 16:53 [PATCH v3 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
@ 2019-10-11 16:53 ` Josh Hunt
2019-10-25 3:26 ` Brown, Aaron F
2019-10-11 16:53 ` [PATCH v3 2/3] ixgbe: " Josh Hunt
2019-10-11 16:53 ` [PATCH v3 3/3] i40e: " Josh Hunt
2 siblings, 1 reply; 11+ messages in thread
From: Josh Hunt @ 2019-10-11 16:53 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..b65e8a063aef 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_UDP : E1000_ADVTXD_TUCMD_L4T_TCP;
/* 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] 11+ messages in thread
* [PATCH v3 2/3] ixgbe: Add UDP segmentation offload support
2019-10-11 16:53 [PATCH v3 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 16:53 ` [PATCH v3 1/3] igb: Add " Josh Hunt
@ 2019-10-11 16:53 ` Josh Hunt
2019-10-16 16:43 ` [Intel-wired-lan] " Bowers, AndrewX
2019-10-11 16:53 ` [PATCH v3 3/3] i40e: " Josh Hunt
2 siblings, 1 reply; 11+ messages in thread
From: Josh Hunt @ 2019-10-11 16:53 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..6c9edd272c7a 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_UDP : IXGBE_ADVTXD_TUCMD_L4T_TCP;
/* 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] 11+ messages in thread
* [PATCH v3 3/3] i40e: Add UDP segmentation offload support
2019-10-11 16:53 [PATCH v3 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 16:53 ` [PATCH v3 1/3] igb: Add " Josh Hunt
2019-10-11 16:53 ` [PATCH v3 2/3] ixgbe: " Josh Hunt
@ 2019-10-11 16:53 ` Josh Hunt
2019-10-11 17:39 ` Samudrala, Sridhar
2019-10-16 16:44 ` [Intel-wired-lan] " Bowers, AndrewX
2 siblings, 2 replies; 11+ messages in thread
From: Josh Hunt @ 2019-10-11 16:53 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..b8496037ef7f 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.udp->check, (__force __wsum)htonl(paylen));
+ /* compute length of segmentation header */
+ *hdr_len = sizeof(*l4.udp) + l4_offset;
+ } else {
+ csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
+ /* compute length of segmentation header */
+ *hdr_len = (l4.tcp->doff * 4) + l4_offset;
+ }
/* pull values out of skb_shinfo */
gso_size = skb_shinfo(skb)->gso_size;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] i40e: Add UDP segmentation offload support
2019-10-11 16:53 ` [PATCH v3 3/3] i40e: " Josh Hunt
@ 2019-10-11 17:39 ` Samudrala, Sridhar
2019-10-11 17:56 ` Alexander Duyck
2019-10-16 16:44 ` [Intel-wired-lan] " Bowers, AndrewX
1 sibling, 1 reply; 11+ messages in thread
From: Samudrala, Sridhar @ 2019-10-11 17:39 UTC (permalink / raw)
To: Josh Hunt, netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, aaron.f.brown, alexander.h.duyck, Alexander Duyck
On 10/11/2019 9:53 AM, Josh Hunt wrote:
> 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..b8496037ef7f 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.udp->check, (__force __wsum)htonl(paylen));
> + /* compute length of segmentation header */
> + *hdr_len = sizeof(*l4.udp) + l4_offset;
> + } else {
> + csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
> + /* compute length of segmentation header */
> + *hdr_len = (l4.tcp->doff * 4) + l4_offset;
> + }
Is it guaranteed that gso_type can be either UDP or TCP only if we reach
here? Don't we need to handle the case where it is neither and return
from this function?
>
> /* pull values out of skb_shinfo */
> gso_size = skb_shinfo(skb)->gso_size;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] i40e: Add UDP segmentation offload support
2019-10-11 17:39 ` Samudrala, Sridhar
@ 2019-10-11 17:56 ` Alexander Duyck
2019-10-11 18:59 ` Samudrala, Sridhar
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Duyck @ 2019-10-11 17:56 UTC (permalink / raw)
To: Samudrala, Sridhar, Josh Hunt, netdev, intel-wired-lan,
jeffrey.t.kirsher
Cc: willemb, aaron.f.brown
On Fri, 2019-10-11 at 10:39 -0700, Samudrala, Sridhar wrote:
>
> On 10/11/2019 9:53 AM, Josh Hunt wrote:
> > 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..b8496037ef7f 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.udp->check, (__force __wsum)htonl(paylen));
> > + /* compute length of segmentation header */
> > + *hdr_len = sizeof(*l4.udp) + l4_offset;
> > + } else {
> > + csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
> > + /* compute length of segmentation header */
> > + *hdr_len = (l4.tcp->doff * 4) + l4_offset;
> > + }
>
> Is it guaranteed that gso_type can be either UDP or TCP only if we reach
> here? Don't we need to handle the case where it is neither and return
> from this function?
We should only reach here if a supported gso_type value is in the packet,
otherwise we should end up with software segmentation taking care of it
and clearing the gso_size value if I recall correctly.
Otherwise the code should have been checking for non-TCP types ages ago,
and we would have experienced all sorts of bugs.
- Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] i40e: Add UDP segmentation offload support
2019-10-11 17:56 ` Alexander Duyck
@ 2019-10-11 18:59 ` Samudrala, Sridhar
0 siblings, 0 replies; 11+ messages in thread
From: Samudrala, Sridhar @ 2019-10-11 18:59 UTC (permalink / raw)
To: Alexander Duyck, Josh Hunt, netdev, intel-wired-lan, jeffrey.t.kirsher
Cc: willemb, aaron.f.brown
On 10/11/2019 10:56 AM, Alexander Duyck wrote:
> On Fri, 2019-10-11 at 10:39 -0700, Samudrala, Sridhar wrote:
>>
>> On 10/11/2019 9:53 AM, Josh Hunt wrote:
>>> 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..b8496037ef7f 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.udp->check, (__force __wsum)htonl(paylen));
>>> + /* compute length of segmentation header */
>>> + *hdr_len = sizeof(*l4.udp) + l4_offset;
>>> + } else {
>>> + csum_replace_by_diff(&l4.tcp->check, (__force __wsum)htonl(paylen));
>>> + /* compute length of segmentation header */
>>> + *hdr_len = (l4.tcp->doff * 4) + l4_offset;
>>> + }
>>
>> Is it guaranteed that gso_type can be either UDP or TCP only if we reach
>> here? Don't we need to handle the case where it is neither and return
>> from this function?
>
> We should only reach here if a supported gso_type value is in the packet,
> otherwise we should end up with software segmentation taking care of it
> and clearing the gso_size value if I recall correctly.
>
> Otherwise the code should have been checking for non-TCP types ages ago,
> and we would have experienced all sorts of bugs.
Yes. Sounds good. Thanks for the clarification.
>
> - Alex
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [Intel-wired-lan] [PATCH v3 2/3] ixgbe: Add UDP segmentation offload support
2019-10-11 16:53 ` [PATCH v3 2/3] ixgbe: " Josh Hunt
@ 2019-10-16 16:43 ` Bowers, AndrewX
2019-10-18 18:04 ` Josh Hunt
0 siblings, 1 reply; 11+ messages in thread
From: Bowers, AndrewX @ 2019-10-16 16:43 UTC (permalink / raw)
To: netdev, intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Josh Hunt
> Sent: Friday, October 11, 2019 9:54 AM
> To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Duyck, Alexander H <alexander.h.duyck@intel.com>;
> willemb@google.com; Josh Hunt <johunt@akamai.com>;
> alexander.h.duyck@linux.intel.com
> Subject: [Intel-wired-lan] [PATCH v3 2/3] ixgbe: Add UDP segmentation
> offload support
>
> 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.l
> ocaldomain/
>
> 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(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [Intel-wired-lan] [PATCH v3 3/3] i40e: Add UDP segmentation offload support
2019-10-11 16:53 ` [PATCH v3 3/3] i40e: " Josh Hunt
2019-10-11 17:39 ` Samudrala, Sridhar
@ 2019-10-16 16:44 ` Bowers, AndrewX
1 sibling, 0 replies; 11+ messages in thread
From: Bowers, AndrewX @ 2019-10-16 16:44 UTC (permalink / raw)
To: netdev, intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Josh Hunt
> Sent: Friday, October 11, 2019 9:54 AM
> To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Duyck, Alexander H <alexander.h.duyck@intel.com>;
> willemb@google.com; Josh Hunt <johunt@akamai.com>;
> alexander.h.duyck@linux.intel.com
> Subject: [Intel-wired-lan] [PATCH v3 3/3] i40e: Add UDP segmentation
> offload support
>
> 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(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-wired-lan] [PATCH v3 2/3] ixgbe: Add UDP segmentation offload support
2019-10-16 16:43 ` [Intel-wired-lan] " Bowers, AndrewX
@ 2019-10-18 18:04 ` Josh Hunt
0 siblings, 0 replies; 11+ messages in thread
From: Josh Hunt @ 2019-10-18 18:04 UTC (permalink / raw)
To: Bowers, AndrewX; +Cc: netdev, intel-wired-lan
On Thu, Oct 17, 2019 at 5:33 AM Bowers, AndrewX
<andrewx.bowers@intel.com> wrote:
>
> > -----Original Message-----
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> > Behalf Of Josh Hunt
> > Sent: Friday, October 11, 2019 9:54 AM
> > To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher,
> > Jeffrey T <jeffrey.t.kirsher@intel.com>
> > Cc: Duyck, Alexander H <alexander.h.duyck@intel.com>;
> > willemb@google.com; Josh Hunt <johunt@akamai.com>;
> > alexander.h.duyck@linux.intel.com
> > Subject: [Intel-wired-lan] [PATCH v3 2/3] ixgbe: Add UDP segmentation
> > offload support
> >
> > 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.l
> > ocaldomain/
> >
> > 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(-)
>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>
>
Andrew, thanks for testing. Were you able to validate the igb driver?
Thanks
--
Josh
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3 1/3] igb: Add UDP segmentation offload support
2019-10-11 16:53 ` [PATCH v3 1/3] igb: Add " Josh Hunt
@ 2019-10-25 3:26 ` Brown, Aaron F
0 siblings, 0 replies; 11+ messages in thread
From: Brown, Aaron F @ 2019-10-25 3:26 UTC (permalink / raw)
To: Josh Hunt, netdev, intel-wired-lan, Kirsher, Jeffrey T
Cc: willemb, Samudrala, Sridhar, alexander.h.duyck, Duyck, Alexander H
> From: Josh Hunt <johunt@akamai.com>
> Sent: Friday, October 11, 2019 9:54 AM
> To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: willemb@google.com; Samudrala, Sridhar
> <sridhar.samudrala@intel.com>; Brown, Aaron F
> <aaron.f.brown@intel.com>; alexander.h.duyck@linux.intel.com; Josh Hunt
> <johunt@akamai.com>; Duyck, Alexander H
> <alexander.h.duyck@intel.com>
> Subject: [PATCH v3 1/3] igb: Add UDP segmentation offload support
>
> 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(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-25 3:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 16:53 [PATCH v3 0/3] igb, ixgbe, i40e UDP segmentation offload support Josh Hunt
2019-10-11 16:53 ` [PATCH v3 1/3] igb: Add " Josh Hunt
2019-10-25 3:26 ` Brown, Aaron F
2019-10-11 16:53 ` [PATCH v3 2/3] ixgbe: " Josh Hunt
2019-10-16 16:43 ` [Intel-wired-lan] " Bowers, AndrewX
2019-10-18 18:04 ` Josh Hunt
2019-10-11 16:53 ` [PATCH v3 3/3] i40e: " Josh Hunt
2019-10-11 17:39 ` Samudrala, Sridhar
2019-10-11 17:56 ` Alexander Duyck
2019-10-11 18:59 ` Samudrala, Sridhar
2019-10-16 16:44 ` [Intel-wired-lan] " Bowers, AndrewX
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).