Re: [Xen-devel] [PATCH net-next v3 5/5] xen-netback: enable IPv6 TCP GSO to the guest

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 11 October 2013 11:10
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Wei Liu; David Vrabel;
> Ian Campbell
> Subject: Re: [PATCH net-next v3 5/5] xen-netback: enable IPv6 TCP GSO to
> the guest
> 
> On Thu, Oct 10, 2013 at 04:25:33PM +0100, Paul Durrant wrote:
> [...]
> >     return max;
> > @@ -312,6 +312,7 @@ static struct xenvif_rx_meta
> *get_next_rx_buffer(struct xenvif *vif,
> >     req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
> >
> >     meta = npo->meta + npo->meta_prod++;
> > +   meta->gso_type = 0;
> 
> Should use XEN_NETIF_GSO_TYPE_NONE here.
> 
> >     meta->gso_size = 0;
> >     meta->size = 0;
> [...]
> > +   if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
> > +           gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> > +           gso_size = skb_shinfo(skb)->gso_size;
> > +   } else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
> > +           gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> > +           gso_size = skb_shinfo(skb)->gso_size;
> > +   } else {
> > +           gso_type = 0;
> 
> Ditto.
> 
> > +           gso_size = 0;
> > +   }
> > +
> [...]
> > @@ -438,10 +461,13 @@ static int xenvif_gop_skb(struct sk_buff *skb,
> >     req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
> >     meta = npo->meta + npo->meta_prod++;
> >
> > -   if (!vif->gso_prefix)
> > -           meta->gso_size = skb_shinfo(skb)->gso_size;
> > -   else
> > +   if ((1 << gso_type) & vif->gso_mask) {
> > +           meta->gso_type = gso_type;
> > +           meta->gso_size = gso_size;
> > +   } else {
> > +           meta->gso_type = 0;
> 
> Ditto.
> 
> >             meta->gso_size = 0;
> > +   }
> >
> >     meta->size = 0;
> >     meta->id = req->id;
> > @@ -587,7 +613,8 @@ void xenvif_rx_action(struct xenvif *vif)
> >
> >             vif = netdev_priv(skb->dev);
> >
> > -           if (vif->meta[npo.meta_cons].gso_size && vif->gso_prefix) {
> > +           if ((1 << vif->meta[npo.meta_cons].gso_type) &
> > +               vif->gso_prefix_mask) {
> 
> The logic is changed here. Why do you remove the test on gso_size?
> 

Sorry, I missed this one before so never responded...

The test on gso_size is no longer necessary, because if gso_size is 0 then 
gso_type will be XEN_NETIF_GSO_TYPE_NONE and that will never appear in the 
masks.

  Paul

> >                     resp = RING_GET_RESPONSE(&vif->rx,
> >                                              vif->rx.rsp_prod_pvt++);
> >
> > @@ -624,7 +651,8 @@ void xenvif_rx_action(struct xenvif *vif)
> >                                     vif->meta[npo.meta_cons].size,
> >                                     flags);
> >
> > -           if (vif->meta[npo.meta_cons].gso_size && !vif->gso_prefix)
> {
> > +           if ((1 << vif->meta[npo.meta_cons].gso_type) &
> > +               vif->gso_mask) {
> 
> Ditto.
> 
> >                     struct xen_netif_extra_info *gso =
> >                             (struct xen_netif_extra_info *)
> >                             RING_GET_RESPONSE(&vif->rx,
> > @@ -632,8 +660,8 @@ void xenvif_rx_action(struct xenvif *vif)
> >
> [...]
> >      * was turned on by a zero value in (or lack of)
> > diff --git a/include/xen/interface/io/netif.h
> b/include/xen/interface/io/netif.h
> > index d7dd8d7..41674bc 100644
> > --- a/include/xen/interface/io/netif.h
> > +++ b/include/xen/interface/io/netif.h
> > @@ -113,6 +113,7 @@ struct xen_netif_tx_request {
> >  #define  XEN_NETIF_EXTRA_FLAG_MORE
>       (1U<<_XEN_NETIF_EXTRA_FLAG_MORE)
> >
> >  /* GSO types */
> > +#define XEN_NETIF_GSO_TYPE_NONE         (0)
> 
> Indentation.
> 
> Wei.

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