Re: [PATCH net-next] ipv6: gro: support sit protocol

[Eric Dumazet]

On Wed, 2015-11-04 at 13:19 +0100, Wolfgang Walter wrote:

> Today I found a problem: on a router forwarding GRE-packets (ipv4) (it is not 
> the endpount) the interface (intel igb) stops sending packets after some 
> time. 
> I think this happens when an ISATAP packet is inside the GRE-packet.
> 
>   gre packets arrives on eth0
>   eth1 stops sending (receiving still works)
>   ethtool -r eth1
>   eth1 works again for some time
> 
> Switching GRO off on eth0 "fixes" the problem.
> 
> I didn't test vanilla 4.1.12 yet, though. Until today 4.1.11 has been running 
> on the router. What I tested was your patch
>   "gre_gso_segment() chokes if SIT frames were aggregated by GRO engine."
> but did not solve the problem.
> 
> So I would not recommend to backport it to longterm 4.1.
> 
> My plans are:
> 
> * test vanilla 4.1.12
> * test 4.3
> 
> I want to test 4.3 on another router first, though.

If the NIC stops sending packets after some time, it might be an igb
issue.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Eric Dumazet]

On Wed, 2015-11-04 at 15:09 +0100, Wolfgang Walter wrote:

> 
> Yes, maybe igb has a problem sending a gro-packet if it is an isatap in gre.

We might detect this condition properly from igb ndo_features_check
method.

It currently uses plain passthru_features_check()



> 
> igb has no problem sending gro-packets which are pure isatap or which are 
> ipv4 
> (tcp/udp) in gre with 4.1.12 + these patches.
> 
> And it had no problem with 4.1.11 with isatap in gre.
> 
> Disabling gso for the interface does help.

My patch was aimed for 4.4, not sure about backports to old kernels...


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Wolfgang Walter]

Am Mittwoch, 4. November 2015, 04:40:51 schrieb Eric Dumazet:
> On Wed, 2015-11-04 at 13:19 +0100, Wolfgang Walter wrote:
> > Today I found a problem: on a router forwarding GRE-packets (ipv4) (it is
> > not the endpount) the interface (intel igb) stops sending packets after
> > some time. I think this happens when an ISATAP packet is inside the
> > GRE-packet.> 
> > gre packets arrives on eth0
> > eth1 stops sending (receiving still works)
> > ethtool -r eth1
> > eth1 works again for some time
> > 
> > Switching GRO off on eth0 "fixes" the problem.
> > 
> > I didn't test vanilla 4.1.12 yet, though. Until today 4.1.11 has been
> > running on the router. What I tested was your patch
> > 
> > "gre_gso_segment() chokes if SIT frames were aggregated by GRO 
engine."
> > 
> > but did not solve the problem.
> > 
> > So I would not recommend to backport it to longterm 4.1.
> > 
> > My plans are:
> > 
> > * test vanilla 4.1.12
> > * test 4.3
> > 
> > I want to test 4.3 on another router first, though.
> 
> If the NIC stops sending packets after some time, it might be an igb
> issue.

Yes, maybe igb has a problem sending a gro-packet if it is an isatap in gre.

igb has no problem sending gro-packets which are pure isatap or which are ipv4 
(tcp/udp) in gre with 4.1.12 + these patches.

And it had no problem with 4.1.11 with isatap in gre.

Disabling gso for the interface does help.

I'll test pure 4.1.12 soon.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Wolfgang Walter]

Am Dienstag, 3. November 2015, 05:07:33 schrieb Eric Dumazet:
> On Tue, 2015-11-03 at 13:57 +0100, Wolfgang Walter wrote:
> > Am Montag, 19. Oktober 2015, 20:40:17 schrieb Eric Dumazet:
> > > From: Eric Dumazet 
> > > 
> > > Tom Herbert added SIT support to GRO with commit
> > > 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
> > > later reverted by Herbert Xu.
> > > 
> > > The problem came because Tom patch was building GRO
> > > packets without proper meta data : If packets were locally
> > > delivered, we would not care.
> > > 
> > > But if packets needed to be forwarded, GSO engine was not
> > > able to segment individual segments.
> > > 
> > > With the following patch, we correctly set skb->encapsulation
> > > and inner network header. We also update gso_type.
> > 
> > I'm running 4.1.11 / 4.1.12 with this patch on top now since over a week.
> > ISATAP works fine.
> 
> Perfect ! thanks a lot for testing !

Today I found a problem: on a router forwarding GRE-packets (ipv4) (it is not 
the endpount) the interface (intel igb) stops sending packets after some time. 
I think this happens when an ISATAP packet is inside the GRE-packet.

gre packets arrives on eth0
eth1 stops sending (receiving still works)
ethtool -r eth1
eth1 works again for some time

Switching GRO off on eth0 "fixes" the problem.

I didn't test vanilla 4.1.12 yet, though. Until today 4.1.11 has been running 
on the router. What I tested was your patch
"gre_gso_segment() chokes if SIT frames were aggregated by GRO engine."
but did not solve the problem.

So I would not recommend to backport it to longterm 4.1.

My plans are:

* test vanilla 4.1.12
* test 4.3

I want to test 4.3 on another router first, though.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Wolfgang Walter]

Am Mittwoch, 4. November 2015, 07:13:07 schrieb Eric Dumazet:
> On Wed, 2015-11-04 at 15:09 +0100, Wolfgang Walter wrote:
> > Yes, maybe igb has a problem sending a gro-packet if it is an isatap in
> > gre.
> We might detect this condition properly from igb ndo_features_check
> method.
> 
> It currently uses plain passthru_features_check()
> 
> > igb has no problem sending gro-packets which are pure isatap or which are
> > ipv4 (tcp/udp) in gre with 4.1.12 + these patches.
> > 
> > And it had no problem with 4.1.11 with isatap in gre.
> > 
> > Disabling gso for the interface does help.
> 
> My patch was aimed for 4.4, not sure about backports to old kernels...

I know. I cannot test 4.4 (or net-next) on that router, though, as I don't 
have easy physical access to it if it crashes or I loose network connectivity. 
For such tests I must send someone in situ.

As 4.4 will be the next longterm kernel I definitivly will do that for 4.4-rc2 
or 4.4-rc3.

I think your patch is correct for 4.1 in the sense that ISATAP is correctly 
handled. Only SIT in GRE triggers this and if it is indeed igb I will see it 
probably in 4.4 ;-), too.

I now tested an unmodified 4.1.12 and it shows no problems.


Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Wolfgang Walter]

Am Montag, 19. Oktober 2015, 20:40:17 schrieb Eric Dumazet:
> From: Eric Dumazet 
> 
> Tom Herbert added SIT support to GRO with commit
> 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
> later reverted by Herbert Xu.
> 
> The problem came because Tom patch was building GRO
> packets without proper meta data : If packets were locally
> delivered, we would not care.
> 
> But if packets needed to be forwarded, GSO engine was not
> able to segment individual segments.
> 
> With the following patch, we correctly set skb->encapsulation
> and inner network header. We also update gso_type.
> 

I'm running 4.1.11 / 4.1.12 with this patch on top now since over a week. 
ISATAP works fine.
 

> Tested:
> 
> Server :
> netserver
> modprobe dummy
> ifconfig dummy0 8.0.0.1 netmask 255.255.255.0 up
> arp -s 8.0.0.100 4e:32:51:04:47:e5
> iptables -I INPUT -s 10.246.7.151 -j TEE --gateway 8.0.0.100
> ifconfig sixtofour0
> sixtofour0 Link encap:IPv6-in-IPv4
>   inet6 addr: 2002:af6:798::1/128 Scope:Global
>   inet6 addr: 2002:af6:798::/128 Scope:Global
>   UP RUNNING NOARP  MTU:1480  Metric:1
>   RX packets:411169 errors:0 dropped:0 overruns:0 frame:0
>   TX packets:409414 errors:0 dropped:0 overruns:0 carrier:0
>   collisions:0 txqueuelen:0
>   RX bytes:20319631739 (20.3 GB)  TX bytes:29529556 (29.5 MB)
> 
> Client :
> netperf -H 2002:af6:798::1 -l 1000 &
> 
> Checked on server traffic copied on dummy0 and verify segments were
> properly rebuilt, with proper IP headers, TCP checksums...
> 
> tcpdump on eth0 shows proper GRO aggregation takes place.
> 
> Signed-off-by: Eric Dumazet 
> ---
>  net/ipv6/ip6_offload.c |   12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 08b62047c67f..eeca943f12dc 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -264,6 +264,9 @@ static int ipv6_gro_complete(struct sk_buff *skb, int
> nhoff) struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
>   int err = -ENOSYS;
> 
> + if (skb->encapsulation)
> + skb_set_inner_network_header(skb, nhoff);
> +
>   iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
> 
>   rcu_read_lock();
> @@ -280,6 +283,13 @@ out_unlock:
>   return err;
>  }
> 
> +static int sit_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> + skb->encapsulation = 1;
> + skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
> + return ipv6_gro_complete(skb, nhoff);
> +}
> +
>  static struct packet_offload ipv6_packet_offload __read_mostly = {
>   .type = cpu_to_be16(ETH_P_IPV6),
>   .callbacks = {
> @@ -292,6 +302,8 @@ static struct packet_offload ipv6_packet_offload
> __read_mostly = { static const struct net_offload sit_offload = {
>   .callbacks = {
>   .gso_segment= ipv6_gso_segment,
> + .gro_receive= ipv6_gro_receive,
> + .gro_complete   = sit_gro_complete,
>   },
>  };

Thanks,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Eric Dumazet]

On Tue, 2015-11-03 at 13:57 +0100, Wolfgang Walter wrote:
> Am Montag, 19. Oktober 2015, 20:40:17 schrieb Eric Dumazet:
> > From: Eric Dumazet 
> > 
> > Tom Herbert added SIT support to GRO with commit
> > 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
> > later reverted by Herbert Xu.
> > 
> > The problem came because Tom patch was building GRO
> > packets without proper meta data : If packets were locally
> > delivered, we would not care.
> > 
> > But if packets needed to be forwarded, GSO engine was not
> > able to segment individual segments.
> > 
> > With the following patch, we correctly set skb->encapsulation
> > and inner network header. We also update gso_type.
> > 
> 
> I'm running 4.1.11 / 4.1.12 with this patch on top now since over a week. 
> ISATAP works fine.

Perfect ! thanks a lot for testing !


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Tom Herbert]

On Tue, Nov 3, 2015 at 10:07 PM, Eric Dumazet  wrote:
> On Tue, 2015-11-03 at 13:57 +0100, Wolfgang Walter wrote:
>> Am Montag, 19. Oktober 2015, 20:40:17 schrieb Eric Dumazet:
>> > From: Eric Dumazet 
>> >
>> > Tom Herbert added SIT support to GRO with commit
>> > 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
>> > later reverted by Herbert Xu.
>> >
>> > The problem came because Tom patch was building GRO
>> > packets without proper meta data : If packets were locally
>> > delivered, we would not care.
>> >
>> > But if packets needed to be forwarded, GSO engine was not
>> > able to segment individual segments.
>> >
>> > With the following patch, we correctly set skb->encapsulation
>> > and inner network header. We also update gso_type.
>> >
>>
>> I'm running 4.1.11 / 4.1.12 with this patch on top now since over a week.
>> ISATAP works fine.
>
> Perfect ! thanks a lot for testing !
>

Thanks for all the work on this!

Tom

>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[David Miller]

From: Eric Dumazet 
Date: Mon, 19 Oct 2015 20:40:17 -0700

> From: Eric Dumazet 
> 
> Tom Herbert added SIT support to GRO with commit
> 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
> later reverted by Herbert Xu.
> 
> The problem came because Tom patch was building GRO
> packets without proper meta data : If packets were locally
> delivered, we would not care.
> 
> But if packets needed to be forwarded, GSO engine was not
> able to segment individual segments.
> 
> With the following patch, we correctly set skb->encapsulation
> and inner network header. We also update gso_type.
> 
> Tested:
 ...
> Checked on server traffic copied on dummy0 and verify segments were
> properly rebuilt, with proper IP headers, TCP checksums...
> 
> tcpdump on eth0 shows proper GRO aggregation takes place.
> 
> Signed-off-by: Eric Dumazet 

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next] ipv6: gro: support sit protocol

[Eric Dumazet]

From: Eric Dumazet 

Tom Herbert added SIT support to GRO with commit
19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
later reverted by Herbert Xu.

The problem came because Tom patch was building GRO
packets without proper meta data : If packets were locally
delivered, we would not care.

But if packets needed to be forwarded, GSO engine was not
able to segment individual segments.

With the following patch, we correctly set skb->encapsulation
and inner network header. We also update gso_type.

Tested:

Server :
netserver
modprobe dummy
ifconfig dummy0 8.0.0.1 netmask 255.255.255.0 up
arp -s 8.0.0.100 4e:32:51:04:47:e5
iptables -I INPUT -s 10.246.7.151 -j TEE --gateway 8.0.0.100
ifconfig sixtofour0
sixtofour0 Link encap:IPv6-in-IPv4  
  inet6 addr: 2002:af6:798::1/128 Scope:Global
  inet6 addr: 2002:af6:798::/128 Scope:Global
  UP RUNNING NOARP  MTU:1480  Metric:1
  RX packets:411169 errors:0 dropped:0 overruns:0 frame:0
  TX packets:409414 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:0 
  RX bytes:20319631739 (20.3 GB)  TX bytes:29529556 (29.5 MB)

Client :
netperf -H 2002:af6:798::1 -l 1000 &

Checked on server traffic copied on dummy0 and verify segments were
properly rebuilt, with proper IP headers, TCP checksums...

tcpdump on eth0 shows proper GRO aggregation takes place.

Signed-off-by: Eric Dumazet 
---
 net/ipv6/ip6_offload.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 08b62047c67f..eeca943f12dc 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -264,6 +264,9 @@ static int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
int err = -ENOSYS;
 
+   if (skb->encapsulation)
+   skb_set_inner_network_header(skb, nhoff);
+
iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
 
rcu_read_lock();
@@ -280,6 +283,13 @@ out_unlock:
return err;
 }
 
+static int sit_gro_complete(struct sk_buff *skb, int nhoff)
+{
+   skb->encapsulation = 1;
+   skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
+   return ipv6_gro_complete(skb, nhoff);
+}
+
 static struct packet_offload ipv6_packet_offload __read_mostly = {
.type = cpu_to_be16(ETH_P_IPV6),
.callbacks = {
@@ -292,6 +302,8 @@ static struct packet_offload ipv6_packet_offload 
__read_mostly = {
 static const struct net_offload sit_offload = {
.callbacks = {
.gso_segment= ipv6_gso_segment,
+   .gro_receive= ipv6_gro_receive,
+   .gro_complete   = sit_gro_complete,
},
 };
 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] ipv6: gro: support sit protocol

[Tom Herbert]

On Mon, Oct 19, 2015 at 8:40 PM, Eric Dumazet  wrote:
> From: Eric Dumazet 
>
> Tom Herbert added SIT support to GRO with commit
> 19424e052fb4 ("sit: Add gro callbacks to sit_offload"),
> later reverted by Herbert Xu.
>
> The problem came because Tom patch was building GRO
> packets without proper meta data : If packets were locally
> delivered, we would not care.
>
> But if packets needed to be forwarded, GSO engine was not
> able to segment individual segments.
>
> With the following patch, we correctly set skb->encapsulation
> and inner network header. We also update gso_type.
>
> Tested:
>
> Server :
> netserver
> modprobe dummy
> ifconfig dummy0 8.0.0.1 netmask 255.255.255.0 up
> arp -s 8.0.0.100 4e:32:51:04:47:e5
> iptables -I INPUT -s 10.246.7.151 -j TEE --gateway 8.0.0.100
> ifconfig sixtofour0
> sixtofour0 Link encap:IPv6-in-IPv4
>   inet6 addr: 2002:af6:798::1/128 Scope:Global
>   inet6 addr: 2002:af6:798::/128 Scope:Global
>   UP RUNNING NOARP  MTU:1480  Metric:1
>   RX packets:411169 errors:0 dropped:0 overruns:0 frame:0
>   TX packets:409414 errors:0 dropped:0 overruns:0 carrier:0
>   collisions:0 txqueuelen:0
>   RX bytes:20319631739 (20.3 GB)  TX bytes:29529556 (29.5 MB)
>
> Client :
> netperf -H 2002:af6:798::1 -l 1000 &
>
> Checked on server traffic copied on dummy0 and verify segments were
> properly rebuilt, with proper IP headers, TCP checksums...
>
> tcpdump on eth0 shows proper GRO aggregation takes place.
>
> Signed-off-by: Eric Dumazet 

Acked-by: Tom Herbert 

> ---
>  net/ipv6/ip6_offload.c |   12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 08b62047c67f..eeca943f12dc 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -264,6 +264,9 @@ static int ipv6_gro_complete(struct sk_buff *skb, int 
> nhoff)
> struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
> int err = -ENOSYS;
>
> +   if (skb->encapsulation)
> +   skb_set_inner_network_header(skb, nhoff);
> +
> iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
>
> rcu_read_lock();
> @@ -280,6 +283,13 @@ out_unlock:
> return err;
>  }
>
> +static int sit_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> +   skb->encapsulation = 1;
> +   skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
> +   return ipv6_gro_complete(skb, nhoff);
> +}
> +
>  static struct packet_offload ipv6_packet_offload __read_mostly = {
> .type = cpu_to_be16(ETH_P_IPV6),
> .callbacks = {
> @@ -292,6 +302,8 @@ static struct packet_offload ipv6_packet_offload 
> __read_mostly = {
>  static const struct net_offload sit_offload = {
> .callbacks = {
> .gso_segment= ipv6_gso_segment,
> +   .gro_receive= ipv6_gro_receive,
> +   .gro_complete   = sit_gro_complete,
> },
>  };
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html