All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/9] net: Unified offload configuration
@ 2011-02-16  2:59 Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 3/9] ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count() Michał Mirosław
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

Here's a v6 of the ethtool unification patch series.

What's in it?
 1..4:
	cleanups for the core patches
 5:
	the patch - implement unified ethtool setting ops
 6..7:
	implement interoperation between old and new ethtool ops
 8:
	include RX checksum in features and plug it into new framework
 9:
	convert loopback device to new framework

What is it good for?
 - unifies driver behaviour wrt hardware offloads
 - removes a lot of boilerplate code from drivers
 - allows better fine-grained control over used offloads

This version is not tested, yet.

Best Regards,
Michał Mirosław


v1: http://marc.info/?l=linux-netdev&m=129245188832643&w=3

Changes from v5:
 - register_netdevice(): avoid warning on GSO for non-SG capable devices
 - rebased on current net-next (introduction of ndo_add/del_slave)

Changes from v4:
 - more split cleanups
 - fix error return for ETHTOOL_SFLAGS
 - fix ETHTOOL_G* compatibility for not converted drivers

Changes from v3:
 - fixed kernel-doc and other comments
 - added HIGHDMA to never-changeable features
 - changed GFEATURES .size interpretation
 - changed feature strings
 - change __ethtool_set_flags() to reject invalid changes

Changes from v2:
 - rebase to net-next after merging v2 leading patches
 - fix missing comma in feature name table
 - force NETIF_F_SOFT_FEATURES in hw_features for simpler code
   (fixes a bug that disallowed changing GSO and GRO state)

Changes from v1:
 - split structures for GFEATURES/SFEATURES
 - naming of feature bits using GSTRINGS ETH_SS_FEATURES
 - strict checking of bits used in SFEATURES call
 - more comments and kernel-doc
 - rebased to net-next after 2.6.37

---

Michał Mirosław (9):
  ethtool: move EXPORT_SYMBOL(ethtool_op_set_tx_csum) to correct place
  ethtool: enable GSO and GRO by default
  ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count()
  ethtool: factorize get/set_one_feature
  net: Introduce new feature setting ops
  net: ethtool: use ndo_fix_features for offload setting
  net: use ndo_fix_features for ethtool_ops->set_flags
  net: introduce NETIF_F_RXCSUM
  loopback: convert to hw_features

 drivers/net/loopback.c    |    9 +-
 include/linux/ethtool.h   |   86 ++++++++-
 include/linux/netdevice.h |   49 ++++-
 net/core/dev.c            |   52 ++++-
 net/core/ethtool.c        |  527 +++++++++++++++++++++++++++++---------------
 5 files changed, 531 insertions(+), 192 deletions(-)

-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v6 1/9] ethtool: move EXPORT_SYMBOL(ethtool_op_set_tx_csum) to correct place
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 3/9] ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count() Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 2/9] ethtool: enable GSO and GRO by default Michał Mirosław
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller


Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/ethtool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 5984ee0..9eb8277 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -55,6 +55,7 @@ int ethtool_op_set_tx_csum(struct net_device *dev, u32 data)
 
 	return 0;
 }
+EXPORT_SYMBOL(ethtool_op_set_tx_csum);
 
 int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data)
 {
@@ -1124,7 +1125,6 @@ static int ethtool_set_tx_csum(struct net_device *dev, char __user *useraddr)
 
 	return dev->ethtool_ops->set_tx_csum(dev, edata.data);
 }
-EXPORT_SYMBOL(ethtool_op_set_tx_csum);
 
 static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
 {
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 2/9] ethtool: enable GSO and GRO by default
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 3/9] ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count() Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 1/9] ethtool: move EXPORT_SYMBOL(ethtool_op_set_tx_csum) to correct place Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 5/9] net: Introduce new feature setting ops Michał Mirosław
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller


Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/netdevice.h |    3 +++
 net/core/dev.c            |   18 ++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d08ef65..168e3ad 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -984,6 +984,9 @@ struct net_device {
 				 NETIF_F_SG | NETIF_F_HIGHDMA |		\
 				 NETIF_F_FRAGLIST)
 
+	/* changeable features with no special hardware requirements */
+#define NETIF_F_SOFT_FEATURES	(NETIF_F_GSO | NETIF_F_GRO)
+
 	/* Interface index. Unique device identifier	*/
 	int			ifindex;
 	int			iflink;
diff --git a/net/core/dev.c b/net/core/dev.c
index 4580460..8686f6f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5274,6 +5274,12 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
 		features &= ~NETIF_F_TSO;
 	}
 
+	/* Software GSO depends on SG. */
+	if ((features & NETIF_F_GSO) && !(features & NETIF_F_SG)) {
+		netdev_info(dev, "Dropping NETIF_F_GSO since no SG feature.\n");
+		features &= ~NETIF_F_GSO;
+	}
+
 	/* UFO needs SG and checksumming */
 	if (features & NETIF_F_UFO) {
 		/* maybe split UFO into V4 and V6? */
@@ -5430,12 +5436,16 @@ int register_netdevice(struct net_device *dev)
 	if (dev->iflink == -1)
 		dev->iflink = dev->ifindex;
 
+	/* Enable software offloads by default - will be stripped in
+	 * netdev_fix_features() if not supported. */
+	dev->features |= NETIF_F_SOFT_FEATURES;
+
+	/* Avoid warning from netdev_fix_features() for GSO without SG */
+	if (!(dev->features & NETIF_F_SG))
+		dev->features &= ~NETIF_F_GSO;
+
 	dev->features = netdev_fix_features(dev, dev->features);
 
-	/* Enable software GSO if SG is supported. */
-	if (dev->features & NETIF_F_SG)
-		dev->features |= NETIF_F_GSO;
-
 	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
 	 * vlan_dev_init() will do the dev->features check, so these features
 	 * are enabled only if supported by underlying device.
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 3/9] ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count()
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 1/9] ethtool: move EXPORT_SYMBOL(ethtool_op_set_tx_csum) to correct place Michał Mirosław
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

This is needed for unified offloads patch.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/ethtool.c |   35 +++++++++++++++++++++++------------
 1 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9eb8277..85aaeab 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -172,6 +172,25 @@ EXPORT_SYMBOL(ethtool_ntuple_flush);
 
 /* Handlers for each ethtool command */
 
+static int __ethtool_get_sset_count(struct net_device *dev, int sset)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+
+	if (ops && ops->get_sset_count && ops->get_strings)
+		return ops->get_sset_count(dev, sset);
+	else
+		return -EOPNOTSUPP;
+}
+
+static void __ethtool_get_strings(struct net_device *dev,
+	u32 stringset, u8 *data)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+
+	/* ops->get_strings is valid because checked earlier */
+	ops->get_strings(dev, stringset, data);
+}
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
@@ -252,14 +271,10 @@ static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev,
 						    void __user *useraddr)
 {
 	struct ethtool_sset_info info;
-	const struct ethtool_ops *ops = dev->ethtool_ops;
 	u64 sset_mask;
 	int i, idx = 0, n_bits = 0, ret, rc;
 	u32 *info_buf = NULL;
 
-	if (!ops->get_sset_count)
-		return -EOPNOTSUPP;
-
 	if (copy_from_user(&info, useraddr, sizeof(info)))
 		return -EFAULT;
 
@@ -286,7 +301,7 @@ static noinline_for_stack int ethtool_get_sset_info(struct net_device *dev,
 		if (!(sset_mask & (1ULL << i)))
 			continue;
 
-		rc = ops->get_sset_count(dev, i);
+		rc = __ethtool_get_sset_count(dev, i);
 		if (rc >= 0) {
 			info.sset_mask |= (1ULL << i);
 			info_buf[idx++] = rc;
@@ -1287,17 +1302,13 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
 static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_gstrings gstrings;
-	const struct ethtool_ops *ops = dev->ethtool_ops;
 	u8 *data;
 	int ret;
 
-	if (!ops->get_strings || !ops->get_sset_count)
-		return -EOPNOTSUPP;
-
 	if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
 		return -EFAULT;
 
-	ret = ops->get_sset_count(dev, gstrings.string_set);
+	ret = __ethtool_get_sset_count(dev, gstrings.string_set);
 	if (ret < 0)
 		return ret;
 
@@ -1307,7 +1318,7 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 	if (!data)
 		return -ENOMEM;
 
-	ops->get_strings(dev, gstrings.string_set, data);
+	__ethtool_get_strings(dev, gstrings.string_set, data);
 
 	ret = -EFAULT;
 	if (copy_to_user(useraddr, &gstrings, sizeof(gstrings)))
@@ -1317,7 +1328,7 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 		goto out;
 	ret = 0;
 
- out:
+out:
 	kfree(data);
 	return ret;
 }
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 4/9] ethtool: factorize get/set_one_feature
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (4 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 6/9] net: ethtool: use ndo_fix_features for offload setting Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 9/9] loopback: convert to hw_features Michał Mirosław
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

This allows to enable GRO even if RX csum is disabled. GRO will not
be used for packets without hardware checksum anyway.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/netdevice.h |    6 +
 net/core/ethtool.c        |  274 ++++++++++++++++++++++-----------------------
 2 files changed, 138 insertions(+), 142 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 168e3ad..dede3fd 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -976,6 +976,12 @@ struct net_device {
 #define NETIF_F_V6_CSUM		(NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM)
 #define NETIF_F_ALL_CSUM	(NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
 
+#define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+
+#define NETIF_F_ALL_TX_OFFLOADS	(NETIF_F_ALL_CSUM | NETIF_F_SG | \
+				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
+				 NETIF_F_SCTP_CSUM | NETIF_F_FCOE_CRC)
+
 	/*
 	 * If one device supports one of these features, then enable them
 	 * for all in netdev_increment_features.
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 85aaeab..c3fb8f9 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -191,6 +191,109 @@ static void __ethtool_get_strings(struct net_device *dev,
 	ops->get_strings(dev, stringset, data);
 }
 
+static u32 ethtool_get_feature_mask(u32 eth_cmd)
+{
+	/* feature masks of legacy discrete ethtool ops */
+
+	switch (eth_cmd) {
+	case ETHTOOL_GTXCSUM:
+	case ETHTOOL_STXCSUM:
+		return NETIF_F_ALL_CSUM | NETIF_F_SCTP_CSUM;
+	case ETHTOOL_GSG:
+	case ETHTOOL_SSG:
+		return NETIF_F_SG;
+	case ETHTOOL_GTSO:
+	case ETHTOOL_STSO:
+		return NETIF_F_ALL_TSO;
+	case ETHTOOL_GUFO:
+	case ETHTOOL_SUFO:
+		return NETIF_F_UFO;
+	case ETHTOOL_GGSO:
+	case ETHTOOL_SGSO:
+		return NETIF_F_GSO;
+	case ETHTOOL_GGRO:
+	case ETHTOOL_SGRO:
+		return NETIF_F_GRO;
+	default:
+		BUG();
+	}
+}
+
+static void *__ethtool_get_one_feature_actor(struct net_device *dev, u32 ethcmd)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+
+	if (!ops)
+		return NULL;
+
+	switch (ethcmd) {
+	case ETHTOOL_GTXCSUM:
+		return ops->get_tx_csum;
+	case ETHTOOL_SSG:
+		return ops->get_sg;
+	case ETHTOOL_STSO:
+		return ops->get_tso;
+	case ETHTOOL_SUFO:
+		return ops->get_ufo;
+	default:
+		return NULL;
+	}
+}
+
+static int ethtool_get_one_feature(struct net_device *dev,
+	char __user *useraddr, u32 ethcmd)
+{
+	struct ethtool_value edata = {
+		.cmd = ethcmd,
+		.data = !!(dev->features & ethtool_get_feature_mask(ethcmd)),
+	};
+	u32 (*actor)(struct net_device *);
+
+	actor = __ethtool_get_one_feature_actor(dev, ethcmd);
+	if (actor)
+		edata.data = actor(dev);
+
+	if (copy_to_user(useraddr, &edata, sizeof(edata)))
+		return -EFAULT;
+	return 0;
+}
+
+static int __ethtool_set_tx_csum(struct net_device *dev, u32 data);
+static int __ethtool_set_sg(struct net_device *dev, u32 data);
+static int __ethtool_set_tso(struct net_device *dev, u32 data);
+static int __ethtool_set_ufo(struct net_device *dev, u32 data);
+
+static int ethtool_set_one_feature(struct net_device *dev,
+	void __user *useraddr, u32 ethcmd)
+{
+	struct ethtool_value edata;
+	u32 mask;
+
+	if (copy_from_user(&edata, useraddr, sizeof(edata)))
+		return -EFAULT;
+
+	switch (ethcmd) {
+	case ETHTOOL_STXCSUM:
+		return __ethtool_set_tx_csum(dev, edata.data);
+	case ETHTOOL_SSG:
+		return __ethtool_set_sg(dev, edata.data);
+	case ETHTOOL_STSO:
+		return __ethtool_set_tso(dev, edata.data);
+	case ETHTOOL_SUFO:
+		return __ethtool_set_ufo(dev, edata.data);
+	case ETHTOOL_SGSO:
+	case ETHTOOL_SGRO:
+		mask = ethtool_get_feature_mask(ethcmd);
+		if (edata.data)
+			dev->features |= mask;
+		else
+			dev->features &= ~mask;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
@@ -1107,6 +1210,9 @@ static int __ethtool_set_sg(struct net_device *dev, u32 data)
 {
 	int err;
 
+	if (data && !(dev->features & NETIF_F_ALL_CSUM))
+		return -EINVAL;
+
 	if (!data && dev->ethtool_ops->set_tso) {
 		err = dev->ethtool_ops->set_tso(dev, 0);
 		if (err)
@@ -1121,24 +1227,20 @@ static int __ethtool_set_sg(struct net_device *dev, u32 data)
 	return dev->ethtool_ops->set_sg(dev, data);
 }
 
-static int ethtool_set_tx_csum(struct net_device *dev, char __user *useraddr)
+static int __ethtool_set_tx_csum(struct net_device *dev, u32 data)
 {
-	struct ethtool_value edata;
 	int err;
 
 	if (!dev->ethtool_ops->set_tx_csum)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (!edata.data && dev->ethtool_ops->set_sg) {
+	if (!data && dev->ethtool_ops->set_sg) {
 		err = __ethtool_set_sg(dev, 0);
 		if (err)
 			return err;
 	}
 
-	return dev->ethtool_ops->set_tx_csum(dev, edata.data);
+	return dev->ethtool_ops->set_tx_csum(dev, data);
 }
 
 static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
@@ -1157,108 +1259,28 @@ static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
 	return dev->ethtool_ops->set_rx_csum(dev, edata.data);
 }
 
-static int ethtool_set_sg(struct net_device *dev, char __user *useraddr)
+static int __ethtool_set_tso(struct net_device *dev, u32 data)
 {
-	struct ethtool_value edata;
-
-	if (!dev->ethtool_ops->set_sg)
-		return -EOPNOTSUPP;
-
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (edata.data &&
-	    !(dev->features & NETIF_F_ALL_CSUM))
-		return -EINVAL;
-
-	return __ethtool_set_sg(dev, edata.data);
-}
-
-static int ethtool_set_tso(struct net_device *dev, char __user *useraddr)
-{
-	struct ethtool_value edata;
-
 	if (!dev->ethtool_ops->set_tso)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (edata.data && !(dev->features & NETIF_F_SG))
+	if (data && !(dev->features & NETIF_F_SG))
 		return -EINVAL;
 
-	return dev->ethtool_ops->set_tso(dev, edata.data);
+	return dev->ethtool_ops->set_tso(dev, data);
 }
 
-static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
+static int __ethtool_set_ufo(struct net_device *dev, u32 data)
 {
-	struct ethtool_value edata;
-
 	if (!dev->ethtool_ops->set_ufo)
 		return -EOPNOTSUPP;
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-	if (edata.data && !(dev->features & NETIF_F_SG))
+	if (data && !(dev->features & NETIF_F_SG))
 		return -EINVAL;
-	if (edata.data && !((dev->features & NETIF_F_GEN_CSUM) ||
+	if (data && !((dev->features & NETIF_F_GEN_CSUM) ||
 		(dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
 			== (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)))
 		return -EINVAL;
-	return dev->ethtool_ops->set_ufo(dev, edata.data);
-}
-
-static int ethtool_get_gso(struct net_device *dev, char __user *useraddr)
-{
-	struct ethtool_value edata = { ETHTOOL_GGSO };
-
-	edata.data = dev->features & NETIF_F_GSO;
-	if (copy_to_user(useraddr, &edata, sizeof(edata)))
-		return -EFAULT;
-	return 0;
-}
-
-static int ethtool_set_gso(struct net_device *dev, char __user *useraddr)
-{
-	struct ethtool_value edata;
-
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-	if (edata.data)
-		dev->features |= NETIF_F_GSO;
-	else
-		dev->features &= ~NETIF_F_GSO;
-	return 0;
-}
-
-static int ethtool_get_gro(struct net_device *dev, char __user *useraddr)
-{
-	struct ethtool_value edata = { ETHTOOL_GGRO };
-
-	edata.data = dev->features & NETIF_F_GRO;
-	if (copy_to_user(useraddr, &edata, sizeof(edata)))
-		return -EFAULT;
-	return 0;
-}
-
-static int ethtool_set_gro(struct net_device *dev, char __user *useraddr)
-{
-	struct ethtool_value edata;
-
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (edata.data) {
-		u32 rxcsum = dev->ethtool_ops->get_rx_csum ?
-				dev->ethtool_ops->get_rx_csum(dev) :
-				ethtool_op_get_rx_csum(dev);
-
-		if (!rxcsum)
-			return -EINVAL;
-		dev->features |= NETIF_F_GRO;
-	} else
-		dev->features &= ~NETIF_F_GRO;
-
-	return 0;
+	return dev->ethtool_ops->set_ufo(dev, data);
 }
 
 static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
@@ -1590,33 +1612,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXCSUM:
 		rc = ethtool_set_rx_csum(dev, useraddr);
 		break;
-	case ETHTOOL_GTXCSUM:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_tx_csum ?
-					dev->ethtool_ops->get_tx_csum :
-					ethtool_op_get_tx_csum));
-		break;
-	case ETHTOOL_STXCSUM:
-		rc = ethtool_set_tx_csum(dev, useraddr);
-		break;
-	case ETHTOOL_GSG:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_sg ?
-					dev->ethtool_ops->get_sg :
-					ethtool_op_get_sg));
-		break;
-	case ETHTOOL_SSG:
-		rc = ethtool_set_sg(dev, useraddr);
-		break;
-	case ETHTOOL_GTSO:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_tso ?
-					dev->ethtool_ops->get_tso :
-					ethtool_op_get_tso));
-		break;
-	case ETHTOOL_STSO:
-		rc = ethtool_set_tso(dev, useraddr);
-		break;
 	case ETHTOOL_TEST:
 		rc = ethtool_self_test(dev, useraddr);
 		break;
@@ -1632,21 +1627,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GPERMADDR:
 		rc = ethtool_get_perm_addr(dev, useraddr);
 		break;
-	case ETHTOOL_GUFO:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_ufo ?
-					dev->ethtool_ops->get_ufo :
-					ethtool_op_get_ufo));
-		break;
-	case ETHTOOL_SUFO:
-		rc = ethtool_set_ufo(dev, useraddr);
-		break;
-	case ETHTOOL_GGSO:
-		rc = ethtool_get_gso(dev, useraddr);
-		break;
-	case ETHTOOL_SGSO:
-		rc = ethtool_set_gso(dev, useraddr);
-		break;
 	case ETHTOOL_GFLAGS:
 		rc = ethtool_get_value(dev, useraddr, ethcmd,
 				       (dev->ethtool_ops->get_flags ?
@@ -1677,12 +1657,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXCLSRLINS:
 		rc = ethtool_set_rxnfc(dev, ethcmd, useraddr);
 		break;
-	case ETHTOOL_GGRO:
-		rc = ethtool_get_gro(dev, useraddr);
-		break;
-	case ETHTOOL_SGRO:
-		rc = ethtool_set_gro(dev, useraddr);
-		break;
 	case ETHTOOL_FLASHDEV:
 		rc = ethtool_flash_device(dev, useraddr);
 		break;
@@ -1704,6 +1678,22 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXFHINDIR:
 		rc = ethtool_set_rxfh_indir(dev, useraddr);
 		break;
+	case ETHTOOL_GTXCSUM:
+	case ETHTOOL_GSG:
+	case ETHTOOL_GTSO:
+	case ETHTOOL_GUFO:
+	case ETHTOOL_GGSO:
+	case ETHTOOL_GGRO:
+		rc = ethtool_get_one_feature(dev, useraddr, ethcmd);
+		break;
+	case ETHTOOL_STXCSUM:
+	case ETHTOOL_SSG:
+	case ETHTOOL_STSO:
+	case ETHTOOL_SUFO:
+	case ETHTOOL_SGSO:
+	case ETHTOOL_SGRO:
+		rc = ethtool_set_one_feature(dev, useraddr, ethcmd);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 6/9] net: ethtool: use ndo_fix_features for offload setting
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (3 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 5/9] net: Introduce new feature setting ops Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 4/9] ethtool: factorize get/set_one_feature Michał Mirosław
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller


Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/ethtool.c |   45 ++++++++++++++++++++++++++++++++-------------
 1 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9577396..6599997 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -357,15 +357,21 @@ static void *__ethtool_get_one_feature_actor(struct net_device *dev, u32 ethcmd)
 static int ethtool_get_one_feature(struct net_device *dev,
 	char __user *useraddr, u32 ethcmd)
 {
+	u32 mask = ethtool_get_feature_mask(ethcmd);
 	struct ethtool_value edata = {
 		.cmd = ethcmd,
-		.data = !!(dev->features & ethtool_get_feature_mask(ethcmd)),
+		.data = !!(dev->features & mask),
 	};
-	u32 (*actor)(struct net_device *);
 
-	actor = __ethtool_get_one_feature_actor(dev, ethcmd);
-	if (actor)
-		edata.data = actor(dev);
+	/* compatibility with discrete get_ ops */
+	if (!(dev->hw_features & mask)) {
+		u32 (*actor)(struct net_device *);
+
+		actor = __ethtool_get_one_feature_actor(dev, ethcmd);
+
+		if (actor)
+			edata.data = actor(dev);
+	}
 
 	if (copy_to_user(useraddr, &edata, sizeof(edata)))
 		return -EFAULT;
@@ -386,6 +392,27 @@ static int ethtool_set_one_feature(struct net_device *dev,
 	if (copy_from_user(&edata, useraddr, sizeof(edata)))
 		return -EFAULT;
 
+	mask = ethtool_get_feature_mask(ethcmd);
+	mask &= dev->hw_features;
+	if (mask) {
+		if (edata.data)
+			dev->wanted_features |= mask;
+		else
+			dev->wanted_features &= ~mask;
+
+		netdev_update_features(dev);
+		return 0;
+	}
+
+	/* Driver is not converted to ndo_fix_features or does not
+	 * support changing this offload. In the latter case it won't
+	 * have corresponding ethtool_ops field set.
+	 *
+	 * Following part is to be removed after all drivers advertise
+	 * their changeable features in netdev->hw_features and stop
+	 * using discrete offload setting ops.
+	 */
+
 	switch (ethcmd) {
 	case ETHTOOL_STXCSUM:
 		return __ethtool_set_tx_csum(dev, edata.data);
@@ -395,14 +422,6 @@ static int ethtool_set_one_feature(struct net_device *dev,
 		return __ethtool_set_tso(dev, edata.data);
 	case ETHTOOL_SUFO:
 		return __ethtool_set_ufo(dev, edata.data);
-	case ETHTOOL_SGSO:
-	case ETHTOOL_SGRO:
-		mask = ethtool_get_feature_mask(ethcmd);
-		if (edata.data)
-			dev->features |= mask;
-		else
-			dev->features &= ~mask;
-		return 0;
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 5/9] net: Introduce new feature setting ops
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (2 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 2/9] ethtool: enable GSO and GRO by default Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 6/9] net: ethtool: use ndo_fix_features for offload setting Michał Mirosław
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

This introduces a new framework to handle device features setting.
It consists of:
  - new fields in struct net_device:
	+ hw_features - features that hw/driver supports toggling
	+ wanted_features - features that user wants enabled, when possible
  - new netdev_ops:
	+ feat = ndo_fix_features(dev, feat) - API checking constraints for
		enabling features or their combinations
	+ ndo_set_features(dev) - API updating hardware state to match
		changed dev->features
  - new ethtool commands:
	+ ETHTOOL_GFEATURES/ETHTOOL_SFEATURES: get/set dev->wanted_features
		and trigger device reconfiguration if resulting dev->features
		changed
	+ ETHTOOL_GSTRINGS(ETH_SS_FEATURES): get feature bits names (meaning)

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/ethtool.h   |   85 ++++++++++++++++++++++++++++++
 include/linux/netdevice.h |   37 +++++++++++++-
 net/core/dev.c            |   46 ++++++++++++++--
 net/core/ethtool.c        |  125 ++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 283 insertions(+), 10 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 1908929..806e716 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -251,6 +251,7 @@ enum ethtool_stringset {
 	ETH_SS_STATS,
 	ETH_SS_PRIV_FLAGS,
 	ETH_SS_NTUPLE_FILTERS,
+	ETH_SS_FEATURES,
 };
 
 /* for passing string sets for data tagging */
@@ -523,6 +524,87 @@ struct ethtool_flash {
 	char	data[ETHTOOL_FLASH_MAX_FILENAME];
 };
 
+/* for returning and changing feature sets */
+
+/**
+ * struct ethtool_get_features_block - block with state of 32 features
+ * @available: mask of changeable features
+ * @requested: mask of features requested to be enabled if possible
+ * @active: mask of currently enabled features
+ * @never_changed: mask of features not changeable for any device
+ */
+struct ethtool_get_features_block {
+	__u32	available;
+	__u32	requested;
+	__u32	active;
+	__u32	never_changed;
+};
+
+/**
+ * struct ethtool_gfeatures - command to get state of device's features
+ * @cmd: command number = %ETHTOOL_GFEATURES
+ * @size: in: number of elements in the features[] array;
+ *       out: number of elements in features[] needed to hold all features
+ * @features: state of features
+ */
+struct ethtool_gfeatures {
+	__u32	cmd;
+	__u32	size;
+	struct ethtool_get_features_block features[0];
+};
+
+/**
+ * struct ethtool_set_features_block - block with request for 32 features
+ * @valid: mask of features to be changed
+ * @requested: values of features to be changed
+ */
+struct ethtool_set_features_block {
+	__u32	valid;
+	__u32	requested;
+};
+
+/**
+ * struct ethtool_sfeatures - command to request change in device's features
+ * @cmd: command number = %ETHTOOL_SFEATURES
+ * @size: array size of the features[] array
+ * @features: feature change masks
+ */
+struct ethtool_sfeatures {
+	__u32	cmd;
+	__u32	size;
+	struct ethtool_set_features_block features[0];
+};
+
+/*
+ * %ETHTOOL_SFEATURES changes features present in features[].valid to the
+ * values of corresponding bits in features[].requested. Bits in .requested
+ * not set in .valid or not changeable are ignored.
+ *
+ * Returns %EINVAL when .valid contains undefined or never-changable bits
+ * or size is not equal to required number of features words (32-bit blocks).
+ * Returns >= 0 if request was completed; bits set in the value mean:
+ *   %ETHTOOL_F_UNSUPPORTED - there were bits set in .valid that are not
+ *	changeable (not present in %ETHTOOL_GFEATURES' features[].available)
+ *	those bits were ignored.
+ *   %ETHTOOL_F_WISH - some or all changes requested were recorded but the
+ *      resulting state of bits masked by .valid is not equal to .requested.
+ *      Probably there are other device-specific constraints on some features
+ *      in the set. When %ETHTOOL_F_UNSUPPORTED is set, .valid is considered
+ *      here as though ignored bits were cleared.
+ *
+ * Meaning of bits in the masks are obtained by %ETHTOOL_GSSET_INFO (number of
+ * bits in the arrays - always multiple of 32) and %ETHTOOL_GSTRINGS commands
+ * for ETH_SS_FEATURES string set. First entry in the table corresponds to least
+ * significant bit in features[0] fields. Empty strings mark undefined features.
+ */
+enum ethtool_sfeatures_retval_bits {
+	ETHTOOL_F_UNSUPPORTED__BIT,
+	ETHTOOL_F_WISH__BIT,
+};
+
+#define ETHTOOL_F_UNSUPPORTED   (1 << ETHTOOL_F_UNSUPPORTED__BIT)
+#define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
+
 #ifdef __KERNEL__
 
 #include <linux/rculist.h>
@@ -744,6 +826,9 @@ struct ethtool_ops {
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
 
+#define ETHTOOL_GFEATURES	0x0000003a /* Get device offload settings */
+#define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
+
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dede3fd..85f67e2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -791,6 +791,18 @@ struct netdev_tc_txq {
  *
  * int (*ndo_del_slave)(struct net_device *dev, struct net_device *slave_dev);
  *	Called to release previously enslaved netdev.
+ *
+ *      Feature/offload setting functions.
+ * u32 (*ndo_fix_features)(struct net_device *dev, u32 features);
+ *	Adjusts the requested feature flags according to device-specific
+ *	constraints, and returns the resulting flags. Must not modify
+ *	the device state.
+ *
+ * int (*ndo_set_features)(struct net_device *dev, u32 features);
+ *	Called to update device configuration to new features. Passed
+ *	feature set might be less than what was returned by ndo_fix_features()).
+ *	Must return >0 or -errno if it changed dev->features itself.
+ *
  */
 #define HAVE_NET_DEVICE_OPS
 struct net_device_ops {
@@ -874,6 +886,10 @@ struct net_device_ops {
 						 struct net_device *slave_dev);
 	int			(*ndo_del_slave)(struct net_device *dev,
 						 struct net_device *slave_dev);
+	u32			(*ndo_fix_features)(struct net_device *dev,
+						    u32 features);
+	int			(*ndo_set_features)(struct net_device *dev,
+						    u32 features);
 };
 
 /*
@@ -925,12 +941,18 @@ struct net_device {
 	struct list_head	napi_list;
 	struct list_head	unreg_list;
 
-	/* Net device features */
+	/* currently active device features */
 	u32			features;
-
+	/* user-changeable features */
+	u32			hw_features;
+	/* user-requested features */
+	u32			wanted_features;
 	/* VLAN feature mask */
 	u32			vlan_features;
 
+	/* Net device feature bits; if you change something,
+	 * also update netdev_features_strings[] in ethtool.c */
+
 #define NETIF_F_SG		1	/* Scatter/gather IO. */
 #define NETIF_F_IP_CSUM		2	/* Can checksum TCP/UDP over IPv4. */
 #define NETIF_F_NO_CSUM		4	/* Does not require checksum. F.e. loopack. */
@@ -966,6 +988,12 @@ struct net_device {
 #define NETIF_F_TSO6		(SKB_GSO_TCPV6 << NETIF_F_GSO_SHIFT)
 #define NETIF_F_FSO		(SKB_GSO_FCOE << NETIF_F_GSO_SHIFT)
 
+	/* Features valid for ethtool to change */
+	/* = all defined minus driver/device-class-related */
+#define NETIF_F_NEVER_CHANGE	(NETIF_F_HIGHDMA | NETIF_F_VLAN_CHALLENGED | \
+				  NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
+#define NETIF_F_ETHTOOL_BITS	(0x1f3fffff & ~NETIF_F_NEVER_CHANGE)
+
 	/* List of features with software fallbacks. */
 #define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
 				 NETIF_F_TSO6 | NETIF_F_UFO)
@@ -2428,8 +2456,13 @@ extern char *netdev_drivername(const struct net_device *dev, char *buffer, int l
 
 extern void linkwatch_run_queue(void);
 
+static inline u32 netdev_get_wanted_features(struct net_device *dev)
+{
+	return (dev->features & ~dev->hw_features) | dev->wanted_features;
+}
 u32 netdev_increment_features(u32 all, u32 one, u32 mask);
 u32 netdev_fix_features(struct net_device *dev, u32 features);
+void netdev_update_features(struct net_device *dev);
 
 void netif_stacked_transfer_operstate(const struct net_device *rootdev,
 					struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 8686f6f..4f69439 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5302,6 +5302,37 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
 }
 EXPORT_SYMBOL(netdev_fix_features);
 
+void netdev_update_features(struct net_device *dev)
+{
+	u32 features;
+	int err = 0;
+
+	features = netdev_get_wanted_features(dev);
+
+	if (dev->netdev_ops->ndo_fix_features)
+		features = dev->netdev_ops->ndo_fix_features(dev, features);
+
+	/* driver might be less strict about feature dependencies */
+	features = netdev_fix_features(dev, features);
+
+	if (dev->features == features)
+		return;
+
+	netdev_info(dev, "Features changed: 0x%08x -> 0x%08x\n",
+		dev->features, features);
+
+	if (dev->netdev_ops->ndo_set_features)
+		err = dev->netdev_ops->ndo_set_features(dev, features);
+
+	if (!err)
+		dev->features = features;
+	else if (err < 0)
+		netdev_err(dev,
+			"set_features() failed (%d); wanted 0x%08x, left 0x%08x\n",
+			err, features, dev->features);
+}
+EXPORT_SYMBOL(netdev_update_features);
+
 /**
  *	netif_stacked_transfer_operstate -	transfer operstate
  *	@rootdev: the root or lower level device to transfer state from
@@ -5436,15 +5467,18 @@ int register_netdevice(struct net_device *dev)
 	if (dev->iflink == -1)
 		dev->iflink = dev->ifindex;
 
-	/* Enable software offloads by default - will be stripped in
-	 * netdev_fix_features() if not supported. */
-	dev->features |= NETIF_F_SOFT_FEATURES;
+	/* Transfer changeable features to wanted_features and enable
+	 * software offloads (GSO and GRO).
+	 */
+	dev->hw_features |= NETIF_F_SOFT_FEATURES;
+	dev->wanted_features = (dev->features & dev->hw_features)
+		| NETIF_F_SOFT_FEATURES;
 
 	/* Avoid warning from netdev_fix_features() for GSO without SG */
-	if (!(dev->features & NETIF_F_SG))
-		dev->features &= ~NETIF_F_GSO;
+	if (!(dev->wanted_features & NETIF_F_SG))
+		dev->wanted_features &= ~NETIF_F_GSO;
 
-	dev->features = netdev_fix_features(dev, dev->features);
+	netdev_update_features(dev);
 
 	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
 	 * vlan_dev_init() will do the dev->features check, so these features
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index c3fb8f9..9577396 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -172,10 +172,120 @@ EXPORT_SYMBOL(ethtool_ntuple_flush);
 
 /* Handlers for each ethtool command */
 
+#define ETHTOOL_DEV_FEATURE_WORDS	1
+
+static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_gfeatures cmd = {
+		.cmd = ETHTOOL_GFEATURES,
+		.size = ETHTOOL_DEV_FEATURE_WORDS,
+	};
+	struct ethtool_get_features_block features[ETHTOOL_DEV_FEATURE_WORDS] = {
+		{
+			.available = dev->hw_features,
+			.requested = dev->wanted_features,
+			.active = dev->features,
+			.never_changed = NETIF_F_NEVER_CHANGE,
+		},
+	};
+	u32 __user *sizeaddr;
+	u32 copy_size;
+
+	sizeaddr = useraddr + offsetof(struct ethtool_gfeatures, size);
+	if (get_user(copy_size, sizeaddr))
+		return -EFAULT;
+
+	if (copy_size > ETHTOOL_DEV_FEATURE_WORDS)
+		copy_size = ETHTOOL_DEV_FEATURE_WORDS;
+
+	if (copy_to_user(useraddr, &cmd, sizeof(cmd)))
+		return -EFAULT;
+	useraddr += sizeof(cmd);
+	if (copy_to_user(useraddr, features, copy_size * sizeof(*features)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_sfeatures cmd;
+	struct ethtool_set_features_block features[ETHTOOL_DEV_FEATURE_WORDS];
+	int ret = 0;
+
+	if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
+		return -EFAULT;
+	useraddr += sizeof(cmd);
+
+	if (cmd.size != ETHTOOL_DEV_FEATURE_WORDS)
+		return -EINVAL;
+
+	if (copy_from_user(features, useraddr, sizeof(features)))
+		return -EFAULT;
+
+	if (features[0].valid & ~NETIF_F_ETHTOOL_BITS)
+		return -EINVAL;
+
+	if (features[0].valid & ~dev->hw_features) {
+		features[0].valid &= dev->hw_features;
+		ret |= ETHTOOL_F_UNSUPPORTED;
+	}
+
+	dev->wanted_features &= ~features[0].valid;
+	dev->wanted_features |= features[0].valid & features[0].requested;
+	netdev_update_features(dev);
+
+	if ((dev->wanted_features ^ dev->features) & features[0].valid)
+		ret |= ETHTOOL_F_WISH;
+
+	return ret;
+}
+
+static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GSTRING_LEN] = {
+	/* NETIF_F_SG */              "tx-scatter-gather",
+	/* NETIF_F_IP_CSUM */         "tx-checksum-ipv4",
+	/* NETIF_F_NO_CSUM */         "tx-checksum-unneeded",
+	/* NETIF_F_HW_CSUM */         "tx-checksum-ip-generic",
+	/* NETIF_F_IPV6_CSUM */       "tx_checksum-ipv6",
+	/* NETIF_F_HIGHDMA */         "highdma",
+	/* NETIF_F_FRAGLIST */        "tx-scatter-gather-fraglist",
+	/* NETIF_F_HW_VLAN_TX */      "tx-vlan-hw-insert",
+
+	/* NETIF_F_HW_VLAN_RX */      "rx-vlan-hw-parse",
+	/* NETIF_F_HW_VLAN_FILTER */  "rx-vlan-filter",
+	/* NETIF_F_VLAN_CHALLENGED */ "vlan-challenged",
+	/* NETIF_F_GSO */             "tx-generic-segmentation",
+	/* NETIF_F_LLTX */            "tx-lockless",
+	/* NETIF_F_NETNS_LOCAL */     "netns-local",
+	/* NETIF_F_GRO */             "rx-gro",
+	/* NETIF_F_LRO */             "rx-lro",
+
+	/* NETIF_F_TSO */             "tx-tcp-segmentation",
+	/* NETIF_F_UFO */             "tx-udp-fragmentation",
+	/* NETIF_F_GSO_ROBUST */      "tx-gso-robust",
+	/* NETIF_F_TSO_ECN */         "tx-tcp-ecn-segmentation",
+	/* NETIF_F_TSO6 */            "tx-tcp6-segmentation",
+	/* NETIF_F_FSO */             "tx-fcoe-segmentation",
+	"",
+	"",
+
+	/* NETIF_F_FCOE_CRC */        "tx-checksum-fcoe-crc",
+	/* NETIF_F_SCTP_CSUM */       "tx-checksum-sctp",
+	/* NETIF_F_FCOE_MTU */        "fcoe-mtu",
+	/* NETIF_F_NTUPLE */          "rx-ntuple-filter",
+	/* NETIF_F_RXHASH */          "rx-hashing",
+	"",
+	"",
+	"",
+};
+
 static int __ethtool_get_sset_count(struct net_device *dev, int sset)
 {
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 
+	if (sset == ETH_SS_FEATURES)
+		return ARRAY_SIZE(netdev_features_strings);
+
 	if (ops && ops->get_sset_count && ops->get_strings)
 		return ops->get_sset_count(dev, sset);
 	else
@@ -187,8 +297,12 @@ static void __ethtool_get_strings(struct net_device *dev,
 {
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 
-	/* ops->get_strings is valid because checked earlier */
-	ops->get_strings(dev, stringset, data);
+	if (stringset == ETH_SS_FEATURES)
+		memcpy(data, netdev_features_strings,
+			sizeof(netdev_features_strings));
+	else
+		/* ops->get_strings is valid because checked earlier */
+		ops->get_strings(dev, stringset, data);
 }
 
 static u32 ethtool_get_feature_mask(u32 eth_cmd)
@@ -1533,6 +1647,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GRXCLSRLCNT:
 	case ETHTOOL_GRXCLSRULE:
 	case ETHTOOL_GRXCLSRLALL:
+	case ETHTOOL_GFEATURES:
 		break;
 	default:
 		if (!capable(CAP_NET_ADMIN))
@@ -1678,6 +1793,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SRXFHINDIR:
 		rc = ethtool_set_rxfh_indir(dev, useraddr);
 		break;
+	case ETHTOOL_GFEATURES:
+		rc = ethtool_get_features(dev, useraddr);
+		break;
+	case ETHTOOL_SFEATURES:
+		rc = ethtool_set_features(dev, useraddr);
+		break;
 	case ETHTOOL_GTXCSUM:
 	case ETHTOOL_GSG:
 	case ETHTOOL_GTSO:
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 8/9] net: introduce NETIF_F_RXCSUM
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (7 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 7/9] net: use ndo_fix_features for ethtool_ops->set_flags Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-17 22:56 ` [PATCH v6 0/9] net: Unified offload configuration David Miller
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

Introduce NETIF_F_RXCSUM to replace device-private flags for RX checksum
offload. Integrate it with ndo_fix_features.

ethtool_op_get_rx_csum() is removed altogether as nothing in-tree uses it.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/ethtool.h   |    1 -
 include/linux/netdevice.h |    5 +++-
 net/core/ethtool.c        |   47 ++++++++++++++++++++++-----------------------
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 806e716..54d776c 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -625,7 +625,6 @@ struct net_device;
 
 /* Some generic methods drivers may use in their ethtool_ops */
 u32 ethtool_op_get_link(struct net_device *dev);
-u32 ethtool_op_get_rx_csum(struct net_device *dev);
 u32 ethtool_op_get_tx_csum(struct net_device *dev);
 int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
 int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 85f67e2..ffe56c1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -977,6 +977,7 @@ struct net_device {
 #define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
 #define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
 #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
+#define NETIF_F_RXCSUM		(1 << 29) /* Receive checksumming offload */
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16
@@ -992,7 +993,7 @@ struct net_device {
 	/* = all defined minus driver/device-class-related */
 #define NETIF_F_NEVER_CHANGE	(NETIF_F_HIGHDMA | NETIF_F_VLAN_CHALLENGED | \
 				  NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
-#define NETIF_F_ETHTOOL_BITS	(0x1f3fffff & ~NETIF_F_NEVER_CHANGE)
+#define NETIF_F_ETHTOOL_BITS	(0x3f3fffff & ~NETIF_F_NEVER_CHANGE)
 
 	/* List of features with software fallbacks. */
 #define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
@@ -2510,6 +2511,8 @@ static inline int dev_ethtool_get_settings(struct net_device *dev,
 
 static inline u32 dev_ethtool_get_rx_csum(struct net_device *dev)
 {
+	if (dev->hw_features & NETIF_F_RXCSUM)
+		return !!(dev->features & NETIF_F_RXCSUM);
 	if (!dev->ethtool_ops || !dev->ethtool_ops->get_rx_csum)
 		return 0;
 	return dev->ethtool_ops->get_rx_csum(dev);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 65b3d50..66cdc76 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -34,12 +34,6 @@ u32 ethtool_op_get_link(struct net_device *dev)
 }
 EXPORT_SYMBOL(ethtool_op_get_link);
 
-u32 ethtool_op_get_rx_csum(struct net_device *dev)
-{
-	return (dev->features & NETIF_F_ALL_CSUM) != 0;
-}
-EXPORT_SYMBOL(ethtool_op_get_rx_csum);
-
 u32 ethtool_op_get_tx_csum(struct net_device *dev)
 {
 	return (dev->features & NETIF_F_ALL_CSUM) != 0;
@@ -274,7 +268,7 @@ static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
 	/* NETIF_F_FCOE_MTU */        "fcoe-mtu",
 	/* NETIF_F_NTUPLE */          "rx-ntuple-filter",
 	/* NETIF_F_RXHASH */          "rx-hashing",
-	"",
+	/* NETIF_F_RXCSUM */          "rx-checksum",
 	"",
 	"",
 };
@@ -313,6 +307,9 @@ static u32 ethtool_get_feature_mask(u32 eth_cmd)
 	case ETHTOOL_GTXCSUM:
 	case ETHTOOL_STXCSUM:
 		return NETIF_F_ALL_CSUM | NETIF_F_SCTP_CSUM;
+	case ETHTOOL_GRXCSUM:
+	case ETHTOOL_SRXCSUM:
+		return NETIF_F_RXCSUM;
 	case ETHTOOL_GSG:
 	case ETHTOOL_SSG:
 		return NETIF_F_SG;
@@ -343,6 +340,8 @@ static void *__ethtool_get_one_feature_actor(struct net_device *dev, u32 ethcmd)
 	switch (ethcmd) {
 	case ETHTOOL_GTXCSUM:
 		return ops->get_tx_csum;
+	case ETHTOOL_GRXCSUM:
+		return ops->get_rx_csum;
 	case ETHTOOL_SSG:
 		return ops->get_sg;
 	case ETHTOOL_STSO:
@@ -354,6 +353,11 @@ static void *__ethtool_get_one_feature_actor(struct net_device *dev, u32 ethcmd)
 	}
 }
 
+static u32 __ethtool_get_rx_csum_oldbug(struct net_device *dev)
+{
+	return !!(dev->features & NETIF_F_ALL_CSUM);
+}
+
 static int ethtool_get_one_feature(struct net_device *dev,
 	char __user *useraddr, u32 ethcmd)
 {
@@ -369,6 +373,10 @@ static int ethtool_get_one_feature(struct net_device *dev,
 
 		actor = __ethtool_get_one_feature_actor(dev, ethcmd);
 
+		/* bug compatibility with old get_rx_csum */
+		if (ethcmd == ETHTOOL_GRXCSUM && !actor)
+			actor = __ethtool_get_rx_csum_oldbug;
+
 		if (actor)
 			edata.data = actor(dev);
 	}
@@ -379,6 +387,7 @@ static int ethtool_get_one_feature(struct net_device *dev,
 }
 
 static int __ethtool_set_tx_csum(struct net_device *dev, u32 data);
+static int __ethtool_set_rx_csum(struct net_device *dev, u32 data);
 static int __ethtool_set_sg(struct net_device *dev, u32 data);
 static int __ethtool_set_tso(struct net_device *dev, u32 data);
 static int __ethtool_set_ufo(struct net_device *dev, u32 data);
@@ -416,6 +425,8 @@ static int ethtool_set_one_feature(struct net_device *dev,
 	switch (ethcmd) {
 	case ETHTOOL_STXCSUM:
 		return __ethtool_set_tx_csum(dev, edata.data);
+	case ETHTOOL_SRXCSUM:
+		return __ethtool_set_rx_csum(dev, edata.data);
 	case ETHTOOL_SSG:
 		return __ethtool_set_sg(dev, edata.data);
 	case ETHTOOL_STSO:
@@ -1404,20 +1415,15 @@ static int __ethtool_set_tx_csum(struct net_device *dev, u32 data)
 	return dev->ethtool_ops->set_tx_csum(dev, data);
 }
 
-static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
+static int __ethtool_set_rx_csum(struct net_device *dev, u32 data)
 {
-	struct ethtool_value edata;
-
 	if (!dev->ethtool_ops->set_rx_csum)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&edata, useraddr, sizeof(edata)))
-		return -EFAULT;
-
-	if (!edata.data && dev->ethtool_ops->set_sg)
+	if (!data)
 		dev->features &= ~NETIF_F_GRO;
 
-	return dev->ethtool_ops->set_rx_csum(dev, edata.data);
+	return dev->ethtool_ops->set_rx_csum(dev, data);
 }
 
 static int __ethtool_set_tso(struct net_device *dev, u32 data)
@@ -1765,15 +1771,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SPAUSEPARAM:
 		rc = ethtool_set_pauseparam(dev, useraddr);
 		break;
-	case ETHTOOL_GRXCSUM:
-		rc = ethtool_get_value(dev, useraddr, ethcmd,
-				       (dev->ethtool_ops->get_rx_csum ?
-					dev->ethtool_ops->get_rx_csum :
-					ethtool_op_get_rx_csum));
-		break;
-	case ETHTOOL_SRXCSUM:
-		rc = ethtool_set_rx_csum(dev, useraddr);
-		break;
 	case ETHTOOL_TEST:
 		rc = ethtool_self_test(dev, useraddr);
 		break;
@@ -1846,6 +1843,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 		rc = ethtool_set_features(dev, useraddr);
 		break;
 	case ETHTOOL_GTXCSUM:
+	case ETHTOOL_GRXCSUM:
 	case ETHTOOL_GSG:
 	case ETHTOOL_GTSO:
 	case ETHTOOL_GUFO:
@@ -1854,6 +1852,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 		rc = ethtool_get_one_feature(dev, useraddr, ethcmd);
 		break;
 	case ETHTOOL_STXCSUM:
+	case ETHTOOL_SRXCSUM:
 	case ETHTOOL_SSG:
 	case ETHTOOL_STSO:
 	case ETHTOOL_SUFO:
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 9/9] loopback: convert to hw_features
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (5 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 4/9] ethtool: factorize get/set_one_feature Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 7/9] net: use ndo_fix_features for ethtool_ops->set_flags Michał Mirosław
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

This also enables TSOv6, TSO-ECN, and UFO as loopback clearly can handle them.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/loopback.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 2d9663a..ea0dc45 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -129,10 +129,6 @@ static u32 always_on(struct net_device *dev)
 
 static const struct ethtool_ops loopback_ethtool_ops = {
 	.get_link		= always_on,
-	.set_tso		= ethtool_op_set_tso,
-	.get_tx_csum		= always_on,
-	.get_sg			= always_on,
-	.get_rx_csum		= always_on,
 };
 
 static int loopback_dev_init(struct net_device *dev)
@@ -169,9 +165,12 @@ static void loopback_setup(struct net_device *dev)
 	dev->type		= ARPHRD_LOOPBACK;	/* 0x0001*/
 	dev->flags		= IFF_LOOPBACK;
 	dev->priv_flags	       &= ~IFF_XMIT_DST_RELEASE;
+	dev->hw_features	= NETIF_F_ALL_TSO | NETIF_F_UFO;
 	dev->features 		= NETIF_F_SG | NETIF_F_FRAGLIST
-		| NETIF_F_TSO
+		| NETIF_F_ALL_TSO
+		| NETIF_F_UFO
 		| NETIF_F_NO_CSUM
+		| NETIF_F_RXCSUM
 		| NETIF_F_HIGHDMA
 		| NETIF_F_LLTX
 		| NETIF_F_NETNS_LOCAL;
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v6 7/9] net: use ndo_fix_features for ethtool_ops->set_flags
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (6 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 9/9] loopback: convert to hw_features Michał Mirosław
@ 2011-02-16  2:59 ` Michał Mirosław
  2011-02-16  2:59 ` [PATCH v6 8/9] net: introduce NETIF_F_RXCSUM Michał Mirosław
  2011-02-17 22:56 ` [PATCH v6 0/9] net: Unified offload configuration David Miller
  9 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-16  2:59 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller


Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/ethtool.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6599997..65b3d50 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -427,6 +427,34 @@ static int ethtool_set_one_feature(struct net_device *dev,
 	}
 }
 
+static int __ethtool_set_flags(struct net_device *dev, u32 data)
+{
+	u32 changed;
+
+	if (data & ~flags_dup_features)
+		return -EINVAL;
+
+	/* legacy set_flags() op */
+	if (dev->ethtool_ops->set_flags) {
+		if (unlikely(dev->hw_features & flags_dup_features))
+			netdev_warn(dev,
+				"driver BUG: mixed hw_features and set_flags()\n");
+		return dev->ethtool_ops->set_flags(dev, data);
+	}
+
+	/* allow changing only bits set in hw_features */
+	changed = (data ^ dev->wanted_features) & flags_dup_features;
+	if (changed & ~dev->hw_features)
+		return (changed & dev->hw_features) ? -EINVAL : -EOPNOTSUPP;
+
+	dev->wanted_features =
+		(dev->wanted_features & ~changed) | data;
+
+	netdev_update_features(dev);
+
+	return 0;
+}
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
@@ -1768,8 +1796,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 					ethtool_op_get_flags));
 		break;
 	case ETHTOOL_SFLAGS:
-		rc = ethtool_set_value(dev, useraddr,
-				       dev->ethtool_ops->set_flags);
+		rc = ethtool_set_value(dev, useraddr, __ethtool_set_flags);
 		break;
 	case ETHTOOL_GPFLAGS:
 		rc = ethtool_get_value(dev, useraddr, ethcmd,
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
                   ` (8 preceding siblings ...)
  2011-02-16  2:59 ` [PATCH v6 8/9] net: introduce NETIF_F_RXCSUM Michał Mirosław
@ 2011-02-17 22:56 ` David Miller
  2011-02-18 14:22   ` Michał Mirosław
  9 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2011-02-17 22:56 UTC (permalink / raw)
  To: mirq-linux; +Cc: netdev, bhutchings

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Wed, 16 Feb 2011 03:59:16 +0100 (CET)

> Here's a v6 of the ethtool unification patch series.
> 
> What's in it?
>  1..4:
> 	cleanups for the core patches
>  5:
> 	the patch - implement unified ethtool setting ops
>  6..7:
> 	implement interoperation between old and new ethtool ops
>  8:
> 	include RX checksum in features and plug it into new framework
>  9:
> 	convert loopback device to new framework
> 
> What is it good for?
>  - unifies driver behaviour wrt hardware offloads
>  - removes a lot of boilerplate code from drivers
>  - allows better fine-grained control over used offloads

Applied to net-next-2.6, please send any bug fixes relative to this.

Please get rid of that annoying message spit out by netif_features_change(),
it's just spam.  If we want notifications for stuff like this, use a
non-unicast netlink message so those who want to hear it can do so.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-17 22:56 ` [PATCH v6 0/9] net: Unified offload configuration David Miller
@ 2011-02-18 14:22   ` Michał Mirosław
  2011-02-18 14:29     ` Ben Hutchings
  0 siblings, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2011-02-18 14:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, bhutchings

On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Wed, 16 Feb 2011 03:59:16 +0100 (CET)
> > Here's a v6 of the ethtool unification patch series.
> > 
> > What's in it?
> >  1..4:
> > 	cleanups for the core patches
> >  5:
> > 	the patch - implement unified ethtool setting ops
> >  6..7:
> > 	implement interoperation between old and new ethtool ops
> >  8:
> > 	include RX checksum in features and plug it into new framework
> >  9:
> > 	convert loopback device to new framework
> > 
> > What is it good for?
> >  - unifies driver behaviour wrt hardware offloads
> >  - removes a lot of boilerplate code from drivers
> >  - allows better fine-grained control over used offloads
> Applied to net-next-2.6, please send any bug fixes relative to this.
> 
> Please get rid of that annoying message spit out by netif_features_change(),
> it's just spam.  If we want notifications for stuff like this, use a
> non-unicast netlink message so those who want to hear it can do so.

You mean netdev_update_features() "Features changed" message? Is it ok
to just demote it to DEBUG level or you want to remove it altogether?
What about netdev_fix_features() messages?

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-18 14:22   ` Michał Mirosław
@ 2011-02-18 14:29     ` Ben Hutchings
  2011-02-18 19:27       ` Michał Mirosław
  2011-02-18 20:06       ` David Miller
  0 siblings, 2 replies; 19+ messages in thread
From: Ben Hutchings @ 2011-02-18 14:29 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: David Miller, netdev

On Fri, 2011-02-18 at 15:22 +0100, Michał Mirosław wrote:
> On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
[...]
> > Please get rid of that annoying message spit out by netif_features_change(),
> > it's just spam.  If we want notifications for stuff like this, use a
> > non-unicast netlink message so those who want to hear it can do so.
> 
> You mean netdev_update_features() "Features changed" message? Is it ok
> to just demote it to DEBUG level or you want to remove it altogether?
> What about netdev_fix_features() messages?

I think you need to emit these messages at 'error' severity when fixing
up features for a newly-added device, but at 'debug' later on.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-18 14:29     ` Ben Hutchings
@ 2011-02-18 19:27       ` Michał Mirosław
  2011-02-18 20:06       ` David Miller
  1 sibling, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-18 19:27 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev

On Fri, Feb 18, 2011 at 02:29:31PM +0000, Ben Hutchings wrote:
> On Fri, 2011-02-18 at 15:22 +0100, Michał Mirosław wrote:
> > On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
> [...]
> > > Please get rid of that annoying message spit out by netif_features_change(),
> > > it's just spam.  If we want notifications for stuff like this, use a
> > > non-unicast netlink message so those who want to hear it can do so.
> > You mean netdev_update_features() "Features changed" message? Is it ok
> > to just demote it to DEBUG level or you want to remove it altogether?
> > What about netdev_fix_features() messages?
> I think you need to emit these messages at 'error' severity when fixing
> up features for a newly-added device, but at 'debug' later on.

Why the difference? Those messages on startup matter mostly to developers
and later mostly to users. Anyway, the conditions in netdev_fix_features()
are all constant (dependent only on net core implementation).

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-18 14:29     ` Ben Hutchings
  2011-02-18 19:27       ` Michał Mirosław
@ 2011-02-18 20:06       ` David Miller
  2011-02-19 12:28         ` Michał Mirosław
                           ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: David Miller @ 2011-02-18 20:06 UTC (permalink / raw)
  To: bhutchings; +Cc: mirq-linux, netdev

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 18 Feb 2011 14:29:31 +0000

> On Fri, 2011-02-18 at 15:22 +0100, Michał Mirosław wrote:
>> On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
> [...]
>> > Please get rid of that annoying message spit out by netif_features_change(),
>> > it's just spam.  If we want notifications for stuff like this, use a
>> > non-unicast netlink message so those who want to hear it can do so.
>> 
>> You mean netdev_update_features() "Features changed" message? Is it ok
>> to just demote it to DEBUG level or you want to remove it altogether?
>> What about netdev_fix_features() messages?
> 
> I think you need to emit these messages at 'error' severity when fixing
> up features for a newly-added device, but at 'debug' later on.

I get one several minutes after every boot for a completely
unregistered device for some reason:

[119704.730965] (unregistered net_device): Features changed: 0x00011065 -> 0x00015065

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v6 0/9] net: Unified offload configuration
  2011-02-18 20:06       ` David Miller
@ 2011-02-19 12:28         ` Michał Mirosław
  2011-02-19 12:46         ` [PATCH 1/1] Fix "(unregistered net_device): Features changed" message Michał Mirosław
  2011-02-22 14:42         ` [PATCH] net: avoid initial "Features " Michał Mirosław
  2 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-19 12:28 UTC (permalink / raw)
  To: David Miller; +Cc: bhutchings, netdev

On Fri, Feb 18, 2011 at 12:06:17PM -0800, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Fri, 18 Feb 2011 14:29:31 +0000
> 
> > On Fri, 2011-02-18 at 15:22 +0100, Michał Mirosław wrote:
> >> On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
> > [...]
> >> > Please get rid of that annoying message spit out by netif_features_change(),
> >> > it's just spam.  If we want notifications for stuff like this, use a
> >> > non-unicast netlink message so those who want to hear it can do so.
> >> You mean netdev_update_features() "Features changed" message? Is it ok
> >> to just demote it to DEBUG level or you want to remove it altogether?
> >> What about netdev_fix_features() messages?
> > I think you need to emit these messages at 'error' severity when fixing
> > up features for a newly-added device, but at 'debug' later on.
> I get one several minutes after every boot for a completely
> unregistered device for some reason:
> 
> [119704.730965] (unregistered net_device): Features changed: 0x00011065 -> 0x00015065

Hmm. That's because netdev_update_features() get's called before changing
netdev->reg_state. I wonder if moving the feature update just after
"dev->reg_state = NETREG_REGISTERED;" will be correct fix.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/1] Fix "(unregistered net_device): Features changed" message
  2011-02-18 20:06       ` David Miller
  2011-02-19 12:28         ` Michał Mirosław
@ 2011-02-19 12:46         ` Michał Mirosław
  2011-02-22 14:42         ` [PATCH] net: avoid initial "Features " Michał Mirosław
  2 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-19 12:46 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

Fix netdev_update_features() messages on register time by moving
the call further in register_netdevice(). When
netdev->reg_state != NETREG_REGISTERED, netdev_name() returns
"(unregistered netdevice)" even if the dev's name is already filled.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/dev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 4f69439..5d8e13e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5478,8 +5478,6 @@ int register_netdevice(struct net_device *dev)
 	if (!(dev->wanted_features & NETIF_F_SG))
 		dev->wanted_features &= ~NETIF_F_GSO;
 
-	netdev_update_features(dev);
-
 	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
 	 * vlan_dev_init() will do the dev->features check, so these features
 	 * are enabled only if supported by underlying device.
@@ -5496,6 +5494,8 @@ int register_netdevice(struct net_device *dev)
 		goto err_uninit;
 	dev->reg_state = NETREG_REGISTERED;
 
+	netdev_update_features(dev);
+
 	/*
 	 *	Default initial state at registry is that the
 	 *	device is present.
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] net: avoid initial "Features changed" message
  2011-02-18 20:06       ` David Miller
  2011-02-19 12:28         ` Michał Mirosław
  2011-02-19 12:46         ` [PATCH 1/1] Fix "(unregistered net_device): Features changed" message Michał Mirosław
@ 2011-02-22 14:42         ` Michał Mirosław
  2011-02-22 14:43           ` Michał Mirosław
  2 siblings, 1 reply; 19+ messages in thread
From: Michał Mirosław @ 2011-02-22 14:42 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

Avoid "Features changed" message and ndo_set_features call on device
registration caused by automatic enabling of GSO and GRO. Driver should
have already enabled hardware offloads it set in features, so the
ndo_set_features() is not needed at registration time unless features
change because of other dependencies.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/core/dev.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 33f4318..039a9a6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5475,12 +5475,14 @@ int register_netdevice(struct net_device *dev)
 	 * software offloads (GSO and GRO).
 	 */
 	dev->hw_features |= NETIF_F_SOFT_FEATURES;
-	dev->wanted_features = (dev->features & dev->hw_features)
-		| NETIF_F_SOFT_FEATURES;
+	dev->features |= NETIF_F_SOFT_FEATURES;
+	dev->wanted_features = dev->features & dev->hw_features;
 
 	/* Avoid warning from netdev_fix_features() for GSO without SG */
-	if (!(dev->wanted_features & NETIF_F_SG))
+	if (!(dev->wanted_features & NETIF_F_SG)) {
 		dev->wanted_features &= ~NETIF_F_GSO;
+		dev->features &= ~NETIF_F_GSO;
+	}
 
 	/* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
 	 * vlan_dev_init() will do the dev->features check, so these features
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: avoid initial "Features changed" message
  2011-02-22 14:42         ` [PATCH] net: avoid initial "Features " Michał Mirosław
@ 2011-02-22 14:43           ` Michał Mirosław
  0 siblings, 0 replies; 19+ messages in thread
From: Michał Mirosław @ 2011-02-22 14:43 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, David Miller

On Tue, Feb 22, 2011 at 03:42:06PM +0100, Michał Mirosław wrote:
> Avoid "Features changed" message and ndo_set_features call on device
> registration caused by automatic enabling of GSO and GRO. Driver should
> have already enabled hardware offloads it set in features, so the
> ndo_set_features() is not needed at registration time unless features
> change because of other dependencies.
[cut patch]

Ah. This depends on 'Fix "(unregistered net_device): Features changed" message'
patch I sent couple days ago.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-02-22 14:43 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-16  2:59 [PATCH v6 0/9] net: Unified offload configuration Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 3/9] ethtool: factorize ethtool_get_strings() and ethtool_get_sset_count() Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 1/9] ethtool: move EXPORT_SYMBOL(ethtool_op_set_tx_csum) to correct place Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 2/9] ethtool: enable GSO and GRO by default Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 5/9] net: Introduce new feature setting ops Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 6/9] net: ethtool: use ndo_fix_features for offload setting Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 4/9] ethtool: factorize get/set_one_feature Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 9/9] loopback: convert to hw_features Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 7/9] net: use ndo_fix_features for ethtool_ops->set_flags Michał Mirosław
2011-02-16  2:59 ` [PATCH v6 8/9] net: introduce NETIF_F_RXCSUM Michał Mirosław
2011-02-17 22:56 ` [PATCH v6 0/9] net: Unified offload configuration David Miller
2011-02-18 14:22   ` Michał Mirosław
2011-02-18 14:29     ` Ben Hutchings
2011-02-18 19:27       ` Michał Mirosław
2011-02-18 20:06       ` David Miller
2011-02-19 12:28         ` Michał Mirosław
2011-02-19 12:46         ` [PATCH 1/1] Fix "(unregistered net_device): Features changed" message Michał Mirosław
2011-02-22 14:42         ` [PATCH] net: avoid initial "Features " Michał Mirosław
2011-02-22 14:43           ` Michał Mirosław

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.