[PATCH v4 3/5] net: use ndo_fix_features for ethtool_ops->set_flags

[PATCH v4 3/5] net: use ndo_fix_features for ethtool_ops->set_flags

[Michał Mirosław]

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 555accf..6e7c6f2 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -240,6 +240,34 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
 	return ret;
 }
 
+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 -EOPNOTSUPP;
+
+	dev->wanted_features =
+		(dev->wanted_features & ~changed) | data;
+
+	netdev_update_features(dev);
+
+	return 0;
+}
+
 static u32 ethtool_get_feature_mask(u32 eth_cmd)
 {
 	/* feature masks of legacy discrete ethtool ops */
@@ -1733,8 +1761,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

[PATCH v4 2/5] net: ethtool: use ndo_fix_features for offload setting

[Michał Mirosław]

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>                                                    

---
 net/core/ethtool.c |  262 ++++++++++++++++++++++++----------------------------
 1 files changed, 119 insertions(+), 143 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 2f1b448..555accf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -240,6 +240,96 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
 	return ret;
 }
 
+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 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)),
+	};
+
+	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;
+
+	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);
+	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);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 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",
@@ -1218,6 +1308,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)
@@ -1232,26 +1325,21 @@ 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);
 }
-EXPORT_SYMBOL(ethtool_op_set_tx_csum);
 
 static int ethtool_set_rx_csum(struct net_device *dev, char __user *useraddr)
 {
@@ -1269,108 +1357,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)
@@ -1703,33 +1711,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;
@@ -1745,21 +1726,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 ?
@@ -1790,12 +1756,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;
@@ -1823,6 +1783,22 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SFEATURES:
 		rc = ethtool_set_features(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

[PATCH v4 0/5] net: Unified offload configuration

[Michał Mirosław]

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

What's in it?
 1:
	the patch - implement unified ethtool setting ops
 2..3:
	implement interoperation between old and new ethtool ops
 4:
	include RX checksum in features and plug it into new framework
 5:
	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

I'm testing this on ARM Gemini arch now. Patch to ethtool userspace tool
will follow this series. I'm not fond of the GFEATURES output I implemented -
please throw some suggestions on it if you can.

Driver conversions stay the same as in v2 - as for v3, I'll keep them
from resending until after the core code gets accepted.

Patches 2,4,5 are unchanged from v3.

Best Regards,
Michał Mirosław


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

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 (5):
  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 |   48 +++++-
 net/core/dev.c            |   49 ++++-
 net/core/ethtool.c        |  481 ++++++++++++++++++++++++++++-----------------
 5 files changed, 480 insertions(+), 193 deletions(-)

-- 
1.7.2.3

[PATCH v4 1/5] net: Introduce new feature setting ops

[Michał Mirosław]

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 |   45 +++++++++++++-
 net/core/dev.c            |   49 +++++++++++++--
 net/core/ethtool.c        |  154 +++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 314 insertions(+), 19 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 c7d7074..4a3e554 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -783,6 +783,17 @@ struct netdev_tc_txq {
  *	Set hardware filter for RFS.  rxq_index is the target queue index;
  *	flow_id is a flow ID to be passed to rps_may_expire_flow() later.
  *	Return the filter ID on success, or a negative error code.
+ *
+ * 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 {
@@ -862,6 +873,10 @@ struct net_device_ops {
 						     u16 rxq_index,
 						     u32 flow_id);
 #endif
+	u32			(*ndo_fix_features)(struct net_device *dev,
+						    u32 features);
+	int			(*ndo_set_features)(struct net_device *dev,
+						    u32 features);
 };
 
 /*
@@ -913,12 +928,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. */
@@ -954,6 +975,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)
@@ -964,6 +991,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.
@@ -972,6 +1005,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;
@@ -2405,8 +2441,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 9109e26..92c690e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5246,6 +5246,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? */
@@ -5268,6 +5274,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
@@ -5402,11 +5439,13 @@ int register_netdevice(struct net_device *dev)
 	if (dev->iflink == -1)
 		dev->iflink = dev->ifindex;
 
-	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;
+	/* 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;
+	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 5984ee0..2f1b448 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)
 {
@@ -171,6 +172,136 @@ 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);
+	else if (ops && ops->get_sset_count)
+		return ops->get_sset_count(dev, sset);
+	else
+		return -EINVAL;
+}
+
+static void __ethtool_get_strings(struct net_device *dev,
+	u32 stringset, u8 *data)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+
+	if (stringset == ETH_SS_FEATURES)
+		memcpy(data, netdev_features_strings,
+			sizeof(netdev_features_strings));
+	else if (ops && ops->get_strings)
+		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 };
@@ -251,14 +382,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;
 
@@ -285,7 +412,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 +1414,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 +1430,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 +1440,7 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
 		goto out;
 	ret = 0;
 
- out:
+out:
 	kfree(data);
 	return ret;
 }
@@ -1500,6 +1623,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))
@@ -1693,6 +1817,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;
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
1.7.2.3

[PATCH v2] ethtool: implement G/SFEATURES calls

[Michał Mirosław]

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---

Changes from v1:
 - removed deprecation of -k/-K
 - mark never-changeable features in userspace

---

 ethtool-copy.h |   89 +++++++++++++++++++++++-
 ethtool.c      |  215 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 296 insertions(+), 8 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 75c3ae7..6430dbd 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.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)
+
 
 /* CMDs currently supported */
 #define ETHTOOL_GSET		0x00000001 /* Get settings. */
@@ -534,7 +616,9 @@ struct ethtool_flash {
 #define ETHTOOL_GMSGLVL		0x00000007 /* Get driver message level */
 #define ETHTOOL_SMSGLVL		0x00000008 /* Set driver msg level. */
 #define ETHTOOL_NWAY_RST	0x00000009 /* Restart autonegotiation. */
-#define ETHTOOL_GLINK		0x0000000a /* Get link status (ethtool_value) */
+/* Get link status for host, i.e. whether the interface *and* the
+ * physical port (if there is one) are up (ethtool_value). */
+#define ETHTOOL_GLINK		0x0000000a
 #define ETHTOOL_GEEPROM		0x0000000b /* Get EEPROM data */
 #define ETHTOOL_SEEPROM		0x0000000c /* Set EEPROM data. */
 #define ETHTOOL_GCOALESCE	0x0000000e /* Get coalesce config */
@@ -585,6 +669,9 @@ struct ethtool_flash {
 #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/ethtool.c b/ethtool.c
index 1afdfe4..af43e47 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -39,6 +39,7 @@
 #include <limits.h>
 #include <ctype.h>
 
+#include <unistd.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
@@ -97,6 +98,9 @@ static int do_gcoalesce(int fd, struct ifreq *ifr);
 static int do_scoalesce(int fd, struct ifreq *ifr);
 static int do_goffload(int fd, struct ifreq *ifr);
 static int do_soffload(int fd, struct ifreq *ifr);
+static void parse_sfeatures_args(int argc, char **argp, int argi);
+static int do_gfeatures(int fd, struct ifreq *ifr);
+static int do_sfeatures(int fd, struct ifreq *ifr);
 static int do_gstats(int fd, struct ifreq *ifr);
 static int rxflow_str_to_type(const char *str);
 static int parse_rxfhashopts(char *optstr, u32 *data);
@@ -133,6 +137,8 @@ static enum {
 	MODE_SRING,
 	MODE_GOFFLOAD,
 	MODE_SOFFLOAD,
+	MODE_GFEATURES,
+	MODE_SFEATURES,
 	MODE_GSTATS,
 	MODE_GNFC,
 	MODE_SNFC,
@@ -211,6 +217,10 @@ static struct option {
 		"		[ ntuple on|off ]\n"
 		"		[ rxhash on|off ]\n"
     },
+    { "-w", "--show-features", MODE_GFEATURES, "Get offload status" },
+    { "-W", "--request-features", MODE_SFEATURES, "Set requested offload",
+		"		[ feature-name on|off [...] ]\n"
+		"		see --show-features output for feature-name strings\n" },
     { "-i", "--driver", MODE_GDRV, "Show driver information" },
     { "-d", "--register-dump", MODE_GREGS, "Do a register dump",
 		"		[ raw on|off ]\n"
@@ -743,8 +753,10 @@ static void parse_generic_cmdline(int argc, char **argp,
 				break;
 			}
 		}
-		if( !found)
+		if( !found) {
+			fprintf(stdout, "bad param: %s\n", argp[i]);
 			show_usage(1);
+		}
 	}
 }
 
@@ -829,6 +841,8 @@ static void parse_cmdline(int argc, char **argp)
 			    (mode == MODE_SRING) ||
 			    (mode == MODE_GOFFLOAD) ||
 			    (mode == MODE_SOFFLOAD) ||
+			    (mode == MODE_GFEATURES) ||
+			    (mode == MODE_SFEATURES) ||
 			    (mode == MODE_GSTATS) ||
 			    (mode == MODE_GNFC) ||
 			    (mode == MODE_SNFC) ||
@@ -919,6 +933,11 @@ static void parse_cmdline(int argc, char **argp)
 				i = argc;
 				break;
 			}
+			if (mode == MODE_SFEATURES) {
+				parse_sfeatures_args(argc, argp, i);
+				i = argc;
+				break;
+			}
 			if (mode == MODE_SNTUPLE) {
 				if (!strcmp(argp[i], "flow-type")) {
 					i += 1;
@@ -1947,21 +1966,30 @@ static int dump_rxfhash(int fhash, u64 val)
 	return 0;
 }
 
-static int doit(void)
+static int get_control_socket(struct ifreq *ifr)
 {
-	struct ifreq ifr;
 	int fd;
 
 	/* Setup our control structures. */
-	memset(&ifr, 0, sizeof(ifr));
-	strcpy(ifr.ifr_name, devname);
+	memset(ifr, 0, sizeof(*ifr));
+	strcpy(ifr->ifr_name, devname);
 
 	/* Open control socket. */
 	fd = socket(AF_INET, SOCK_DGRAM, 0);
-	if (fd < 0) {
+	if (fd < 0)
 		perror("Cannot get control socket");
+
+	return fd;
+}
+
+static int doit(void)
+{
+	struct ifreq ifr;
+	int fd;
+
+	fd = get_control_socket(&ifr);
+	if (fd < 0)
 		return 70;
-	}
 
 	/* all of these are expected to populate ifr->ifr_data as needed */
 	if (mode == MODE_GDRV) {
@@ -1998,6 +2026,10 @@ static int doit(void)
 		return do_goffload(fd, &ifr);
 	} else if (mode == MODE_SOFFLOAD) {
 		return do_soffload(fd, &ifr);
+	} else if (mode == MODE_GFEATURES) {
+		return do_gfeatures(fd, &ifr);
+	} else if (mode == MODE_SFEATURES) {
+		return do_sfeatures(fd, &ifr);
 	} else if (mode == MODE_GSTATS) {
 		return do_gstats(fd, &ifr);
 	} else if (mode == MODE_GNFC) {
@@ -2435,6 +2467,175 @@ static int do_soffload(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+static int get_feature_strings(int fd, struct ifreq *ifr,
+	struct ethtool_gstrings **strs)
+{
+	struct ethtool_sset_info *sset_info;
+	struct ethtool_gstrings *strings;
+	int sz_str, n_strings, err;
+
+	sset_info = malloc(sizeof(struct ethtool_sset_info) + sizeof(u32));
+	sset_info->cmd = ETHTOOL_GSSET_INFO;
+	sset_info->sset_mask = (1ULL << ETH_SS_FEATURES);
+	ifr->ifr_data = (caddr_t)sset_info;
+	err = send_ioctl(fd, ifr);
+
+	if ((err < 0) ||
+	    (!(sset_info->sset_mask & (1ULL << ETH_SS_FEATURES)))) {
+		perror("Cannot get driver strings info");
+		return -100;
+	}
+
+	n_strings = sset_info->data[0];
+	free(sset_info);
+	sz_str = n_strings * ETH_GSTRING_LEN;
+
+	strings = calloc(1, sz_str + sizeof(struct ethtool_gstrings));
+	if (!strings) {
+		fprintf(stderr, "no memory available\n");
+		return -95;
+	}
+
+	strings->cmd = ETHTOOL_GSTRINGS;
+	strings->string_set = ETH_SS_FEATURES;
+	strings->len = n_strings;
+	ifr->ifr_data = (caddr_t) strings;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Cannot get feature strings information");
+		free(strings);
+		return -96;
+	}
+
+	*strs = strings;
+	return n_strings;
+}
+
+struct ethtool_sfeatures *features_req;
+
+static void parse_sfeatures_args(int argc, char **argp, int argi)
+{
+	struct cmdline_info *cmdline_desc, *cp;
+	struct ethtool_gstrings *strings;
+	struct ifreq ifr;
+	int n_strings, sz_features, i;
+	int fd, changed = 0;
+
+	fd = get_control_socket(&ifr);
+	if (fd < 0)
+		exit(100);
+
+	n_strings = get_feature_strings(fd, &ifr, &strings);
+	if (n_strings < 0)
+		exit(-n_strings);
+
+	sz_features = sizeof(*features_req->features) * ((n_strings + 31) / 32);
+
+	cp = cmdline_desc = calloc(n_strings, sizeof(*cmdline_desc));
+	features_req = calloc(1, sizeof(*features_req) + sz_features);
+	if (!cmdline_desc || !features_req) {
+		fprintf(stderr, "no memory available\n");
+		exit(95);
+	}
+
+	features_req->size = (n_strings + 31) / 32;
+
+	for (i = 0; i < n_strings; ++i) {
+		if (!strings->data[i*ETH_GSTRING_LEN])
+			continue;
+
+		strings->data[i*ETH_GSTRING_LEN + ETH_GSTRING_LEN-1] = 0;
+		cp->name = (const char *)strings->data + i * ETH_GSTRING_LEN;
+		cp->type = CMDL_FLAG;
+		cp->flag_val = 1 << (i % 32);
+		cp->wanted_val = &features_req->features[i / 32].requested;
+		cp->seen_val = &features_req->features[i / 32].valid;
+		++cp;
+	}
+
+	parse_generic_cmdline(argc, argp, argi, &changed,
+		cmdline_desc, cp - cmdline_desc);
+
+	free(cmdline_desc);
+	free(strings);
+	close(fd);
+
+	if (!changed) {
+		free(features_req);
+		features_req = NULL;
+	}
+}
+
+static int do_gfeatures(int fd, struct ifreq *ifr)
+{
+	struct ethtool_gstrings *strings;
+	struct ethtool_gfeatures *features;
+	int n_strings, sz_features, err, i;
+
+	n_strings = get_feature_strings(fd, ifr, &strings);
+	if (n_strings < 0)
+		return -n_strings;
+
+	sz_features = sizeof(*features->features) * ((n_strings + 31) / 32);
+	features = calloc(1, sz_features + sizeof(*features));
+	if (!features) {
+		fprintf(stderr, "no memory available\n");
+		return 95;
+	}
+
+	features->cmd = ETHTOOL_GFEATURES;
+	features->size = (n_strings + 31) / 32;
+	ifr->ifr_data = (caddr_t) features;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Cannot get feature status");
+		free(strings);
+		free(features);
+		return 97;
+	}
+
+	fprintf(stdout, "Offload state:  (name: enabled,wanted,changable)\n");
+	for (i = 0; i < n_strings; i++) {
+		if (!strings->data[i * ETH_GSTRING_LEN])
+			continue;	/* empty */
+#define P_FLAG(f) \
+	(features->features[i / 32].f & (1 << (i % 32))) ? "yes" : " no"
+#define PN_FLAG(f) \
+	(features->features[i / 32].never_changed & (1 << (i % 32))) ? "---" : P_FLAG(f)
+		fprintf(stdout, "     %-*.*s %s,%s,%s\n",
+			ETH_GSTRING_LEN, ETH_GSTRING_LEN,
+			&strings->data[i * ETH_GSTRING_LEN],
+			P_FLAG(active), PN_FLAG(requested), PN_FLAG(available));
+#undef P_FLAG
+	}
+	free(strings);
+	free(features);
+
+	return 0;
+}
+
+static int do_sfeatures(int fd, struct ifreq *ifr)
+{
+	int err;
+
+	if (!features_req) {
+		fprintf(stderr, "no features changed\n");
+		return 97;
+	}
+
+	features_req->cmd = ETHTOOL_SFEATURES;
+	ifr->ifr_data = (caddr_t) features_req;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Cannot change features");
+		free(features_req);
+		return 97;
+	}
+
+	return 0;
+}
+
+
 static int do_gset(int fd, struct ifreq *ifr)
 {
 	int err;
-- 
1.7.2.3

[PATCH v4 4/5] net: introduce NETIF_F_RXCSUM

[Michał Mirosław]

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>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>                                                    

---
 include/linux/ethtool.h   |    1 -
 include/linux/netdevice.h |    5 ++++-
 net/core/ethtool.c        |   36 ++++++++++++------------------------
 3 files changed, 16 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 4a3e554..45080bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -964,6 +964,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
@@ -979,7 +980,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 | \
@@ -2501,6 +2502,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 6e7c6f2..52e4272 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;
@@ -276,6 +270,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;
@@ -310,6 +307,7 @@ static int ethtool_get_one_feature(struct net_device *dev, char __user *useraddr
 }
 
 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);
@@ -347,6 +345,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:
@@ -391,7 +391,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",
 	"",
 	"",
 };
@@ -1369,20 +1369,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)
@@ -1730,15 +1725,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;
@@ -1811,6 +1797,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:
@@ -1819,6 +1806,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

[PATCH v4 5/5] loopback: convert to hw_features

[Michał Mirosław]

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..97b116b 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;
 	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

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[Ben Hutchings]

On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
> 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>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>

-- 
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.

Re: [PATCH v4 3/5] net: use ndo_fix_features for ethtool_ops->set_flags

[Ben Hutchings]

On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
> 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 555accf..6e7c6f2 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -240,6 +240,34 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
>  	return ret;
>  }
>  
> +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 -EOPNOTSUPP;
[...]

The error code should only be EOPNOTSUPP if (dev->hw_features &
flags_dup_features) == 0.  Otherwise it should be EINVAL.

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.

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 07 Feb 2011 19:39:57 +0000

> On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
>> 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>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>

Applied, thanks.

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 07 Feb 2011 12:51:20 -0800 (PST)

> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 07 Feb 2011 19:39:57 +0000
> 
>> On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
>>> 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>
>> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> Applied, thanks.

I had to make a fix to this patch, there were duplicate EXPORT_SYMBOL()
lines in net/core/ethtool.c for ethtool_op_set_tx_csum() after your
changes.

How did this build successfully for you?

Re: [PATCH v4 2/5] net: ethtool: use ndo_fix_features for offload setting

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu,  3 Feb 2011 15:21:21 +0100 (CET)

> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>                                                    

Applied, and now I see why the tree "built" successfully for
you.

You remove the duplicate EXPORT_SYMBOL() in this patch.

You absolutely cannot test your patch sets like this, only
build testing at the end.

Every single individual change must not introduce any functional
or build regressions, therefore you must make sure the build works
fine after each and every patch in your series, not just after they
are all applied.

What disturbs me even more, is that really this problem was introduced
because you mixed functional and cleanup changes in the first patch.
Something you should also never do.

Re: [PATCH v4 3/5] net: use ndo_fix_features for ethtool_ops->set_flags

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 07 Feb 2011 19:46:12 +0000

> On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
>> 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 555accf..6e7c6f2 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -240,6 +240,34 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
>>  	return ret;
>>  }
>>  
>> +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 -EOPNOTSUPP;
> [...]
> 
> The error code should only be EOPNOTSUPP if (dev->hw_features &
> flags_dup_features) == 0.  Otherwise it should be EINVAL.

I'll fix this up when I apply his patch, thanks Ben.

Re: [PATCH v4 4/5] net: introduce NETIF_F_RXCSUM

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu,  3 Feb 2011 15:21:22 +0100 (CET)

> 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>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>                                                    

Applied.

Re: [PATCH v4 5/5] loopback: convert to hw_features

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu,  3 Feb 2011 15:21:22 +0100 (CET)

> 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>

Applied.

Re: [PATCH v4 0/5] net: Unified offload configuration

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Thu,  3 Feb 2011 15:21:21 +0100 (CET)

> Here's a v4 of the ethtool unification patch series.
> 
> What's in it?
>  1:
> 	the patch - implement unified ethtool setting ops
>  2..3:
> 	implement interoperation between old and new ethtool ops
>  4:
> 	include RX checksum in features and plug it into new framework
>  5:
> 	convert loopback device to new framework

After these changes the ethtool output is now inaccurate for
RX checksumming.

Before:

davem@maramba:~$ /usr/sbin/ethtool -k eth0
Offload parameters for eth0:
rx-checksumming: on
tx-checksumming: on
scatter-gather: on
tcp segmentation offload: off
udp fragmentation offload: off
generic segmentation offload: on
large receive offload: off
davem@maramba:~$ 

After:

davem@maramba:~$ /usr/sbin/ethtool -k eth0
Offload parameters for eth0:
rx-checksumming: off
tx-checksumming: on
scatter-gather: on
tcp segmentation offload: off
udp fragmentation offload: off
generic segmentation offload: on
large receive offload: off

If the issue is that you require driver or ethtool utility changes in
order for things to keep working properly, then that is not
acceptable.

I'm reverting all of these changes, resubmit them when you have them
in a state such that no regressions will be introduced.

Thanks.

Re: [PATCH v4 0/5] net: Unified offload configuration

[Michał Mirosław]

On Mon, Feb 07, 2011 at 01:37:21PM -0800, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu,  3 Feb 2011 15:21:21 +0100 (CET)
> > Here's a v4 of the ethtool unification patch series.
[cut list]
> After these changes the ethtool output is now inaccurate for
> RX checksumming.
> 
> Before:
> 
> davem@maramba:~$ /usr/sbin/ethtool -k eth0
> Offload parameters for eth0:
> rx-checksumming: on
> tx-checksumming: on
[...]
 
> After:
> 
> davem@maramba:~$ /usr/sbin/ethtool -k eth0
> Offload parameters for eth0:
> rx-checksumming: off
> tx-checksumming: on
[...]
> 
> If the issue is that you require driver or ethtool utility changes in
> order for things to keep working properly, then that is not
> acceptable.

> I'm reverting all of these changes, resubmit them when you have them
> in a state such that no regressions will be introduced.

What driver did you get this output from?  All drivers that implement
set_rx_csum also implement their own get_rx_csum and so should not be
affected by patch #4. For others that don't implement get_rx_csum
rx-checksumming status is unreliable. Looking at random drivers:
 - via-rhine: will advertise rx-checksumming when it doesn't support
	it (as a side effect of hardware workaround - checksum in driver)
 - sunhme: has no way to disable rx- and tx-checksumming so was
	correctly showing rx-checksumming enabled
 - 8139too: will show rx-checksumming enabled but doesn't support it
	(side effect of hardware workaround - checksumming in driver)

I wouldn't be suprised if there was a driver which doesn't advertise
RX checksumming but use it anyway.

So yes - this patchset uncovers bugs in drivers. I'll see how can I
make the RXCSUM patch retain the previous behaviour for this case.

Best Regards,
Michał Mirosław

Re: [PATCH v4 0/5] net: Unified offload configuration

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Mon, 7 Feb 2011 23:49:37 +0100

> What driver did you get this output from?

NIU, which sets NETIF_F_HW_CSUM in netdev->features, which is
absolutely correct.

Re: [PATCH v4 0/5] net: Unified offload configuration

[Michał Mirosław]

On Mon, Feb 07, 2011 at 02:52:59PM -0800, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Mon, 7 Feb 2011 23:49:37 +0100
> > What driver did you get this output from?
> NIU, which sets NETIF_F_HW_CSUM in netdev->features, which is
> absolutely correct.

Quick look at niu_process_rx_pkt() reveals that if you disable
TX checksumming you will also get 'rx-checksumming: off' from ethtool,
but RX checksumming will still be used.

Best Regards,
Michał Mirosław

Re: [PATCH v4 0/5] net: Unified offload configuration

[David Miller]

BTW, none of your patch postings from today made it to the mailing
lists because there were syntax errors in your email headers.

Therefore, you'll need to resend them.

Re: [PATCH v4 0/5] net: Unified offload configuration

[Michał Mirosław]

On Tue, Feb 08, 2011 at 11:40:18AM -0800, David Miller wrote:
> BTW, none of your patch postings from today made it to the mailing
> lists because there were syntax errors in your email headers.
> 
> Therefore, you'll need to resend them.

Hmm, that's weird. My mailer said:

mail.info: Feb  8 15:23:59 postfix/smtp[6076]: 7138813909: to=<netdev@vger.kernel.org>, relay=vger.kernel.org[209.132.180.67]:25, delay=3, delays=0.09/0.08/2.2/0.7, dsn=2.7.0, status=sent (250 2.7.0 nothing apparently wrong in the message. BF:<H 0>; S1754489Ab1BHOX7)
mail.info: Feb  8 15:23:59 postfix/smtp[6110]: BA99413A5F: to=<netdev@vger.kernel.org>, relay=vger.kernel.org[209.132.180.67]:25, delay=3, delays=0.2/0.45/1.5/0.84, dsn=2.7.1, status=sent (250 2.7.1 Looks like Linux source DIFF email.. BF:<H 0>; S1754575Ab1BHOX7)
[same for other patches in series]

And I got no bounces. What were the errors?

Best Regards,
Michał Mirosław

Re: [PATCH v4 0/5] net: Unified offload configuration

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Wed, 9 Feb 2011 00:55:43 +0100

> And I got no bounces. What were the errors?

The bounces only go to me.

You had mal-formed email addresses in your TO or CC line,
you forgot a comma between two of them.

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[Eric Dumazet]

Le lundi 07 février 2011 à 19:39 +0000, Ben Hutchings a écrit :
> On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
> > 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>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> 


Hi guys

Do we have any ethtool patch to use these 'new' features ?

Thanks

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[Ben Hutchings]

On Tue, 2012-01-24 at 14:54 +0100, Eric Dumazet wrote:
> Le lundi 07 février 2011 à 19:39 +0000, Ben Hutchings a écrit :
> > On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
> > > 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>
> > Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> > 
> 
> 
> Hi guys
> 
> Do we have any ethtool patch to use these 'new' features ?

I have some unfinished changes to support this.  What I don't want to do
is to add an entirely separate set of options; I want -k/-K to work with
old and new kernel versions, supporting named features as an extension
where available.

Ben.

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

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[Eric Dumazet]

Le mardi 24 janvier 2012 à 15:30 +0000, Ben Hutchings a écrit :
> On Tue, 2012-01-24 at 14:54 +0100, Eric Dumazet wrote:

> > Hi guys
> > 
> > Do we have any ethtool patch to use these 'new' features ?
> 
> I have some unfinished changes to support this.  What I don't want to do
> is to add an entirely separate set of options; I want -k/-K to work with
> old and new kernel versions, supporting named features as an extension
> where available.

Excellent !

Thanks

Re: [PATCH v4 1/5] net: Introduce new feature setting ops

[Michał Mirosław]

On Tue, Jan 24, 2012 at 03:30:02PM +0000, Ben Hutchings wrote:
> On Tue, 2012-01-24 at 14:54 +0100, Eric Dumazet wrote:
> > Le lundi 07 février 2011 à 19:39 +0000, Ben Hutchings a écrit :
> > > On Thu, 2011-02-03 at 15:21 +0100, Michał Mirosław wrote:
> > > > 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>
> > > Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> > Do we have any ethtool patch to use these 'new' features ?
> I have some unfinished changes to support this.  What I don't want to do
> is to add an entirely separate set of options; I want -k/-K to work with
> old and new kernel versions, supporting named features as an extension
> where available.

Ben, what are your patches like?  I mean their state?  I'm asking, because I
found a bit of time and rebased my earlier patch to current ethtool 3.2
this weekend.  Haven't sent them yet, because I'm seeing -EOPNOTSUPP from
kernel on x86_64 for GFEATURES and I'm trying to figure out where's the bug.

Best Regards,
Michał Mirosław

[RFC PATCH] ethtool: implement [GS]FEATURES handling

[Michał Mirosław]

On Tue, Jan 24, 2012 at 08:05:13PM +0100, Michał Mirosław wrote:
> On Tue, Jan 24, 2012 at 03:30:02PM +0000, Ben Hutchings wrote:
> > I have some unfinished changes to support this.  What I don't want to do
> > is to add an entirely separate set of options; I want -k/-K to work with
> > old and new kernel versions, supporting named features as an extension
> > where available.
>
> Ben, what are your patches like?  I mean their state?  I'm asking, because I
> found a bit of time and rebased my earlier patch to current ethtool 3.2
> this weekend.  Haven't sent them yet, because I'm seeing -EOPNOTSUPP from
> kernel on x86_64 for GFEATURES and I'm trying to figure out where's the bug.

Turned out it was a typo in userspace code. Resulting patch below.

The more I look at this the more I think that never_changed field was a mistake
and it would be better to include vlan_features there instead. What do you
think? Can we change it (it's not widely used, yet)? That would include changing
GFEATURES command number of course.

Best Regards,
Michał Mirosław

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

diff --git a/ethtool.c b/ethtool.c
index d0cc7d4..b2ce7f2 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1036,9 +1036,15 @@ static int dump_coalesce(const struct ethtool_coalesce *ecoal)
 	return 0;
 }
 
-static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
-			int gro, int lro, int rxvlan, int txvlan, int ntuple,
-			int rxhash)
+struct offload_state {
+	int rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan, ntuple, rxhash;
+};
+
+static const char *const old_feature_names[] = {
+	"rx", "tx", "sg", "tso", "ufo", "gso", "gro", "lro", "rxvlan", "txvlan", "ntuple", "rxhash"
+};
+
+static int dump_offload(const struct offload_state *offload)
 {
 	fprintf(stdout,
 		"rx-checksumming: %s\n"
@@ -1053,18 +1059,18 @@ static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
 		"tx-vlan-offload: %s\n"
 		"ntuple-filters: %s\n"
 		"receive-hashing: %s\n",
-		rx ? "on" : "off",
-		tx ? "on" : "off",
-		sg ? "on" : "off",
-		tso ? "on" : "off",
-		ufo ? "on" : "off",
-		gso ? "on" : "off",
-		gro ? "on" : "off",
-		lro ? "on" : "off",
-		rxvlan ? "on" : "off",
-		txvlan ? "on" : "off",
-		ntuple ? "on" : "off",
-		rxhash ? "on" : "off");
+		offload->rx ? "on" : "off",
+		offload->tx ? "on" : "off",
+		offload->sg ? "on" : "off",
+		offload->tso ? "on" : "off",
+		offload->ufo ? "on" : "off",
+		offload->gso ? "on" : "off",
+		offload->gro ? "on" : "off",
+		offload->lro ? "on" : "off",
+		offload->rxvlan ? "on" : "off",
+		offload->txvlan ? "on" : "off",
+		offload->ntuple ? "on" : "off",
+		offload->rxhash ? "on" : "off");
 
 	return 0;
 }
@@ -1547,24 +1553,20 @@ static int do_scoalesce(struct cmd_context *ctx)
 	return 0;
 }
 
-static int do_goffload(struct cmd_context *ctx)
+static int send_goffloads(struct cmd_context *ctx,
+	struct offload_state *offload)
 {
 	struct ethtool_value eval;
-	int err, allfail = 1, rx = 0, tx = 0, sg = 0;
-	int tso = 0, ufo = 0, gso = 0, gro = 0, lro = 0, rxvlan = 0, txvlan = 0,
-	    ntuple = 0, rxhash = 0;
-
-	if (ctx->argc != 0)
-		exit_bad_args();
+	int err, allfail = 1;
 
-	fprintf(stdout, "Offload parameters for %s:\n", ctx->devname);
+	memset(offload, 0, sizeof(*offload));
 
 	eval.cmd = ETHTOOL_GRXCSUM;
 	err = send_ioctl(ctx, &eval);
 	if (err)
 		perror("Cannot get device rx csum settings");
 	else {
-		rx = eval.data;
+		offload->rx = eval.data;
 		allfail = 0;
 	}
 
@@ -1573,7 +1575,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device tx csum settings");
 	else {
-		tx = eval.data;
+		offload->tx = eval.data;
 		allfail = 0;
 	}
 
@@ -1582,7 +1584,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device scatter-gather settings");
 	else {
-		sg = eval.data;
+		offload->sg = eval.data;
 		allfail = 0;
 	}
 
@@ -1591,7 +1593,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device tcp segmentation offload settings");
 	else {
-		tso = eval.data;
+		offload->tso = eval.data;
 		allfail = 0;
 	}
 
@@ -1600,7 +1602,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device udp large send offload settings");
 	else {
-		ufo = eval.data;
+		offload->ufo = eval.data;
 		allfail = 0;
 	}
 
@@ -1609,7 +1611,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device generic segmentation offload settings");
 	else {
-		gso = eval.data;
+		offload->gso = eval.data;
 		allfail = 0;
 	}
 
@@ -1618,11 +1620,11 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err) {
 		perror("Cannot get device flags");
 	} else {
-		lro = (eval.data & ETH_FLAG_LRO) != 0;
-		rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
-		txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
-		ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
-		rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
+		offload->lro = (eval.data & ETH_FLAG_LRO) != 0;
+		offload->rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
+		offload->txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
+		offload->ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
+		offload->rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
 		allfail = 0;
 	}
 
@@ -1631,7 +1633,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device GRO settings");
 	else {
-		gro = eval.data;
+		offload->gro = eval.data;
 		allfail = 0;
 	}
 
@@ -1640,144 +1642,509 @@ static int do_goffload(struct cmd_context *ctx)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
-			    ntuple, rxhash);
+	return 0;
 }
 
-static int do_soffload(struct cmd_context *ctx)
+static int send_soffloads(struct cmd_context *ctx,
+	const struct offload_state *wanted)
 {
-	int goffload_changed = 0;
-	int off_csum_rx_wanted = -1;
-	int off_csum_tx_wanted = -1;
-	int off_sg_wanted = -1;
-	int off_tso_wanted = -1;
-	int off_ufo_wanted = -1;
-	int off_gso_wanted = -1;
+	struct ethtool_value eval;
+	int changed = 0, err;
 	u32 off_flags_wanted = 0;
 	u32 off_flags_mask = 0;
-	int off_gro_wanted = -1;
-	struct cmdline_info cmdline_offload[] = {
-		{ "rx", CMDL_BOOL, &off_csum_rx_wanted, NULL },
-		{ "tx", CMDL_BOOL, &off_csum_tx_wanted, NULL },
-		{ "sg", CMDL_BOOL, &off_sg_wanted, NULL },
-		{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
-		{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
-		{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
-		{ "lro", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_LRO, &off_flags_mask },
-		{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
-		{ "rxvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_RXVLAN, &off_flags_mask },
-		{ "txvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_TXVLAN, &off_flags_mask },
-		{ "ntuple", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_NTUPLE, &off_flags_mask },
-		{ "rxhash", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_RXHASH, &off_flags_mask },
-	};
-	struct ethtool_value eval;
-	int err, changed = 0;
 
-	parse_generic_cmdline(ctx, &goffload_changed,
-			      cmdline_offload, ARRAY_SIZE(cmdline_offload));
+	if (wanted->lro >= 0) {
+		off_flags_mask |= ETH_FLAG_LRO;
+		if (wanted->lro)
+			off_flags_wanted |= ETH_FLAG_LRO;
+	}
+
+	if (wanted->rxvlan >= 0) {
+		off_flags_mask |= ETH_FLAG_RXVLAN;
+		if (wanted->rxvlan)
+			off_flags_wanted |= ETH_FLAG_RXVLAN;
+	}
 
-	if (off_csum_rx_wanted >= 0) {
-		changed = 1;
+	if (wanted->txvlan >= 0) {
+		off_flags_mask |= ETH_FLAG_TXVLAN;
+		if (wanted->txvlan)
+			off_flags_wanted |= ETH_FLAG_TXVLAN;
+	}
+
+	if (wanted->ntuple >= 0) {
+		off_flags_mask |= ETH_FLAG_NTUPLE;
+		if (wanted->ntuple)
+			off_flags_wanted |= ETH_FLAG_NTUPLE;
+	}
+
+	if (wanted->rxhash >= 0) {
+		off_flags_mask |= ETH_FLAG_RXHASH;
+		if (wanted->rxhash)
+			off_flags_wanted |= ETH_FLAG_RXHASH;
+	}
+
+	if (wanted->rx >= 0) {
 		eval.cmd = ETHTOOL_SRXCSUM;
-		eval.data = (off_csum_rx_wanted == 1);
+		eval.data = !!wanted->rx;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device rx csum settings");
-			return 84;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_csum_tx_wanted >= 0) {
-		changed = 1;
+	if (wanted->tx >= 0) {
 		eval.cmd = ETHTOOL_STXCSUM;
-		eval.data = (off_csum_tx_wanted == 1);
+		eval.data = !!wanted->tx;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device tx csum settings");
-			return 85;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_sg_wanted >= 0) {
-		changed = 1;
+	if (wanted->sg >= 0) {
 		eval.cmd = ETHTOOL_SSG;
-		eval.data = (off_sg_wanted == 1);
+		eval.data = !!wanted->sg;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device scatter-gather settings");
-			return 86;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_tso_wanted >= 0) {
-		changed = 1;
+	if (wanted->tso >= 0) {
 		eval.cmd = ETHTOOL_STSO;
-		eval.data = (off_tso_wanted == 1);
+		eval.data = !!wanted->tso;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device tcp segmentation offload settings");
-			return 88;
-		}
+		else
+			changed = 1;
 	}
-	if (off_ufo_wanted >= 0) {
-		changed = 1;
+	if (wanted->ufo >= 0) {
 		eval.cmd = ETHTOOL_SUFO;
-		eval.data = (off_ufo_wanted == 1);
+		eval.data = !!wanted->ufo;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device udp large send offload settings");
-			return 89;
-		}
+		else
+			changed = 1;
 	}
-	if (off_gso_wanted >= 0) {
-		changed = 1;
+	if (wanted->gso >= 0) {
 		eval.cmd = ETHTOOL_SGSO;
-		eval.data = (off_gso_wanted == 1);
+		eval.data = !!wanted->gso;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device generic segmentation offload settings");
-			return 90;
-		}
+		else
+			changed = 1;
 	}
 	if (off_flags_mask) {
-		changed = 1;
 		eval.cmd = ETHTOOL_GFLAGS;
 		eval.data = 0;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot get device flag settings");
-			return 91;
-		}
-
-		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data = ((eval.data & ~off_flags_mask) |
-			     off_flags_wanted);
-
-		err = send_ioctl(ctx, &eval);
-		if (err) {
-			perror("Cannot set device flag settings");
-			return 92;
+		else {
+			eval.cmd = ETHTOOL_SFLAGS;
+			eval.data = ((eval.data & ~off_flags_mask) |
+				     off_flags_wanted);
+
+			err = send_ioctl(ctx, &eval);
+			if (err)
+				perror("Cannot set device flag settings");
+			else
+				changed = 1;
 		}
 	}
-	if (off_gro_wanted >= 0) {
-		changed = 1;
+	if (wanted->gro >= 0) {
 		eval.cmd = ETHTOOL_SGRO;
-		eval.data = (off_gro_wanted == 1);
+		eval.data = !!wanted->gro;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device GRO settings");
-			return 93;
+		else
+			changed = 1;
+	}
+
+	return changed;
+}
+
+static int n_feature_strings;
+static const char **feature_strings;
+
+static int init_feature_strings(struct cmd_context *ctx)
+{
+	struct ethtool_gstrings *strings;
+	int i, n;
+
+	if (feature_strings)
+		return n_feature_strings;
+
+	strings = get_stringset(ctx, ETH_SS_FEATURES, 0);
+	if (!strings)
+		return -100;
+
+	n = n_feature_strings = strings->len;
+	feature_strings = calloc(n, sizeof(*feature_strings));
+	if (!feature_strings) {
+		fprintf(stderr, "no memory available for string table [size=%d]\n", n);
+		exit(95);
+	}
+
+	for (i = 0; i < n; ++i) {
+		if (!strings->data[i*ETH_GSTRING_LEN])
+			continue;
+
+		feature_strings[i] = strndup(
+			(const char *)&strings->data[i * ETH_GSTRING_LEN],
+			ETH_GSTRING_LEN);
+
+		if (!feature_strings[i]) {
+			fprintf(stderr, "no memory available for a string\n");
+			exit(95);
 		}
 	}
 
+	free(strings);
+	return n;
+}
+
+static void parse_sfeatures_args(struct cmd_context *ctx,
+	struct offload_state *offload,
+	struct ethtool_sfeatures **features_req_p)
+{
+	struct ethtool_sfeatures *features_req;
+	struct cmdline_info *cmdline_desc, *cp;
+	int sz_features, i;
+	int changed = 0;
+
+	struct cmdline_info cmdline_offload[] = {
+		{ "rx", CMDL_BOOL, &offload->rx, NULL },
+		{ "tx", CMDL_BOOL, &offload->tx, NULL },
+		{ "sg", CMDL_BOOL, &offload->sg, NULL },
+		{ "tso", CMDL_BOOL, &offload->tso, NULL },
+		{ "ufo", CMDL_BOOL, &offload->ufo, NULL },
+		{ "gso", CMDL_BOOL, &offload->gso, NULL },
+		{ "lro", CMDL_FLAG, &offload->lro, NULL },
+		{ "gro", CMDL_FLAG, &offload->gro, NULL },
+		{ "rxvlan", CMDL_FLAG, &offload->rxvlan, NULL },
+		{ "txvlan", CMDL_FLAG, &offload->txvlan, NULL },
+		{ "ntuple", CMDL_FLAG, &offload->ntuple, NULL },
+		{ "rxhash", CMDL_FLAG, &offload->rxhash, NULL },
+	};
+
+	for (i = 0; i < ARRAY_SIZE(old_feature_names); ++i)
+		((int *)offload)[i] = -1;
+	*features_req_p = NULL;
+
+	if (init_feature_strings(ctx) < 0) {
+		/* ETHTOOL_GFEATURES unavailable */
+		parse_generic_cmdline(ctx, &changed,
+			cmdline_offload, ARRAY_SIZE(cmdline_offload));
+		return;
+	}
+
+	sz_features = sizeof(*features_req->features) * ((n_feature_strings + 31) / 32);
+
+	cp = cmdline_desc = calloc(n_feature_strings + ARRAY_SIZE(cmdline_offload),
+		sizeof(*cmdline_desc));
+	memcpy(cp, cmdline_offload, sizeof(cmdline_offload));
+	cp += ARRAY_SIZE(cmdline_offload);
+
+	features_req = calloc(1, sizeof(*features_req) + sz_features);
+	if (!cmdline_desc || !features_req) {
+		fprintf(stderr, "no memory available\n");
+		exit(95);
+	}
+
+	features_req->size = (n_feature_strings + 31) / 32;
+
+	for (i = 0; i < n_feature_strings; ++i) {
+		if (!feature_strings[i])
+			continue;
+
+		cp->name = feature_strings[i];
+		cp->type = CMDL_FLAG;
+		cp->flag_val = 1 << (i % 32);
+		cp->wanted_val = &features_req->features[i / 32].requested;
+		cp->seen_val = &features_req->features[i / 32].valid;
+		++cp;
+	}
+
+	parse_generic_cmdline(ctx, &changed, cmdline_desc, cp - cmdline_desc);
+
+	free(cmdline_desc);
+
+	if (changed)
+		*features_req_p = features_req;
+	else
+		free(features_req);
+}
+
+static int send_gfeatures(struct cmd_context *ctx,
+	struct ethtool_gfeatures **features_p)
+{
+	struct ethtool_gfeatures *features;
+	int err, sz_features;
+
+	sz_features = sizeof(*features->features) * ((n_feature_strings + 31) / 32);
+	features = calloc(1, sz_features + sizeof(*features));
+	if (!features) {
+		fprintf(stderr, "no memory available\n");
+		return 95;
+	}
+
+	features->cmd = ETHTOOL_GFEATURES;
+	features->size = (n_feature_strings + 31) / 32;
+	err = send_ioctl(ctx, features);
+
+	if (err < 0) {
+		perror("Cannot get feature status");
+		free(features);
+		return 97;
+	}
+
+	*features_p = features;
+	return 0;
+}
+
+static const char *get_feature_state(struct ethtool_get_features_block *gfb,
+	uint32_t bit)
+{
+	if (gfb->never_changed & bit)
+		return "fixed";
+	if (!(gfb->available & bit))
+		return "driver-controlled";
+
+	if ((gfb->active ^ gfb->requested) & bit)
+		return (gfb->requested & bit) ? "requested on" : "requested off";
+	else
+		return "changeable";
+}
+
+static int do_gfeatures(struct cmd_context *ctx)
+{
+	struct ethtool_get_features_block *gfb;
+	struct ethtool_gfeatures *features;
+	uint32_t bit;
+	int err, i;
+
+	err = init_feature_strings(ctx);
+	if (err < 0)
+		return -err;
+
+	err = send_gfeatures(ctx, &features);
+	if (err)
+		return err;
+
+	fprintf(stdout, "\nExtended offload state for %s:\n", ctx->devname);
+	for (i = 0; i < n_feature_strings; i++) {
+		if (!feature_strings[i])
+			continue;	/* empty */
+
+		gfb = features->features + i / 32;
+		bit = 1 << (i % 32);
+
+		fprintf(stdout, "%s: %s (%s)\n", feature_strings[i],
+			(gfb->active & bit) ? "on" : "off",
+			get_feature_state(gfb, bit));
+	}
+	free(features);
+
+	return 0;
+}
+
+static void print_gfeatures_diff(
+	const struct ethtool_get_features_block *expected,
+	const struct ethtool_get_features_block *set,
+	const char **strings, int n_strings)
+{
+	int i;
+
+	if (n_strings > 32)
+		n_strings = 32;
+
+	for (i = 0; i < n_strings; ++i) {
+		u32 mask = 1 << i;
+
+		if (!strings[i])
+			continue;
+
+		if (!((expected->active ^ set->active) & mask))
+			continue;
+
+		fprintf(stdout, "feature %.*s is %s (expected: %s, saved: %s)\n",
+			ETH_GSTRING_LEN, strings[i],
+			set->active & mask ? "enabled" : "disabled",
+			expected->active & mask ? "enabled" : "disabled",
+			!(set->available & mask) ? "not user-changeable" :
+				set->requested & mask ? "enabled" : "disabled"
+		);
+	}
+}
+
+static int get_offload_state(struct cmd_context *ctx,
+	struct ethtool_gfeatures **features,
+	struct offload_state *offload)
+{
+	int err, allfail;
+
+	if (ctx->argc != 0)
+		exit_bad_args();
+
+	allfail = send_goffloads(ctx, offload);
+
+	err = init_feature_strings(ctx);
+	if (err < 0)
+		return allfail ? err : 0;
+
+	err = send_gfeatures(ctx, features);
+	if (err)
+		perror("Cannot read features");
+
+	return allfail ? -err : 0;
+}
+
+static int send_sfeatures(struct cmd_context *ctx,
+	struct ethtool_sfeatures *features)
+{
+	int err;
+
+	features->cmd = ETHTOOL_SFEATURES;
+	err = send_ioctl(ctx, features);
+	if (err < 0) {
+		perror("Cannot change features");
+		return 97;
+	}
+
+	return 0;
+}
+
+static void compare_offload_state(struct offload_state *offload0,
+	struct offload_state *offload_req, struct offload_state *offload1)
+{
+	int *old = (int *)offload0;
+	int *req = (int *)offload_req;
+	int *new = (int *)offload1;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(old_feature_names); i++) {
+		if (req[i] < 0)
+			req[i] = old[i];
+		if (req[i] == new[i])
+			continue;
+
+		fprintf(stdout, "feature group %s is %s (expected: %s)\n",
+			old_feature_names[i],
+			new[i] ? "enabled" : "disabled",
+			req[i] ? "enabled" : "disabled"
+		);
+	}
+}
+
+static void compare_features(struct ethtool_gfeatures *features0,
+	struct ethtool_sfeatures *features_req,
+	struct ethtool_gfeatures *features1)
+{
+	int i;
+
+	if (features_req) {
+		/* make features0 .active what we expect to be set */
+		i = (n_feature_strings + 31) / 32;
+		while (i--) {
+			features0->features[i].active &= ~features_req->features[i].valid;
+			features0->features[i].active |=
+				features_req->features[i].requested &
+				features_req->features[i].valid;
+		}
+	}
+
+	for (i = 0; i < n_feature_strings; i += 32)
+		print_gfeatures_diff(&features0->features[i / 32],
+			&features1->features[i / 32],
+			feature_strings + i,
+			n_feature_strings - i);
+}
+
+static int do_goffload(struct cmd_context *ctx)
+{
+	struct offload_state offload;
+	int err, allfail;
+
+	allfail = send_goffloads(ctx, &offload);
+
+	if (!allfail) {
+		fprintf(stdout, "Offload parameters for %s:\n", ctx->devname);
+
+		dump_offload(&offload);
+	}
+
+	err = do_gfeatures(ctx);
+	if (!err)
+		allfail = 0;
+
+	if (allfail) {
+		fprintf(stdout, "no offload info available\n");
+		return 83;
+	}
+
+	return 0;
+}
+
+static int do_soffload(struct cmd_context *ctx)
+{
+	struct ethtool_gfeatures *features_old, *features_new;
+	struct ethtool_sfeatures *features_req;
+	struct offload_state offload_old, offload_new, offload_req;
+	int err, changed;
+
+	parse_sfeatures_args(ctx, &offload_req, &features_req);
+
+	err = get_offload_state(ctx, &features_old, &offload_old);
+	if (err)
+		return -err;
+
+	changed = send_soffloads(ctx, &offload_req);
+
+	if (features_req) {
+		err = send_sfeatures(ctx, features_req);
+		if (!err)
+			changed = 1;
+	}
+
 	if (!changed) {
 		fprintf(stdout, "no offload settings changed\n");
+		return err;
+	}
+
+	err = get_offload_state(ctx, &features_new, &offload_new);
+	if (err) {
+		perror("can't verify offload setting");
+		return 101;
+	}
+
+	if ((!features_old) ^ (!features_new)) {
+		fprintf(stderr, "can't compare features (one GFEATURES failed)\n");
+		if (features_old) {
+			free(features_old);
+			features_old = NULL;
+		}
+		if (features_new) {
+			free(features_new);
+			features_new = NULL;
+		}
+	}
+
+	compare_offload_state(&offload_old, &offload_req, &offload_new);
+	if (features_old) {
+		compare_features(features_old, features_req, features_new);
+		free(features_old);
+		free(features_new);
 	}
+	if (features_req)
+		free(features_req);
 
 	return 0;
 }
-- 
1.7.8.3

[RFC PATCH v2] ethtool: implement [GS]FEATURES handling

[Michał Mirosław]

v2: fix missing change CMDL_FLAG -> CMDL_BOOL for old offloads parsing

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

diff --git a/ethtool.c b/ethtool.c
index d0cc7d4..8df2ce8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1036,9 +1036,15 @@ static int dump_coalesce(const struct ethtool_coalesce *ecoal)
 	return 0;
 }
 
-static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
-			int gro, int lro, int rxvlan, int txvlan, int ntuple,
-			int rxhash)
+struct offload_state {
+	int rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan, ntuple, rxhash;
+};
+
+static const char *const old_feature_names[] = {
+	"rx", "tx", "sg", "tso", "ufo", "gso", "gro", "lro", "rxvlan", "txvlan", "ntuple", "rxhash"
+};
+
+static int dump_offload(const struct offload_state *offload)
 {
 	fprintf(stdout,
 		"rx-checksumming: %s\n"
@@ -1053,18 +1059,18 @@ static int dump_offload(int rx, int tx, int sg, int tso, int ufo, int gso,
 		"tx-vlan-offload: %s\n"
 		"ntuple-filters: %s\n"
 		"receive-hashing: %s\n",
-		rx ? "on" : "off",
-		tx ? "on" : "off",
-		sg ? "on" : "off",
-		tso ? "on" : "off",
-		ufo ? "on" : "off",
-		gso ? "on" : "off",
-		gro ? "on" : "off",
-		lro ? "on" : "off",
-		rxvlan ? "on" : "off",
-		txvlan ? "on" : "off",
-		ntuple ? "on" : "off",
-		rxhash ? "on" : "off");
+		offload->rx ? "on" : "off",
+		offload->tx ? "on" : "off",
+		offload->sg ? "on" : "off",
+		offload->tso ? "on" : "off",
+		offload->ufo ? "on" : "off",
+		offload->gso ? "on" : "off",
+		offload->gro ? "on" : "off",
+		offload->lro ? "on" : "off",
+		offload->rxvlan ? "on" : "off",
+		offload->txvlan ? "on" : "off",
+		offload->ntuple ? "on" : "off",
+		offload->rxhash ? "on" : "off");
 
 	return 0;
 }
@@ -1547,24 +1553,20 @@ static int do_scoalesce(struct cmd_context *ctx)
 	return 0;
 }
 
-static int do_goffload(struct cmd_context *ctx)
+static int send_goffloads(struct cmd_context *ctx,
+	struct offload_state *offload)
 {
 	struct ethtool_value eval;
-	int err, allfail = 1, rx = 0, tx = 0, sg = 0;
-	int tso = 0, ufo = 0, gso = 0, gro = 0, lro = 0, rxvlan = 0, txvlan = 0,
-	    ntuple = 0, rxhash = 0;
-
-	if (ctx->argc != 0)
-		exit_bad_args();
+	int err, allfail = 1;
 
-	fprintf(stdout, "Offload parameters for %s:\n", ctx->devname);
+	memset(offload, 0, sizeof(*offload));
 
 	eval.cmd = ETHTOOL_GRXCSUM;
 	err = send_ioctl(ctx, &eval);
 	if (err)
 		perror("Cannot get device rx csum settings");
 	else {
-		rx = eval.data;
+		offload->rx = eval.data;
 		allfail = 0;
 	}
 
@@ -1573,7 +1575,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device tx csum settings");
 	else {
-		tx = eval.data;
+		offload->tx = eval.data;
 		allfail = 0;
 	}
 
@@ -1582,7 +1584,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device scatter-gather settings");
 	else {
-		sg = eval.data;
+		offload->sg = eval.data;
 		allfail = 0;
 	}
 
@@ -1591,7 +1593,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device tcp segmentation offload settings");
 	else {
-		tso = eval.data;
+		offload->tso = eval.data;
 		allfail = 0;
 	}
 
@@ -1600,7 +1602,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device udp large send offload settings");
 	else {
-		ufo = eval.data;
+		offload->ufo = eval.data;
 		allfail = 0;
 	}
 
@@ -1609,7 +1611,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device generic segmentation offload settings");
 	else {
-		gso = eval.data;
+		offload->gso = eval.data;
 		allfail = 0;
 	}
 
@@ -1618,11 +1620,11 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err) {
 		perror("Cannot get device flags");
 	} else {
-		lro = (eval.data & ETH_FLAG_LRO) != 0;
-		rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
-		txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
-		ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
-		rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
+		offload->lro = (eval.data & ETH_FLAG_LRO) != 0;
+		offload->rxvlan = (eval.data & ETH_FLAG_RXVLAN) != 0;
+		offload->txvlan = (eval.data & ETH_FLAG_TXVLAN) != 0;
+		offload->ntuple = (eval.data & ETH_FLAG_NTUPLE) != 0;
+		offload->rxhash = (eval.data & ETH_FLAG_RXHASH) != 0;
 		allfail = 0;
 	}
 
@@ -1631,7 +1633,7 @@ static int do_goffload(struct cmd_context *ctx)
 	if (err)
 		perror("Cannot get device GRO settings");
 	else {
-		gro = eval.data;
+		offload->gro = eval.data;
 		allfail = 0;
 	}
 
@@ -1640,144 +1642,509 @@ static int do_goffload(struct cmd_context *ctx)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
-			    ntuple, rxhash);
+	return 0;
 }
 
-static int do_soffload(struct cmd_context *ctx)
+static int send_soffloads(struct cmd_context *ctx,
+	const struct offload_state *wanted)
 {
-	int goffload_changed = 0;
-	int off_csum_rx_wanted = -1;
-	int off_csum_tx_wanted = -1;
-	int off_sg_wanted = -1;
-	int off_tso_wanted = -1;
-	int off_ufo_wanted = -1;
-	int off_gso_wanted = -1;
+	struct ethtool_value eval;
+	int changed = 0, err;
 	u32 off_flags_wanted = 0;
 	u32 off_flags_mask = 0;
-	int off_gro_wanted = -1;
-	struct cmdline_info cmdline_offload[] = {
-		{ "rx", CMDL_BOOL, &off_csum_rx_wanted, NULL },
-		{ "tx", CMDL_BOOL, &off_csum_tx_wanted, NULL },
-		{ "sg", CMDL_BOOL, &off_sg_wanted, NULL },
-		{ "tso", CMDL_BOOL, &off_tso_wanted, NULL },
-		{ "ufo", CMDL_BOOL, &off_ufo_wanted, NULL },
-		{ "gso", CMDL_BOOL, &off_gso_wanted, NULL },
-		{ "lro", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_LRO, &off_flags_mask },
-		{ "gro", CMDL_BOOL, &off_gro_wanted, NULL },
-		{ "rxvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_RXVLAN, &off_flags_mask },
-		{ "txvlan", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_TXVLAN, &off_flags_mask },
-		{ "ntuple", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_NTUPLE, &off_flags_mask },
-		{ "rxhash", CMDL_FLAG, &off_flags_wanted, NULL,
-		  ETH_FLAG_RXHASH, &off_flags_mask },
-	};
-	struct ethtool_value eval;
-	int err, changed = 0;
 
-	parse_generic_cmdline(ctx, &goffload_changed,
-			      cmdline_offload, ARRAY_SIZE(cmdline_offload));
+	if (wanted->lro >= 0) {
+		off_flags_mask |= ETH_FLAG_LRO;
+		if (wanted->lro)
+			off_flags_wanted |= ETH_FLAG_LRO;
+	}
+
+	if (wanted->rxvlan >= 0) {
+		off_flags_mask |= ETH_FLAG_RXVLAN;
+		if (wanted->rxvlan)
+			off_flags_wanted |= ETH_FLAG_RXVLAN;
+	}
 
-	if (off_csum_rx_wanted >= 0) {
-		changed = 1;
+	if (wanted->txvlan >= 0) {
+		off_flags_mask |= ETH_FLAG_TXVLAN;
+		if (wanted->txvlan)
+			off_flags_wanted |= ETH_FLAG_TXVLAN;
+	}
+
+	if (wanted->ntuple >= 0) {
+		off_flags_mask |= ETH_FLAG_NTUPLE;
+		if (wanted->ntuple)
+			off_flags_wanted |= ETH_FLAG_NTUPLE;
+	}
+
+	if (wanted->rxhash >= 0) {
+		off_flags_mask |= ETH_FLAG_RXHASH;
+		if (wanted->rxhash)
+			off_flags_wanted |= ETH_FLAG_RXHASH;
+	}
+
+	if (wanted->rx >= 0) {
 		eval.cmd = ETHTOOL_SRXCSUM;
-		eval.data = (off_csum_rx_wanted == 1);
+		eval.data = !!wanted->rx;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device rx csum settings");
-			return 84;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_csum_tx_wanted >= 0) {
-		changed = 1;
+	if (wanted->tx >= 0) {
 		eval.cmd = ETHTOOL_STXCSUM;
-		eval.data = (off_csum_tx_wanted == 1);
+		eval.data = !!wanted->tx;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device tx csum settings");
-			return 85;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_sg_wanted >= 0) {
-		changed = 1;
+	if (wanted->sg >= 0) {
 		eval.cmd = ETHTOOL_SSG;
-		eval.data = (off_sg_wanted == 1);
+		eval.data = !!wanted->sg;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device scatter-gather settings");
-			return 86;
-		}
+		else
+			changed = 1;
 	}
 
-	if (off_tso_wanted >= 0) {
-		changed = 1;
+	if (wanted->tso >= 0) {
 		eval.cmd = ETHTOOL_STSO;
-		eval.data = (off_tso_wanted == 1);
+		eval.data = !!wanted->tso;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device tcp segmentation offload settings");
-			return 88;
-		}
+		else
+			changed = 1;
 	}
-	if (off_ufo_wanted >= 0) {
-		changed = 1;
+	if (wanted->ufo >= 0) {
 		eval.cmd = ETHTOOL_SUFO;
-		eval.data = (off_ufo_wanted == 1);
+		eval.data = !!wanted->ufo;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device udp large send offload settings");
-			return 89;
-		}
+		else
+			changed = 1;
 	}
-	if (off_gso_wanted >= 0) {
-		changed = 1;
+	if (wanted->gso >= 0) {
 		eval.cmd = ETHTOOL_SGSO;
-		eval.data = (off_gso_wanted == 1);
+		eval.data = !!wanted->gso;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device generic segmentation offload settings");
-			return 90;
-		}
+		else
+			changed = 1;
 	}
 	if (off_flags_mask) {
-		changed = 1;
 		eval.cmd = ETHTOOL_GFLAGS;
 		eval.data = 0;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot get device flag settings");
-			return 91;
-		}
-
-		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data = ((eval.data & ~off_flags_mask) |
-			     off_flags_wanted);
-
-		err = send_ioctl(ctx, &eval);
-		if (err) {
-			perror("Cannot set device flag settings");
-			return 92;
+		else {
+			eval.cmd = ETHTOOL_SFLAGS;
+			eval.data = ((eval.data & ~off_flags_mask) |
+				     off_flags_wanted);
+
+			err = send_ioctl(ctx, &eval);
+			if (err)
+				perror("Cannot set device flag settings");
+			else
+				changed = 1;
 		}
 	}
-	if (off_gro_wanted >= 0) {
-		changed = 1;
+	if (wanted->gro >= 0) {
 		eval.cmd = ETHTOOL_SGRO;
-		eval.data = (off_gro_wanted == 1);
+		eval.data = !!wanted->gro;
 		err = send_ioctl(ctx, &eval);
-		if (err) {
+		if (err)
 			perror("Cannot set device GRO settings");
-			return 93;
+		else
+			changed = 1;
+	}
+
+	return changed;
+}
+
+static int n_feature_strings;
+static const char **feature_strings;
+
+static int init_feature_strings(struct cmd_context *ctx)
+{
+	struct ethtool_gstrings *strings;
+	int i, n;
+
+	if (feature_strings)
+		return n_feature_strings;
+
+	strings = get_stringset(ctx, ETH_SS_FEATURES, 0);
+	if (!strings)
+		return -100;
+
+	n = n_feature_strings = strings->len;
+	feature_strings = calloc(n, sizeof(*feature_strings));
+	if (!feature_strings) {
+		fprintf(stderr, "no memory available for string table [size=%d]\n", n);
+		exit(95);
+	}
+
+	for (i = 0; i < n; ++i) {
+		if (!strings->data[i*ETH_GSTRING_LEN])
+			continue;
+
+		feature_strings[i] = strndup(
+			(const char *)&strings->data[i * ETH_GSTRING_LEN],
+			ETH_GSTRING_LEN);
+
+		if (!feature_strings[i]) {
+			fprintf(stderr, "no memory available for a string\n");
+			exit(95);
 		}
 	}
 
+	free(strings);
+	return n;
+}
+
+static void parse_sfeatures_args(struct cmd_context *ctx,
+	struct offload_state *offload,
+	struct ethtool_sfeatures **features_req_p)
+{
+	struct ethtool_sfeatures *features_req;
+	struct cmdline_info *cmdline_desc, *cp;
+	int sz_features, i;
+	int changed = 0;
+
+	struct cmdline_info cmdline_offload[] = {
+		{ "rx", CMDL_BOOL, &offload->rx, NULL },
+		{ "tx", CMDL_BOOL, &offload->tx, NULL },
+		{ "sg", CMDL_BOOL, &offload->sg, NULL },
+		{ "tso", CMDL_BOOL, &offload->tso, NULL },
+		{ "ufo", CMDL_BOOL, &offload->ufo, NULL },
+		{ "gso", CMDL_BOOL, &offload->gso, NULL },
+		{ "lro", CMDL_BOOL, &offload->lro, NULL },
+		{ "gro", CMDL_BOOL, &offload->gro, NULL },
+		{ "rxvlan", CMDL_BOOL, &offload->rxvlan, NULL },
+		{ "txvlan", CMDL_BOOL, &offload->txvlan, NULL },
+		{ "ntuple", CMDL_BOOL, &offload->ntuple, NULL },
+		{ "rxhash", CMDL_BOOL, &offload->rxhash, NULL },
+	};
+
+	for (i = 0; i < ARRAY_SIZE(old_feature_names); ++i)
+		((int *)offload)[i] = -1;
+	*features_req_p = NULL;
+
+	if (init_feature_strings(ctx) < 0) {
+		/* ETHTOOL_GFEATURES unavailable */
+		parse_generic_cmdline(ctx, &changed,
+			cmdline_offload, ARRAY_SIZE(cmdline_offload));
+		return;
+	}
+
+	sz_features = sizeof(*features_req->features) * ((n_feature_strings + 31) / 32);
+
+	cp = cmdline_desc = calloc(n_feature_strings + ARRAY_SIZE(cmdline_offload),
+		sizeof(*cmdline_desc));
+	memcpy(cp, cmdline_offload, sizeof(cmdline_offload));
+	cp += ARRAY_SIZE(cmdline_offload);
+
+	features_req = calloc(1, sizeof(*features_req) + sz_features);
+	if (!cmdline_desc || !features_req) {
+		fprintf(stderr, "no memory available\n");
+		exit(95);
+	}
+
+	features_req->size = (n_feature_strings + 31) / 32;
+
+	for (i = 0; i < n_feature_strings; ++i) {
+		if (!feature_strings[i])
+			continue;
+
+		cp->name = feature_strings[i];
+		cp->type = CMDL_FLAG;
+		cp->flag_val = 1 << (i % 32);
+		cp->wanted_val = &features_req->features[i / 32].requested;
+		cp->seen_val = &features_req->features[i / 32].valid;
+		++cp;
+	}
+
+	parse_generic_cmdline(ctx, &changed, cmdline_desc, cp - cmdline_desc);
+
+	free(cmdline_desc);
+
+	if (changed)
+		*features_req_p = features_req;
+	else
+		free(features_req);
+}
+
+static int send_gfeatures(struct cmd_context *ctx,
+	struct ethtool_gfeatures **features_p)
+{
+	struct ethtool_gfeatures *features;
+	int err, sz_features;
+
+	sz_features = sizeof(*features->features) * ((n_feature_strings + 31) / 32);
+	features = calloc(1, sz_features + sizeof(*features));
+	if (!features) {
+		fprintf(stderr, "no memory available\n");
+		return 95;
+	}
+
+	features->cmd = ETHTOOL_GFEATURES;
+	features->size = (n_feature_strings + 31) / 32;
+	err = send_ioctl(ctx, features);
+
+	if (err < 0) {
+		perror("Cannot get feature status");
+		free(features);
+		return 97;
+	}
+
+	*features_p = features;
+	return 0;
+}
+
+static const char *get_feature_state(struct ethtool_get_features_block *gfb,
+	uint32_t bit)
+{
+	if (gfb->never_changed & bit)
+		return "fixed";
+	if (!(gfb->available & bit))
+		return "driver-controlled";
+
+	if ((gfb->active ^ gfb->requested) & bit)
+		return (gfb->requested & bit) ? "requested on" : "requested off";
+	else
+		return "changeable";
+}
+
+static int do_gfeatures(struct cmd_context *ctx)
+{
+	struct ethtool_get_features_block *gfb;
+	struct ethtool_gfeatures *features;
+	uint32_t bit;
+	int err, i;
+
+	err = init_feature_strings(ctx);
+	if (err < 0)
+		return -err;
+
+	err = send_gfeatures(ctx, &features);
+	if (err)
+		return err;
+
+	fprintf(stdout, "\nExtended offload state for %s:\n", ctx->devname);
+	for (i = 0; i < n_feature_strings; i++) {
+		if (!feature_strings[i])
+			continue;	/* empty */
+
+		gfb = features->features + i / 32;
+		bit = 1 << (i % 32);
+
+		fprintf(stdout, "%s: %s (%s)\n", feature_strings[i],
+			(gfb->active & bit) ? "on" : "off",
+			get_feature_state(gfb, bit));
+	}
+	free(features);
+
+	return 0;
+}
+
+static void print_gfeatures_diff(
+	const struct ethtool_get_features_block *expected,
+	const struct ethtool_get_features_block *set,
+	const char **strings, int n_strings)
+{
+	int i;
+
+	if (n_strings > 32)
+		n_strings = 32;
+
+	for (i = 0; i < n_strings; ++i) {
+		u32 mask = 1 << i;
+
+		if (!strings[i])
+			continue;
+
+		if (!((expected->active ^ set->active) & mask))
+			continue;
+
+		fprintf(stdout, "feature %.*s is %s (expected: %s, saved: %s)\n",
+			ETH_GSTRING_LEN, strings[i],
+			set->active & mask ? "enabled" : "disabled",
+			expected->active & mask ? "enabled" : "disabled",
+			!(set->available & mask) ? "not user-changeable" :
+				set->requested & mask ? "enabled" : "disabled"
+		);
+	}
+}
+
+static int get_offload_state(struct cmd_context *ctx,
+	struct ethtool_gfeatures **features,
+	struct offload_state *offload)
+{
+	int err, allfail;
+
+	if (ctx->argc != 0)
+		exit_bad_args();
+
+	allfail = send_goffloads(ctx, offload);
+
+	err = init_feature_strings(ctx);
+	if (err < 0)
+		return allfail ? err : 0;
+
+	err = send_gfeatures(ctx, features);
+	if (err)
+		perror("Cannot read features");
+
+	return allfail ? -err : 0;
+}
+
+static int send_sfeatures(struct cmd_context *ctx,
+	struct ethtool_sfeatures *features)
+{
+	int err;
+
+	features->cmd = ETHTOOL_SFEATURES;
+	err = send_ioctl(ctx, features);
+	if (err < 0) {
+		perror("Cannot change features");
+		return 97;
+	}
+
+	return 0;
+}
+
+static void compare_offload_state(struct offload_state *offload0,
+	struct offload_state *offload_req, struct offload_state *offload1)
+{
+	int *old = (int *)offload0;
+	int *req = (int *)offload_req;
+	int *new = (int *)offload1;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(old_feature_names); i++) {
+		if (req[i] < 0)
+			req[i] = old[i];
+		if (req[i] == new[i])
+			continue;
+
+		fprintf(stdout, "feature group %s is %s (expected: %s)\n",
+			old_feature_names[i],
+			new[i] ? "enabled" : "disabled",
+			req[i] ? "enabled" : "disabled"
+		);
+	}
+}
+
+static void compare_features(struct ethtool_gfeatures *features0,
+	struct ethtool_sfeatures *features_req,
+	struct ethtool_gfeatures *features1)
+{
+	int i;
+
+	if (features_req) {
+		/* make features0 .active what we expect to be set */
+		i = (n_feature_strings + 31) / 32;
+		while (i--) {
+			features0->features[i].active &= ~features_req->features[i].valid;
+			features0->features[i].active |=
+				features_req->features[i].requested &
+				features_req->features[i].valid;
+		}
+	}
+
+	for (i = 0; i < n_feature_strings; i += 32)
+		print_gfeatures_diff(&features0->features[i / 32],
+			&features1->features[i / 32],
+			feature_strings + i,
+			n_feature_strings - i);
+}
+
+static int do_goffload(struct cmd_context *ctx)
+{
+	struct offload_state offload;
+	int err, allfail;
+
+	allfail = send_goffloads(ctx, &offload);
+
+	if (!allfail) {
+		fprintf(stdout, "Offload parameters for %s:\n", ctx->devname);
+
+		dump_offload(&offload);
+	}
+
+	err = do_gfeatures(ctx);
+	if (!err)
+		allfail = 0;
+
+	if (allfail) {
+		fprintf(stdout, "no offload info available\n");
+		return 83;
+	}
+
+	return 0;
+}
+
+static int do_soffload(struct cmd_context *ctx)
+{
+	struct ethtool_gfeatures *features_old, *features_new;
+	struct ethtool_sfeatures *features_req;
+	struct offload_state offload_old, offload_new, offload_req;
+	int err, changed;
+
+	parse_sfeatures_args(ctx, &offload_req, &features_req);
+
+	err = get_offload_state(ctx, &features_old, &offload_old);
+	if (err)
+		return -err;
+
+	changed = send_soffloads(ctx, &offload_req);
+
+	if (features_req) {
+		err = send_sfeatures(ctx, features_req);
+		if (!err)
+			changed = 1;
+	}
+
 	if (!changed) {
 		fprintf(stdout, "no offload settings changed\n");
+		return err;
+	}
+
+	err = get_offload_state(ctx, &features_new, &offload_new);
+	if (err) {
+		perror("can't verify offload setting");
+		return 101;
+	}
+
+	if ((!features_old) ^ (!features_new)) {
+		fprintf(stderr, "can't compare features (one GFEATURES failed)\n");
+		if (features_old) {
+			free(features_old);
+			features_old = NULL;
+		}
+		if (features_new) {
+			free(features_new);
+			features_new = NULL;
+		}
+	}
+
+	compare_offload_state(&offload_old, &offload_req, &offload_new);
+	if (features_old) {
+		compare_features(features_old, features_req, features_new);
+		free(features_old);
+		free(features_new);
 	}
+	if (features_req)
+		free(features_req);
 
 	return 0;
 }
-- 
1.7.8.3

Re: [RFC PATCH v2] ethtool: implement [GS]FEATURES handling

[Eric Dumazet]

Le samedi 28 janvier 2012 à 09:37 +0100, Michał Mirosław a écrit :
> v2: fix missing change CMDL_FLAG -> CMDL_BOOL for old offloads parsing
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  ethtool.c |  613 ++++++++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 490 insertions(+), 123 deletions(-)

Seems fine to me, here is the result for the new section :

I can now 'see' my tg3 doesnt support tx-tcp6-segmentation without
having to read driver source...

Extended offload state for eth3:
tx-scatter-gather: on (changeable)
tx-checksum-ipv4: on (changeable)
tx-checksum-ip-generic: off (driver-controlled)
tx-checksum-ipv6: off (driver-controlled)
highdma: on (changeable)
tx-scatter-gather-fraglist: off (driver-controlled)
tx-vlan-hw-insert: on (changeable)
rx-vlan-hw-parse: on (changeable)
rx-vlan-filter: off (driver-controlled)
vlan-challenged: off (fixed)
tx-generic-segmentation: on (changeable)
tx-lockless: off (fixed)
netns-local: off (fixed)
rx-gro: on (changeable)
rx-lro: off (driver-controlled)
tx-tcp-segmentation: on (changeable)
tx-udp-fragmentation: off (driver-controlled)
tx-gso-robust: off (driver-controlled)
tx-tcp-ecn-segmentation: off (driver-controlled)
tx-tcp6-segmentation: off (driver-controlled)
tx-fcoe-segmentation: off (driver-controlled)
tx-checksum-fcoe-crc: off (driver-controlled)
tx-checksum-sctp: off (driver-controlled)
fcoe-mtu: off (driver-controlled)
rx-ntuple-filter: off (driver-controlled)
rx-hashing: off (driver-controlled)
rx-checksum: on (changeable)
tx-nocache-copy: on (changeable)
loopback: off (changeable)

Thanks !