[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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
extra or prefix segments to pass the large packet to the frontend. New
xenstore flags, feature-gso-tcpv6 and feature-gso-tcpv6-prefix, are
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-
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-
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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.