| Message ID | 1389799111-8372-1-git-send-email-paul.durrant@citrix.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 15/01/14 15:18, Paul Durrant wrote: > This patch adds support for IPv6 checksum offload and GSO when those > features are available in the backend. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: David Vrabel <david.vrabel@citrix.com> > --- > drivers/net/xen-netfront.c | 48 +++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > index c41537b..321759f 100644 > --- a/drivers/net/xen-netfront.c > +++ b/drivers/net/xen-netfront.c > @@ -617,7 +617,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) > tx->flags |= XEN_NETTXF_extra_info; > > gso->u.gso.size = skb_shinfo(skb)->gso_size; > - gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4; > + gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ? > + XEN_NETIF_GSO_TYPE_TCPV6 : > + XEN_NETIF_GSO_TYPE_TCPV4; > gso->u.gso.pad = 0; > gso->u.gso.features = 0; > > @@ -809,15 +811,18 @@ static int xennet_set_skb_gso(struct sk_buff *skb, > return -EINVAL; > } > > - /* Currently only TCPv4 S.O. is supported. */ > - if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) { > + if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 && > + gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) { > if (net_ratelimit()) > pr_warn("Bad GSO type %d\n", gso->u.gso.type); > return -EINVAL; > } > > skb_shinfo(skb)->gso_size = gso->u.gso.size; > - skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4; > + skb_shinfo(skb)->gso_type = > + (gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ? > + SKB_GSO_TCPV4 : > + SKB_GSO_TCPV6; > > /* Header must be checked, and gso_segs computed. */ > skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY; > @@ -1191,6 +1196,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev, > features &= ~NETIF_F_SG; > } > > + if (features & NETIF_F_IPV6_CSUM) { > + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > + "feature-ipv6-csum-offload", "%d", &val) < 0) > + val = 0; > + > + if (!val) > + features &= ~NETIF_F_IPV6_CSUM; > + } > + > if (features & NETIF_F_TSO) { > if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > "feature-gso-tcpv4", "%d", &val) < 0) > @@ -1200,6 +1214,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev, > features &= ~NETIF_F_TSO; > } > > + if (features & NETIF_F_TSO6) { > + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > + "feature-gso-tcpv6", "%d", &val) < 0) > + val = 0; > + > + if (!val) > + features &= ~NETIF_F_TSO6; > + } > + > return features; > } > > @@ -1338,7 +1361,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev) > netif_napi_add(netdev, &np->napi, xennet_poll, 64); > netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM | > NETIF_F_GSO_ROBUST; > - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO; > + netdev->hw_features = NETIF_F_SG | > + NETIF_F_IPV6_CSUM | > + NETIF_F_TSO | NETIF_F_TSO6; > > /* > * Assume that all hw features are available for now. This set > @@ -1716,6 +1741,19 @@ again: > goto abort_transaction; > } > > + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1); "%d", 1 results in a constant string. xenbus_write() would avoid a transitory memory allocation. ~Andrew > + if (err) { > + message = "writing feature-gso-tcpv6"; > + goto abort_transaction; > + } > + > + err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-offload", > + "%d", 1); > + if (err) { > + message = "writing feature-ipv6-csum-offload"; > + goto abort_transaction; > + } > + > err = xenbus_transaction_end(xbt, 0); > if (err) { > if (err == -EAGAIN) -- 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
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: 15 January 2014 15:25 > To: Paul Durrant > Cc: netdev@vger.kernel.org; xen-devel@lists.xen.org; Boris Ostrovsky; David > Vrabel > Subject: Re: [Xen-devel] [PATCH net-next] xen-netfront: add support for > IPv6 offloads > > On 15/01/14 15:18, Paul Durrant wrote: > > This patch adds support for IPv6 checksum offload and GSO when those > > features are available in the backend. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > Cc: David Vrabel <david.vrabel@citrix.com> > > --- > > drivers/net/xen-netfront.c | 48 > +++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 43 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > > index c41537b..321759f 100644 > > --- a/drivers/net/xen-netfront.c > > +++ b/drivers/net/xen-netfront.c > > @@ -617,7 +617,9 @@ static int xennet_start_xmit(struct sk_buff *skb, > struct net_device *dev) > > tx->flags |= XEN_NETTXF_extra_info; > > > > gso->u.gso.size = skb_shinfo(skb)->gso_size; > > - gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4; > > + gso->u.gso.type = (skb_shinfo(skb)->gso_type & > SKB_GSO_TCPV6) ? > > + XEN_NETIF_GSO_TYPE_TCPV6 : > > + XEN_NETIF_GSO_TYPE_TCPV4; > > gso->u.gso.pad = 0; > > gso->u.gso.features = 0; > > > > @@ -809,15 +811,18 @@ static int xennet_set_skb_gso(struct sk_buff > *skb, > > return -EINVAL; > > } > > > > - /* Currently only TCPv4 S.O. is supported. */ > > - if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) { > > + if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 && > > + gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) { > > if (net_ratelimit()) > > pr_warn("Bad GSO type %d\n", gso->u.gso.type); > > return -EINVAL; > > } > > > > skb_shinfo(skb)->gso_size = gso->u.gso.size; > > - skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4; > > + skb_shinfo(skb)->gso_type = > > + (gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ? > > + SKB_GSO_TCPV4 : > > + SKB_GSO_TCPV6; > > > > /* Header must be checked, and gso_segs computed. */ > > skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY; > > @@ -1191,6 +1196,15 @@ static netdev_features_t > xennet_fix_features(struct net_device *dev, > > features &= ~NETIF_F_SG; > > } > > > > + if (features & NETIF_F_IPV6_CSUM) { > > + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > > + "feature-ipv6-csum-offload", "%d", &val) < > 0) > > + val = 0; > > + > > + if (!val) > > + features &= ~NETIF_F_IPV6_CSUM; > > + } > > + > > if (features & NETIF_F_TSO) { > > if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > > "feature-gso-tcpv4", "%d", &val) < 0) > > @@ -1200,6 +1214,15 @@ static netdev_features_t > xennet_fix_features(struct net_device *dev, > > features &= ~NETIF_F_TSO; > > } > > > > + if (features & NETIF_F_TSO6) { > > + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, > > + "feature-gso-tcpv6", "%d", &val) < 0) > > + val = 0; > > + > > + if (!val) > > + features &= ~NETIF_F_TSO6; > > + } > > + > > return features; > > } > > > > @@ -1338,7 +1361,9 @@ static struct net_device > *xennet_create_dev(struct xenbus_device *dev) > > netif_napi_add(netdev, &np->napi, xennet_poll, 64); > > netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM | > > NETIF_F_GSO_ROBUST; > > - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | > NETIF_F_TSO; > > + netdev->hw_features = NETIF_F_SG | > > + NETIF_F_IPV6_CSUM | > > + NETIF_F_TSO | NETIF_F_TSO6; > > > > /* > > * Assume that all hw features are available for now. This set > > @@ -1716,6 +1741,19 @@ again: > > goto abort_transaction; > > } > > > > + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", > "%d", 1); > > "%d", 1 results in a constant string. xenbus_write() would avoid a > transitory memory allocation. > > ~Andrew > This code is consistent with all the other xenbus_printf()s in the neighbourhood and does it really matter? Paul > > + if (err) { > > + message = "writing feature-gso-tcpv6"; > > + goto abort_transaction; > > + } > > + > > + err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum- > offload", > > + "%d", 1); > > + if (err) { > > + message = "writing feature-ipv6-csum-offload"; > > + goto abort_transaction; > > + } > > + > > err = xenbus_transaction_end(xbt, 0); > > if (err) { > > if (err == -EAGAIN) -- 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
>>> On 15.01.14 at 16:54, Paul Durrant <Paul.Durrant@citrix.com> wrote: >> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] >> On 15/01/14 15:18, Paul Durrant wrote: >> > + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1); >> >> "%d", 1 results in a constant string. xenbus_write() would avoid a >> transitory memory allocation. > > This code is consistent with all the other xenbus_printf()s in the > neighbourhood and does it really matter? I think we should always strive to have the simplest possible code that fulfills the purpose. And hence we shouldn't be setting further bad precedents. (In fact I have a patch queued to replace all the unnecessary xenbus_printf()s with xenbus_write()s on linux-2.6.18-xen.hg, and may look into porting this to the respective upstream components.) Jan -- 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
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 15 January 2014 16:04 > To: Andrew Cooper; Paul Durrant > Cc: David Vrabel; xen-devel@lists.xen.org; Boris Ostrovsky; > netdev@vger.kernel.org > Subject: Re: [Xen-devel] [PATCH net-next] xen-netfront: add support for > IPv6 offloads > > >>> On 15.01.14 at 16:54, Paul Durrant <Paul.Durrant@citrix.com> wrote: > >> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > >> On 15/01/14 15:18, Paul Durrant wrote: > >> > + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", > "%d", 1); > >> > >> "%d", 1 results in a constant string. xenbus_write() would avoid a > >> transitory memory allocation. > > > > This code is consistent with all the other xenbus_printf()s in the > > neighbourhood and does it really matter? > > I think we should always strive to have the simplest possible code > that fulfills the purpose. And hence we shouldn't be setting further > bad precedents. (In fact I have a patch queued to replace all the > unnecessary xenbus_printf()s with xenbus_write()s on > linux-2.6.18-xen.hg, and may look into porting this to the > respective upstream components.) > Ok. Personally I'd go for code consistency with this patch and then a full replacement... but I'll re-work it. Paul -- 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
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index c41537b..321759f 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -617,7 +617,9 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) tx->flags |= XEN_NETTXF_extra_info; gso->u.gso.size = skb_shinfo(skb)->gso_size; - gso->u.gso.type = XEN_NETIF_GSO_TYPE_TCPV4; + gso->u.gso.type = (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) ? + XEN_NETIF_GSO_TYPE_TCPV6 : + XEN_NETIF_GSO_TYPE_TCPV4; gso->u.gso.pad = 0; gso->u.gso.features = 0; @@ -809,15 +811,18 @@ static int xennet_set_skb_gso(struct sk_buff *skb, return -EINVAL; } - /* Currently only TCPv4 S.O. is supported. */ - if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) { + if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4 && + gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV6) { if (net_ratelimit()) pr_warn("Bad GSO type %d\n", gso->u.gso.type); return -EINVAL; } skb_shinfo(skb)->gso_size = gso->u.gso.size; - skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4; + skb_shinfo(skb)->gso_type = + (gso->u.gso.type == XEN_NETIF_GSO_TYPE_TCPV4) ? + SKB_GSO_TCPV4 : + SKB_GSO_TCPV6; /* Header must be checked, and gso_segs computed. */ skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY; @@ -1191,6 +1196,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev, features &= ~NETIF_F_SG; } + if (features & NETIF_F_IPV6_CSUM) { + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, + "feature-ipv6-csum-offload", "%d", &val) < 0) + val = 0; + + if (!val) + features &= ~NETIF_F_IPV6_CSUM; + } + if (features & NETIF_F_TSO) { if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-gso-tcpv4", "%d", &val) < 0) @@ -1200,6 +1214,15 @@ static netdev_features_t xennet_fix_features(struct net_device *dev, features &= ~NETIF_F_TSO; } + if (features & NETIF_F_TSO6) { + if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, + "feature-gso-tcpv6", "%d", &val) < 0) + val = 0; + + if (!val) + features &= ~NETIF_F_TSO6; + } + return features; } @@ -1338,7 +1361,9 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev) netif_napi_add(netdev, &np->napi, xennet_poll, 64); netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM | NETIF_F_GSO_ROBUST; - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO; + netdev->hw_features = NETIF_F_SG | + NETIF_F_IPV6_CSUM | + NETIF_F_TSO | NETIF_F_TSO6; /* * Assume that all hw features are available for now. This set @@ -1716,6 +1741,19 @@ again: goto abort_transaction; } + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", "%d", 1); + if (err) { + message = "writing feature-gso-tcpv6"; + goto abort_transaction; + } + + err = xenbus_printf(xbt, dev->nodename, "feature-ipv6-csum-offload", + "%d", 1); + if (err) { + message = "writing feature-ipv6-csum-offload"; + goto abort_transaction; + } + err = xenbus_transaction_end(xbt, 0); if (err) { if (err == -EAGAIN)
This patch adds support for IPv6 checksum offload and GSO when those features are available in the backend. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: David Vrabel <david.vrabel@citrix.com> --- drivers/net/xen-netfront.c | 48 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-)