Re: [PATCH RESEND] net: convert xen-netfront to hw_features
On Sat, 2011-04-02 at 04:54 +0100, David Miller wrote: From: Michał Mirosław mirq-li...@rere.qmqm.pl Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST) Not tested in any way. The original code for offload setting seems broken as it resets the features on every netback reconnect. This will set GSO_ROBUST at device creation time (earlier than connect time). RX checksum offload is forced on - so advertise as it is. Signed-off-by: Michał Mirosław mirq-li...@rere.qmqm.pl Applied. Thanks, but unfortunately the patch results in the features all being disabled by default, since they are not set in the initial dev-features and the initial dev-wanted_features is based on features hw_features. The ndo_fix_features hook only clears features and doesn't add new features (nor should it AFAICT). Features cannot be negotiated with the backend until xennet_connect(). The carrier is not enabled until the end of that function, therefore I think it is safe to start with a full set of features in dev-features and rely on the call to netdev_update_features() in xennet_connect() to clear those which turn out to be unavailable. The following works for me, I guess the alternative is for xennet_connect() to expand dev-features based on what it detects? Or is there a mechanism for a driver to inform the core that a new hardware feature has become available (I doubt that really happens on physical h/w so I guess not). Ian. 8- From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001 From: Ian Campbell ian.campb...@citrix.com Date: Mon, 4 Apr 2011 10:58:50 +0100 Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup We need to assume that all features will be available when registering the netdev otherwise they are ommitted from the initial set of dev-wanted_features. When we connect to the backed we reduce the set as necessary due to the call to netdev_update_features() in xennet_connect(). Signed-off-by: Ian Campbell ian.campb...@citrix.com Cc: mirq-li...@rere.qmqm.pl Cc: net...@vger.kernel.org net...@vger.kernel.org Cc: Jeremy Fitzhardinge jer...@goop.org Cc: konrad.w...@oracle.com Cc: Eric Dumazet eric.duma...@gmail.com Cc: xen-de...@lists.xensource.com --- drivers/net/xen-netfront.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 0cfe4cc..db9a763 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1251,6 +1251,14 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev NETIF_F_GSO_ROBUST; netdev-hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO; + /* + * Assume that all hw features are available for now. This set + * will be adjusted by the call to netdev_update_features() in + * xennet_connect() which is the earliest point where we can + * negotiate with the backend regarding supported features. + */ + netdev-features |= netdev-hw_features; + SET_ETHTOOL_OPS(netdev, xennet_ethtool_ops); SET_NETDEV_DEV(netdev, dev-dev); -- 1.7.2.5 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH RESEND] net: convert xen-netfront to hw_features
From: Ian Campbell ian.campb...@eu.citrix.com Date: Mon, 4 Apr 2011 13:29:19 +0100 From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001 From: Ian Campbell ian.campb...@citrix.com Date: Mon, 4 Apr 2011 10:58:50 +0100 Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup We need to assume that all features will be available when registering the netdev otherwise they are ommitted from the initial set of dev-wanted_features. When we connect to the backed we reduce the set as necessary due to the call to netdev_update_features() in xennet_connect(). Signed-off-by: Ian Campbell ian.campb...@citrix.com I've applied this, thanks Ian. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH RESEND] net: convert xen-netfront to hw_features
From: Michał Mirosław mirq-li...@rere.qmqm.pl Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST) Not tested in any way. The original code for offload setting seems broken as it resets the features on every netback reconnect. This will set GSO_ROBUST at device creation time (earlier than connect time). RX checksum offload is forced on - so advertise as it is. Signed-off-by: Michał Mirosław mirq-li...@rere.qmqm.pl Applied. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH RESEND] net: convert xen-netfront to hw_features
On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
Not tested in any way. The original code for offload setting seems broken
as it resets the features on every netback reconnect.
Thanks, I've got a pending TODO item to test this and propagate similar
changes to netback. I hope to get to it soon...
Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
a long time now...
Ian.
This will set GSO_ROBUST at device creation time (earlier than connect time).
RX checksum offload is forced on - so advertise as it is.
Signed-off-by: Michał Mirosław mirq-li...@rere.qmqm.pl
---
[I don't know Xen code enough to say this is correct. There is Xen netback
driver coming in, that has similar changes to be made. Please match
them up if you can.]
drivers/net/xen-netfront.c | 57 +--
1 files changed, 23 insertions(+), 34 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5c8d9c3..2a71c9f 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
.ndo_change_mtu = xennet_change_mtu,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
+ .ndo_fix_features= xennet_fix_features,
+ .ndo_set_features= xennet_set_features,
};
static struct net_device * __devinit xennet_create_dev(struct xenbus_device
*dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit
xennet_create_dev(struct xenbus_device *dev
netdev-netdev_ops = xennet_netdev_ops;
netif_napi_add(netdev, np-napi, xennet_poll, 64);
- netdev-features= NETIF_F_IP_CSUM;
+ netdev-features= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+ NETIF_F_GSO_ROBUST;
+ netdev-hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
SET_ETHTOOL_OPS(netdev, xennet_ethtool_ops);
SET_NETDEV_DEV(netdev, dev-dev);
@@ -1510,52 +1514,40 @@ again:
return err;
}
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
{
- if (data) {
- struct netfront_info *np = netdev_priv(dev);
- int val;
+ struct netfront_info *np = netdev_priv(dev);
+ int val;
+ if (features NETIF_F_SG) {
if (xenbus_scanf(XBT_NIL, np-xbdev-otherend, feature-sg,
%d, val) 0)
val = 0;
+
if (!val)
- return -ENOSYS;
- } else if (dev-mtu ETH_DATA_LEN)
- dev-mtu = ETH_DATA_LEN;
-
- return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
- if (data) {
- struct netfront_info *np = netdev_priv(dev);
- int val;
+ features = ~NETIF_F_SG;
+ }
+ if (features NETIF_F_TSO) {
if (xenbus_scanf(XBT_NIL, np-xbdev-otherend,
feature-gso-tcpv4, %d, val) 0)
val = 0;
+
if (!val)
- return -ENOSYS;
+ features = ~NETIF_F_TSO;
}
- return ethtool_op_set_tso(dev, data);
+ return features;
}
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
{
- /* Turn off all GSO bits except ROBUST. */
- dev-features = ~NETIF_F_GSO_MASK;
- dev-features |= NETIF_F_GSO_ROBUST;
- xennet_set_sg(dev, 0);
+ if (!(features NETIF_F_SG) dev-mtu ETH_DATA_LEN) {
+ netdev_info(dev, Reducing MTU because no SG offload);
+ dev-mtu = ETH_DATA_LEN;
+ }
- /* We need checksum offload to enable scatter/gather and TSO. */
- if (!(dev-features NETIF_F_IP_CSUM))
- return;
-
- if (!xennet_set_sg(dev, 1))
- xennet_set_tso(dev, 1);
+ return 0;
}
static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
if (err)
return err;
- xennet_set_features(dev);
+ netdev_update_features(dev);
spin_lock_bh(np-rx_lock);
spin_lock_irq(np-tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev,
u32 stringset, u8 * data)
static const struct ethtool_ops xennet_ethtool_ops =
{
- .set_tx_csum = ethtool_op_set_tx_csum,
- .set_sg = xennet_set_sg,
- .set_tso = xennet_set_tso,
.get_link = ethtool_op_get_link,
.get_sset_count = xennet_get_sset_count,
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
Re: [PATCH RESEND] net: convert xen-netfront to hw_features
On Thu, Mar 31, 2011 at 12:13:30PM +0100, Ian Campbell wrote: On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote: Not tested in any way. The original code for offload setting seems broken as it resets the features on every netback reconnect. Thanks, I've got a pending TODO item to test this and propagate similar changes to netback. I hope to get to it soon... Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for a long time now... This probably affects only people who are using suspending VM's to disk or [live] migration. The bug is hard to notice if you're not looking for it. I only noticed it after trying to change the code. Best Regards, Michał Mirosław ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
