Patchwork [net-next,v2,2/2] ip_gre: propogate target device GSO capability to the tunnel device

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

Comments

Dmitry Kravkov - Feb. 18, 2013, 7:50 p.m.
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(-)
David Miller - Feb. 19, 2013, 5:53 a.m.
From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Mon, 18 Feb 2013 21:50:53 +0200

> 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:39 p.m.
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
--
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:31 p.m.
> -----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


--
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, 10:51 p.m.
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
>
>
--
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 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;