| Message ID | 1381229896-18657-2-git-send-email-paul.durrant@citrix.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Oct 08, 2013 at 11:58:12AM +0100, Paul Durrant wrote: > Check xenstore flag feature-ipv6-csum-offload to determine if a > guest is happy to accept IPv6 packets with only partial checksum. > Also check analogous feature-ip-csum-offload to determone if a determone -> determine > guest is happy to accept IPv4 packets with only partial checksum > as a replacement for a negated feature-no-csum-offload value and > add a comment to deprecate use of feature-no-csum-offload. > [...] > @@ -306,7 +308,7 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid, > vif->domid = domid; > vif->handle = handle; > vif->can_sg = 1; > - vif->csum = 1; > + vif->ip_csum = 1; Not setting a default value for vif->ipv6_csum? > vif->dev = dev; > [...] > + /* Before feature-ipv6-csum-offload was introduced, checksum offload > + * was turned on by a zero value in (or lack of) > + * feature-no-csum-offload. Therefore, for compatibility, assume > + * that if feature-ip-csum-offload is missing that IP (v4) offload > + * is turned on. If this is not the case then the flag will be > + * cleared when we subsequently sample feature-no-csum-offload. > + */ > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ip-csum-offload", > + "%d", &val) < 0) > + val = 1; > + vif->ip_csum = !!val; > + > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-offload", > + "%d", &val) < 0) > + val = 0; > + vif->ipv6_csum = !!val; > + > + /* This is deprecated. Frontends should set IP v4 and v6 checksum > + * offload using feature-ip-csum-offload and > + * feature-ipv6-csum-offload respectively. > + */ These comments on feature flags should also be synchronized to master netif.h in Xen and Linux's header. We've been trying to document thing along the way for quite some time and the long term benifit is big. > if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload", > "%d", &val) < 0) > val = 0; > - vif->csum = !val; > + if (val) > + vif->ip_csum = 0; Do you also need to set vif->ipv6_csum to 0? I don't think a frontend which cannot deal with V4 csum offload has the ability to deal with V6 csum offload. Wei. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 08 October 2013 14:10 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; David Vrabel; > Ian Campbell > Subject: Re: [PATCH net-next v2 1/5] xen-netback: add IPv6 checksum > offload to guest > > On Tue, Oct 08, 2013 at 11:58:12AM +0100, Paul Durrant wrote: > > Check xenstore flag feature-ipv6-csum-offload to determine if a > > guest is happy to accept IPv6 packets with only partial checksum. > > Also check analogous feature-ip-csum-offload to determone if a > > determone -> determine > Ok. > > guest is happy to accept IPv4 packets with only partial checksum > > as a replacement for a negated feature-no-csum-offload value and > > add a comment to deprecate use of feature-no-csum-offload. > > > [...] > > @@ -306,7 +308,7 @@ struct xenvif *xenvif_alloc(struct device *parent, > domid_t domid, > > vif->domid = domid; > > vif->handle = handle; > > vif->can_sg = 1; > > - vif->csum = 1; > > + vif->ip_csum = 1; > > Not setting a default value for vif->ipv6_csum? > The default is 0 so no need. > > vif->dev = dev; > > > [...] > > + /* Before feature-ipv6-csum-offload was introduced, checksum > offload > > + * was turned on by a zero value in (or lack of) > > + * feature-no-csum-offload. Therefore, for compatibility, assume > > + * that if feature-ip-csum-offload is missing that IP (v4) offload > > + * is turned on. If this is not the case then the flag will be > > + * cleared when we subsequently sample feature-no-csum-offload. > > + */ > > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ip-csum- > offload", > > + "%d", &val) < 0) > > + val = 1; > > + vif->ip_csum = !!val; > > + > > + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum- > offload", > > + "%d", &val) < 0) > > + val = 0; > > + vif->ipv6_csum = !!val; > > + > > + /* This is deprecated. Frontends should set IP v4 and v6 checksum > > + * offload using feature-ip-csum-offload and > > + * feature-ipv6-csum-offload respectively. > > + */ > > These comments on feature flags should also be synchronized to master > netif.h in Xen and Linux's header. We've been trying to document thing > along the way for quite some time and the long term benifit is big. > Ok. That sounds like a good idea. I'll add a patch to do that. > > if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum- > offload", > > "%d", &val) < 0) > > val = 0; > > - vif->csum = !val; > > + if (val) > > + vif->ip_csum = 0; > > Do you also need to set vif->ipv6_csum to 0? I don't think a frontend > which cannot deal with V4 csum offload has the ability to deal with V6 > csum offload. > I was just trying to preserve the existing semantic of feature-no-csum-offload, which only affects v4. Since it's deprecated, should we add a new semantic? Paul -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 5715318..b4a9a3c 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -153,7 +153,8 @@ struct xenvif { u8 can_sg:1; u8 gso:1; u8 gso_prefix:1; - u8 csum:1; + u8 ip_csum:1; + u8 ipv6_csum:1; /* Internal feature information. */ u8 can_queue:1; /* can queue packets for receiver? */ diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index 01bb854..8e92783 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -216,8 +216,10 @@ static netdev_features_t xenvif_fix_features(struct net_device *dev, features &= ~NETIF_F_SG; if (!vif->gso && !vif->gso_prefix) features &= ~NETIF_F_TSO; - if (!vif->csum) + if (!vif->ip_csum) features &= ~NETIF_F_IP_CSUM; + if (!vif->ipv6_csum) + features &= ~NETIF_F_IPV6_CSUM; return features; } @@ -306,7 +308,7 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid, vif->domid = domid; vif->handle = handle; vif->can_sg = 1; - vif->csum = 1; + vif->ip_csum = 1; vif->dev = dev; vif->credit_bytes = vif->remaining_credit = ~0UL; @@ -316,7 +318,9 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid, vif->credit_timeout.expires = jiffies; dev->netdev_ops = &xenvif_netdev_ops; - dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO; + dev->hw_features = NETIF_F_SG | + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | + NETIF_F_TSO; dev->features = dev->hw_features; SET_ETHTOOL_OPS(dev, &xenvif_ethtool_ops); diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index b45bce2..5e45b00 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -567,10 +567,32 @@ static int connect_rings(struct backend_info *be) val = 0; vif->gso_prefix = !!val; + /* Before feature-ipv6-csum-offload was introduced, checksum offload + * was turned on by a zero value in (or lack of) + * feature-no-csum-offload. Therefore, for compatibility, assume + * that if feature-ip-csum-offload is missing that IP (v4) offload + * is turned on. If this is not the case then the flag will be + * cleared when we subsequently sample feature-no-csum-offload. + */ + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ip-csum-offload", + "%d", &val) < 0) + val = 1; + vif->ip_csum = !!val; + + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-offload", + "%d", &val) < 0) + val = 0; + vif->ipv6_csum = !!val; + + /* This is deprecated. Frontends should set IP v4 and v6 checksum + * offload using feature-ip-csum-offload and + * feature-ipv6-csum-offload respectively. + */ if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload", "%d", &val) < 0) val = 0; - vif->csum = !val; + if (val) + vif->ip_csum = 0; /* Map the shared frame, irq etc. */ err = xenvif_connect(vif, tx_ring_ref, rx_ring_ref,
Check xenstore flag feature-ipv6-csum-offload to determine if a guest is happy to accept IPv6 packets with only partial checksum. Also check analogous feature-ip-csum-offload to determone if a guest is happy to accept IPv4 packets with only partial checksum as a replacement for a negated feature-no-csum-offload value and add a comment to deprecate use of feature-no-csum-offload. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> --- drivers/net/xen-netback/common.h | 3 ++- drivers/net/xen-netback/interface.c | 10 +++++++--- drivers/net/xen-netback/xenbus.c | 24 +++++++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-)