netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
@ 2013-02-18 19:50 Dmitry Kravkov
  2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-18 19:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: Dmitry Kravkov

If device is not able to handle checksumming it will
be handled in dev_xmit

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
---
Changes from v1: fixed email address

 net/ipv4/ip_gre.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index a56f118..cdc31ac 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
 			goto error;
 		skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
 		return skb;
-	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		err = skb_checksum_help(skb);
-		if (unlikely(err))
-			goto error;
 	}
-	skb->ip_summed = CHECKSUM_NONE;
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		skb->ip_summed = CHECKSUM_NONE;
 
 	return skb;
 
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-18 19:50 [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets Dmitry Kravkov
@ 2013-02-18 19:50 ` Dmitry Kravkov
  2013-02-19  5:53   ` David Miller
  2013-02-19 18:39   ` pravin
  2013-02-19  5:53 ` [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets David Miller
  2013-02-19 18:28 ` pravin
  2 siblings, 2 replies; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-18 19:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: Dmitry Kravkov


Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
---
Changes from v1: fixed email address


 net/ipv4/ip_gre.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index cdc31ac..31bc941 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
 	tunnel->hlen = addend;
 	/* TCP offload with GRE SEQ is not supported. */
 	if (!(tunnel->parms.o_flags & GRE_SEQ)) {
-		dev->features		|= NETIF_F_GSO_SOFTWARE;
-		dev->hw_features	|= NETIF_F_GSO_SOFTWARE;
+		/* device supports enc gso offload*/
+		if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
+			dev->features		|= NETIF_F_TSO;
+			dev->hw_features	|= NETIF_F_TSO;
+		} else {
+			dev->features		|= NETIF_F_GSO_SOFTWARE;
+			dev->hw_features	|= NETIF_F_GSO_SOFTWARE;
+		}
 	}
 
 	return mtu;
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-18 19:50 [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets Dmitry Kravkov
  2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
@ 2013-02-19  5:53 ` David Miller
  2013-02-19 18:28 ` pravin
  2 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2013-02-19  5:53 UTC (permalink / raw)
  To: dmitry; +Cc: netdev

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Mon, 18 Feb 2013 21:50:52 +0200

> If device is not able to handle checksumming it will
> be handled in dev_xmit
> 
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>

Applied.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
@ 2013-02-19  5:53   ` David Miller
  2013-02-19 18:39   ` pravin
  1 sibling, 0 replies; 20+ messages in thread
From: David Miller @ 2013-02-19  5:53 UTC (permalink / raw)
  To: dmitry; +Cc: netdev

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Mon, 18 Feb 2013 21:50:53 +0200

> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>

Applied.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-18 19:50 [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets Dmitry Kravkov
  2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
  2013-02-19  5:53 ` [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets David Miller
@ 2013-02-19 18:28 ` pravin
  2013-02-19 19:20   ` Dmitry Kravkov
  2 siblings, 1 reply; 20+ messages in thread
From: pravin @ 2013-02-19 18:28 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> If device is not able to handle checksumming it will
> be handled in dev_xmit
>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> ---
> Changes from v1: fixed email address
>
>  net/ipv4/ip_gre.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index a56f118..cdc31ac 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
>                         goto error;
>                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>                 return skb;
> -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -               err = skb_checksum_help(skb);
> -               if (unlikely(err))
> -                       goto error;
>         }
> -       skb->ip_summed = CHECKSUM_NONE;
> +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> +               skb->ip_summed = CHECKSUM_NONE;
>
>         return skb;
>
> --
> 1.7.7.2
>
>

This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
complete IP packet to checksum entire GRE payload.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
  2013-02-19  5:53   ` David Miller
@ 2013-02-19 18:39   ` pravin
  2013-02-19 19:31     ` Dmitry Kravkov
  2013-02-21 22:30     ` Dmitry Kravkov
  1 sibling, 2 replies; 20+ messages in thread
From: pravin @ 2013-02-19 18:39 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> ---
> Changes from v1: fixed email address
>
>
>  net/ipv4/ip_gre.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index cdc31ac..31bc941 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
>         tunnel->hlen = addend;
>         /* TCP offload with GRE SEQ is not supported. */
>         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> -               dev->features           |= NETIF_F_GSO_SOFTWARE;
> -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> +               /* device supports enc gso offload*/
> +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> +                       dev->features           |= NETIF_F_TSO;
> +                       dev->hw_features        |= NETIF_F_TSO;
> +               } else {
> +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> +               }
>         }

I am not sure about this change, Are you trying to limit GRE TSO to
just IPV4  in case of GRE offload in hardware?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-19 18:28 ` pravin
@ 2013-02-19 19:20   ` Dmitry Kravkov
  2013-02-19 23:03     ` pravin
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-19 19:20 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev


> -----Original Message-----
> From: pravin [mailto:pravin.shelar@gmail.com]
> Sent: Tuesday, February 19, 2013 8:28 PM
> To: Dmitry Kravkov
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> handle packets
> 
> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> wrote:
> > If device is not able to handle checksumming it will
> > be handled in dev_xmit
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > ---
> > Changes from v1: fixed email address
> >
> >  net/ipv4/ip_gre.c |    7 ++-----
> >  1 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index a56f118..cdc31ac 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
> *skb)
> >                         goto error;
> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> >                 return skb;
> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > -               err = skb_checksum_help(skb);
> > -               if (unlikely(err))
> > -                       goto error;
> >         }
> > -       skb->ip_summed = CHECKSUM_NONE;
> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> > +               skb->ip_summed = CHECKSUM_NONE;
> >
> >         return skb;
> >
> > --
> > 1.7.7.2
> >
> >
> 
> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
> complete IP packet to checksum entire GRE payload.

Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
This is the only problematic case, right?
 
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-19 18:39   ` pravin
@ 2013-02-19 19:31     ` Dmitry Kravkov
  2013-02-19 22:51       ` pravin
  2013-02-21 22:30     ` Dmitry Kravkov
  1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-19 19:31 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

> -----Original Message-----
> From: pravin [mailto:pravin.shelar@gmail.com]
> Sent: Tuesday, February 19, 2013 8:40 PM
> To: Dmitry Kravkov
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO
> capability to the tunnel device
> 
> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> wrote:
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > ---
> > Changes from v1: fixed email address
> >
> >
> >  net/ipv4/ip_gre.c |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index cdc31ac..31bc941 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device
> *dev)
> >         tunnel->hlen = addend;
> >         /* TCP offload with GRE SEQ is not supported. */
> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> > +               /* device supports enc gso offload*/
> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> > +                       dev->features           |= NETIF_F_TSO;
> > +                       dev->hw_features        |= NETIF_F_TSO;
> > +               } else {
> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> > +               }
> >         }
> 
> I am not sure about this change, Are you trying to limit GRE TSO to
> just IPV4  in case of GRE offload in hardware?
> 
You're right,
It probably should be fixed to:
	if (tdev && tdev->hw_enc_features & NETIF_F_GRE_GSO) {
		dev->features           |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;
		dev->hw_features        |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-19 19:31     ` Dmitry Kravkov
@ 2013-02-19 22:51       ` pravin
  0 siblings, 0 replies; 20+ messages in thread
From: pravin @ 2013-02-19 22:51 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Tue, Feb 19, 2013 at 11:31 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> -----Original Message-----
>> From: pravin [mailto:pravin.shelar@gmail.com]
>> Sent: Tuesday, February 19, 2013 8:40 PM
>> To: Dmitry Kravkov
>> Cc: davem@davemloft.net; netdev@vger.kernel.org
>> Subject: Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO
>> capability to the tunnel device
>>
>> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>> wrote:
>> >
>> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> > ---
>> > Changes from v1: fixed email address
>> >
>> >
>> >  net/ipv4/ip_gre.c |   10 ++++++++--
>> >  1 files changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> > index cdc31ac..31bc941 100644
>> > --- a/net/ipv4/ip_gre.c
>> > +++ b/net/ipv4/ip_gre.c
>> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device
>> *dev)
>> >         tunnel->hlen = addend;
>> >         /* TCP offload with GRE SEQ is not supported. */
>> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> > +               /* device supports enc gso offload*/
>> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> > +                       dev->features           |= NETIF_F_TSO;
>> > +                       dev->hw_features        |= NETIF_F_TSO;
>> > +               } else {
>> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> > +               }
>> >         }
>>
>> I am not sure about this change, Are you trying to limit GRE TSO to
>> just IPV4  in case of GRE offload in hardware?
>>
> You're right,
> It probably should be fixed to:
>         if (tdev && tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>                 dev->features           |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;
>                 dev->hw_features        |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;
>
>> >
>> >         return mtu;
>> > --
>> > 1.7.7.2
>> >

I think if device features are set right, dev_hard_start_xmit() should
do right thing without any of these changes. i.e. it will GSO ipv6
packets if hardware only supports GRE-ipv4 packets.
This patch changes that at encapsulated dev layer and disables rest of
GSO if one type of TSO available which is not optimal.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-19 19:20   ` Dmitry Kravkov
@ 2013-02-19 23:03     ` pravin
  2013-02-20 14:45       ` Dmitry Kravkov
  2013-02-21 22:26       ` Dmitry Kravkov
  0 siblings, 2 replies; 20+ messages in thread
From: pravin @ 2013-02-19 23:03 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>
>> -----Original Message-----
>> From: pravin [mailto:pravin.shelar@gmail.com]
>> Sent: Tuesday, February 19, 2013 8:28 PM
>> To: Dmitry Kravkov
>> Cc: davem@davemloft.net; netdev@vger.kernel.org
>> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
>> handle packets
>>
>> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>> wrote:
>> > If device is not able to handle checksumming it will
>> > be handled in dev_xmit
>> >
>> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> > ---
>> > Changes from v1: fixed email address
>> >
>> >  net/ipv4/ip_gre.c |    7 ++-----
>> >  1 files changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> > index a56f118..cdc31ac 100644
>> > --- a/net/ipv4/ip_gre.c
>> > +++ b/net/ipv4/ip_gre.c
>> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
>> *skb)
>> >                         goto error;
>> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >                 return skb;
>> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> > -               err = skb_checksum_help(skb);
>> > -               if (unlikely(err))
>> > -                       goto error;
>> >         }
>> > -       skb->ip_summed = CHECKSUM_NONE;
>> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
>> > +               skb->ip_summed = CHECKSUM_NONE;
>> >
>> >         return skb;
>> >
>> > --
>> > 1.7.7.2
>> >
>> >
>>
>> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
>> complete IP packet to checksum entire GRE payload.
>
> Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
> This is the only problematic case, right?
>

It does not work for me. I have GRE device with csum on. Ping works
fine but Netperf is not working.
Looking at code, I am not sure how tcp will work if inner packet TCP
checksum is calculated after GRE_CSUM calculation.

Thanks,
Pravin.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-19 23:03     ` pravin
@ 2013-02-20 14:45       ` Dmitry Kravkov
  2013-02-21 22:26       ` Dmitry Kravkov
  1 sibling, 0 replies; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-20 14:45 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

> -----Original Message-----
> From: pravin [mailto:pravin.shelar@gmail.com]
> Sent: Wednesday, February 20, 2013 1:04 AM
> To: Dmitry Kravkov
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> handle packets
> 
> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com>
> wrote:
> >
> >> -----Original Message-----
> >> From: pravin [mailto:pravin.shelar@gmail.com]
> >> Sent: Tuesday, February 19, 2013 8:28 PM
> >> To: Dmitry Kravkov
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> >> handle packets
> >>
> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> >> wrote:
> >> > If device is not able to handle checksumming it will
> >> > be handled in dev_xmit
> >> >
> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> >> > ---
> >> > Changes from v1: fixed email address
> >> >
> >> >  net/ipv4/ip_gre.c |    7 ++-----
> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> >> > index a56f118..cdc31ac 100644
> >> > --- a/net/ipv4/ip_gre.c
> >> > +++ b/net/ipv4/ip_gre.c
> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
> >> *skb)
> >> >                         goto error;
> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> >> >                 return skb;
> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >> > -               err = skb_checksum_help(skb);
> >> > -               if (unlikely(err))
> >> > -                       goto error;
> >> >         }
> >> > -       skb->ip_summed = CHECKSUM_NONE;
> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> >> > +               skb->ip_summed = CHECKSUM_NONE;
> >> >
> >> >         return skb;
> >> >
> >> > --
> >> > 1.7.7.2
> >> >
> >> >
> >>
> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
> >> complete IP packet to checksum entire GRE payload.
> >
> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be
> used in ipgre_tunnel_xmit() later on
> > This is the only problematic case, right?
> >
> 
> It does not work for me. I have GRE device with csum on. Ping works
> fine but Netperf is not working.

I will test this mode also  ...

> Looking at code, I am not sure how tcp will work if inner packet TCP
> checksum is calculated after GRE_CSUM calculation.

Not sure if there is HW that supports it today

> Thanks,
> Pravin.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-19 23:03     ` pravin
  2013-02-20 14:45       ` Dmitry Kravkov
@ 2013-02-21 22:26       ` Dmitry Kravkov
  2013-02-21 23:46         ` pravin
  1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-21 22:26 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> >
> >> -----Original Message-----
> >> From: pravin [mailto:pravin.shelar@gmail.com]
> >> Sent: Tuesday, February 19, 2013 8:28 PM
> >> To: Dmitry Kravkov
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> >> handle packets
> >>
> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> >> wrote:
> >> > If device is not able to handle checksumming it will
> >> > be handled in dev_xmit
> >> >
> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> >> > ---
> >> > Changes from v1: fixed email address
> >> >
> >> >  net/ipv4/ip_gre.c |    7 ++-----
> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> >> > index a56f118..cdc31ac 100644
> >> > --- a/net/ipv4/ip_gre.c
> >> > +++ b/net/ipv4/ip_gre.c
> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
> >> *skb)
> >> >                         goto error;
> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> >> >                 return skb;
> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >> > -               err = skb_checksum_help(skb);
> >> > -               if (unlikely(err))
> >> > -                       goto error;
> >> >         }
> >> > -       skb->ip_summed = CHECKSUM_NONE;
> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> >> > +               skb->ip_summed = CHECKSUM_NONE;
> >> >
> >> >         return skb;
> >> >
> >> > --
> >> > 1.7.7.2
> >> >
> >> >
> >>
> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
> >> complete IP packet to checksum entire GRE payload.
> >
> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
> > This is the only problematic case, right?
> >
> 
> It does not work for me. I have GRE device with csum on. Ping works
> fine but Netperf is not working.
> Looking at code, I am not sure how tcp will work if inner packet TCP
> checksum is calculated after GRE_CSUM calculation.


Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
---
Tested with each mode (separately)

 net/ipv4/ip_gre.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 5ef4da7..0de54cd 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -735,7 +735,8 @@ drop:
 	return 0;
 }
 
-static struct sk_buff *handle_offloads(struct sk_buff *skb)
+static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
+				       struct sk_buff *skb)
 {
 	int err;
 
@@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
sk_buff *skb)
 			goto error;
 		skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
 		return skb;
+	} else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
+		   skb->ip_summed == CHECKSUM_PARTIAL) {
+		err = skb_checksum_help(skb);
+		if (unlikely(err))
+			goto error;
 	}
-	if (skb->ip_summed != CHECKSUM_PARTIAL)
+
+	if (skb->ip_summed != CHECKSUM_PARTIAL ||
+	    tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM))
 		skb->ip_summed = CHECKSUM_NONE;
 
 	return skb;
@@ -776,7 +784,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff
*skb, struct net_device *dev
 	int    err;
 	int    pkt_len;
 
-	skb = handle_offloads(skb);
+	skb = handle_offloads(tunnel, skb);
 	if (IS_ERR(skb)) {
 		dev->stats.tx_dropped++;
 		return NETDEV_TX_OK;
-- 
1.7.7.2


> Thanks,
> Pravin.
> 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-19 18:39   ` pravin
  2013-02-19 19:31     ` Dmitry Kravkov
@ 2013-02-21 22:30     ` Dmitry Kravkov
  2013-02-21 23:53       ` pravin
  1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-21 22:30 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

On Tue, 2013-02-19 at 10:39 -0800, pravin wrote:
> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > ---
> > Changes from v1: fixed email address
> >
> >
> >  net/ipv4/ip_gre.c |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index cdc31ac..31bc941 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
> >         tunnel->hlen = addend;
> >         /* TCP offload with GRE SEQ is not supported. */
> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> > +               /* device supports enc gso offload*/
> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> > +                       dev->features           |= NETIF_F_TSO;
> > +                       dev->hw_features        |= NETIF_F_TSO;
> > +               } else {
> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> > +               }
> >         }
> 
> I am not sure about this change, Are you trying to limit GRE TSO to
> just IPV4  in case of GRE offload in hardware?


This mostly reverts previous patch, but also disables GSO when CSUM is
set.

---
 net/ipv4/ip_gre.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 0de54cd..aae2d4b 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1107,16 +1107,10 @@ static int ipgre_tunnel_bind_dev(struct
net_device *dev)
 		mtu = 68;
 
 	tunnel->hlen = addend;
-	/* TCP offload with GRE SEQ is not supported. */
-	if (!(tunnel->parms.o_flags & GRE_SEQ)) {
-		/* device supports enc gso offload*/
-		if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
-			dev->features		|= NETIF_F_TSO;
-			dev->hw_features	|= NETIF_F_TSO;
-		} else {
-			dev->features		|= NETIF_F_GSO_SOFTWARE;
-			dev->hw_features	|= NETIF_F_GSO_SOFTWARE;
-		}
+	/* TCP offload with GRE SEQ or CSUM is not supported. */
+	if (!(tunnel->parms.o_flags & (GRE_SEQ|GRE_CSUM))) {
+		dev->features		|= NETIF_F_GSO_SOFTWARE;
+		dev->hw_features	|= NETIF_F_GSO_SOFTWARE;
 	}
 
 	return mtu;
-- 
1.7.7.2


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-21 22:26       ` Dmitry Kravkov
@ 2013-02-21 23:46         ` pravin
  2013-02-22  7:19           ` pravin
  0 siblings, 1 reply; 20+ messages in thread
From: pravin @ 2013-02-21 23:46 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Thu, Feb 21, 2013 at 2:26 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
>> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >
>> >> -----Original Message-----
>> >> From: pravin [mailto:pravin.shelar@gmail.com]
>> >> Sent: Tuesday, February 19, 2013 8:28 PM
>> >> To: Dmitry Kravkov
>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
>> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
>> >> handle packets
>> >>
>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>> >> wrote:
>> >> > If device is not able to handle checksumming it will
>> >> > be handled in dev_xmit
>> >> >
>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> >> > ---
>> >> > Changes from v1: fixed email address
>> >> >
>> >> >  net/ipv4/ip_gre.c |    7 ++-----
>> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
>> >> >
>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >> > index a56f118..cdc31ac 100644
>> >> > --- a/net/ipv4/ip_gre.c
>> >> > +++ b/net/ipv4/ip_gre.c
>> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
>> >> *skb)
>> >> >                         goto error;
>> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >> >                 return skb;
>> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> >> > -               err = skb_checksum_help(skb);
>> >> > -               if (unlikely(err))
>> >> > -                       goto error;
>> >> >         }
>> >> > -       skb->ip_summed = CHECKSUM_NONE;
>> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
>> >> > +               skb->ip_summed = CHECKSUM_NONE;
>> >> >
>> >> >         return skb;
>> >> >
>> >> > --
>> >> > 1.7.7.2
>> >> >
>> >> >
>> >>
>> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
>> >> complete IP packet to checksum entire GRE payload.
>> >
>> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
>> > This is the only problematic case, right?
>> >
>>
>> It does not work for me. I have GRE device with csum on. Ping works
>> fine but Netperf is not working.
>> Looking at code, I am not sure how tcp will work if inner packet TCP
>> checksum is calculated after GRE_CSUM calculation.
>
>
> Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
> ---
> Tested with each mode (separately)
>
>  net/ipv4/ip_gre.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 5ef4da7..0de54cd 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -735,7 +735,8 @@ drop:
>         return 0;
>  }
>
> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
> +                                      struct sk_buff *skb)
>  {
>         int err;
>
> @@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
> sk_buff *skb)
>                         goto error;
>                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>                 return skb;
> +       } else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
> +                  skb->ip_summed == CHECKSUM_PARTIAL) {
> +               err = skb_checksum_help(skb);
> +               if (unlikely(err))
> +                       goto error;
>         }
> -       if (skb->ip_summed != CHECKSUM_PARTIAL)
> +
> +       if (skb->ip_summed != CHECKSUM_PARTIAL ||
> +           tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM))
>                 skb->ip_summed = CHECKSUM_NONE;
>
We can simplify this by dropping check of o_flags.

>         return skb;
> @@ -776,7 +784,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff
> *skb, struct net_device *dev
>         int    err;
>         int    pkt_len;
>
> -       skb = handle_offloads(skb);
> +       skb = handle_offloads(tunnel, skb);
>         if (IS_ERR(skb)) {
>                 dev->stats.tx_dropped++;
>                 return NETDEV_TX_OK;
> --
> 1.7.7.2
>
>
>> Thanks,
>> Pravin.
>>
>
>
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-21 22:30     ` Dmitry Kravkov
@ 2013-02-21 23:53       ` pravin
  2013-02-24 16:45         ` Dmitry Kravkov
  0 siblings, 1 reply; 20+ messages in thread
From: pravin @ 2013-02-21 23:53 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Thu, Feb 21, 2013 at 2:30 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Tue, 2013-02-19 at 10:39 -0800, pravin wrote:
>> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >
>> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> > ---
>> > Changes from v1: fixed email address
>> >
>> >
>> >  net/ipv4/ip_gre.c |   10 ++++++++--
>> >  1 files changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> > index cdc31ac..31bc941 100644
>> > --- a/net/ipv4/ip_gre.c
>> > +++ b/net/ipv4/ip_gre.c
>> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
>> >         tunnel->hlen = addend;
>> >         /* TCP offload with GRE SEQ is not supported. */
>> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> > +               /* device supports enc gso offload*/
>> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> > +                       dev->features           |= NETIF_F_TSO;
>> > +                       dev->hw_features        |= NETIF_F_TSO;
>> > +               } else {
>> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> > +               }
>> >         }
>>
>> I am not sure about this change, Are you trying to limit GRE TSO to
>> just IPV4  in case of GRE offload in hardware?
>
>
> This mostly reverts previous patch, but also disables GSO when CSUM is
> set.
>
> ---
>  net/ipv4/ip_gre.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 0de54cd..aae2d4b 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1107,16 +1107,10 @@ static int ipgre_tunnel_bind_dev(struct
> net_device *dev)
>                 mtu = 68;
>
>         tunnel->hlen = addend;
> -       /* TCP offload with GRE SEQ is not supported. */
> -       if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> -               /* device supports enc gso offload*/
> -               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> -                       dev->features           |= NETIF_F_TSO;
> -                       dev->hw_features        |= NETIF_F_TSO;
> -               } else {
> -                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> -                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> -               }
> +       /* TCP offload with GRE SEQ or CSUM is not supported. */
> +       if (!(tunnel->parms.o_flags & (GRE_SEQ|GRE_CSUM))) {
> +               dev->features           |= NETIF_F_GSO_SOFTWARE;
> +               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>         }
>
>         return mtu;

This narrows down definition of NETIF_F_GRE_GSO. Is it required for
hardware offload?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-21 23:46         ` pravin
@ 2013-02-22  7:19           ` pravin
  2013-02-24 16:56             ` Dmitry Kravkov
  0 siblings, 1 reply; 20+ messages in thread
From: pravin @ 2013-02-22  7:19 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Thu, Feb 21, 2013 at 3:46 PM, pravin <pravin.shelar@gmail.com> wrote:
> On Thu, Feb 21, 2013 at 2:26 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
>>> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>>> >
>>> >> -----Original Message-----
>>> >> From: pravin [mailto:pravin.shelar@gmail.com]
>>> >> Sent: Tuesday, February 19, 2013 8:28 PM
>>> >> To: Dmitry Kravkov
>>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
>>> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
>>> >> handle packets
>>> >>
>>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>>> >> wrote:
>>> >> > If device is not able to handle checksumming it will
>>> >> > be handled in dev_xmit
>>> >> >
>>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>>> >> > ---
>>> >> > Changes from v1: fixed email address
>>> >> >
>>> >> >  net/ipv4/ip_gre.c |    7 ++-----
>>> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
>>> >> >
>>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>>> >> > index a56f118..cdc31ac 100644
>>> >> > --- a/net/ipv4/ip_gre.c
>>> >> > +++ b/net/ipv4/ip_gre.c
>>> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
>>> >> *skb)
>>> >> >                         goto error;
>>> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>>> >> >                 return skb;
>>> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>>> >> > -               err = skb_checksum_help(skb);
>>> >> > -               if (unlikely(err))
>>> >> > -                       goto error;
>>> >> >         }
>>> >> > -       skb->ip_summed = CHECKSUM_NONE;
>>> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
>>> >> > +               skb->ip_summed = CHECKSUM_NONE;
>>> >> >
>>> >> >         return skb;
>>> >> >
>>> >> > --
>>> >> > 1.7.7.2
>>> >> >
>>> >> >
>>> >>
>>> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
>>> >> complete IP packet to checksum entire GRE payload.
>>> >
>>> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
>>> > This is the only problematic case, right?
>>> >
>>>
>>> It does not work for me. I have GRE device with csum on. Ping works
>>> fine but Netperf is not working.
>>> Looking at code, I am not sure how tcp will work if inner packet TCP
>>> checksum is calculated after GRE_CSUM calculation.
>>
>>
>> Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
>> ---
>> Tested with each mode (separately)
>>
>>  net/ipv4/ip_gre.c |   14 +++++++++++---
>>  1 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> index 5ef4da7..0de54cd 100644
>> --- a/net/ipv4/ip_gre.c
>> +++ b/net/ipv4/ip_gre.c
>> @@ -735,7 +735,8 @@ drop:
>>         return 0;
>>  }
>>
>> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
>> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
>> +                                      struct sk_buff *skb)
>>  {
>>         int err;
>>
>> @@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
>> sk_buff *skb)
>>                         goto error;
>>                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>>                 return skb;
>> +       } else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
>> +                  skb->ip_summed == CHECKSUM_PARTIAL) {
>> +               err = skb_checksum_help(skb);
>> +               if (unlikely(err))
>> +                       goto error;
>>         }

one more thing, there is no need to do csum for GRE_SEQ case.

>> -       if (skb->ip_summed != CHECKSUM_PARTIAL)
>> +
>> +       if (skb->ip_summed != CHECKSUM_PARTIAL ||
>> +           tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM))
>>                 skb->ip_summed = CHECKSUM_NONE;
>>
> We can simplify this by dropping check of o_flags.
>
>>         return skb;
>> @@ -776,7 +784,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff
>> *skb, struct net_device *dev
>>         int    err;
>>         int    pkt_len;
>>
>> -       skb = handle_offloads(skb);
>> +       skb = handle_offloads(tunnel, skb);
>>         if (IS_ERR(skb)) {
>>                 dev->stats.tx_dropped++;
>>                 return NETDEV_TX_OK;
>> --
>> 1.7.7.2
>>
>>
>>> Thanks,
>>> Pravin.
>>>
>>
>>
>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-21 23:53       ` pravin
@ 2013-02-24 16:45         ` Dmitry Kravkov
  2013-02-25  6:07           ` Pravin Shelar
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-24 16:45 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

On Thu, 2013-02-21 at 15:53 -0800, pravin wrote:
> On Thu, Feb 21, 2013 at 2:30 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> > On Tue, 2013-02-19 at 10:39 -0800, pravin wrote:
> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> >> >
> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> >> > ---
> >> > Changes from v1: fixed email address
> >> >
> >> >
> >> >  net/ipv4/ip_gre.c |   10 ++++++++--
> >> >  1 files changed, 8 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> >> > index cdc31ac..31bc941 100644
> >> > --- a/net/ipv4/ip_gre.c
> >> > +++ b/net/ipv4/ip_gre.c
> >> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
> >> >         tunnel->hlen = addend;
> >> >         /* TCP offload with GRE SEQ is not supported. */
> >> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> >> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
> >> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> >> > +               /* device supports enc gso offload*/
> >> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> >> > +                       dev->features           |= NETIF_F_TSO;
> >> > +                       dev->hw_features        |= NETIF_F_TSO;
> >> > +               } else {
> >> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> >> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> >> > +               }
> >> >         }
> >>
> >> I am not sure about this change, Are you trying to limit GRE TSO to
> >> just IPV4  in case of GRE offload in hardware?
> >
> >
> > This mostly reverts previous patch, but also disables GSO when CSUM is
> > set.
> >
> > ---
> >  net/ipv4/ip_gre.c |   14 ++++----------
> >  1 files changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index 0de54cd..aae2d4b 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1107,16 +1107,10 @@ static int ipgre_tunnel_bind_dev(struct
> > net_device *dev)
> >                 mtu = 68;
> >
> >         tunnel->hlen = addend;
> > -       /* TCP offload with GRE SEQ is not supported. */
> > -       if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> > -               /* device supports enc gso offload*/
> > -               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> > -                       dev->features           |= NETIF_F_TSO;
> > -                       dev->hw_features        |= NETIF_F_TSO;
> > -               } else {
> > -                       dev->features           |= NETIF_F_GSO_SOFTWARE;
> > -                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> > -               }
> > +       /* TCP offload with GRE SEQ or CSUM is not supported. */
> > +       if (!(tunnel->parms.o_flags & (GRE_SEQ|GRE_CSUM))) {
> > +               dev->features           |= NETIF_F_GSO_SOFTWARE;
> > +               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
> >         }
> >
> >         return mtu;
> 
> This narrows down definition of NETIF_F_GRE_GSO. Is it required for
> hardware offload?

This is required to prevent dev_hard_xmit() to perform soft GSO.
W/o it device receives divided packets.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-22  7:19           ` pravin
@ 2013-02-24 16:56             ` Dmitry Kravkov
  2013-02-25  6:09               ` Pravin Shelar
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Kravkov @ 2013-02-24 16:56 UTC (permalink / raw)
  To: pravin; +Cc: davem, netdev

On Thu, 2013-02-21 at 23:19 -0800, pravin wrote:
> On Thu, Feb 21, 2013 at 3:46 PM, pravin <pravin.shelar@gmail.com> wrote:
> > On Thu, Feb 21, 2013 at 2:26 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> >> On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
> >>> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> >>> >
> >>> >> -----Original Message-----
> >>> >> From: pravin [mailto:pravin.shelar@gmail.com]
> >>> >> Sent: Tuesday, February 19, 2013 8:28 PM
> >>> >> To: Dmitry Kravkov
> >>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
> >>> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> >>> >> handle packets
> >>> >>
> >>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> >>> >> wrote:
> >>> >> > If device is not able to handle checksumming it will
> >>> >> > be handled in dev_xmit
> >>> >> >
> >>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> >>> >> > ---
> >>> >> > Changes from v1: fixed email address
> >>> >> >
> >>> >> >  net/ipv4/ip_gre.c |    7 ++-----
> >>> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
> >>> >> >
> >>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> >>> >> > index a56f118..cdc31ac 100644
> >>> >> > --- a/net/ipv4/ip_gre.c
> >>> >> > +++ b/net/ipv4/ip_gre.c
> >>> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
> >>> >> *skb)
> >>> >> >                         goto error;
> >>> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> >>> >> >                 return skb;
> >>> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >>> >> > -               err = skb_checksum_help(skb);
> >>> >> > -               if (unlikely(err))
> >>> >> > -                       goto error;
> >>> >> >         }
> >>> >> > -       skb->ip_summed = CHECKSUM_NONE;
> >>> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
> >>> >> > +               skb->ip_summed = CHECKSUM_NONE;
> >>> >> >
> >>> >> >         return skb;
> >>> >> >
> >>> >> > --
> >>> >> > 1.7.7.2
> >>> >> >
> >>> >> >
> >>> >>
> >>> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
> >>> >> complete IP packet to checksum entire GRE payload.
> >>> >
> >>> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
> >>> > This is the only problematic case, right?
> >>> >
> >>>
> >>> It does not work for me. I have GRE device with csum on. Ping works
> >>> fine but Netperf is not working.
> >>> Looking at code, I am not sure how tcp will work if inner packet TCP
> >>> checksum is calculated after GRE_CSUM calculation.
> >>
> >>
> >> Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
> >> ---
> >> Tested with each mode (separately)
> >>
> >>  net/ipv4/ip_gre.c |   14 +++++++++++---
> >>  1 files changed, 11 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> >> index 5ef4da7..0de54cd 100644
> >> --- a/net/ipv4/ip_gre.c
> >> +++ b/net/ipv4/ip_gre.c
> >> @@ -735,7 +735,8 @@ drop:
> >>         return 0;
> >>  }
> >>
> >> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
> >> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
> >> +                                      struct sk_buff *skb)
> >>  {
> >>         int err;
> >>
> >> @@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
> >> sk_buff *skb)
> >>                         goto error;
> >>                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> >>                 return skb;
> >> +       } else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
> >> +                  skb->ip_summed == CHECKSUM_PARTIAL) {
> >> +               err = skb_checksum_help(skb);
> >> +               if (unlikely(err))
> >> +                       goto error;
> >>         }
> 
> one more thing, there is no need to do csum for GRE_SEQ case.
If csum is not performed TCP csum become incorrect for offload
capable and incapable devices.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
  2013-02-24 16:45         ` Dmitry Kravkov
@ 2013-02-25  6:07           ` Pravin Shelar
  0 siblings, 0 replies; 20+ messages in thread
From: Pravin Shelar @ 2013-02-25  6:07 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: pravin, davem, netdev

On Sun, Feb 24, 2013 at 8:45 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Thu, 2013-02-21 at 15:53 -0800, pravin wrote:
>> On Thu, Feb 21, 2013 at 2:30 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> > On Tue, 2013-02-19 at 10:39 -0800, pravin wrote:
>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >> >
>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> >> > ---
>> >> > Changes from v1: fixed email address
>> >> >
>> >> >
>> >> >  net/ipv4/ip_gre.c |   10 ++++++++--
>> >> >  1 files changed, 8 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >> > index cdc31ac..31bc941 100644
>> >> > --- a/net/ipv4/ip_gre.c
>> >> > +++ b/net/ipv4/ip_gre.c
>> >> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
>> >> >         tunnel->hlen = addend;
>> >> >         /* TCP offload with GRE SEQ is not supported. */
>> >> >         if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> >> > -               dev->features           |= NETIF_F_GSO_SOFTWARE;
>> >> > -               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> >> > +               /* device supports enc gso offload*/
>> >> > +               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> >> > +                       dev->features           |= NETIF_F_TSO;
>> >> > +                       dev->hw_features        |= NETIF_F_TSO;
>> >> > +               } else {
>> >> > +                       dev->features           |= NETIF_F_GSO_SOFTWARE;
>> >> > +                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> >> > +               }
>> >> >         }
>> >>
>> >> I am not sure about this change, Are you trying to limit GRE TSO to
>> >> just IPV4  in case of GRE offload in hardware?
>> >
>> >
>> > This mostly reverts previous patch, but also disables GSO when CSUM is
>> > set.
>> >
>> > ---
>> >  net/ipv4/ip_gre.c |   14 ++++----------
>> >  1 files changed, 4 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> > index 0de54cd..aae2d4b 100644
>> > --- a/net/ipv4/ip_gre.c
>> > +++ b/net/ipv4/ip_gre.c
>> > @@ -1107,16 +1107,10 @@ static int ipgre_tunnel_bind_dev(struct
>> > net_device *dev)
>> >                 mtu = 68;
>> >
>> >         tunnel->hlen = addend;
>> > -       /* TCP offload with GRE SEQ is not supported. */
>> > -       if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> > -               /* device supports enc gso offload*/
>> > -               if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> > -                       dev->features           |= NETIF_F_TSO;
>> > -                       dev->hw_features        |= NETIF_F_TSO;
>> > -               } else {
>> > -                       dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > -                       dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> > -               }
>> > +       /* TCP offload with GRE SEQ or CSUM is not supported. */
>> > +       if (!(tunnel->parms.o_flags & (GRE_SEQ|GRE_CSUM))) {
>> > +               dev->features           |= NETIF_F_GSO_SOFTWARE;
>> > +               dev->hw_features        |= NETIF_F_GSO_SOFTWARE;
>> >         }
>> >
>> >         return mtu;
>>
>> This narrows down definition of NETIF_F_GRE_GSO. Is it required for
>> hardware offload?
>
> This is required to prevent dev_hard_xmit() to perform soft GSO.
> W/o it device receives divided packets.
>
If NIC features are set correctly dev_hard_xmit() will not perform GSO.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
  2013-02-24 16:56             ` Dmitry Kravkov
@ 2013-02-25  6:09               ` Pravin Shelar
  0 siblings, 0 replies; 20+ messages in thread
From: Pravin Shelar @ 2013-02-25  6:09 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: davem, netdev

On Sun, Feb 24, 2013 at 8:56 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Thu, 2013-02-21 at 23:19 -0800, pravin wrote:
>> On Thu, Feb 21, 2013 at 3:46 PM, pravin <pravin.shelar@gmail.com> wrote:
>> > On Thu, Feb 21, 2013 at 2:26 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >> On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
>> >>> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >>> >
>> >>> >> -----Original Message-----
>> >>> >> From: pravin [mailto:pravin.shelar@gmail.com]
>> >>> >> Sent: Tuesday, February 19, 2013 8:28 PM
>> >>> >> To: Dmitry Kravkov
>> >>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
>> >>> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
>> >>> >> handle packets
>> >>> >>
>> >>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>> >>> >> wrote:
>> >>> >> > If device is not able to handle checksumming it will
>> >>> >> > be handled in dev_xmit
>> >>> >> >
>> >>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> >>> >> > ---
>> >>> >> > Changes from v1: fixed email address
>> >>> >> >
>> >>> >> >  net/ipv4/ip_gre.c |    7 ++-----
>> >>> >> >  1 files changed, 2 insertions(+), 5 deletions(-)
>> >>> >> >
>> >>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >>> >> > index a56f118..cdc31ac 100644
>> >>> >> > --- a/net/ipv4/ip_gre.c
>> >>> >> > +++ b/net/ipv4/ip_gre.c
>> >>> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
>> >>> >> *skb)
>> >>> >> >                         goto error;
>> >>> >> >                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >>> >> >                 return skb;
>> >>> >> > -       } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> >>> >> > -               err = skb_checksum_help(skb);
>> >>> >> > -               if (unlikely(err))
>> >>> >> > -                       goto error;
>> >>> >> >         }
>> >>> >> > -       skb->ip_summed = CHECKSUM_NONE;
>> >>> >> > +       if (skb->ip_summed != CHECKSUM_PARTIAL)
>> >>> >> > +               skb->ip_summed = CHECKSUM_NONE;
>> >>> >> >
>> >>> >> >         return skb;
>> >>> >> >
>> >>> >> > --
>> >>> >> > 1.7.7.2
>> >>> >> >
>> >>> >> >
>> >>> >>
>> >>> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
>> >>> >> complete IP packet to checksum entire GRE payload.
>> >>> >
>> >>> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
>> >>> > This is the only problematic case, right?
>> >>> >
>> >>>
>> >>> It does not work for me. I have GRE device with csum on. Ping works
>> >>> fine but Netperf is not working.
>> >>> Looking at code, I am not sure how tcp will work if inner packet TCP
>> >>> checksum is calculated after GRE_CSUM calculation.
>> >>
>> >>
>> >> Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
>> >> ---
>> >> Tested with each mode (separately)
>> >>
>> >>  net/ipv4/ip_gre.c |   14 +++++++++++---
>> >>  1 files changed, 11 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >> index 5ef4da7..0de54cd 100644
>> >> --- a/net/ipv4/ip_gre.c
>> >> +++ b/net/ipv4/ip_gre.c
>> >> @@ -735,7 +735,8 @@ drop:
>> >>         return 0;
>> >>  }
>> >>
>> >> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
>> >> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
>> >> +                                      struct sk_buff *skb)
>> >>  {
>> >>         int err;
>> >>
>> >> @@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
>> >> sk_buff *skb)
>> >>                         goto error;
>> >>                 skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >>                 return skb;
>> >> +       } else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
>> >> +                  skb->ip_summed == CHECKSUM_PARTIAL) {
>> >> +               err = skb_checksum_help(skb);
>> >> +               if (unlikely(err))
>> >> +                       goto error;
>> >>         }
>>
>> one more thing, there is no need to do csum for GRE_SEQ case.
> If csum is not performed TCP csum become incorrect for offload
> capable and incapable devices.
>
>
that csum computation is only required for GRE_CSUM case, not for GRE_SEQ.
I have send out patches to fix GRE issues, please have a look.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-02-25  6:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-18 19:50 [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets Dmitry Kravkov
2013-02-18 19:50 ` [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device Dmitry Kravkov
2013-02-19  5:53   ` David Miller
2013-02-19 18:39   ` pravin
2013-02-19 19:31     ` Dmitry Kravkov
2013-02-19 22:51       ` pravin
2013-02-21 22:30     ` Dmitry Kravkov
2013-02-21 23:53       ` pravin
2013-02-24 16:45         ` Dmitry Kravkov
2013-02-25  6:07           ` Pravin Shelar
2013-02-19  5:53 ` [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets David Miller
2013-02-19 18:28 ` pravin
2013-02-19 19:20   ` Dmitry Kravkov
2013-02-19 23:03     ` pravin
2013-02-20 14:45       ` Dmitry Kravkov
2013-02-21 22:26       ` Dmitry Kravkov
2013-02-21 23:46         ` pravin
2013-02-22  7:19           ` pravin
2013-02-24 16:56             ` Dmitry Kravkov
2013-02-25  6:09               ` Pravin Shelar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).