* [next-queue 0/4] ixgbe: Enable tso and checksum offload with ipsec @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, steffen.klassert This patchset fixes up the bits for supporting TSO and checksum offload in conjunction with IPsec offload. This brings the throughput of a simple iperf test back up to nearly line rate. Shannon Nelson (4): ixgbe: no need for ipsec csum feature check ixgbe: remove unneeded ipsec test in Tx path ixgbe: no need for esp trailer if gso ixgbe: enable tso with ipsec offload drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 45 +++++++++++++++----------- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 30 ++++++++--------- 2 files changed, 42 insertions(+), 33 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 0/4] ixgbe: Enable tso and checksum offload with ipsec @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan This patchset fixes up the bits for supporting TSO and checksum offload in conjunction with IPsec offload. This brings the throughput of a simple iperf test back up to nearly line rate. Shannon Nelson (4): ixgbe: no need for ipsec csum feature check ixgbe: remove unneeded ipsec test in Tx path ixgbe: no need for esp trailer if gso ixgbe: enable tso with ipsec offload drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 45 +++++++++++++++----------- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 30 ++++++++--------- 2 files changed, 42 insertions(+), 33 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [next-queue 1/4] ixgbe: no need for ipsec csum feature check 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson @ 2018-03-15 21:23 ` Shannon Nelson -1 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, steffen.klassert With the patch commit f8aa2696b4af ("esp: check the NETIF_F_HW_ESP_TX_CSUM bit before segmenting") we no longer need to protect ourself from checksum offload requests on IPsec packets, so we can remove the check in our .ndo_features_check callback. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 8536942..153cd9e 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -9908,12 +9908,6 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) features &= ~NETIF_F_TSO; -#ifdef CONFIG_XFRM_OFFLOAD - /* IPsec offload doesn't get along well with others *yet* */ - if (skb->sp) - features &= ~(NETIF_F_TSO | NETIF_F_HW_CSUM); -#endif - return features; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 1/4] ixgbe: no need for ipsec csum feature check @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan With the patch commit f8aa2696b4af ("esp: check the NETIF_F_HW_ESP_TX_CSUM bit before segmenting") we no longer need to protect ourself from checksum offload requests on IPsec packets, so we can remove the check in our .ndo_features_check callback. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 8536942..153cd9e 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -9908,12 +9908,6 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) features &= ~NETIF_F_TSO; -#ifdef CONFIG_XFRM_OFFLOAD - /* IPsec offload doesn't get along well with others *yet* */ - if (skb->sp) - features &= ~(NETIF_F_TSO | NETIF_F_HW_CSUM); -#endif - return features; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [next-queue 2/4] ixgbe: remove unneeded ipsec test in TX path 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson @ 2018-03-15 21:23 ` Shannon Nelson -1 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, steffen.klassert Since the ipsec data fields will be zero anyway in the non-ipsec case, we can remove the conditional jump. Suggested-by: Alexander Duyck <alexander.duyck@gmail.com> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 153cd9e..a54f3d8 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -7864,10 +7864,8 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring, vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT; vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; - if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) { - fceof_saidx |= itd->sa_idx; - type_tucmd |= itd->flags | itd->trailer_len; - } + fceof_saidx |= itd->sa_idx; + type_tucmd |= itd->flags | itd->trailer_len; ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, 0); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 2/4] ixgbe: remove unneeded ipsec test in TX path @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan Since the ipsec data fields will be zero anyway in the non-ipsec case, we can remove the conditional jump. Suggested-by: Alexander Duyck <alexander.duyck@gmail.com> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 153cd9e..a54f3d8 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -7864,10 +7864,8 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring, vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT; vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; - if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) { - fceof_saidx |= itd->sa_idx; - type_tucmd |= itd->flags | itd->trailer_len; - } + fceof_saidx |= itd->sa_idx; + type_tucmd |= itd->flags | itd->trailer_len; ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, 0); } -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [next-queue 3/4] ixgbe: no need for esp trailer if gso 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson @ 2018-03-15 21:23 ` Shannon Nelson -1 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, steffen.klassert There is no need to calculate the trailer length if we're doing a GSO/TSO, as there is no trailer added to the packet data. Also, don't bother clearing the flags field as it was already cleared earlier. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 37 +++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c index f225452..5ddea43 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c @@ -774,11 +774,7 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, first->tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC; - itd->flags = 0; if (xs->id.proto == IPPROTO_ESP) { - struct sk_buff *skb = first->skb; - int ret, authlen, trailerlen; - u8 padlen; itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP | IXGBE_ADVTXD_TUCMD_L4T_TCP; @@ -790,19 +786,28 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, * padlen bytes of padding. This ends up not the same * as the static value found in xs->props.trailer_len (21). * - * The "correct" way to get the auth length would be to use - * authlen = crypto_aead_authsize(xs->data); - * but since we know we only have one size to worry about - * we can let the compiler use the constant and save us a - * few CPU cycles. + * ... but if we're doing GSO, don't bother as the stack + * doesn't add a trailer for those. */ - authlen = IXGBE_IPSEC_AUTH_BITS / 8; - - ret = skb_copy_bits(skb, skb->len - (authlen + 2), &padlen, 1); - if (unlikely(ret)) - return 0; - trailerlen = authlen + 2 + padlen; - itd->trailer_len = trailerlen; + if (!skb_is_gso(first->skb)) { + /* The "correct" way to get the auth length would be + * to use + * authlen = crypto_aead_authsize(xs->data); + * but since we know we only have one size to worry + * about * we can let the compiler use the constant + * and save us a few CPU cycles. + */ + const int authlen = IXGBE_IPSEC_AUTH_BITS / 8; + struct sk_buff *skb = first->skb; + u8 padlen; + int ret; + + ret = skb_copy_bits(skb, skb->len - (authlen + 2), + &padlen, 1); + if (unlikely(ret)) + return 0; + itd->trailer_len = authlen + 2 + padlen; + } } if (tsa->encrypt) itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN; -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 3/4] ixgbe: no need for esp trailer if gso @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan There is no need to calculate the trailer length if we're doing a GSO/TSO, as there is no trailer added to the packet data. Also, don't bother clearing the flags field as it was already cleared earlier. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 37 +++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c index f225452..5ddea43 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c @@ -774,11 +774,7 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, first->tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC; - itd->flags = 0; if (xs->id.proto == IPPROTO_ESP) { - struct sk_buff *skb = first->skb; - int ret, authlen, trailerlen; - u8 padlen; itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP | IXGBE_ADVTXD_TUCMD_L4T_TCP; @@ -790,19 +786,28 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, * padlen bytes of padding. This ends up not the same * as the static value found in xs->props.trailer_len (21). * - * The "correct" way to get the auth length would be to use - * authlen = crypto_aead_authsize(xs->data); - * but since we know we only have one size to worry about - * we can let the compiler use the constant and save us a - * few CPU cycles. + * ... but if we're doing GSO, don't bother as the stack + * doesn't add a trailer for those. */ - authlen = IXGBE_IPSEC_AUTH_BITS / 8; - - ret = skb_copy_bits(skb, skb->len - (authlen + 2), &padlen, 1); - if (unlikely(ret)) - return 0; - trailerlen = authlen + 2 + padlen; - itd->trailer_len = trailerlen; + if (!skb_is_gso(first->skb)) { + /* The "correct" way to get the auth length would be + * to use + * authlen = crypto_aead_authsize(xs->data); + * but since we know we only have one size to worry + * about * we can let the compiler use the constant + * and save us a few CPU cycles. + */ + const int authlen = IXGBE_IPSEC_AUTH_BITS / 8; + struct sk_buff *skb = first->skb; + u8 padlen; + int ret; + + ret = skb_copy_bits(skb, skb->len - (authlen + 2), + &padlen, 1); + if (unlikely(ret)) + return 0; + itd->trailer_len = authlen + 2 + padlen; + } } if (tsa->encrypt) itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN; -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [next-queue 4/4] ixgbe: enable tso with ipsec offload 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson @ 2018-03-15 21:23 ` Shannon Nelson -1 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan; +Cc: netdev, steffen.klassert Fix things up to support TSO offload in conjunction with IPsec hw offload. This raises throughput with IPsec offload on to nearly line rate. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c index 5ddea43..bfbcfc2 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { struct ixgbe_ipsec *ipsec; + netdev_features_t features; size_t size; if (adapter->hw.mac.type == ixgbe_mac_82598EB) @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) ixgbe_ipsec_clear_hw_tables(adapter); adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; - adapter->netdev->features |= NETIF_F_HW_ESP; - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; + + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; + adapter->netdev->features |= features; + adapter->netdev->hw_enc_features |= features; return; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index a54f3d8..6022666 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) static int ixgbe_tso(struct ixgbe_ring *tx_ring, struct ixgbe_tx_buffer *first, - u8 *hdr_len) + u8 *hdr_len, + struct ixgbe_ipsec_tx_data *itd) { u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; + u32 fceof_saidx = 0; struct sk_buff *skb = first->skb; union { struct iphdr *v4; @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); /* IP header will have to cancel out any data that - * is not a part of the outer IP header + * is not a part of the outer IP header, except for + * IPsec where we want the IP+ESP header. */ - ip.v4->check = csum_fold(csum_partial(trans_start, + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) + ip.v4->check = 0; + else + ip.v4->check = csum_fold(csum_partial(trans_start, csum_start - trans_start, 0)); type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; + fceof_saidx |= itd->sa_idx; + type_tucmd |= itd->flags | itd->trailer_len; + /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ vlan_macip_lens = l4.hdr - ip.hdr; vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, mss_l4len_idx); return 1; @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) goto out_drop; #endif - tso = ixgbe_tso(tx_ring, first, &hdr_len); + + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); if (tso < 0) goto out_drop; else if (!tso) @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, /* We can only support IPV4 TSO in tunnels if we can mangle the * inner IP ID field, so strip TSO if MANGLEID is not supported. + * IPsec offoad sets skb->encapsulation but still can handle + * the TSO, so it's the exception. */ - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && + !skb->sp) features &= ~NETIF_F_TSO; return features; -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 4/4] ixgbe: enable tso with ipsec offload @ 2018-03-15 21:23 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 21:23 UTC (permalink / raw) To: intel-wired-lan Fix things up to support TSO offload in conjunction with IPsec hw offload. This raises throughput with IPsec offload on to nearly line rate. Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c index 5ddea43..bfbcfc2 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { struct ixgbe_ipsec *ipsec; + netdev_features_t features; size_t size; if (adapter->hw.mac.type == ixgbe_mac_82598EB) @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) ixgbe_ipsec_clear_hw_tables(adapter); adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; - adapter->netdev->features |= NETIF_F_HW_ESP; - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; + + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; + adapter->netdev->features |= features; + adapter->netdev->hw_enc_features |= features; return; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index a54f3d8..6022666 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) static int ixgbe_tso(struct ixgbe_ring *tx_ring, struct ixgbe_tx_buffer *first, - u8 *hdr_len) + u8 *hdr_len, + struct ixgbe_ipsec_tx_data *itd) { u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; + u32 fceof_saidx = 0; struct sk_buff *skb = first->skb; union { struct iphdr *v4; @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); /* IP header will have to cancel out any data that - * is not a part of the outer IP header + * is not a part of the outer IP header, except for + * IPsec where we want the IP+ESP header. */ - ip.v4->check = csum_fold(csum_partial(trans_start, + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) + ip.v4->check = 0; + else + ip.v4->check = csum_fold(csum_partial(trans_start, csum_start - trans_start, 0)); type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; + fceof_saidx |= itd->sa_idx; + type_tucmd |= itd->flags | itd->trailer_len; + /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ vlan_macip_lens = l4.hdr - ip.hdr; vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, mss_l4len_idx); return 1; @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) goto out_drop; #endif - tso = ixgbe_tso(tx_ring, first, &hdr_len); + + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); if (tso < 0) goto out_drop; else if (!tso) @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, /* We can only support IPV4 TSO in tunnels if we can mangle the * inner IP ID field, so strip TSO if MANGLEID is not supported. + * IPsec offoad sets skb->encapsulation but still can handle + * the TSO, so it's the exception. */ - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && + !skb->sp) features &= ~NETIF_F_TSO; return features; -- 2.7.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Intel-wired-lan] [next-queue 4/4] ixgbe: enable tso with ipsec offload 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson @ 2018-03-15 22:03 ` Alexander Duyck -1 siblings, 0 replies; 14+ messages in thread From: Alexander Duyck @ 2018-03-15 22:03 UTC (permalink / raw) To: Shannon Nelson; +Cc: intel-wired-lan, Steffen Klassert, Netdev On Thu, Mar 15, 2018 at 2:23 PM, Shannon Nelson <shannon.nelson@oracle.com> wrote: > Fix things up to support TSO offload in conjunction > with IPsec hw offload. This raises throughput with > IPsec offload on to nearly line rate. > > Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > index 5ddea43..bfbcfc2 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, > void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) > { > struct ixgbe_ipsec *ipsec; > + netdev_features_t features; > size_t size; > > if (adapter->hw.mac.type == ixgbe_mac_82598EB) > @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) > ixgbe_ipsec_clear_hw_tables(adapter); > > adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; > - adapter->netdev->features |= NETIF_F_HW_ESP; > - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; > + > + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; > + adapter->netdev->features |= features; > + adapter->netdev->hw_enc_features |= features; Instead of adding the local variable you might just create a new define that includes these 3 feature flags and then use that here. You could use the way I did IXGBE_GSO_PARTIAL_FEATURES as an example. > return; > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index a54f3d8..6022666 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) > > static int ixgbe_tso(struct ixgbe_ring *tx_ring, > struct ixgbe_tx_buffer *first, > - u8 *hdr_len) > + u8 *hdr_len, > + struct ixgbe_ipsec_tx_data *itd) > { > u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; > + u32 fceof_saidx = 0; > struct sk_buff *skb = first->skb; Reverse xmas tree this. It should probably be moved down to just past the declaration of paylen and l4_offset. > union { > struct iphdr *v4; > @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, > unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); > > /* IP header will have to cancel out any data that > - * is not a part of the outer IP header > + * is not a part of the outer IP header, except for > + * IPsec where we want the IP+ESP header. > */ > - ip.v4->check = csum_fold(csum_partial(trans_start, > + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) > + ip.v4->check = 0; > + else > + ip.v4->check = csum_fold(csum_partial(trans_start, > csum_start - trans_start, > 0)); > type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; I would say this should be flipped like so: ip.v4->check = (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) ? csum_fold(csum_partial(trans_start, csum_start - trans_start, 0) : 0; > @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, > mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; > mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; > > + fceof_saidx |= itd->sa_idx; > + type_tucmd |= itd->flags | itd->trailer_len; > + > /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ > vlan_macip_lens = l4.hdr - ip.hdr; > vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; > vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; > > - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, > + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, > mss_l4len_idx); > > return 1; > @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, > if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) > goto out_drop; > #endif > - tso = ixgbe_tso(tx_ring, first, &hdr_len); > + > + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); > if (tso < 0) > goto out_drop; > else if (!tso) No need for the extra blank line. I would say just leave it as is and add your extra argument. > @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, > > /* We can only support IPV4 TSO in tunnels if we can mangle the > * inner IP ID field, so strip TSO if MANGLEID is not supported. > + * IPsec offoad sets skb->encapsulation but still can handle > + * the TSO, so it's the exception. > */ > - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) > + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && > + !skb->sp) > features &= ~NETIF_F_TSO; > > return features; > -- > 2.7.4 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 4/4] ixgbe: enable tso with ipsec offload @ 2018-03-15 22:03 ` Alexander Duyck 0 siblings, 0 replies; 14+ messages in thread From: Alexander Duyck @ 2018-03-15 22:03 UTC (permalink / raw) To: intel-wired-lan On Thu, Mar 15, 2018 at 2:23 PM, Shannon Nelson <shannon.nelson@oracle.com> wrote: > Fix things up to support TSO offload in conjunction > with IPsec hw offload. This raises throughput with > IPsec offload on to nearly line rate. > > Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ > 2 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > index 5ddea43..bfbcfc2 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c > @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, > void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) > { > struct ixgbe_ipsec *ipsec; > + netdev_features_t features; > size_t size; > > if (adapter->hw.mac.type == ixgbe_mac_82598EB) > @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) > ixgbe_ipsec_clear_hw_tables(adapter); > > adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; > - adapter->netdev->features |= NETIF_F_HW_ESP; > - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; > + > + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; > + adapter->netdev->features |= features; > + adapter->netdev->hw_enc_features |= features; Instead of adding the local variable you might just create a new define that includes these 3 feature flags and then use that here. You could use the way I did IXGBE_GSO_PARTIAL_FEATURES as an example. > return; > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index a54f3d8..6022666 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) > > static int ixgbe_tso(struct ixgbe_ring *tx_ring, > struct ixgbe_tx_buffer *first, > - u8 *hdr_len) > + u8 *hdr_len, > + struct ixgbe_ipsec_tx_data *itd) > { > u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; > + u32 fceof_saidx = 0; > struct sk_buff *skb = first->skb; Reverse xmas tree this. It should probably be moved down to just past the declaration of paylen and l4_offset. > union { > struct iphdr *v4; > @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, > unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); > > /* IP header will have to cancel out any data that > - * is not a part of the outer IP header > + * is not a part of the outer IP header, except for > + * IPsec where we want the IP+ESP header. > */ > - ip.v4->check = csum_fold(csum_partial(trans_start, > + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) > + ip.v4->check = 0; > + else > + ip.v4->check = csum_fold(csum_partial(trans_start, > csum_start - trans_start, > 0)); > type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; I would say this should be flipped like so: ip.v4->check = (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) ? csum_fold(csum_partial(trans_start, csum_start - trans_start, 0) : 0; > @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, > mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; > mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; > > + fceof_saidx |= itd->sa_idx; > + type_tucmd |= itd->flags | itd->trailer_len; > + > /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ > vlan_macip_lens = l4.hdr - ip.hdr; > vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; > vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; > > - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, > + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, > mss_l4len_idx); > > return 1; > @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, > if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) > goto out_drop; > #endif > - tso = ixgbe_tso(tx_ring, first, &hdr_len); > + > + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); > if (tso < 0) > goto out_drop; > else if (!tso) No need for the extra blank line. I would say just leave it as is and add your extra argument. > @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, > > /* We can only support IPV4 TSO in tunnels if we can mangle the > * inner IP ID field, so strip TSO if MANGLEID is not supported. > + * IPsec offoad sets skb->encapsulation but still can handle > + * the TSO, so it's the exception. > */ > - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) > + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && > + !skb->sp) > features &= ~NETIF_F_TSO; > > return features; > -- > 2.7.4 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan at osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-wired-lan] [next-queue 4/4] ixgbe: enable tso with ipsec offload 2018-03-15 22:03 ` Alexander Duyck @ 2018-03-15 23:33 ` Shannon Nelson -1 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 23:33 UTC (permalink / raw) To: Alexander Duyck; +Cc: intel-wired-lan, Steffen Klassert, Netdev On 3/15/2018 3:03 PM, Alexander Duyck wrote: > On Thu, Mar 15, 2018 at 2:23 PM, Shannon Nelson > <shannon.nelson@oracle.com> wrote: >> Fix things up to support TSO offload in conjunction >> with IPsec hw offload. This raises throughput with >> IPsec offload on to nearly line rate. >> >> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ >> 2 files changed, 24 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> index 5ddea43..bfbcfc2 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, >> void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) >> { >> struct ixgbe_ipsec *ipsec; >> + netdev_features_t features; >> size_t size; >> >> if (adapter->hw.mac.type == ixgbe_mac_82598EB) >> @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) >> ixgbe_ipsec_clear_hw_tables(adapter); >> >> adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; >> - adapter->netdev->features |= NETIF_F_HW_ESP; >> - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; >> + >> + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; >> + adapter->netdev->features |= features; >> + adapter->netdev->hw_enc_features |= features; > > Instead of adding the local variable you might just create a new > define that includes these 3 feature flags and then use that here. You > could use the way I did IXGBE_GSO_PARTIAL_FEATURES as an example. > >> return; >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index a54f3d8..6022666 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) >> >> static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> struct ixgbe_tx_buffer *first, >> - u8 *hdr_len) >> + u8 *hdr_len, >> + struct ixgbe_ipsec_tx_data *itd) >> { >> u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; >> + u32 fceof_saidx = 0; >> struct sk_buff *skb = first->skb; > > Reverse xmas tree this. It should probably be moved down to just past > the declaration of paylen and l4_offset. > >> union { >> struct iphdr *v4; >> @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); >> >> /* IP header will have to cancel out any data that >> - * is not a part of the outer IP header >> + * is not a part of the outer IP header, except for >> + * IPsec where we want the IP+ESP header. >> */ >> - ip.v4->check = csum_fold(csum_partial(trans_start, >> + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) >> + ip.v4->check = 0; >> + else >> + ip.v4->check = csum_fold(csum_partial(trans_start, >> csum_start - trans_start, >> 0)); >> type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; > > I would say this should be flipped like so: > ip.v4->check = (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) ? > csum_fold(csum_partial(trans_start, > csum_start - trans_start, 0) : 0; > >> @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; >> mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; >> >> + fceof_saidx |= itd->sa_idx; >> + type_tucmd |= itd->flags | itd->trailer_len; >> + >> /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ >> vlan_macip_lens = l4.hdr - ip.hdr; >> vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; >> vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; >> >> - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, >> + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, >> mss_l4len_idx); >> >> return 1; >> @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, >> if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) >> goto out_drop; >> #endif >> - tso = ixgbe_tso(tx_ring, first, &hdr_len); >> + >> + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); >> if (tso < 0) >> goto out_drop; >> else if (!tso) > > No need for the extra blank line. I would say just leave it as is and > add your extra argument. Yep, you're right on all counts. That SKB_GSO_PARTIAL bit looks a bit interesting... I'll respin and repost. sln > >> @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, >> >> /* We can only support IPV4 TSO in tunnels if we can mangle the >> * inner IP ID field, so strip TSO if MANGLEID is not supported. >> + * IPsec offoad sets skb->encapsulation but still can handle >> + * the TSO, so it's the exception. >> */ >> - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) >> + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && >> + !skb->sp) >> features &= ~NETIF_F_TSO; >> >> return features; >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-wired-lan mailing list >> Intel-wired-lan@osuosl.org >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [next-queue 4/4] ixgbe: enable tso with ipsec offload @ 2018-03-15 23:33 ` Shannon Nelson 0 siblings, 0 replies; 14+ messages in thread From: Shannon Nelson @ 2018-03-15 23:33 UTC (permalink / raw) To: intel-wired-lan On 3/15/2018 3:03 PM, Alexander Duyck wrote: > On Thu, Mar 15, 2018 at 2:23 PM, Shannon Nelson > <shannon.nelson@oracle.com> wrote: >> Fix things up to support TSO offload in conjunction >> with IPsec hw offload. This raises throughput with >> IPsec offload on to nearly line rate. >> >> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com> >> --- >> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 7 +++++-- >> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++------ >> 2 files changed, 24 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> index 5ddea43..bfbcfc2 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c >> @@ -896,6 +896,7 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring, >> void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) >> { >> struct ixgbe_ipsec *ipsec; >> + netdev_features_t features; >> size_t size; >> >> if (adapter->hw.mac.type == ixgbe_mac_82598EB) >> @@ -929,8 +930,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) >> ixgbe_ipsec_clear_hw_tables(adapter); >> >> adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops; >> - adapter->netdev->features |= NETIF_F_HW_ESP; >> - adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP; >> + >> + features = NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | NETIF_F_GSO_ESP; >> + adapter->netdev->features |= features; >> + adapter->netdev->hw_enc_features |= features; > > Instead of adding the local variable you might just create a new > define that includes these 3 feature flags and then use that here. You > could use the way I did IXGBE_GSO_PARTIAL_FEATURES as an example. > >> return; >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> index a54f3d8..6022666 100644 >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c >> @@ -7721,9 +7721,11 @@ static void ixgbe_service_task(struct work_struct *work) >> >> static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> struct ixgbe_tx_buffer *first, >> - u8 *hdr_len) >> + u8 *hdr_len, >> + struct ixgbe_ipsec_tx_data *itd) >> { >> u32 vlan_macip_lens, type_tucmd, mss_l4len_idx; >> + u32 fceof_saidx = 0; >> struct sk_buff *skb = first->skb; > > Reverse xmas tree this. It should probably be moved down to just past > the declaration of paylen and l4_offset. > >> union { >> struct iphdr *v4; >> @@ -7762,9 +7764,13 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> unsigned char *trans_start = ip.hdr + (ip.v4->ihl * 4); >> >> /* IP header will have to cancel out any data that >> - * is not a part of the outer IP header >> + * is not a part of the outer IP header, except for >> + * IPsec where we want the IP+ESP header. >> */ >> - ip.v4->check = csum_fold(csum_partial(trans_start, >> + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) >> + ip.v4->check = 0; >> + else >> + ip.v4->check = csum_fold(csum_partial(trans_start, >> csum_start - trans_start, >> 0)); >> type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4; > > I would say this should be flipped like so: > ip.v4->check = (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) ? > csum_fold(csum_partial(trans_start, > csum_start - trans_start, 0) : 0; > >> @@ -7797,12 +7803,15 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring, >> mss_l4len_idx = (*hdr_len - l4_offset) << IXGBE_ADVTXD_L4LEN_SHIFT; >> mss_l4len_idx |= skb_shinfo(skb)->gso_size << IXGBE_ADVTXD_MSS_SHIFT; >> >> + fceof_saidx |= itd->sa_idx; >> + type_tucmd |= itd->flags | itd->trailer_len; >> + >> /* vlan_macip_lens: HEADLEN, MACLEN, VLAN tag */ >> vlan_macip_lens = l4.hdr - ip.hdr; >> vlan_macip_lens |= (ip.hdr - skb->data) << IXGBE_ADVTXD_MACLEN_SHIFT; >> vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK; >> >> - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, >> + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, >> mss_l4len_idx); >> >> return 1; >> @@ -8493,7 +8502,8 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb, >> if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx)) >> goto out_drop; >> #endif >> - tso = ixgbe_tso(tx_ring, first, &hdr_len); >> + >> + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx); >> if (tso < 0) >> goto out_drop; >> else if (!tso) > > No need for the extra blank line. I would say just leave it as is and > add your extra argument. Yep, you're right on all counts. That SKB_GSO_PARTIAL bit looks a bit interesting... I'll respin and repost. sln > >> @@ -9902,8 +9912,11 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev, >> >> /* We can only support IPV4 TSO in tunnels if we can mangle the >> * inner IP ID field, so strip TSO if MANGLEID is not supported. >> + * IPsec offoad sets skb->encapsulation but still can handle >> + * the TSO, so it's the exception. >> */ >> - if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID)) >> + if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID) && >> + !skb->sp) >> features &= ~NETIF_F_TSO; >> >> return features; >> -- >> 2.7.4 >> >> _______________________________________________ >> Intel-wired-lan mailing list >> Intel-wired-lan at osuosl.org >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-03-15 23:33 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-15 21:23 [next-queue 0/4] ixgbe: Enable tso and checksum offload with ipsec Shannon Nelson 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson 2018-03-15 21:23 ` [next-queue 1/4] ixgbe: no need for ipsec csum feature check Shannon Nelson 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson 2018-03-15 21:23 ` [next-queue 2/4] ixgbe: remove unneeded ipsec test in TX path Shannon Nelson 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson 2018-03-15 21:23 ` [next-queue 3/4] ixgbe: no need for esp trailer if gso Shannon Nelson 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson 2018-03-15 21:23 ` [next-queue 4/4] ixgbe: enable tso with ipsec offload Shannon Nelson 2018-03-15 21:23 ` [Intel-wired-lan] " Shannon Nelson 2018-03-15 22:03 ` Alexander Duyck 2018-03-15 22:03 ` Alexander Duyck 2018-03-15 23:33 ` Shannon Nelson 2018-03-15 23:33 ` Shannon Nelson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.