[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
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx] > Sent: 08 October 2013 14:34 > To: Paul Durrant > Cc: xen-devel@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Wei Liu; David Vrabel; > Ian Campbell > Subject: Re: [PATCH net-next v2 5/5] xen-netback: enable IPv6 TCP GSO to > the guest > > On Tue, Oct 08, 2013 at 11:58:16AM +0100, Paul Durrant wrote: > [...] > > /* Data must not cross a page boundary. */ > > BUG_ON(size + offset > PAGE_SIZE<<compound_order(page)); > > @@ -392,7 +393,14 @@ static void xenvif_gop_frag_copy(struct xenvif > *vif, struct sk_buff *skb, > > } > > > > /* Leave a gap for the GSO descriptor. */ > > - if (*head && skb_shinfo(skb)->gso_size && !vif->gso_prefix) > > + if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) > > + gso_type = XEN_NETIF_GSO_TYPE_TCPV4; > > + else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) > > + gso_type = XEN_NETIF_GSO_TYPE_TCPV6; > > + else > > + gso_type = 0; > > Should probably #define XEN_NETIF_GSO_TYPE_NONE 0 instead of using > plain > 0? All I need is a bit shift that's not going to hit in the mask. Probably worth reserving the value in the header. > > > + 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. > > > - 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; > [...] > > @@ -573,15 +573,40 @@ static int connect_rings(struct backend_info *be) > > val = 0; > > vif->can_sg = !!val; > > > > + vif->gso_mask = 0; > > + vif->gso_prefix_mask = 0; > > + > > if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4", > > "%d", &val) < 0) > > val = 0; > > - vif->gso = !!val; > > + if (val) > > + vif->gso_mask = 1 << XEN_NETIF_GSO_TYPE_TCPV4; > > Not using "|=" ? > > > > > if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4- > prefix", > > "%d", &val) < 0) > > val = 0; > > - vif->gso_prefix = !!val; > > + if (val) > > + vif->gso_prefix_mask = 1 << XEN_NETIF_GSO_TYPE_TCPV4; > > + > > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6", > > + "%d", &val) < 0) > > + val = 0; > > + if (val) > > + vif->gso_mask = 1 << XEN_NETIF_GSO_TYPE_TCPV6; > > + > > Not using "|="? Are feature-gso_tcpv{4,6} mutually exclusive? > > Same question goes to the setting of gso_prefix_mask. > That's very odd. You're correct and I could have sworn the code did have |= in these places; thanks for catching that. Paul _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |