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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: annie li [mailto:annie.li@xxxxxxxxxx]
> Sent: 09 October 2013 05:42
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Wei Liu; David Vrabel;
> Ian Campbell
> Subject: Re: [Xen-devel] [PATCH net-next v2 5/5] xen-netback: enable IPv6
> TCP GSO to the guest
> 
> 
> On 2013-10-8 18:58, Paul Durrant wrote:
> > This patch adds code to handle SKB_GSO_TCPV6 skbs and construct
> appropriate
> > extra or prefix segments to pass the large packet to the frontend. New
> > xenstore flags, feature-gso-tcpv6 and feature-gso-tcpv6-prefix, are
> sampled
> > to determine if the frontend is capable of handling such packets.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >   drivers/net/xen-netback/common.h    |    6 +++--
> >   drivers/net/xen-netback/interface.c |    8 ++++--
> >   drivers/net/xen-netback/netback.c   |   47
> +++++++++++++++++++++++++++--------
> >   drivers/net/xen-netback/xenbus.c    |   29 +++++++++++++++++++--
> >   4 files changed, 74 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index b4a9a3c..720b1ca 100644
> > --- a/drivers/net/xen-netback/common.h
> > +++ b/drivers/net/xen-netback/common.h
> > @@ -87,6 +87,7 @@ struct pending_tx_info {
> >   struct xenvif_rx_meta {
> >     int id;
> >     int size;
> > +   int gso_type;
> >     int gso_size;
> >   };
> >
> > @@ -150,9 +151,10 @@ struct xenvif {
> >     u8               fe_dev_addr[6];
> >
> >     /* Frontend feature information. */
> > +   int gso_mask;
> > +   int gso_prefix_mask;
> I assume it is a flag instead of mask here, right? If it is mask, then 1
> means disabling the gso.

I don't understand what you're saying here. I'm just swapping from bit flags to 
a couple of masks. Masks without either of the requisite bits for v4 or v6 gso 
mean it is disabled.

> > +
> >     u8 can_sg:1;
> > -   u8 gso:1;
> > -   u8 gso_prefix:1;
> >     u8 ip_csum:1;
> >     u8 ipv6_csum:1;
> >
> > diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-
> netback/interface.c
> > index cb0d8ea..3d11387 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -214,8 +214,12 @@ static netdev_features_t
> xenvif_fix_features(struct net_device *dev,
> >
> >     if (!vif->can_sg)
> >             features &= ~NETIF_F_SG;
> > -   if (!vif->gso && !vif->gso_prefix)
> > +   if (~(vif->gso_mask | vif->gso_prefix_mask) &
> > +       (1 << XEN_NETIF_GSO_TYPE_TCPV4))
> 
> Is it better to use XEN_NETIF_GSO_TYPE_TCPV4 directly and setting
> gso_mask(gso_prefix_mask) with "|= XEN_NETIF_GSO_TYPE_TCPV4" or "|=
> XEN_NETIF_GSO_TYPE_TCPV6" instead of "1 <<"?
> 

I thought about it but decided it was best to leave XEN_NETIF_GSO_TYPE_xxx as a 
list of types rather than bits in a mask as there's no intrinsic reason why 
you'd ever want to OR them together (unlike the tx or rx flags). That fact I 
use them as bit shifts in netback is purely for convenience of coding - I guess 
I could define macros to make it a little tidier though.

  Paul

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