[net-next PATCH v2 0/9] Fix Tunnel features and enable GSO partial for several drivers

[net-next PATCH v2 0/9] Fix Tunnel features and enable GSO partial for several drivers

[Alexander Duyck]

This patch series is meant to allow us to get the best performance possible
for Mellanox ConnectX-3/4 and Broadcom NetXtreme-C/E adapters in terms of
VXLAN and GRE tunnels.

The first few patches address issues I found when just trying to collect
performance numbers.  Specifically I was unable to get rates of any more
than 1 or 2 Mb/s if I was using a tunnel that ran over IPv6.  In addition I
found a few other items related to GSO_PARTIAL and the TSO_MANGLEID that
needed to be addressed.

The next 4 patches go through and enable GSO_PARTIAL for VXLAN tunnels that
have an outer checksum enabled, and then enable IPv6 support where I can.
One outstanding issue is that I wasn't able to get offloads working with
outer IPv6 headers on mlx4.  However that wasn't a feature that was enabled
before so it isn't technically a regression, however I believe Engineers
from Mellanox said they would look into it since they thought it should be
supported.

The last patch enables GSO_PARTIAL for VXLAN and GRE tunnels on the bnxt
driver.  One piece of feedback I received on the patch was that the
hardware has globally set IPv6 UDP tunnels to always have the checksum
field computed.  I plan to work with Broadcom to get that addressed so that
we only populate the checksum field if it was requested by the network
stack.

v2: Rebased patches off of latest changes to the mlx4/mlx5 drivers.
    Added bnxt driver patch as I received feedback on the RFC.

    There are outstanding issues that need to be addressed, however they
    were present before these patches so it isn't as if they introduce a
    regression.  In addition gains can be easily seen so there should be no
    issue with applying the driver patches while the IPv6 mlx4 and bnxt
    issues are being researched.

---

Alexander Duyck (9):
      net: Disable segmentation if checksumming is not supported
      gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
      net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
      vxlan: Add checksum check to the features check function
      mlx4: Add support for UDP tunnel segmentation with outer checksum offload
      mlx4: Add support for inner IPv6 checksum offloads and TSO
      mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
      mlx5e: Fix IPv6 tunnel checksum offload
      bnxt: Add support for segmentation of tunnels with outer checksums


 drivers/net/ethernet/broadcom/bnxt/bnxt.c         |    9 ++++-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c    |   38 +++++++++++++++++----
 drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   15 +++++++-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |   10 ++++--
 include/linux/if_ether.h                          |    5 +++
 include/net/vxlan.h                               |    4 ++
 net/core/dev.c                                    |    6 +++
 net/core/skbuff.c                                 |    6 ++-
 8 files changed, 74 insertions(+), 19 deletions(-)

[net-next PATCH v2 1/9] net: Disable segmentation if checksumming is not supported

[Alexander Duyck]

In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
offload for tunnels.  With this being the case we should disable GSO in
addition to the checksum offload features when we find that a device cannot
perform a checksum on a given packet type.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 net/core/dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index d91dfbec0fc6..673d1f118bfb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2815,7 +2815,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
 
 	if (skb->ip_summed != CHECKSUM_NONE &&
 	    !can_checksum_protocol(features, type)) {
-		features &= ~NETIF_F_CSUM_MASK;
+		features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 	} else if (illegal_highdma(skb->dev, skb)) {
 		features &= ~NETIF_F_SG;
 	}

Re: [net-next PATCH v2 1/9] net: Disable segmentation if checksumming is not supported

[Or Gerlitz]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
> offload for tunnels.

Alex,

To clarify, when you say "not support IPv6 checksum for tunnels", you
refer to the offloading of the outer or inner checksum?

Still (me and I think also Tariq from our driver team) catching up on
the series, the primitives and conventions you are introducing using
and how this applies on mlx5. I saw that Saeed acked the the mlx5e
patches (7 and 8).

Specifically, the mlx4 patches are practically fixes so if they don't
land in 4.7 via net-next we can get them there through net, lets give
us the few more days needed to catch up from our side.

Or.

Re: [net-next PATCH v2 1/9] net: Disable segmentation if checksumming is not supported

[Alexander Duyck]

On Sun, May 1, 2016 at 1:30 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
>> offload for tunnels.
>
> Alex,
>
> To clarify, when you say "not support IPv6 checksum for tunnels", you
> refer to the offloading of the outer or inner checksum?

In the case of mlx4 it was an issue with both inner and outer due to
IPv6 checksum.  The issue was that the feature was not exposed and yet
the stack was attempting to make use of it in various ways.  The fixes
that resolved the issues are in patches 1 and 4.  If we wanted to we
could move those to net, but then it would be difficult to test the
existing patches on the mlx4 until the net tree containing those
patches was merged back in.

> Still (me and I think also Tariq from our driver team) catching up on
> the series, the primitives and conventions you are introducing using
> and how this applies on mlx5. I saw that Saeed acked the the mlx5e
> patches (7 and 8).

The concept for all this is pretty simple.  What I am doing is
restricting TSO so that we have a fixed size that is used for all
outgoing frames.  Then we precompute the outer headers and use those
values when performing GSO.  By doing this we can populate the UDP
checksum field instead of forcing it to 0 which allows us to perform
TSO for tunnels with outer checksums.

> Specifically, the mlx4 patches are practically fixes so if they don't
> land in 4.7 via net-next we can get them there through net, lets give
> us the few more days needed to catch up from our side.

Actually the mlx4 specific portion of these patches are not really
fixes, they are enabling features.  Specifically IPv6 checksum, TSOv6,
and TSO for VXLAN tunnels with outer checksums.  It is patches 1 and 4
that contain the fix and it likely applies to more than just the mlx4
driver as I believe there is a qlogic driver with a similar feature
flag layout.  If we want we could recommend those patches for stable
and they could probably be back ported into 4.6 after it is released
because I don't see the urgency to push a fix in for something that
has obviously been broken for quite some time now that likely nobody
is ever testing.

- Alex

[net-next PATCH v2 2/9] gso: Only allow GSO_PARTIAL if we can checksum the inner protocol

[Alexander Duyck]

This patch addresses a possible issue that can occur if we get into any odd
corner cases where we support TSO for a given protocol but not the checksum
or scatter-gather offload.  There are few drivers floating around that
setup their tunnels this way and by enforcing the checksum piece we can
avoid mangling any frames.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 net/core/skbuff.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7a1d48983f81..954741873877 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3080,8 +3080,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 	unsigned int headroom;
 	unsigned int len = head_skb->len;
 	__be16 proto;
-	bool csum;
-	int sg = !!(features & NETIF_F_SG);
+	bool csum, sg;
 	int nfrags = skb_shinfo(head_skb)->nr_frags;
 	int err = -ENOMEM;
 	int i = 0;
@@ -3093,13 +3092,14 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 	if (unlikely(!proto))
 		return ERR_PTR(-EINVAL);
 
+	sg = !!(features & NETIF_F_SG);
 	csum = !!can_checksum_protocol(features, proto);
 
 	/* GSO partial only requires that we trim off any excess that
 	 * doesn't fit into an MSS sized block, so take care of that
 	 * now.
 	 */
-	if (features & NETIF_F_GSO_PARTIAL) {
+	if (sg && csum && (features & NETIF_F_GSO_PARTIAL)) {
 		partial_segs = len / mss;
 		mss *= partial_segs;
 	}

[net-next PATCH v2 3/9] net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO

[Alexander Duyck]

This change makes it so that we will strip the TSO_MANGLEID bit if TSO is
not present.  This way we will also handle ECN correctly of TSO is not
present.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 net/core/dev.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 673d1f118bfb..e98ba63fe280 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6721,6 +6721,10 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
 		features &= ~NETIF_F_TSO6;
 	}
 
+	/* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */
+	if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO))
+		features &= ~NETIF_F_TSO_MANGLEID;
+
 	/* TSO ECN requires that TSO is present as well. */
 	if ((features & NETIF_F_ALL_TSO) == NETIF_F_TSO_ECN)
 		features &= ~NETIF_F_TSO_ECN;

[net-next PATCH v2 4/9] vxlan: Add checksum check to the features check function

[Alexander Duyck]

We need to perform an additional check on the inner headers to determine if
we can offload the checksum for them.  Previously this check didn't occur
so we would generate an invalid frame in the case of an IPv6 header
encapsulated inside of an IPv4 tunnel.  To fix this I added a secondary
check to vxlan_features_check so that we can verify that we can offload the
inner checksum.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 include/linux/if_ether.h |    5 +++++
 include/net/vxlan.h      |    4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index d5569734f672..548fd535fd02 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -28,6 +28,11 @@ static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
 	return (struct ethhdr *)skb_mac_header(skb);
 }
 
+static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb)
+{
+	return (struct ethhdr *)skb_inner_mac_header(skb);
+}
+
 int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
 
 extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 673e9f9e6da7..b8803165df91 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -317,7 +317,9 @@ static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
 	    (skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
 	     skb->inner_protocol != htons(ETH_P_TEB) ||
 	     (skb_inner_mac_header(skb) - skb_transport_header(skb) !=
-	      sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+	      sizeof(struct udphdr) + sizeof(struct vxlanhdr)) ||
+	     (skb->ip_summed != CHECKSUM_NONE &&
+	      !can_checksum_protocol(features, inner_eth_hdr(skb)->h_proto))))
 		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 
 	return features;

[net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8bd143dda95d..bce37cbfde24 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2358,7 +2358,9 @@ out:
 
 	/* set offloads */
 	priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
-				      NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
+				      NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+				      NETIF_F_GSO_UDP_TUNNEL_CSUM |
+				      NETIF_F_GSO_PARTIAL;
 }
 
 static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
@@ -2368,7 +2370,9 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
 						 vxlan_del_task);
 	/* unset offloads */
 	priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
-				      NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL);
+					NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+					NETIF_F_GSO_UDP_TUNNEL_CSUM |
+					NETIF_F_GSO_PARTIAL);
 
 	ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
 				  VXLAN_STEER_BY_OUTER_MAC, 0);
@@ -2992,8 +2996,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	}
 
 	if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
-		dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
-		dev->features    |= NETIF_F_GSO_UDP_TUNNEL;
+		dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+				    NETIF_F_GSO_UDP_TUNNEL_CSUM |
+				    NETIF_F_GSO_PARTIAL;
+		dev->features    |= NETIF_F_GSO_UDP_TUNNEL |
+				    NETIF_F_GSO_UDP_TUNNEL_CSUM |
+				    NETIF_F_GSO_PARTIAL;
+		dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
 	}
 
 	mdev->pndev[port] = dev;

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Saeed Mahameed]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8bd143dda95d..bce37cbfde24 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2358,7 +2358,9 @@ out:
>
>         /* set offloads */
>         priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> -                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
> +                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
> +                                     NETIF_F_GSO_UDP_TUNNEL_CSUM |
> +                                     NETIF_F_GSO_PARTIAL;
>  }
>
>  static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
> @@ -2368,7 +2370,9 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
>                                                  vxlan_del_task);
>         /* unset offloads */
>         priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> -                                     NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL);
> +                                       NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
> +                                       NETIF_F_GSO_UDP_TUNNEL_CSUM |
> +                                       NETIF_F_GSO_PARTIAL);

I know it is not related to your patch, but is it ok to dynamically
modify priv->dev->hw_enc_features every vxlan add/del_port request ?
especially on a deferred work !
Shouldn't we at least notify the stack ?

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Or Gerlitz]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.

Hi Alex,

I see now the above text appearing in bunch of similar commit of
yours, specifically to Intel drivers and mlx5... could you please
elaborate a bit more what you mean here and what are the practical
consequences of that characteristics?

I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
where the outer checksum is not zero. AFAIK, any other outer checksum
value can't correctly be a constant... are you assuming here  RCO or
LCO?

Or.

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

On Sun, May 1, 2016 at 1:35 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
>> header fields for length and checksum as well as the length and checksum
>> fields for outer UDP headers.
>
> Hi Alex,
>
> I see now the above text appearing in bunch of similar commit of
> yours, specifically to Intel drivers and mlx5... could you please
> elaborate a bit more what you mean here and what are the practical
> consequences of that characteristics?
>
> I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
> segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
> where the outer checksum is not zero. AFAIK, any other outer checksum
> value can't correctly be a constant... are you assuming here  RCO or
> LCO?

Actually it is really easy for outer UDP checksum to be constant as
long as we keep the length of all segments constant.  This all ties
back into LCO.  As long as the fields between the start of the UDP
header and the start of the TCP header are either constant, as in the
case of IPv6, or have their own checksum as in the case of IPv4 we
will end up with the checksum of the outer header being constant.

So in effect as long as we can trust the hardware to segment every
frame to the specified size and that it won't insert any extra data
anywhere in that region that we aren't expecting we can guarantee that
each frame will have the same checksum for the outer UDP header.

- Alex

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Or Gerlitz]

On Mon, May 2, 2016 at 5:25 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Sun, May 1, 2016 at 1:35 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
>>> header fields for length and checksum as well as the length and checksum
>>> fields for outer UDP headers.

>> I see now the above text appearing in bunch of similar commit of
>> yours, specifically to Intel drivers and mlx5... could you please
>> elaborate a bit more what you mean here and what are the practical
>> consequences of that characteristics?

>> I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
>> segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
>> where the outer checksum is not zero. AFAIK, any other outer checksum
>> value can't correctly be a constant... are you assuming here  RCO or
>> LCO?

> Actually it is really easy for outer UDP checksum to be constant as
> long as we keep the length of all segments constant.  This all ties
> back into LCO.  As long as the fields between the start of the UDP
> header and the start of the TCP header are either constant, as in the
> case of IPv6, or have their own checksum as in the case of IPv4 we
> will end up with the checksum of the outer header being constant.

cool. I would love seeing this documented somewhere, either in the
change log if you do a respin or on some kernel networking
documentation, is that part of the LCO documentation?

> So in effect as long as we can trust the hardware to segment every
> frame to the specified size and that it won't insert any extra data
> anywhere in that region that we aren't expecting we can guarantee that
> each frame will have the same checksum for the outer UDP header.

Wow, that is really cool, thanks for taking the time and explaining it over.

Just one more piece to clarify... in the general case (e.g inner
packet size 1.5k...64k), the last segment would not have the same
length as the other segments, what happens on that case?

Or.

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

On Mon, May 2, 2016 at 12:19 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Mon, May 2, 2016 at 5:25 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Sun, May 1, 2016 at 1:35 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>>> This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
>>>> header fields for length and checksum as well as the length and checksum
>>>> fields for outer UDP headers.
>
>>> I see now the above text appearing in bunch of similar commit of
>>> yours, specifically to Intel drivers and mlx5... could you please
>>> elaborate a bit more what you mean here and what are the practical
>>> consequences of that characteristics?
>
>>> I got that right NETIF_F_GSO_UDP_TUNNEL_CSUM means that the HW can do
>>> segmentation for TCP packets encapsulated by UDP tunnel e.g VXLAN
>>> where the outer checksum is not zero. AFAIK, any other outer checksum
>>> value can't correctly be a constant... are you assuming here  RCO or
>>> LCO?
>
>> Actually it is really easy for outer UDP checksum to be constant as
>> long as we keep the length of all segments constant.  This all ties
>> back into LCO.  As long as the fields between the start of the UDP
>> header and the start of the TCP header are either constant, as in the
>> case of IPv6, or have their own checksum as in the case of IPv4 we
>> will end up with the checksum of the outer header being constant.
>
> cool. I would love seeing this documented somewhere, either in the
> change log if you do a respin or on some kernel networking
> documentation, is that part of the LCO documentation?

I have some documentation in
Documentation/networking/segmentation-offloads.txt.  Feel free to
review it and provide any additional feedback and/or patches you
believe it needs.  To me it made sense but I already understood how
all this stuff worked.

>> So in effect as long as we can trust the hardware to segment every
>> frame to the specified size and that it won't insert any extra data
>> anywhere in that region that we aren't expecting we can guarantee that
>> each frame will have the same checksum for the outer UDP header.
>
> Wow, that is really cool, thanks for taking the time and explaining it over.
>
> Just one more piece to clarify... in the general case (e.g inner
> packet size 1.5k...64k), the last segment would not have the same
> length as the other segments, what happens on that case?

Actually in the case of GSO partial we have go through the software
segmentation code and trim off any last bit that doesn't match the MSS
of the rest of the frame.  That way you end up with one frame that has
some number of MSS sized chunks, and then one remainder if there is a
frame that would be a different size.

- Alex

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Or Gerlitz]

On 5/2/2016 6:41 PM, Alexander Duyck wrote:
>> >Just one more piece to clarify... in the general case (e.g inner
>> >packet size 1.5k...64k), the last segment would not have the same
>> >length as the other segments, what happens on that case?
> Actually in the case of GSO partial we have go through the software
> segmentation code and trim off any last bit that doesn't match the MSS
> of the rest of the frame.  That way you end up with one frame that has
> some number of MSS sized chunks, and then one remainder if there is a
> frame that would be a different size.

OK, thanks for further clarifying this, will look on the docs you 
pointed etc. From what you wrote here I understand it's indeed possible 
for one frame to be of different size from the rest, but the LCO thing 
still works somehow..

Or.

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

On Tue, May 3, 2016 at 5:41 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> On 5/2/2016 6:41 PM, Alexander Duyck wrote:
>>>
>>> >Just one more piece to clarify... in the general case (e.g inner
>>> >packet size 1.5k...64k), the last segment would not have the same
>>> >length as the other segments, what happens on that case?
>>
>> Actually in the case of GSO partial we have go through the software
>> segmentation code and trim off any last bit that doesn't match the MSS
>> of the rest of the frame.  That way you end up with one frame that has
>> some number of MSS sized chunks, and then one remainder if there is a
>> frame that would be a different size.
>
>
> OK, thanks for further clarifying this, will look on the docs you pointed
> etc. From what you wrote here I understand it's indeed possible for one
> frame to be of different size from the rest, but the LCO thing still works
> somehow..

We split the one that would be a different size off via GSO.  So we
end up sending up 2 frames to the device if there is going to be one
piece that doesn't quite match.  We split that one piece off via GSO.
That is one of the reasons why I referred to it as partial GSO as all
we are using the software segmentation code for is to make sure we
have the GSO block consists of segments that are all the same size.

- Alex

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Or Gerlitz]

On 5/3/2016 6:29 PM, Alexander Duyck wrote:
> We split the one that would be a different size off via GSO.  So we
> end up sending up 2 frames to the device if there is going to be one
> piece that doesn't quite match.  We split that one piece off via GSO.
> That is one of the reasons why I referred to it as partial GSO as all
> we are using the software segmentation code for is to make sure we
> have the GSO block consists of segments that are all the same size.

I see, so if somehow it happens a lot that the TCP stack sends down 
something which once segmented ends up with the last segment being of 
different size from the other ones we would have to call the NIC xmit 
function twice (BTW can we use xmit_more here?)  -- which could be 
effecting performance, I guess.

GSO_UDP_TUNNEL_CSUM (commit  0f4f4ffa7 "net: Add GSO support for UDP 
tunnels with checksum") came to mark "that a device is capable of 
computing the UDP checksum in the encapsulating header of a UDP tunnel" 
-- and the way we use it here is that we do advertize that bit towards 
the stack for devices whose HW can **not** do that, and things work b/c 
of LCO (this is my understanding).

I miss something in the bigger picture here, what does this buy us? e.g 
vs just letting this (say) vxlan tunnel use zero checksum on the outer 
UDP packet, is that has something to do with RCO?

Or.

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alex Duyck]

On Wed, May 4, 2016 at 8:50 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> On 5/3/2016 6:29 PM, Alexander Duyck wrote:
>>
>> We split the one that would be a different size off via GSO.  So we
>> end up sending up 2 frames to the device if there is going to be one
>> piece that doesn't quite match.  We split that one piece off via GSO.
>> That is one of the reasons why I referred to it as partial GSO as all
>> we are using the software segmentation code for is to make sure we
>> have the GSO block consists of segments that are all the same size.
>
>
> I see, so if somehow it happens a lot that the TCP stack sends down
> something which once segmented ends up with the last segment being of
> different size from the other ones we would have to call the NIC xmit
> function twice (BTW can we use xmit_more here?)  -- which could be effecting
> performance, I guess.
>
> GSO_UDP_TUNNEL_CSUM (commit  0f4f4ffa7 "net: Add GSO support for UDP tunnels
> with checksum") came to mark "that a device is capable of computing the UDP
> checksum in the encapsulating header of a UDP tunnel" -- and the way we use
> it here is that we do advertize that bit towards the stack for devices whose
> HW can **not** do that, and things work b/c of LCO (this is my
> understanding).
>
> I miss something in the bigger picture here, what does this buy us? e.g vs
> just letting this (say) vxlan tunnel use zero checksum on the outer UDP
> packet, is that has something to do with RCO?

I think the piece you are missing is GSO_PARTIAL.  Basically
GSO_PARTIAL indicates that we can perform GSO as long as all segments
are the same size and also allows for ignoring one level of headers.
So in the case of ixgbe for instance we can support tunnel offloads as
long as we allow for the inner IPv4 ID to be a fixed value which is
identified by enabling TSO_MANGLEID.  In the case of i40e, mlx4, and
mlx5 the key bit is that we just have to have the frames the same size
for all segments and then we can support tunnels with outer checksum
because the checksum has been computed once and can be applied to all
of the segmented frames.

Hope that helps.

- Alex

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Or Gerlitz]

On Wed, May 4, 2016 at 7:06 PM, Alex Duyck <aduyck@mirantis.com> wrote:
> On Wed, May 4, 2016 at 8:50 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> On 5/3/2016 6:29 PM, Alexander Duyck wrote:
>>>
>>> We split the one that would be a different size off via GSO.  So we
>>> end up sending up 2 frames to the device if there is going to be one
>>> piece that doesn't quite match.  We split that one piece off via GSO.
>>> That is one of the reasons why I referred to it as partial GSO as all
>>> we are using the software segmentation code for is to make sure we
>>> have the GSO block consists of segments that are all the same size.
>>
>>
>> I see, so if somehow it happens a lot that the TCP stack sends down
>> something which once segmented ends up with the last segment being of
>> different size from the other ones we would have to call the NIC xmit
>> function twice (BTW can we use xmit_more here?)  -- which could be effecting
>> performance, I guess.
>>
>> GSO_UDP_TUNNEL_CSUM (commit  0f4f4ffa7 "net: Add GSO support for UDP tunnels
>> with checksum") came to mark "that a device is capable of computing the UDP
>> checksum in the encapsulating header of a UDP tunnel" -- and the way we use
>> it here is that we do advertize that bit towards the stack for devices whose
>> HW can **not** do that, and things work b/c of LCO (this is my
>> understanding).
>>
>> I miss something in the bigger picture here, what does this buy us? e.g vs
>> just letting this (say) vxlan tunnel use zero checksum on the outer UDP
>> packet, is that has something to do with RCO?
>
> I think the piece you are missing is GSO_PARTIAL.  Basically
> GSO_PARTIAL indicates that we can perform GSO as long as all segments
> are the same size and also allows for ignoring one level of headers.
> So in the case of ixgbe for instance we can support tunnel offloads as
> long as we allow for the inner IPv4 ID to be a fixed value which is
> identified by enabling TSO_MANGLEID.  In the case of i40e, mlx4, and
> mlx5 the key bit is that we just have to have the frames the same size
> for all segments and then we can support tunnels with outer checksum
> because the checksum has been computed once and can be applied to all
> of the segmented frames.

Yep, I think to basically follow on the PARTIAL thing, which once
advertised by i40e, mlx4 and mlx5 allow them support udp (and GRE in
i40e case) tunnels with outer checksum.

My question was what this buy us for the UDP case vs. using zero
checksum for the tunnel (outer packet), I tried to figure out if it
has something to do with the remote side, e.g for RCO or alike.
Basically, under PARTIAL, on the worst case we could have ending up
with 2x packet xmitted to the NIC - e.g if each TCP message which is
to be encapsulated by the stack and later segmented by the NIC HW is
broken to two b/c otherwise the last segmented packet will not be of
equal size as of the all the preceding ones.

Or being a bit more positive... is there an expected performance  gain
when you use MANGLEID and/or PARTIAL to enable supporting UDP tunnel
segmentation checksum offload towards the stack? what is the reason
for that gain?

As for GRE tunnel segmentation checksum offload, I saw in your i40e
patch that it made your testbed to go from 12Gbs to 20Gbs, is this b/c
the stack can not actually let the HW do the segmentation w.o checksum
offload? if not, can you help understand the source of the gain?

> Hope that helps.

yes, your notes are very helpful, thanks for sparing the time..

Or.

Re: [net-next PATCH v2 5/9] mlx4: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

On Thu, May 5, 2016 at 2:39 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Wed, May 4, 2016 at 7:06 PM, Alex Duyck <aduyck@mirantis.com> wrote:
>> On Wed, May 4, 2016 at 8:50 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>>> On 5/3/2016 6:29 PM, Alexander Duyck wrote:
>>>>
>>>> We split the one that would be a different size off via GSO.  So we
>>>> end up sending up 2 frames to the device if there is going to be one
>>>> piece that doesn't quite match.  We split that one piece off via GSO.
>>>> That is one of the reasons why I referred to it as partial GSO as all
>>>> we are using the software segmentation code for is to make sure we
>>>> have the GSO block consists of segments that are all the same size.
>>>
>>>
>>> I see, so if somehow it happens a lot that the TCP stack sends down
>>> something which once segmented ends up with the last segment being of
>>> different size from the other ones we would have to call the NIC xmit
>>> function twice (BTW can we use xmit_more here?)  -- which could be effecting
>>> performance, I guess.
>>>
>>> GSO_UDP_TUNNEL_CSUM (commit  0f4f4ffa7 "net: Add GSO support for UDP tunnels
>>> with checksum") came to mark "that a device is capable of computing the UDP
>>> checksum in the encapsulating header of a UDP tunnel" -- and the way we use
>>> it here is that we do advertize that bit towards the stack for devices whose
>>> HW can **not** do that, and things work b/c of LCO (this is my
>>> understanding).
>>>
>>> I miss something in the bigger picture here, what does this buy us? e.g vs
>>> just letting this (say) vxlan tunnel use zero checksum on the outer UDP
>>> packet, is that has something to do with RCO?
>>
>> I think the piece you are missing is GSO_PARTIAL.  Basically
>> GSO_PARTIAL indicates that we can perform GSO as long as all segments
>> are the same size and also allows for ignoring one level of headers.
>> So in the case of ixgbe for instance we can support tunnel offloads as
>> long as we allow for the inner IPv4 ID to be a fixed value which is
>> identified by enabling TSO_MANGLEID.  In the case of i40e, mlx4, and
>> mlx5 the key bit is that we just have to have the frames the same size
>> for all segments and then we can support tunnels with outer checksum
>> because the checksum has been computed once and can be applied to all
>> of the segmented frames.
>
> Yep, I think to basically follow on the PARTIAL thing, which once
> advertised by i40e, mlx4 and mlx5 allow them support udp (and GRE in
> i40e case) tunnels with outer checksum.
>
> My question was what this buy us for the UDP case vs. using zero
> checksum for the tunnel (outer packet), I tried to figure out if it
> has something to do with the remote side, e.g for RCO or alike.
> Basically, under PARTIAL, on the worst case we could have ending up
> with 2x packet xmitted to the NIC - e.g if each TCP message which is
> to be encapsulated by the stack and later segmented by the NIC HW is
> broken to two b/c otherwise the last segmented packet will not be of
> equal size as of the all the preceding ones.

There ends up being a few pieces to this.  In the case of i40e the Tx
gain seen is mostly for just transmitting the tunnel types with
checksums.  This is because without that we have to use software
segmentation and that is expensive because it requires 40+ frames to
transmit a single 64K block of TCP data.  In the case of GSO_PARTIAL
this data is usually all sent in a single packet because the TCP stack
tries to send MSS aligned blocks.

On the Rx side a gain can be seen if we exceed the number of ports
that can be used to support tunnels on the device.  This is because
the hardware can still offload the outer UDP checksum and as a result
it can still go GRO on the frame thanks to the code Tom Herbert added
that converts validated outer UDP checksums to checksum complete.
Without the outer UDP checksum present we wouldn't be able to do GRO
and throughput drops to the 6 - 9Gb/s range.

> Or being a bit more positive... is there an expected performance  gain
> when you use MANGLEID and/or PARTIAL to enable supporting UDP tunnel
> segmentation checksum offload towards the stack? what is the reason
> for that gain?

The TSO_MANGLEID bit is only really needed for igb and ixgbe.  Those
drivers don't support tunnel offloads directly.  Instead they can
support checksum offloads or a segmentation offload with an arbitrary
IP header size up to 511 bytes.  So in order to do segmentation for
tunnels what we are doing is repeating everything from the outer
transport header through the inner network header for each frame.  As
such we can only perform segmentation offloads for IPv4 in any type of
tunnel if we can repeat the IP ID for the inner header.  If we are
allowed to do that then we can move packets between functions on the
same device at 15Gb/s which is the upper limits of non-encapsulated
traffic for VF to VF.  Without that we are at 12Gb/s with outer
checksums and software segmentation, and only 6Gb/s with software
segmentation and outer checksum forced to 0.

> As for GRE tunnel segmentation checksum offload, I saw in your i40e
> patch that it made your testbed to go from 12Gbs to 20Gbs, is this b/c
> the stack can not actually let the HW do the segmentation w.o checksum
> offload? if not, can you help understand the source of the gain?

The device didn't advertise NETIF_F_GRE_CSUM so if there was a
checksum in the GRE header the packet had to be segmented in software.
By using the GSO_PARTIAL approach the speed is improved and comes up
to about 20Gb/s which is what the hardware does for standard GRE
tunnels.  Basically the best software segmentation can do is 12Gb/s
for most NICs on a single flow.  With hardware segment or GSO partial
we can push somewhere around 20Gb/s or more depending on the
configuration.

>> Hope that helps.
>
> yes, your notes are very helpful, thanks for sparing the time..

No problem.

- Alex

[net-next PATCH v2 6/9] mlx4: Add support for inner IPv6 checksum offloads and TSO

[Alexander Duyck]

>From what I can tell the ConnectX-3 will support an inner IPv6 checksum and
segmentation offload, however it cannot support outer IPv6 headers.  This
assumption is based on the fact that I could see the checksum being
offloaded for inner header on IPv4 tunnels, but not on IPv6 tunnels.

For this reason I am adding the feature to the hw_enc_features and adding
an extra check to the features_check call that will disable GSO and
checksum offload in the case that the encapsulated frame has an outer IP
version of that is not 4.  The check in mlx4_en_features_check could be
removed if at some point in the future a fix is found that allows the
hardware to offload segmentation/checksum on tunnels with an outer IPv6
header.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   25 +++++++++++++++++++-----
 drivers/net/ethernet/mellanox/mlx4/en_tx.c     |   15 ++++++++++++--
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bce37cbfde24..6f28ac58251c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2357,8 +2357,10 @@ out:
 	}
 
 	/* set offloads */
-	priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
-				      NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+	priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+				      NETIF_F_RXCSUM |
+				      NETIF_F_TSO | NETIF_F_TSO6 |
+				      NETIF_F_GSO_UDP_TUNNEL |
 				      NETIF_F_GSO_UDP_TUNNEL_CSUM |
 				      NETIF_F_GSO_PARTIAL;
 }
@@ -2369,8 +2371,10 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
 	struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
 						 vxlan_del_task);
 	/* unset offloads */
-	priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
-					NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+	priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+					NETIF_F_RXCSUM |
+					NETIF_F_TSO | NETIF_F_TSO6 |
+					NETIF_F_GSO_UDP_TUNNEL |
 					NETIF_F_GSO_UDP_TUNNEL_CSUM |
 					NETIF_F_GSO_PARTIAL);
 
@@ -2431,7 +2435,18 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
 						netdev_features_t features)
 {
 	features = vlan_features_check(skb, features);
-	return vxlan_features_check(skb, features);
+	features = vxlan_features_check(skb, features);
+
+	/* The ConnectX-3 doesn't support outer IPv6 checksums but it does
+	 * support inner IPv6 checksums and segmentation so  we need to
+	 * strip that feature if this is an IPv6 encapsulated frame.
+	 */
+	if (skb->encapsulation &&
+	    (skb->ip_summed == CHECKSUM_PARTIAL) &&
+	    (ip_hdr(skb)->version != 4))
+		features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
+
+	return features;
 }
 #endif
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index a386f047c1af..0f206a95429c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -41,6 +41,7 @@
 #include <linux/vmalloc.h>
 #include <linux/tcp.h>
 #include <linux/ip.h>
+#include <linux/ipv6.h>
 #include <linux/moduleparam.h>
 
 #include "mlx4_en.h"
@@ -920,8 +921,18 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 				 tx_ind, fragptr);
 
 	if (skb->encapsulation) {
-		struct iphdr *ipv4 = (struct iphdr *)skb_inner_network_header(skb);
-		if (ipv4->protocol == IPPROTO_TCP || ipv4->protocol == IPPROTO_UDP)
+		union {
+			struct iphdr *v4;
+			struct ipv6hdr *v6;
+			unsigned char *hdr;
+		} ip;
+		u8 proto;
+
+		ip.hdr = skb_inner_network_header(skb);
+		proto = (ip.v4->version == 4) ? ip.v4->protocol :
+						ip.v6->nexthdr;
+
+		if (proto == IPPROTO_TCP || proto == IPPROTO_UDP)
 			op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP | MLX4_WQE_CTRL_ILP);
 		else
 			op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP);

Re: [net-next PATCH v2 6/9] mlx4: Add support for inner IPv6 checksum offloads and TSO

[Saeed Mahameed]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> >From what I can tell the ConnectX-3 will support an inner IPv6 checksum and
> segmentation offload, however it cannot support outer IPv6 headers.  This
> assumption is based on the fact that I could see the checksum being
> offloaded for inner header on IPv4 tunnels, but not on IPv6 tunnels.
>
> For this reason I am adding the feature to the hw_enc_features and adding
> an extra check to the features_check call that will disable GSO and
> checksum offload in the case that the encapsulated frame has an outer IP
> version of that is not 4.  The check in mlx4_en_features_check could be
> removed if at some point in the future a fix is found that allows the
> hardware to offload segmentation/checksum on tunnels with an outer IPv6
> header.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

[net-next PATCH v2 7/9] mlx5e: Add support for UDP tunnel segmentation with outer checksum offload

[Alexander Duyck]

This patch assumes that the mlx5 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 4ccfc1ac62c5..2d6aaad77d62 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2803,13 +2803,18 @@ static void mlx5e_build_netdev(struct net_device *netdev)
 	netdev->hw_features      |= NETIF_F_HW_VLAN_CTAG_FILTER;
 
 	if (mlx5e_vxlan_allowed(mdev)) {
-		netdev->hw_features     |= NETIF_F_GSO_UDP_TUNNEL;
+		netdev->hw_features     |= NETIF_F_GSO_UDP_TUNNEL |
+					   NETIF_F_GSO_UDP_TUNNEL_CSUM |
+					   NETIF_F_GSO_PARTIAL;
 		netdev->hw_enc_features |= NETIF_F_IP_CSUM;
 		netdev->hw_enc_features |= NETIF_F_RXCSUM;
 		netdev->hw_enc_features |= NETIF_F_TSO;
 		netdev->hw_enc_features |= NETIF_F_TSO6;
 		netdev->hw_enc_features |= NETIF_F_RXHASH;
 		netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
+		netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
+					   NETIF_F_GSO_PARTIAL;
+		netdev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
 	}
 
 	mlx5_query_port_fcs(mdev, &fcs_supported, &fcs_enabled);

Re: [net-next PATCH v2 7/9] mlx5e: Add support for UDP tunnel segmentation with outer checksum offload

[Saeed Mahameed]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch assumes that the mlx5 hardware will ignore existing IPv4/v6
> header fields for length and checksum as well as the length and checksum
> fields for outer UDP headers.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

[net-next PATCH v2 8/9] mlx5e: Fix IPv6 tunnel checksum offload

[Alexander Duyck]

The mlx5 driver exposes support for TSO6 but not IPv6 csum for hardware
encapsulated tunnels.  This leads to issues as it triggers warnings in
skb_checksum_help as it ends up being called as we report supporting the
segmentation but not the checksumming for IPv6 frames.

This patch corrects that and drops 2 features that don't actually need to
be supported in hw_enc_features since they are Rx features and don't
actually impact anything by being present in hw_enc_features.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 2d6aaad77d62..409916c18b86 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2807,10 +2807,9 @@ static void mlx5e_build_netdev(struct net_device *netdev)
 					   NETIF_F_GSO_UDP_TUNNEL_CSUM |
 					   NETIF_F_GSO_PARTIAL;
 		netdev->hw_enc_features |= NETIF_F_IP_CSUM;
-		netdev->hw_enc_features |= NETIF_F_RXCSUM;
+		netdev->hw_enc_features |= NETIF_F_IPV6_CSUM;
 		netdev->hw_enc_features |= NETIF_F_TSO;
 		netdev->hw_enc_features |= NETIF_F_TSO6;
-		netdev->hw_enc_features |= NETIF_F_RXHASH;
 		netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
 		netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
 					   NETIF_F_GSO_PARTIAL;

Re: [net-next PATCH v2 8/9] mlx5e: Fix IPv6 tunnel checksum offload

[Saeed Mahameed]

On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The mlx5 driver exposes support for TSO6 but not IPv6 csum for hardware
> encapsulated tunnels.  This leads to issues as it triggers warnings in
> skb_checksum_help as it ends up being called as we report supporting the
> segmentation but not the checksumming for IPv6 frames.
>
> This patch corrects that and drops 2 features that don't actually need to
> be supported in hw_enc_features since they are Rx features and don't
> actually impact anything by being present in hw_enc_features.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

[net-next PATCH v2 9/9] bnxt: Add support for segmentation of tunnels with outer checksums

[Alexander Duyck]

This patch assumes that the bnxt hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP and GRE headers.

I have been told by Michael Chan that this is working.  Though this might
be somewhat redundant for IPv6 as they are forcing the checksum to be
computed for all IPv6 frames that are offloaded.  A follow-up patch may be
necessary in order to fix this as it is essentially mangling the outer IPv6
headers to add a checksum where none was requested.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4645c44e7c15..ae668476fff0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6194,14 +6194,19 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			   NETIF_F_TSO | NETIF_F_TSO6 |
 			   NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
 			   NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
-			   NETIF_F_RXHASH |
+			   NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
+			   NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH |
 			   NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO;
 
 	dev->hw_enc_features =
 			NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
 			NETIF_F_TSO | NETIF_F_TSO6 |
 			NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
-			NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT;
+			NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
+			NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
+			NETIF_F_GSO_PARTIAL;
+	dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
+				    NETIF_F_GSO_GRE_CSUM;
 	dev->vlan_features = dev->hw_features | NETIF_F_HIGHDMA;
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX |
 			    NETIF_F_HW_VLAN_STAG_RX | NETIF_F_HW_VLAN_STAG_TX;

Re: [net-next PATCH v2 1/9] net: Disable segmentation if checksumming is not supported

[Or Gerlitz]

On Mon, May 2, 2016 at 5:16 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Sun, May 1, 2016 at 1:30 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>> On Sat, Apr 30, 2016 at 1:43 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
>>> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
>>> offload for tunnels.
>>
>> Alex,
>>
>> To clarify, when you say "not support IPv6 checksum for tunnels", you
>> refer to the offloading of the outer or inner checksum?
>
> In the case of mlx4 it was an issue with both inner and outer due to
> IPv6 checksum.  The issue was that the feature was not exposed and yet
> the stack was attempting to make use of it in various ways.  The fixes
> that resolved the issues are in patches 1 and 4.  If we wanted to we
> could move those to net, but then it would be difficult to test the
> existing patches on the mlx4 until the net tree containing those
> patches was merged back in.
>
>> Still (me and I think also Tariq from our driver team) catching up on
>> the series, the primitives and conventions you are introducing using
>> and how this applies on mlx5. I saw that Saeed acked the the mlx5e
>> patches (7 and 8).
>
> The concept for all this is pretty simple.  What I am doing is
> restricting TSO so that we have a fixed size that is used for all
> outgoing frames.

excellent... how exactly this is done? I wasn't sure if this is
existing facility in the kernel and which or somehow introduced now
(where)? would appreciate if you can drop a note on that.

> Then we precompute the outer headers and use those
> values when performing GSO.  By doing this we can populate the UDP
> checksum field instead of forcing it to 0 which allows us to perform
> TSO for tunnels with outer checksums.

now.. understood.

>> Specifically, the mlx4 patches are practically fixes so if they don't
>> land in 4.7 via net-next we can get them there through net, lets give
>> us the few more days needed to catch up from our side.

> Actually the mlx4 specific portion of these patches are not really
> fixes, they are enabling features.  Specifically IPv6 checksum, TSOv6,
> and TSO for VXLAN tunnels with outer checksums.  It is patches 1 and 4
> that contain the fix and it likely applies to more than just the mlx4
> driver as I believe there is a qlogic driver with a similar feature
> flag layout.  If we want we could recommend those patches for stable

All makes sense, I will be able to ack the mlx4 patches tomorrow or
the latest Wed so we are just fine for 4.7 - I don't see urgency to
put them on 4.6

Or.