* [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 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 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 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@davemloft.net, netdev@vger.kernel.org > -----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@davemloft.net, netdev@vger.kernel.org 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 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 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 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 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-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 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 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@davemloft.net, netdev@vger.kernel.org > -----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 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@davemloft.net, netdev@vger.kernel.org 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@davemloft.net, netdev@vger.kernel.org > -----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@davemloft.net, netdev@vger.kernel.org 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 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@davemloft.net, netdev@vger.kernel.org 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 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@davemloft.net, netdev@vger.kernel.org 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 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@davemloft.net, netdev@vger.kernel.org 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 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@davemloft.net, netdev@vger.kernel.org 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).