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

[annie li <annie.li@xxxxxxxxxx>]

On 2013-10-9 18:26, Paul Durrant wrote:
> > -----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.
It is just about semantics,  my understanding is masks WITH bits for v4 
or v6 means disabling.
> > > +
> > >         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.
Macros would be fine.

Thanks
Annie

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