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

[annie li <annie.li@xxxxxxxxxx>]

On 2013/11/25 23:19, Konrad Rzeszutek Wilk wrote:
> On Fri, Nov 15, 2013 at 05:52:59PM +0000, Paul Durrant wrote:
> > This patch adds support for IPv6 checksum offload and GSO when those
> > features are available in the backend.
> Wei, Annie, thoughts?
Seems fine except for following two comments :-)

> > 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>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >
> > v2:
> >   - Addressed comments raised by Ian Campbell
> >
> >   drivers/net/xen-netfront.c |  226 ++++++++++++++++++++++++++++++++++++++++----
> >   include/linux/ipv6.h       |    2 +
> >   2 files changed, 211 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index dd1011e..afadfb5 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -616,7 +616,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;
...snip...

> >   static int handle_incoming_queue(struct net_device *dev,
> >                                  struct sk_buff_head *rxq)
> >   {
> > @@ -1232,6 +1392,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)
> > @@ -1241,6 +1410,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;
> >   }
> >   
> > @@ -1373,7 +1551,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 |
Are you missing NETIF_F_IP_CSUM here?

> > +                                 NETIF_F_TSO | NETIF_F_TSO6;
> >   
> >   	/*
> >            * Assume that all hw features are available for now. This set
> > @@ -1751,6 +1931,18 @@ 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-gso-tcpv6";
Change "feature-gso-tcpv6" to "feature-ipv6-csum-offload"?

Thanks
Annie

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