Re: [Xen-devel] [PATCH net-next] xen-netfront: add support for IPv6 offloads

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 15 January 2014 15:25
> To: Paul Durrant
> Cc: netdev@xxxxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; 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@xxxxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
> > ---
> >  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)


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel