All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH ethtool 0/3] Regularise handling of offload flags
@ 2011-02-21 16:54 Ben Hutchings
  2011-02-21 16:57 ` [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6 Ben Hutchings
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ben Hutchings @ 2011-02-21 16:54 UTC (permalink / raw)
  To: netdev, Michał Mirosław

This patch series is partly inspired by the recent changes to regularise
the relationship between ethtool offload control and net_device feature
flags, though I started on it earlier.

I don't intend to apply these changes until after the kernel and ethtool
v2.6.38 releases.  In the mean time, I would appreciate testing and
feedback.

Ben.

Ben Hutchings (3):
  ethtool-copy.h: sync with net-next-2.6
  ethtool: Regularise handling of offload flags
  ethtool: Report additional offload flag changes made automatically

 ethtool-copy.h |   89 +++++++++++-
 ethtool-util.h |   30 ++++
 ethtool.c      |  465 ++++++++++++++++++++++++++++----------------------------
 3 files changed, 352 insertions(+), 232 deletions(-)

-- 
1.7.3.4


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


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

* [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6
  2011-02-21 16:54 [PATCH ethtool 0/3] Regularise handling of offload flags Ben Hutchings
@ 2011-02-21 16:57 ` Ben Hutchings
  2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
  2011-02-21 16:59 ` [PATCH ethtool 3/3] ethtool: Report additional offload flag changes made automatically Ben Hutchings
  2 siblings, 0 replies; 9+ messages in thread
From: Ben Hutchings @ 2011-02-21 16:57 UTC (permalink / raw)
  To: netdev, Michał Mirosław

This covers kernel changes up to:

commit e83d360d9a7e5d71d55c13e96b19109a2ea23bf0
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date:   Tue Feb 15 16:59:18 2011 +0000

    net: introduce NETIF_F_RXCSUM

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool-copy.h |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 88 insertions(+), 1 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
-- 
1.7.3.4



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


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

* [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-21 16:54 [PATCH ethtool 0/3] Regularise handling of offload flags Ben Hutchings
  2011-02-21 16:57 ` [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6 Ben Hutchings
@ 2011-02-21 16:59 ` Ben Hutchings
  2011-02-22 14:50   ` Michał Mirosław
                     ` (2 more replies)
  2011-02-21 16:59 ` [PATCH ethtool 3/3] ethtool: Report additional offload flag changes made automatically Ben Hutchings
  2 siblings, 3 replies; 9+ messages in thread
From: Ben Hutchings @ 2011-02-21 16:59 UTC (permalink / raw)
  To: netdev, Michał Mirosław

Use the new ETHTOOL_{G,S}FEATURES operations where available, and
use the new structure and netif feature flags in any case.

Replace repetitive code for getting/setting offload flags with data-
driven loops.

This changes error messages to use the same long names for offload
flags as in dump_offload(), and changes various exit codes to 1.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
NEITF_F_* flags are copied into ethtool-util.h for now.  I think in
future they should be exposed from <linux/netdevice.h> (hence the
#ifndef).

Ben.

 ethtool-util.h |   30 ++++
 ethtool.c      |  441 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 243 insertions(+), 228 deletions(-)

diff --git a/ethtool-util.h b/ethtool-util.h
index f053028..a118202 100644
--- a/ethtool-util.h
+++ b/ethtool-util.h
@@ -40,6 +40,36 @@ static inline u32 cpu_to_be32(u32 value)
 }
 #endif
 
+#ifndef NETIF_F_SG
+#define NETIF_F_SG		(1 << 0)
+#define NETIF_F_IP_CSUM		(1 << 1)
+#define NETIF_F_NO_CSUM		(1 << 2)
+#define NETIF_F_HW_CSUM		(1 << 3)
+#define NETIF_F_IPV6_CSUM	(1 << 4)
+#define NETIF_F_FRAGLIST	(1 << 6)
+#define NETIF_F_HW_VLAN_TX	(1 << 7)
+#define NETIF_F_HW_VLAN_RX	(1 << 8)
+#define NETIF_F_HW_VLAN_FILTER	(1 << 9)
+#define NETIF_F_GSO		(1 << 11)
+#define NETIF_F_GRO		(1 << 14)
+#define NETIF_F_LRO		(1 << 15)
+#define NETIF_F_TSO		(1 << 16)
+#define NETIF_F_UFO		(1 << 17)
+#define NETIF_F_GSO_ROBUST	(1 << 18)
+#define NETIF_F_TSO_ECN		(1 << 19)
+#define NETIF_F_TSO6		(1 << 20)
+#define NETIF_F_FSO		(1 << 21)
+#define NETIF_F_FCOE_CRC	(1 << 24)
+#define NETIF_F_SCTP_CSUM	(1 << 25)
+#define NETIF_F_FCOE_MTU	(1 << 26)
+#define NETIF_F_NTUPLE		(1 << 27)
+#define NETIF_F_RXHASH		(1 << 28)
+#define NETIF_F_RXCSUM		(1 << 29)
+#define NETIF_F_ALL_CSUM	(NETIF_F_NO_CSUM | NETIF_F_HW_CSUM | \
+				 NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)
+#define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
+#endif
+
 /* National Semiconductor DP83815, DP83816 */
 int natsemi_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
 int natsemi_dump_eeprom(struct ethtool_drvinfo *info,
diff --git a/ethtool.c b/ethtool.c
index f680b6d..25601a5 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -299,15 +299,7 @@ static void show_usage(int badarg)
 static char *devname = NULL;
 
 static int goffload_changed = 0;
-static int off_csum_rx_wanted = -1;
-static int off_csum_tx_wanted = -1;
-static int off_sg_wanted = -1;
-static int off_tso_wanted = -1;
-static int off_ufo_wanted = -1;
-static int off_gso_wanted = -1;
-static u32 off_flags_wanted = 0;
-static u32 off_flags_mask = 0;
-static int off_gro_wanted = -1;
+struct ethtool_set_features_block off_features;
 
 static struct ethtool_pauseparam epause;
 static int gpause_changed = 0;
@@ -463,23 +455,30 @@ static struct cmdline_info cmdline_seeprom[] = {
 };
 
 static 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 },
+	{ "rx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXCSUM, &off_features.valid },
+	{ "tx", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_CSUM, &off_features.valid },
+	{ "sg", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_SG, &off_features.valid },
+	{ "tso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_ALL_TSO, &off_features.valid },
+	{ "ufo", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_UFO, &off_features.valid },
+	{ "gso", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GSO, &off_features.valid },
+	{ "lro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_LRO, &off_features.valid },
+	{ "gro", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_GRO, &off_features.valid },
+	{ "rxvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "txvlan", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_HW_VLAN_TX, &off_features.valid },
+	{ "ntuple", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_NTUPLE, &off_features.valid },
+	{ "rxhash", CMDL_FLAG, &off_features.requested, NULL,
+	  NETIF_F_RXHASH, &off_features.valid },
 };
 
 static struct cmdline_info cmdline_pause[] = {
@@ -1872,35 +1871,39 @@ static int dump_coalesce(void)
 	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)
+static const struct {
+	const char *long_name;
+	u32 cmd;
+	u32 value;
+} off_feature_def[] = {
+	{ "rx-checksumming",		  ETHTOOL_GRXCSUM, NETIF_F_RXCSUM },
+	{ "tx-checksumming",		  ETHTOOL_GTXCSUM, NETIF_F_ALL_CSUM },
+	{ "scatter-gather",		  ETHTOOL_GSG,	   NETIF_F_SG },
+	{ "tcp-segmentation-offload",	  ETHTOOL_GTSO,	   NETIF_F_ALL_TSO },
+	{ "udp-fragmentation-offload",	  ETHTOOL_GUFO,	   NETIF_F_UFO },
+	{ "generic-segmentation-offload", ETHTOOL_GGSO,	   NETIF_F_GSO },
+	{ "generic-receive-offload",	  ETHTOOL_GGRO,	   NETIF_F_GRO },
+	{ "large-receive-offload",	  0,		   NETIF_F_LRO },
+	{ "rx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_RX },
+	{ "tx-vlan-offload",		  0,		   NETIF_F_HW_VLAN_TX },
+	{ "ntuple-filters",		  0,		   NETIF_F_NTUPLE },
+	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
+};
+
+static int dump_offload(const struct ethtool_get_features_block *features)
 {
-	fprintf(stdout,
-		"rx-checksumming: %s\n"
-		"tx-checksumming: %s\n"
-		"scatter-gather: %s\n"
-		"tcp-segmentation-offload: %s\n"
-		"udp-fragmentation-offload: %s\n"
-		"generic-segmentation-offload: %s\n"
-		"generic-receive-offload: %s\n"
-		"large-receive-offload: %s\n"
-		"rx-vlan-offload: %s\n"
-		"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");
+	u32 value;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		printf("%s: %s%s%s\n",
+		       off_feature_def[i].long_name,
+		       (features->active & value) ? "on" : "off",
+		       (features->requested & ~features->active & value) ?
+		       " [requested on]" : "",
+		       (~features->available & value) ? " [unchangeable]" : "");
+	}
 
 	return 0;
 }
@@ -2219,97 +2222,72 @@ static int do_scoalesce(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+/* the following list of flags are the same as their associated
+ * NETIF_F_xxx values in include/linux/netdevice.h
+ */
+static const u32 flags_dup_features =
+	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
+	 ETH_FLAG_RXHASH);
+
 static int do_goffload(int fd, struct ifreq *ifr)
 {
+	struct {
+		struct ethtool_gfeatures cmd;
+		struct ethtool_get_features_block data[1];
+	} features;
 	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;
+	int err, allfail = 1;
+	u32 value;
+	int i;
 
 	fprintf(stdout, "Offload parameters for %s:\n", devname);
 
-	eval.cmd = ETHTOOL_GRXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device rx csum settings");
-	else {
-		rx = eval.data;
+	features.cmd.cmd = ETHTOOL_GFEATURES;
+	features.cmd.size = ARRAY_SIZE(features.data);
+	ifr->ifr_data = (caddr_t)&features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err == 0) {
 		allfail = 0;
-	}
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot get device offload settings");
+	} else {
+		memset(&features.data, 0, sizeof(features.data));
 
-	eval.cmd = ETHTOOL_GTXCSUM;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tx csum settings");
-	else {
-		tx = eval.data;
-		allfail = 0;
-	}
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			value = off_feature_def[i].value;
 
-	eval.cmd = ETHTOOL_GSG;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device scatter-gather settings");
-	else {
-		sg = eval.data;
-		allfail = 0;
-	}
+			/* Assume that anything we can get is changeable */
+			features.data[0].available |= value;
 
-	eval.cmd = ETHTOOL_GTSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = send_ioctl(fd, ifr);
-	if (err)
-		perror("Cannot get device tcp segmentation offload settings");
-	else {
-		tso = eval.data;
-		allfail = 0;
-	}
+			if (!off_feature_def[i].cmd)
+				continue;
 
-	eval.cmd = ETHTOOL_GUFO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device udp large send offload settings");
-	else {
-		ufo = eval.data;
-		allfail = 0;
-	}
-
-	eval.cmd = ETHTOOL_GGSO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device generic segmentation offload settings");
-	else {
-		gso = eval.data;
-		allfail = 0;
-	}
+			eval.cmd = off_feature_def[i].cmd;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = send_ioctl(fd, ifr);
+			if (err) {
+				fprintf(stderr,
+					"Cannot get device %s settings: %m\n",
+					off_feature_def[i].long_name);
+			} else {
+				if (eval.data)
+					features.data[0].active |= value;
+				allfail = 0;
+			}
+		}
 
-	eval.cmd = ETHTOOL_GFLAGS;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	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;
-		allfail = 0;
-	}
+		eval.cmd = ETHTOOL_GFLAGS;
+		ifr->ifr_data = (caddr_t)&eval;
+		err = ioctl(fd, SIOCETHTOOL, ifr);
+		if (err) {
+			perror("Cannot get device flags");
+		} else {
+			features.data[0].active |=
+				eval.data & flags_dup_features;
+			allfail = 0;
+		}
 
-	eval.cmd = ETHTOOL_GGRO;
-	ifr->ifr_data = (caddr_t)&eval;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err)
-		perror("Cannot get device GRO settings");
-	else {
-		gro = eval.data;
-		allfail = 0;
+		features.data[0].requested = features.data[0].active;
 	}
 
 	if (allfail) {
@@ -2317,114 +2295,121 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		return 83;
 	}
 
-	return dump_offload(rx, tx, sg, tso, ufo, gso, gro, lro, rxvlan, txvlan,
-			    ntuple, rxhash);
+	return dump_offload(features.data);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
+	struct {
+		struct ethtool_gfeatures cmd;
+		struct ethtool_get_features_block data[1];
+	} get_features;
+	struct {
+		struct ethtool_sfeatures cmd;
+		struct ethtool_set_features_block data[1];
+	} set_features;
 	struct ethtool_value eval;
 	int err, changed = 0;
+	u32 value;
+	int i;
 
-	if (off_csum_rx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SRXCSUM;
-		eval.data = (off_csum_rx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device rx csum settings");
-			return 84;
+	get_features.cmd.cmd = ETHTOOL_GFEATURES;
+	get_features.cmd.size = ARRAY_SIZE(get_features.data);
+	ifr->ifr_data = (caddr_t)&get_features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err == 0) {
+		set_features.cmd.cmd = ETHTOOL_SFEATURES;
+		set_features.cmd.size = ARRAY_SIZE(set_features.data);
+		set_features.data[0] = off_features;
+
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			value = off_feature_def[i].value;
+			if (!(off_features.valid & value))
+				continue;
+			if (!(get_features.data[0].available & value)) {
+				/* None of these features can be changed */
+				fprintf(stderr,
+					"Cannot set device %s settings: "
+					"Operation not supported\n",
+					off_feature_def[i].long_name);
+			} else if (off_features.requested & value) {
+				/* Some of these features can be
+				 * enabled; mask out any that cannot
+				 */
+				set_features.data[0].requested &=
+					~(value &
+					  ~get_features.data[0].available);
+			}
 		}
-	}
 
-	if (off_csum_tx_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STXCSUM;
-		eval.data = (off_csum_tx_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tx csum settings");
-			return 85;
+		ifr->ifr_data = (caddr_t)&set_features;
+		err = ioctl(fd, SIOCETHTOOL, ifr);
+		if (err < 0) {
+			perror("Cannot set device offload settings");
+			return 1;
 		}
-	}
 
-	if (off_sg_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SSG;
-		eval.data = (off_sg_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device scatter-gather settings");
-			return 86;
-		}
-	}
+		changed = !!set_features.data[0].valid;
 
-	if (off_tso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_STSO;
-		eval.data = (off_tso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = send_ioctl(fd, ifr);
-		if (err) {
-			perror("Cannot set device tcp segmentation offload settings");
-			return 88;
-		}
-	}
-	if (off_ufo_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SUFO;
-		eval.data = (off_ufo_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device udp large send offload settings");
-			return 89;
-		}
-	}
-	if (off_gso_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGSO;
-		eval.data = (off_gso_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device generic segmentation offload settings");
-			return 90;
-		}
-	}
-	if (off_flags_mask) {
-		changed = 1;
-		eval.cmd = ETHTOOL_GFLAGS;
-		eval.data = 0;
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot get device flag settings");
-			return 91;
+		if (err & ETHTOOL_F_WISH)
+			fprintf(stderr,
+				"Cannot set device offload settings: "
+				"Some requested features depend on others "
+				"that are not currently enabled\n");
+
+		/* ETHTOOL_F_UNSUPPORTED should never be set as we
+		 * checked for unsupported flags above.  Treat any
+		 * other warning flags as unknown.
+		 */
+		if (err & ~ETHTOOL_F_WISH)
+			fprintf(stderr,
+				"Cannot set device offload settings: "
+				"warning flags %#x",
+				err & ~ETHTOOL_F_WISH);
+	} else if (errno != EOPNOTSUPP && errno != EPERM) {
+		perror("Cannot get device offload settings");
+		return 1;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+			if (!off_feature_def[i].cmd)
+				continue;
+			if (off_features.valid & off_feature_def[i].value) {
+				changed = 1;
+				eval.cmd = off_feature_def[i].cmd + 1;
+				eval.data = !!(off_features.requested &
+					       off_feature_def[i].value);
+				ifr->ifr_data = (caddr_t)&eval;
+				err = send_ioctl(fd, ifr);
+				if (err) {
+					fprintf(stderr,
+						"Cannot set device %s settings: %m\n",
+						off_feature_def[i].long_name);
+					return 1;
+				}
+			}
 		}
 
-		eval.cmd = ETHTOOL_SFLAGS;
-		eval.data = ((eval.data & ~off_flags_mask) |
-			     off_flags_wanted);
+		if (off_features.valid & flags_dup_features) {
+			changed = 1;
+			eval.cmd = ETHTOOL_GFLAGS;
+			eval.data = 0;
+			ifr->ifr_data = (caddr_t)&eval;
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot get device flag settings");
+				return 91;
+			}
 
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device flag settings");
-			return 92;
-		}
-	}
-	if (off_gro_wanted >= 0) {
-		changed = 1;
-		eval.cmd = ETHTOOL_SGRO;
-		eval.data = (off_gro_wanted == 1);
-		ifr->ifr_data = (caddr_t)&eval;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err) {
-			perror("Cannot set device GRO settings");
-			return 93;
+			eval.cmd = ETHTOOL_SFLAGS;
+			eval.data &= ~(off_features.valid & flags_dup_features);
+			eval.data |= (off_features.requested &
+				      flags_dup_features);
+
+			err = ioctl(fd, SIOCETHTOOL, ifr);
+			if (err) {
+				perror("Cannot set device flag settings");
+				return 92;
+			}
 		}
 	}
 
-- 
1.7.3.4



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


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

* [PATCH ethtool 3/3] ethtool: Report additional offload flag changes made automatically
  2011-02-21 16:54 [PATCH ethtool 0/3] Regularise handling of offload flags Ben Hutchings
  2011-02-21 16:57 ` [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6 Ben Hutchings
  2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
@ 2011-02-21 16:59 ` Ben Hutchings
  2 siblings, 0 replies; 9+ messages in thread
From: Ben Hutchings @ 2011-02-21 16:59 UTC (permalink / raw)
  To: netdev, Michał Mirosław

Turning off checksum offloads or SG will cause the kernel to turn off
other offloads that depend on them.  On some hardware, other offload
features may have to be turned on or off together.  Therefore, check
the offload flags before and after making changes and report any
additional changes beyond those requested.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 ethtool.c |  114 +++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 25601a5..4e6bd41 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1890,13 +1890,16 @@ static const struct {
 	{ "receive-hashing",		  0,		   NETIF_F_RXHASH },
 };
 
-static int dump_offload(const struct ethtool_get_features_block *features)
+static int
+dump_offload(const struct ethtool_get_features_block *features, u32 mask)
 {
 	u32 value;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 		value = off_feature_def[i].value;
+		if (!(mask & value))
+			continue;
 		printf("%s: %s%s%s\n",
 		       off_feature_def[i].long_name,
 		       (features->active & value) ? "on" : "off",
@@ -2229,7 +2232,8 @@ static const u32 flags_dup_features =
 	(ETH_FLAG_LRO | ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |
 	 ETH_FLAG_RXHASH);
 
-static int do_goffload(int fd, struct ifreq *ifr)
+static int get_offload(int fd, struct ifreq *ifr,
+		       struct ethtool_get_features_block *pfeatures)
 {
 	struct {
 		struct ethtool_gfeatures cmd;
@@ -2240,24 +2244,23 @@ static int do_goffload(int fd, struct ifreq *ifr)
 	u32 value;
 	int i;
 
-	fprintf(stdout, "Offload parameters for %s:\n", devname);
-
 	features.cmd.cmd = ETHTOOL_GFEATURES;
 	features.cmd.size = ARRAY_SIZE(features.data);
 	ifr->ifr_data = (caddr_t)&features;
 	err = ioctl(fd, SIOCETHTOOL, ifr);
 	if (err == 0) {
 		allfail = 0;
+		pfeatures[0] = features.data[0];
 	} else if (errno != EOPNOTSUPP && errno != EPERM) {
 		perror("Cannot get device offload settings");
 	} else {
-		memset(&features.data, 0, sizeof(features.data));
+		memset(pfeatures, 0, sizeof(*pfeatures));
 
 		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
 			value = off_feature_def[i].value;
 
 			/* Assume that anything we can get is changeable */
-			features.data[0].available |= value;
+			pfeatures[0].available |= value;
 
 			if (!off_feature_def[i].cmd)
 				continue;
@@ -2271,7 +2274,7 @@ static int do_goffload(int fd, struct ifreq *ifr)
 					off_feature_def[i].long_name);
 			} else {
 				if (eval.data)
-					features.data[0].active |= value;
+					pfeatures[0].active |= value;
 				allfail = 0;
 			}
 		}
@@ -2282,28 +2285,34 @@ static int do_goffload(int fd, struct ifreq *ifr)
 		if (err) {
 			perror("Cannot get device flags");
 		} else {
-			features.data[0].active |=
+			pfeatures[0].active |=
 				eval.data & flags_dup_features;
 			allfail = 0;
 		}
 
-		features.data[0].requested = features.data[0].active;
+		pfeatures[0].requested = pfeatures[0].active;
 	}
 
-	if (allfail) {
+	return allfail;
+}
+
+static int do_goffload(int fd, struct ifreq *ifr)
+{
+	struct ethtool_get_features_block features;
+
+	fprintf(stdout, "Offload parameters for %s:\n", devname);
+
+	if (get_offload(fd, ifr, &features)) {
 		fprintf(stdout, "no offload info available\n");
 		return 83;
 	}
 
-	return dump_offload(features.data);
+	return dump_offload(&features, ~(u32)0);
 }
 
 static int do_soffload(int fd, struct ifreq *ifr)
 {
-	struct {
-		struct ethtool_gfeatures cmd;
-		struct ethtool_get_features_block data[1];
-	} get_features;
+	struct ethtool_get_features_block old_features, new_features;
 	struct {
 		struct ethtool_sfeatures cmd;
 		struct ethtool_set_features_block data[1];
@@ -2313,42 +2322,37 @@ static int do_soffload(int fd, struct ifreq *ifr)
 	u32 value;
 	int i;
 
-	get_features.cmd.cmd = ETHTOOL_GFEATURES;
-	get_features.cmd.size = ARRAY_SIZE(get_features.data);
-	ifr->ifr_data = (caddr_t)&get_features;
-	err = ioctl(fd, SIOCETHTOOL, ifr);
-	if (err == 0) {
-		set_features.cmd.cmd = ETHTOOL_SFEATURES;
-		set_features.cmd.size = ARRAY_SIZE(set_features.data);
-		set_features.data[0] = off_features;
+	if (get_offload(fd, ifr, &old_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
 
-		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
-			value = off_feature_def[i].value;
-			if (!(off_features.valid & value))
-				continue;
-			if (!(get_features.data[0].available & value)) {
-				/* None of these features can be changed */
-				fprintf(stderr,
-					"Cannot set device %s settings: "
-					"Operation not supported\n",
-					off_feature_def[i].long_name);
-			} else if (off_features.requested & value) {
-				/* Some of these features can be
-				 * enabled; mask out any that cannot
-				 */
-				set_features.data[0].requested &=
-					~(value &
-					  ~get_features.data[0].available);
-			}
-		}
+	set_features.cmd.cmd = ETHTOOL_SFEATURES;
+	set_features.cmd.size = ARRAY_SIZE(set_features.data);
+	set_features.data[0] = off_features;
 
-		ifr->ifr_data = (caddr_t)&set_features;
-		err = ioctl(fd, SIOCETHTOOL, ifr);
-		if (err < 0) {
-			perror("Cannot set device offload settings");
-			return 1;
+	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
+		value = off_feature_def[i].value;
+		if (!(off_features.valid & value))
+			continue;
+		if (!(old_features.available & value)) {
+			/* None of these features can be changed */
+			fprintf(stderr,
+				"Cannot set device %s settings: "
+				"Operation not supported\n",
+				off_feature_def[i].long_name);
+		} else if (off_features.requested & value) {
+			/* Some of these features can be
+			 * enabled; mask out any that cannot
+			 */
+			set_features.data[0].requested &=
+				~(value & ~old_features.available);
 		}
+	}
 
+	ifr->ifr_data = (caddr_t)&set_features;
+	err = ioctl(fd, SIOCETHTOOL, ifr);
+	if (err >= 0) {
 		changed = !!set_features.data[0].valid;
 
 		if (err & ETHTOOL_F_WISH)
@@ -2367,7 +2371,7 @@ static int do_soffload(int fd, struct ifreq *ifr)
 				"warning flags %#x",
 				err & ~ETHTOOL_F_WISH);
 	} else if (errno != EOPNOTSUPP && errno != EPERM) {
-		perror("Cannot get device offload settings");
+		perror("Cannot set device offload settings");
 		return 1;
 	} else {
 		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
@@ -2415,6 +2419,20 @@ static int do_soffload(int fd, struct ifreq *ifr)
 
 	if (!changed) {
 		fprintf(stdout, "no offload settings changed\n");
+		return 0;
+	}
+
+	/* Were any additional changes made automatically? */
+	if (get_offload(fd, ifr, &new_features)) {
+		fprintf(stderr, "no offload info available\n");
+		return 1;
+	}
+	value = ((old_features.active & ~off_features.valid) |
+		 off_features.requested) ^
+		new_features.active;
+	if (value) {
+		printf("Additional changes:\n");
+		dump_offload(&new_features, value);
 	}
 
 	return 0;
-- 
1.7.3.4


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


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

* Re: [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
@ 2011-02-22 14:50   ` Michał Mirosław
  2011-02-22 16:44     ` Ben Hutchings
  2011-02-22 21:17   ` Michał Mirosław
  2011-02-23  3:03   ` Michał Mirosław
  2 siblings, 1 reply; 9+ messages in thread
From: Michał Mirosław @ 2011-02-22 14:50 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

On Mon, Feb 21, 2011 at 04:59:08PM +0000, Ben Hutchings wrote:
> Use the new ETHTOOL_{G,S}FEATURES operations where available, and
> use the new structure and netif feature flags in any case.
> 
> Replace repetitive code for getting/setting offload flags with data-
> driven loops.
> 
> This changes error messages to use the same long names for offload
> flags as in dump_offload(), and changes various exit codes to 1.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> NEITF_F_* flags are copied into ethtool-util.h for now.  I think in
> future they should be exposed from <linux/netdevice.h> (hence the
> #ifndef).

I tried to avoid making NETIF_F_ flags an ABI. That's why there's new
ETH_SS_FEATURES ethtool string set. When bits in features get used up
it might be desirable to reorder them while introducing a new field
in struct net_device (eg. move non-changeable bits out of features).

Best Regards,
Michał Mirosław


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

* Re: [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-22 14:50   ` Michał Mirosław
@ 2011-02-22 16:44     ` Ben Hutchings
  2011-02-22 17:03       ` Michał Mirosław
  0 siblings, 1 reply; 9+ messages in thread
From: Ben Hutchings @ 2011-02-22 16:44 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev

On Tue, 2011-02-22 at 15:50 +0100, Michał Mirosław wrote:
> On Mon, Feb 21, 2011 at 04:59:08PM +0000, Ben Hutchings wrote:
> > Use the new ETHTOOL_{G,S}FEATURES operations where available, and
> > use the new structure and netif feature flags in any case.
> > 
> > Replace repetitive code for getting/setting offload flags with data-
> > driven loops.
> > 
> > This changes error messages to use the same long names for offload
> > flags as in dump_offload(), and changes various exit codes to 1.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > NEITF_F_* flags are copied into ethtool-util.h for now.  I think in
> > future they should be exposed from <linux/netdevice.h> (hence the
> > #ifndef).
> 
> I tried to avoid making NETIF_F_ flags an ABI. That's why there's new
> ETH_SS_FEATURES ethtool string set. When bits in features get used up
> it might be desirable to reorder them while introducing a new field
> in struct net_device (eg. move non-changeable bits out of features).

The order is unimportant.  And feature flags are already exposed through
sysfs, so they are part of the kernel ABI.

Ben.

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


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

* Re: [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-22 16:44     ` Ben Hutchings
@ 2011-02-22 17:03       ` Michał Mirosław
  0 siblings, 0 replies; 9+ messages in thread
From: Michał Mirosław @ 2011-02-22 17:03 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

On Tue, Feb 22, 2011 at 04:44:33PM +0000, Ben Hutchings wrote:
> On Tue, 2011-02-22 at 15:50 +0100, Michał Mirosław wrote:
> > On Mon, Feb 21, 2011 at 04:59:08PM +0000, Ben Hutchings wrote:
> > > Use the new ETHTOOL_{G,S}FEATURES operations where available, and
> > > use the new structure and netif feature flags in any case.
> > > 
> > > Replace repetitive code for getting/setting offload flags with data-
> > > driven loops.
> > > 
> > > This changes error messages to use the same long names for offload
> > > flags as in dump_offload(), and changes various exit codes to 1.
> > > 
> > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > > ---
> > > NEITF_F_* flags are copied into ethtool-util.h for now.  I think in
> > > future they should be exposed from <linux/netdevice.h> (hence the
> > > #ifndef).
> > I tried to avoid making NETIF_F_ flags an ABI. That's why there's new
> > ETH_SS_FEATURES ethtool string set. When bits in features get used up
> > it might be desirable to reorder them while introducing a new field
> > in struct net_device (eg. move non-changeable bits out of features).
> The order is unimportant.  And feature flags are already exposed through
> sysfs, so they are part of the kernel ABI.

Since NETIF_F flags are not exposed in the headers I would assume that
/sys/class/net/*/features is a debugging aid and not part of an ABI.
(And I think that's good.)

Best Regards,
Michał Mirosław

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

* Re: [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
  2011-02-22 14:50   ` Michał Mirosław
@ 2011-02-22 21:17   ` Michał Mirosław
  2011-02-23  3:03   ` Michał Mirosław
  2 siblings, 0 replies; 9+ messages in thread
From: Michał Mirosław @ 2011-02-22 21:17 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

On Mon, Feb 21, 2011 at 04:59:08PM +0000, Ben Hutchings wrote:
> Use the new ETHTOOL_{G,S}FEATURES operations where available, and
> use the new structure and netif feature flags in any case.
[...]
> --- a/ethtool.c
> +++ b/ethtool.c
[...]
> +static int dump_offload(const struct ethtool_get_features_block *features)
>  {
[...]
> +	u32 value;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
> +		value = off_feature_def[i].value;
> +		printf("%s: %s%s%s\n",
> +		       off_feature_def[i].long_name,
> +		       (features->active & value) ? "on" : "off",
> +		       (features->requested & ~features->active & value) ?
> +		       " [requested on]" : "",
> +		       (~features->available & value) ? " [unchangeable]" : "");
> +	}

This would be clearer if '[requested XXX]' part was shown only when the
offload's state is different than requested.

Best Regards,
Michał Mirosław 

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

* Re: [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags
  2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
  2011-02-22 14:50   ` Michał Mirosław
  2011-02-22 21:17   ` Michał Mirosław
@ 2011-02-23  3:03   ` Michał Mirosław
  2 siblings, 0 replies; 9+ messages in thread
From: Michał Mirosław @ 2011-02-23  3:03 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

On Mon, Feb 21, 2011 at 04:59:08PM +0000, Ben Hutchings wrote:
> Use the new ETHTOOL_{G,S}FEATURES operations where available, and
> use the new structure and netif feature flags in any case.
[...]
> --- a/ethtool.c
> +++ b/ethtool.c
[...]  
>  static int do_soffload(int fd, struct ifreq *ifr)
>  {
> +	struct {
> +		struct ethtool_gfeatures cmd;
> +		struct ethtool_get_features_block data[1];
> +	} get_features;
> +	struct {
> +		struct ethtool_sfeatures cmd;
> +		struct ethtool_set_features_block data[1];
> +	} set_features;
>  	struct ethtool_value eval;
>  	int err, changed = 0;
> +	u32 value;
> +	int i;
[-]
> +	get_features.cmd.cmd = ETHTOOL_GFEATURES;
> +	get_features.cmd.size = ARRAY_SIZE(get_features.data);
> +	ifr->ifr_data = (caddr_t)&get_features;
> +	err = ioctl(fd, SIOCETHTOOL, ifr);
> +	if (err == 0) {
> +		set_features.cmd.cmd = ETHTOOL_SFEATURES;
> +		set_features.cmd.size = ARRAY_SIZE(set_features.data);
> +		set_features.data[0] = off_features;
> +
> +		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
> +			value = off_feature_def[i].value;
> +			if (!(off_features.valid & value))
> +				continue;
> +			if (!(get_features.data[0].available & value)) {
> +				/* None of these features can be changed */
> +				fprintf(stderr,
> +					"Cannot set device %s settings: "
> +					"Operation not supported\n",
> +					off_feature_def[i].long_name);
> +			} else if (off_features.requested & value) {
> +				/* Some of these features can be
> +				 * enabled; mask out any that cannot
> +				 */
> +				set_features.data[0].requested &=
> +					~(value &
> +					  ~get_features.data[0].available);
> +			}
>  		}
[-]
> +		ifr->ifr_data = (caddr_t)&set_features;
> +		err = ioctl(fd, SIOCETHTOOL, ifr);
> +		if (err < 0) {
> +			perror("Cannot set device offload settings");
> +			return 1;
>  		}
[-]
> +		changed = !!set_features.data[0].valid;
[-]
> +		if (err & ETHTOOL_F_WISH)
> +			fprintf(stderr,
> +				"Cannot set device offload settings: "
> +				"Some requested features depend on others "
> +				"that are not currently enabled\n");
> +
> +		/* ETHTOOL_F_UNSUPPORTED should never be set as we
> +		 * checked for unsupported flags above.  Treat any
> +		 * other warning flags as unknown.
> +		 */
> +		if (err & ~ETHTOOL_F_WISH)
> +			fprintf(stderr,
> +				"Cannot set device offload settings: "
> +				"warning flags %#x",
> +				err & ~ETHTOOL_F_WISH);
> +	} else if (errno != EOPNOTSUPP && errno != EPERM) {
> +		perror("Cannot get device offload settings");
> +		return 1;
> +	} else {
> +		for (i = 0; i < ARRAY_SIZE(off_feature_def); i++) {
> +			if (!off_feature_def[i].cmd)
> +				continue;
> +			if (off_features.valid & off_feature_def[i].value) {
> +				changed = 1;
> +				eval.cmd = off_feature_def[i].cmd + 1;
> +				eval.data = !!(off_features.requested &
> +					       off_feature_def[i].value);
> +				ifr->ifr_data = (caddr_t)&eval;
> +				err = send_ioctl(fd, ifr);
> +				if (err) {
> +					fprintf(stderr,
> +						"Cannot set device %s settings: %m\n",
> +						off_feature_def[i].long_name);
> +					return 1;
> +				}
> +			}
>  		}
[-]
> +		if (off_features.valid & flags_dup_features) {
> +			changed = 1;
> +			eval.cmd = ETHTOOL_GFLAGS;
> +			eval.data = 0;
> +			ifr->ifr_data = (caddr_t)&eval;
> +			err = ioctl(fd, SIOCETHTOOL, ifr);
> +			if (err) {
> +				perror("Cannot get device flag settings");
> +				return 91;
> +			}
[-]
> +			eval.cmd = ETHTOOL_SFLAGS;
> +			eval.data &= ~(off_features.valid & flags_dup_features);
> +			eval.data |= (off_features.requested &
> +				      flags_dup_features);
> +
> +			err = ioctl(fd, SIOCETHTOOL, ifr);
> +			if (err) {
> +				perror("Cannot set device flag settings");
> +				return 92;
> +			}
>  		}
>  	}

I noticed that you assumed that ETHTOOL_SFEATURES can change all features
changeable. This was not the case for drivers not yet converted to
ndo_fix_features() as there could be features changeable only by old ops.
I implemented the kernel part for fixing that in patches sent earlier today.
This introduces ETHTOOL_F_COMPAT bit that indicates when the compatibility
fallback was used (requested features are not fully saved by kernel in
that case).

Best Regards,
Michał Mirosław


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

end of thread, other threads:[~2011-02-23  3:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-21 16:54 [PATCH ethtool 0/3] Regularise handling of offload flags Ben Hutchings
2011-02-21 16:57 ` [PATCH ethtool 1/3] ethtool-copy.h: sync with net-next-2.6 Ben Hutchings
2011-02-21 16:59 ` [PATCH ethtool 2/3] ethtool: Regularise handling of offload flags Ben Hutchings
2011-02-22 14:50   ` Michał Mirosław
2011-02-22 16:44     ` Ben Hutchings
2011-02-22 17:03       ` Michał Mirosław
2011-02-22 21:17   ` Michał Mirosław
2011-02-23  3:03   ` Michał Mirosław
2011-02-21 16:59 ` [PATCH ethtool 3/3] ethtool: Report additional offload flag changes made automatically Ben Hutchings

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