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

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.

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

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.

[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;
 	}