Patchwork [net-next,v2,1/2] ip_gre: allow CSUM capable devices to handle packets

login
register
mail settings
Submitter Dmitry Kravkov
Date Feb. 18, 2013, 7:50 p.m.
Message ID <1361217053-16984-1-git-send-email-dmitry@broadcom.com>
Download mbox | patch
Permalink /patch/221462/
State Accepted
Delegated to: David Miller
Headers show

Comments

Dmitry Kravkov - Feb. 18, 2013, 7:50 p.m.
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(-)
David Miller - Feb. 19, 2013, 5:53 a.m.
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.
--
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
pravin - Feb. 19, 2013, 6:28 p.m.
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
--
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
Dmitry Kravkov - Feb. 19, 2013, 7:20 p.m.
> -----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


--
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
pravin - Feb. 19, 2013, 11:03 p.m.
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.
--
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
Dmitry Kravkov - Feb. 20, 2013, 2:45 p.m.
> -----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.


--
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

Patch

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;