[ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls

[ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls

[David Decotigny]

From: David Decotigny <decot@googlers.com>

This adds support for the new ETHTOOL_xLINKSETTINGS ioctls. This also
fixes a few compilation warnings as well as a heap corruption bug.

History:
  v4
    review Ben Hutchings:
      using AF_UNIX instead of INET6 in the absence of v4 sockets
      use stdbool.h
      do_seeprom always fails when offset/length out of bounds
      sync to latest ethtool.h + kernel.h from net-next
      __SANE_USERSPACE_TYPES__ always defined
      cosmetic updates for var == const tests
      cosmetic updates for associativity in tests
  v3
    TRUE/FALSE obvious-ification
  v2
    added do_seeprom patch
    added netdev@ as recipient
  v1
    initial submission

############################################
# Patch Set Summary:

David Decotigny (7):
  ethtool.c: don't ignore fread() return value
  ethtool.c: fix dump_regs heap corruption
  ethtool.c: do_seeprom checks for params & stdin sanity
  kernel-copy.h: import kernel.h from net-next and use it
  ethtool-copy.h: sync with net-next
  ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls
  ethtool.c: support absence of v4 sockets

Maciej Żenczykowski (4):
  internal.h: change to new sane kernel headers on 64-bit archs
  marvell.c: fix strict alias warnings
  test-common.c: fix test_realloc(NULL, ...)
  test-features.c: add braces around array initialization

 ethtool-copy.h  | 478 ++++++++++++++++++++++++++++++------
 ethtool.c       | 751 ++++++++++++++++++++++++++++++++++++++++++--------------
 internal.h      |  77 +++++-
 kernel-copy.h   |  14 ++
 marvell.c       |  21 +-
 test-cmdline.c  |  12 +
 test-common.c   |   2 +-
 test-features.c |   2 +-
 8 files changed, 1086 insertions(+), 271 deletions(-)
 create mode 100644 kernel-copy.h

-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs

[David Decotigny]

From: Maciej Żenczykowski <maze@google.com>

On ppc64, this fixes:
  In file included from ethtool-copy.h:22:0,
                   from internal.h:32,
                   from ethtool.c:29:
  .../include/linux/types.h:32:25: error: conflicting types for '__be64'
   typedef __u64 __bitwise __be64;
                           ^
  In file included from ethtool.c:29:0:
  internal.h:23:28: note: previous declaration of '__be64' was here
   typedef unsigned long long __be64;
                              ^
  ethtool.c: In function 'do_gstats':
  ethtool.c:3166:4: error: format '%llu' expects argument of type 'long long unsigned int', but argument 5 has type '__u64' [-Werror=format=]
      stats->data[i]);
      ^
  ethtool.c: In function 'print_indir_table':
  ethtool.c:3293:9: error: format '%llu' expects argument of type 'long long unsigned int', but argument 3 has type '__u64' [-Werror=format=]
           ctx->devname, ring_count->data);
           ^


Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
 internal.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/internal.h b/internal.h
index b5ef646..e38d305 100644
--- a/internal.h
+++ b/internal.h
@@ -3,6 +3,12 @@
 #ifndef ETHTOOL_INTERNAL_H__
 #define ETHTOOL_INTERNAL_H__
 
+/* Some platforms (eg. ppc64) need __SANE_USERSPACE_TYPES__ before
+ * <linux/types.h> to select 'int-ll64.h' and avoid compile warnings
+ * when printing __u64 with %llu.
+ */
+#define __SANE_USERSPACE_TYPES__
+
 #ifdef HAVE_CONFIG_H
 #include "ethtool-config.h"
 #endif
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 02/11] ethtool.c: don't ignore fread() return value

[David Decotigny]

From: David Decotigny <decot@googlers.com>

This addresses:
  ethtool.c:1116:8: warning: ignoring return value of ‘fread’, declared with attribute warn_unused_result [-Wunused-result]


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ethtool.c b/ethtool.c
index 92c40b8..9f80d5f 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1007,6 +1007,7 @@ static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
 	if (gregs_dump_file) {
 		FILE *f = fopen(gregs_dump_file, "r");
 		struct stat st;
+		size_t nread;
 
 		if (!f || fstat(fileno(f), &st) < 0) {
 			fprintf(stderr, "Can't open '%s': %s\n",
@@ -1016,8 +1017,10 @@ static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
 
 		regs = realloc(regs, sizeof(*regs) + st.st_size);
 		regs->len = st.st_size;
-		fread(regs->data, regs->len, 1, f);
+		nread = fread(regs->data, regs->len, 1, f);
 		fclose(f);
+		if (nread != 1)
+			return -1;
 	}
 
 	if (!gregs_dump_hex)
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 03/11] ethtool.c: fix dump_regs heap corruption

[David Decotigny]

From: David Decotigny <decot@googlers.com>

The 'regs' pointer is owned by do_gregs(), but updated internally inside
dump_regs() without propagating it back to do_gregs(): later free(regs)
in do_gregs() reclaims the wrong area. This commit moves the realloc()
inside do_gregs().


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 9f80d5f..7c2b5cb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -994,7 +994,6 @@ void dump_hex(FILE *file, const u8 *data, int len, int offset)
 }
 
 static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
-		     const char *gregs_dump_file,
 		     struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 {
 	int i;
@@ -1004,25 +1003,6 @@ static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
 		return 0;
 	}
 
-	if (gregs_dump_file) {
-		FILE *f = fopen(gregs_dump_file, "r");
-		struct stat st;
-		size_t nread;
-
-		if (!f || fstat(fileno(f), &st) < 0) {
-			fprintf(stderr, "Can't open '%s': %s\n",
-				gregs_dump_file, strerror(errno));
-			return -1;
-		}
-
-		regs = realloc(regs, sizeof(*regs) + st.st_size);
-		regs->len = st.st_size;
-		nread = fread(regs->data, regs->len, 1, f);
-		fclose(f);
-		if (nread != 1)
-			return -1;
-	}
-
 	if (!gregs_dump_hex)
 		for (i = 0; i < ARRAY_SIZE(driver_list); i++)
 			if (!strncmp(driver_list[i].name, info->driver,
@@ -2711,7 +2691,31 @@ static int do_gregs(struct cmd_context *ctx)
 		free(regs);
 		return 74;
 	}
-	if (dump_regs(gregs_dump_raw, gregs_dump_hex, gregs_dump_file,
+
+	if (!gregs_dump_raw && gregs_dump_file != NULL) {
+		/* overwrite reg values from file dump */
+		FILE *f = fopen(gregs_dump_file, "r");
+		struct stat st;
+		size_t nread;
+
+		if (!f || fstat(fileno(f), &st) < 0) {
+			fprintf(stderr, "Can't open '%s': %s\n",
+				gregs_dump_file, strerror(errno));
+			free(regs);
+			return 75;
+		}
+
+		regs = realloc(regs, sizeof(*regs) + st.st_size);
+		regs->len = st.st_size;
+		nread = fread(regs->data, regs->len, 1, f);
+		fclose(f);
+		if (nread != 1) {
+			free(regs);
+			return 75;
+		}
+        }
+
+	if (dump_regs(gregs_dump_raw, gregs_dump_hex,
 		      &drvinfo, regs) < 0) {
 		fprintf(stderr, "Cannot dump registers\n");
 		free(regs);
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 04/11] ethtool.c: do_seeprom checks for params & stdin sanity

[David Decotigny]

From: David Decotigny <decot@googlers.com>

Tested:
  On qemu e1000:
  $ dd if=/dev/zero bs=2 count=5 | /mnt/ethtool -E eth0 length 9
  too much data from stdin
  $ dd if=/dev/zero bs=2 count=5 | /mnt/ethtool -E eth0 length 11
  not enough data from stdin
  $ dd if=/dev/zero bs=2 count=5 | /mnt/ethtool -E eth0 length 10
  Cannot set EEPROM data: Bad address


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 7c2b5cb..d349bee 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2828,8 +2828,10 @@ static int do_seeprom(struct cmd_context *ctx)
 	if (seeprom_length == -1)
 		seeprom_length = drvinfo.eedump_len;
 
-	if (drvinfo.eedump_len < seeprom_offset + seeprom_length)
-		seeprom_length = drvinfo.eedump_len - seeprom_offset;
+	if (drvinfo.eedump_len < seeprom_offset + seeprom_length) {
+		fprintf(stderr, "offset & length out of bounds\n");
+		return 1;
+	}
 
 	eeprom = calloc(1, sizeof(*eeprom)+seeprom_length);
 	if (!eeprom) {
@@ -2844,8 +2846,18 @@ static int do_seeprom(struct cmd_context *ctx)
 	eeprom->data[0] = seeprom_value;
 
 	/* Multi-byte write: read input from stdin */
-	if (!seeprom_value_seen)
-		eeprom->len = fread(eeprom->data, 1, eeprom->len, stdin);
+	if (!seeprom_value_seen) {
+		if (1 != fread(eeprom->data, eeprom->len, 1, stdin)) {
+			fprintf(stderr, "not enough data from stdin\n");
+			free(eeprom);
+			return 75;
+		}
+		if ((fgetc(stdin) != EOF) || !feof(stdin)) {
+			fprintf(stderr, "too much data from stdin\n");
+			free(eeprom);
+			return 75;
+		}
+	}
 
 	err = send_ioctl(ctx, eeprom);
 	if (err < 0) {
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings

[David Decotigny]

From: Maciej Żenczykowski <maze@google.com>

Addresses the following warnings:
  marvell.c:426:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  marvell.c:427:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  marvell.c:428:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

Note: code appears endian-dependent, not fixed by this commit.


Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
 marvell.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/marvell.c b/marvell.c
index e583c82..af21188 100644
--- a/marvell.c
+++ b/marvell.c
@@ -381,7 +381,8 @@ static void dump_prefetch(const char *name, const void *r)
 
 int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 {
-	const u32 *r = (const u32 *) regs->data;
+	const u16 *r16 = (const u16 *) regs->data;
+	const u32 *r32 = (const u32 *) regs->data;
 	int dual;
 
 	dump_pci(regs->data + 0x1c00);
@@ -390,15 +391,15 @@ int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 
 	printf("\nBus Management Unit\n");
 	printf("-------------------\n");
-	printf("CSR Receive Queue 1              0x%08X\n", r[24]);
-	printf("CSR Sync Queue 1                 0x%08X\n", r[26]);
-	printf("CSR Async Queue 1                0x%08X\n", r[27]);
+	printf("CSR Receive Queue 1              0x%08X\n", r32[24]);
+	printf("CSR Sync Queue 1                 0x%08X\n", r32[26]);
+	printf("CSR Async Queue 1                0x%08X\n", r32[27]);
 
 	dual = (regs->data[0x11e] & 2) != 0;
 	if (dual) {
-		printf("CSR Receive Queue 2              0x%08X\n", r[25]);
-		printf("CSR Async Queue 2                0x%08X\n", r[29]);
-		printf("CSR Sync Queue 2                 0x%08X\n", r[28]);
+		printf("CSR Receive Queue 2              0x%08X\n", r32[25]);
+		printf("CSR Async Queue 2                0x%08X\n", r32[29]);
+		printf("CSR Sync Queue 2                 0x%08X\n", r32[28]);
 	}
 
 	dump_mac(regs->data);
@@ -423,9 +424,9 @@ int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 	dump_timer("TX status", regs->data + 0xec0);
 	dump_timer("ISR", regs->data + 0xed0);
 
-	printf("\nGMAC control             0x%04X\n", *(u32 *)(regs->data + 0xf00));
-	printf("GPHY control             0x%04X\n", *(u32 *)(regs->data + 0xf04));
-	printf("LINK control             0x%02hX\n", *(u16 *)(regs->data + 0xf10));
+	printf("\nGMAC control             0x%04X\n", r32[0xf00 >> 2]);
+	printf("GPHY control             0x%04X\n", r32[0xf04 >> 2]);
+	printf("LINK control             0x%02hX\n", r16[0xf10 >> 1]);
 
 	dump_gmac("GMAC 1", regs->data + 0x2800);
 	dump_gmac_fifo("Rx GMAC 1", regs->data + 0xc40);
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 06/11] test-common.c: fix test_realloc(NULL, ...)

[David Decotigny]

From: Maciej Żenczykowski <maze@google.com>

This fixes:
  test-common.c: In function 'test_realloc':
  test-common.c:109:8: error: 'block' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    block = realloc(block, sizeof(*block) + size);
          ^


Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
 test-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test-common.c b/test-common.c
index adc3cd4..cd63d1d 100644
--- a/test-common.c
+++ b/test-common.c
@@ -100,7 +100,7 @@ void test_free(void *ptr)
 
 void *test_realloc(void *ptr, size_t size)
 {
-	struct list_head *block;
+	struct list_head *block = NULL;
 
 	if (ptr) {
 		block = (struct list_head *)ptr - 1;
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 07/11] test-features.c: add braces around array initialization

[David Decotigny]

From: Maciej Żenczykowski <maze@google.com>

This fixes:
  test-features.c:21:1: error: missing braces around initializer [-Werror=missing-braces]
   cmd_gssetinfo = { { ETHTOOL_GSSET_INFO, 0, 1ULL << ETH_SS_FEATURES }, 34 };
   ^


Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
 test-features.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test-features.c b/test-features.c
index d7bd994..6ebb364 100644
--- a/test-features.c
+++ b/test-features.c
@@ -18,7 +18,7 @@ static const struct {
 	struct ethtool_sset_info cmd;
 	u32 data[1];
 }
-cmd_gssetinfo = { { ETHTOOL_GSSET_INFO, 0, 1ULL << ETH_SS_FEATURES }, 34 };
+cmd_gssetinfo = { { ETHTOOL_GSSET_INFO, 0, 1ULL << ETH_SS_FEATURES }, { 34 } };
 
 static const struct ethtool_value
 cmd_grxcsum_off = { ETHTOOL_GRXCSUM, 0 },
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 08/11] kernel-copy.h: import kernel.h from net-next and use it

[David Decotigny]

From: David Decotigny <decot@googlers.com>

This is required for recent version of ethtool.h .

This covers kernel.h up to:

  commit b5d3755a22e0cc4c369c0985aef0c52c2477c1e7
  Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
  Date:   Fri Mar 4 11:52:16 2016 +0100

      uapi: define DIV_ROUND_UP for userland


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c     |  3 ++-
 internal.h    |  4 ++--
 kernel-copy.h | 14 ++++++++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 kernel-copy.h

diff --git a/ethtool.c b/ethtool.c
index d349bee..47f0259 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -227,7 +227,8 @@ struct feature_defs {
 	struct feature_def def[0];
 };
 
-#define FEATURE_BITS_TO_BLOCKS(n_bits)		DIV_ROUND_UP(n_bits, 32U)
+#define FEATURE_BITS_TO_BLOCKS(n_bits)		\
+	__KERNEL_DIV_ROUND_UP(n_bits, 32U)
 #define FEATURE_WORD(blocks, index, field)	((blocks)[(index) / 32U].field)
 #define FEATURE_FIELD_FLAG(index)		(1U << (index) % 32U)
 #define FEATURE_BIT_SET(blocks, index, field)			\
diff --git a/internal.h b/internal.h
index e38d305..1c64306 100644
--- a/internal.h
+++ b/internal.h
@@ -35,6 +35,7 @@ typedef uint16_t u16;
 typedef uint8_t u8;
 typedef int32_t s32;
 
+#include "kernel-copy.h"
 #include "ethtool-copy.h"
 #include "net_tstamp-copy.h"
 
@@ -71,8 +72,7 @@ static inline u64 cpu_to_be64(u64 value)
 
 #define BITS_PER_BYTE		8
 #define BITS_PER_LONG		(BITS_PER_BYTE * sizeof(long))
-#define DIV_ROUND_UP(n, d)	(((n) + (d) - 1) / (d))
-#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_LONG)
+#define BITS_TO_LONGS(nr)	__KERNEL_DIV_ROUND_UP(nr, BITS_PER_LONG)
 
 static inline void set_bit(unsigned int nr, unsigned long *addr)
 {
diff --git a/kernel-copy.h b/kernel-copy.h
new file mode 100644
index 0000000..527549f
--- /dev/null
+++ b/kernel-copy.h
@@ -0,0 +1,14 @@
+#ifndef _LINUX_KERNEL_H
+#define _LINUX_KERNEL_H
+
+#include <linux/sysinfo.h>
+
+/*
+ * 'kernel.h' contains some often-used function prototypes etc
+ */
+#define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)
+#define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
+
+#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
+
+#endif /* _LINUX_KERNEL_H */
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 09/11] ethtool-copy.h: sync with net-next

[David Decotigny]

From: David Decotigny <decot@googlers.com>

This cover changes up to:

  commit 14e2037902d65213842b4e40305ff54a64abbcb6
  Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
  Date:   Fri Mar 4 11:52:19 2016 +0100

      ethtool.h: define INT_MAX for userland


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool-copy.h | 478 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 403 insertions(+), 75 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index d23ffc4..7c581ea 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -13,15 +13,19 @@
 #ifndef _LINUX_ETHTOOL_H
 #define _LINUX_ETHTOOL_H
 
+#include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/if_ether.h>
 
+#include <limits.h> /* for INT_MAX */
+
 /* All structures exposed to userland should be defined such that they
  * have the same layout for 32-bit and 64-bit userland.
  */
 
 /**
- * struct ethtool_cmd - link control and status
+ * struct ethtool_cmd - DEPRECATED, link control and status
+ * This structure is DEPRECATED, please use struct ethtool_link_settings.
  * @cmd: Command number = %ETHTOOL_GSET or %ETHTOOL_SSET
  * @supported: Bitmask of %SUPPORTED_* flags for the link modes,
  *	physical connectors and other link features for which the
@@ -31,7 +35,7 @@
  *	physical connectors and other link features that are
  *	advertised through autonegotiation or enabled for
  *	auto-detection.
- * @speed: Low bits of the speed
+ * @speed: Low bits of the speed, 1Mb units, 0 to INT_MAX or SPEED_UNKNOWN
  * @duplex: Duplex mode; one of %DUPLEX_*
  * @port: Physical connector type; one of %PORT_*
  * @phy_address: MDIO address of PHY (transceiver); 0 or 255 if not
@@ -47,7 +51,7 @@
  *	obsoleted by &struct ethtool_coalesce.  Read-only; deprecated.
  * @maxrxpkt: Historically used to report RX IRQ coalescing; now
  *	obsoleted by &struct ethtool_coalesce.  Read-only; deprecated.
- * @speed_hi: High bits of the speed
+ * @speed_hi: High bits of the speed, 1Mb units, 0 to INT_MAX or SPEED_UNKNOWN
  * @eth_tp_mdix: Ethernet twisted-pair MDI(-X) status; one of
  *	%ETH_TP_MDI_*.  If the status is unknown or not applicable, the
  *	value will be %ETH_TP_MDI_INVALID.  Read-only.
@@ -215,6 +219,11 @@ enum tunable_id {
 	ETHTOOL_ID_UNSPEC,
 	ETHTOOL_RX_COPYBREAK,
 	ETHTOOL_TX_COPYBREAK,
+	/*
+	 * Add your fresh new tubale attribute above and remember to update
+	 * tunable_strings[] in net/core/ethtool.c
+	 */
+	__ETHTOOL_TUNABLE_COUNT,
 };
 
 enum tunable_type_id {
@@ -537,6 +546,7 @@ struct ethtool_pauseparam {
  *	now deprecated
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
+ * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -545,6 +555,8 @@ enum ethtool_stringset {
 	ETH_SS_NTUPLE_FILTERS,
 	ETH_SS_FEATURES,
 	ETH_SS_RSS_HASH_FUNCS,
+	ETH_SS_TUNABLES,
+	ETH_SS_PHY_STATS,
 };
 
 /**
@@ -740,6 +752,56 @@ struct ethtool_usrip4_spec {
 	__u8    proto;
 };
 
+/**
+ * struct ethtool_tcpip6_spec - flow specification for TCP/IPv6 etc.
+ * @ip6src: Source host
+ * @ip6dst: Destination host
+ * @psrc: Source port
+ * @pdst: Destination port
+ * @tclass: Traffic Class
+ *
+ * This can be used to specify a TCP/IPv6, UDP/IPv6 or SCTP/IPv6 flow.
+ */
+struct ethtool_tcpip6_spec {
+	__be32	ip6src[4];
+	__be32	ip6dst[4];
+	__be16	psrc;
+	__be16	pdst;
+	__u8    tclass;
+};
+
+/**
+ * struct ethtool_ah_espip6_spec - flow specification for IPsec/IPv6
+ * @ip6src: Source host
+ * @ip6dst: Destination host
+ * @spi: Security parameters index
+ * @tclass: Traffic Class
+ *
+ * This can be used to specify an IPsec transport or tunnel over IPv6.
+ */
+struct ethtool_ah_espip6_spec {
+	__be32	ip6src[4];
+	__be32	ip6dst[4];
+	__be32	spi;
+	__u8    tclass;
+};
+
+/**
+ * struct ethtool_usrip6_spec - general flow specification for IPv6
+ * @ip6src: Source host
+ * @ip6dst: Destination host
+ * @l4_4_bytes: First 4 bytes of transport (layer 4) header
+ * @tclass: Traffic Class
+ * @l4_proto: Transport protocol number (nexthdr after any Extension Headers)
+ */
+struct ethtool_usrip6_spec {
+	__be32	ip6src[4];
+	__be32	ip6dst[4];
+	__be32	l4_4_bytes;
+	__u8    tclass;
+	__u8    l4_proto;
+};
+
 union ethtool_flow_union {
 	struct ethtool_tcpip4_spec		tcp_ip4_spec;
 	struct ethtool_tcpip4_spec		udp_ip4_spec;
@@ -747,6 +809,12 @@ union ethtool_flow_union {
 	struct ethtool_ah_espip4_spec		ah_ip4_spec;
 	struct ethtool_ah_espip4_spec		esp_ip4_spec;
 	struct ethtool_usrip4_spec		usr_ip4_spec;
+	struct ethtool_tcpip6_spec		tcp_ip6_spec;
+	struct ethtool_tcpip6_spec		udp_ip6_spec;
+	struct ethtool_tcpip6_spec		sctp_ip6_spec;
+	struct ethtool_ah_espip6_spec		ah_ip6_spec;
+	struct ethtool_ah_espip6_spec		esp_ip6_spec;
+	struct ethtool_usrip6_spec		usr_ip6_spec;
 	struct ethhdr				ether_spec;
 	__u8					hdata[52];
 };
@@ -796,6 +864,31 @@ struct ethtool_rx_flow_spec {
 	__u32		location;
 };
 
+/* How rings are layed out when accessing virtual functions or
+ * offloaded queues is device specific. To allow users to do flow
+ * steering and specify these queues the ring cookie is partitioned
+ * into a 32bit queue index with an 8 bit virtual function id.
+ * This also leaves the 3bytes for further specifiers. It is possible
+ * future devices may support more than 256 virtual functions if
+ * devices start supporting PCIe w/ARI. However at the moment I
+ * do not know of any devices that support this so I do not reserve
+ * space for this at this time. If a future patch consumes the next
+ * byte it should be aware of this possiblity.
+ */
+#define ETHTOOL_RX_FLOW_SPEC_RING	0x00000000FFFFFFFFLL
+#define ETHTOOL_RX_FLOW_SPEC_RING_VF	0x000000FF00000000LL
+#define ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF 32
+static __inline__ __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie)
+{
+	return ETHTOOL_RX_FLOW_SPEC_RING & ring_cookie;
+};
+
+static __inline__ __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie)
+{
+	return (ETHTOOL_RX_FLOW_SPEC_RING_VF & ring_cookie) >>
+				ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF;
+};
+
 /**
  * struct ethtool_rxnfc - command to get or set RX flow classification rules
  * @cmd: Specific command number - %ETHTOOL_GRXFH, %ETHTOOL_SRXFH,
@@ -1062,6 +1155,11 @@ struct ethtool_sfeatures {
  * the 'hwtstamp_tx_types' and 'hwtstamp_rx_filters' enumeration values,
  * respectively.  For example, if the device supports HWTSTAMP_TX_ON,
  * then (1 << HWTSTAMP_TX_ON) in 'tx_types' will be set.
+ *
+ * Drivers should only report the filters they actually support without
+ * upscaling in the SIOCSHWTSTAMP ioctl. If the SIOCSHWSTAMP request for
+ * HWTSTAMP_FILTER_V1_SYNC is supported by HWTSTAMP_FILTER_V1_EVENT, then the
+ * driver should only report HWTSTAMP_FILTER_V1_EVENT in this op.
  */
 struct ethtool_ts_info {
 	__u32	cmd;
@@ -1108,10 +1206,29 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
 #define ETHTOOL_F_COMPAT        (1 << ETHTOOL_F_COMPAT__BIT)
 
+#define MAX_NUM_QUEUE		4096
+
+/**
+ * struct ethtool_per_queue_op - apply sub command to the queues in mask.
+ * @cmd: ETHTOOL_PERQUEUE
+ * @sub_command: the sub command which apply to each queues
+ * @queue_mask: Bitmap of the queues which sub command apply to
+ * @data: A complete command structure following for each of the queues addressed
+ */
+struct ethtool_per_queue_op {
+	__u32	cmd;
+	__u32	sub_command;
+	__u32	queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
+	char	data[];
+};
 
 /* CMDs currently supported */
-#define ETHTOOL_GSET		0x00000001 /* Get settings. */
-#define ETHTOOL_SSET		0x00000002 /* Set settings. */
+#define ETHTOOL_GSET		0x00000001 /* DEPRECATED, Get settings.
+					    * Please use ETHTOOL_GLINKSETTINGS
+					    */
+#define ETHTOOL_SSET		0x00000002 /* DEPRECATED, Set settings.
+					    * Please use ETHTOOL_SLINKSETTINGS
+					    */
 #define ETHTOOL_GDRVINFO	0x00000003 /* Get driver info. */
 #define ETHTOOL_GREGS		0x00000004 /* Get NIC registers. */
 #define ETHTOOL_GWOL		0x00000005 /* Get wake-on-lan options. */
@@ -1189,74 +1306,143 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_SRSSH		0x00000047 /* Set RX flow hash configuration */
 #define ETHTOOL_GTUNABLE	0x00000048 /* Get tunable configuration */
 #define ETHTOOL_STUNABLE	0x00000049 /* Set tunable configuration */
+#define ETHTOOL_GPHYSTATS	0x0000004a /* get PHY-specific statistics */
+
+#define ETHTOOL_PERQUEUE	0x0000004b /* Set per queue options */
+
+#define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
+#define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
+
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
 #define SPARC_ETH_SSET		ETHTOOL_SSET
 
-#define SUPPORTED_10baseT_Half		(1 << 0)
-#define SUPPORTED_10baseT_Full		(1 << 1)
-#define SUPPORTED_100baseT_Half		(1 << 2)
-#define SUPPORTED_100baseT_Full		(1 << 3)
-#define SUPPORTED_1000baseT_Half	(1 << 4)
-#define SUPPORTED_1000baseT_Full	(1 << 5)
-#define SUPPORTED_Autoneg		(1 << 6)
-#define SUPPORTED_TP			(1 << 7)
-#define SUPPORTED_AUI			(1 << 8)
-#define SUPPORTED_MII			(1 << 9)
-#define SUPPORTED_FIBRE			(1 << 10)
-#define SUPPORTED_BNC			(1 << 11)
-#define SUPPORTED_10000baseT_Full	(1 << 12)
-#define SUPPORTED_Pause			(1 << 13)
-#define SUPPORTED_Asym_Pause		(1 << 14)
-#define SUPPORTED_2500baseX_Full	(1 << 15)
-#define SUPPORTED_Backplane		(1 << 16)
-#define SUPPORTED_1000baseKX_Full	(1 << 17)
-#define SUPPORTED_10000baseKX4_Full	(1 << 18)
-#define SUPPORTED_10000baseKR_Full	(1 << 19)
-#define SUPPORTED_10000baseR_FEC	(1 << 20)
-#define SUPPORTED_20000baseMLD2_Full	(1 << 21)
-#define SUPPORTED_20000baseKR2_Full	(1 << 22)
-#define SUPPORTED_40000baseKR4_Full	(1 << 23)
-#define SUPPORTED_40000baseCR4_Full	(1 << 24)
-#define SUPPORTED_40000baseSR4_Full	(1 << 25)
-#define SUPPORTED_40000baseLR4_Full	(1 << 26)
-#define SUPPORTED_56000baseKR4_Full	(1 << 27)
-#define SUPPORTED_56000baseCR4_Full	(1 << 28)
-#define SUPPORTED_56000baseSR4_Full	(1 << 29)
-#define SUPPORTED_56000baseLR4_Full	(1 << 30)
-
-#define ADVERTISED_10baseT_Half		(1 << 0)
-#define ADVERTISED_10baseT_Full		(1 << 1)
-#define ADVERTISED_100baseT_Half	(1 << 2)
-#define ADVERTISED_100baseT_Full	(1 << 3)
-#define ADVERTISED_1000baseT_Half	(1 << 4)
-#define ADVERTISED_1000baseT_Full	(1 << 5)
-#define ADVERTISED_Autoneg		(1 << 6)
-#define ADVERTISED_TP			(1 << 7)
-#define ADVERTISED_AUI			(1 << 8)
-#define ADVERTISED_MII			(1 << 9)
-#define ADVERTISED_FIBRE		(1 << 10)
-#define ADVERTISED_BNC			(1 << 11)
-#define ADVERTISED_10000baseT_Full	(1 << 12)
-#define ADVERTISED_Pause		(1 << 13)
-#define ADVERTISED_Asym_Pause		(1 << 14)
-#define ADVERTISED_2500baseX_Full	(1 << 15)
-#define ADVERTISED_Backplane		(1 << 16)
-#define ADVERTISED_1000baseKX_Full	(1 << 17)
-#define ADVERTISED_10000baseKX4_Full	(1 << 18)
-#define ADVERTISED_10000baseKR_Full	(1 << 19)
-#define ADVERTISED_10000baseR_FEC	(1 << 20)
-#define ADVERTISED_20000baseMLD2_Full	(1 << 21)
-#define ADVERTISED_20000baseKR2_Full	(1 << 22)
-#define ADVERTISED_40000baseKR4_Full	(1 << 23)
-#define ADVERTISED_40000baseCR4_Full	(1 << 24)
-#define ADVERTISED_40000baseSR4_Full	(1 << 25)
-#define ADVERTISED_40000baseLR4_Full	(1 << 26)
-#define ADVERTISED_56000baseKR4_Full	(1 << 27)
-#define ADVERTISED_56000baseCR4_Full	(1 << 28)
-#define ADVERTISED_56000baseSR4_Full	(1 << 29)
-#define ADVERTISED_56000baseLR4_Full	(1 << 30)
+/* Link mode bit indices */
+enum ethtool_link_mode_bit_indices {
+	ETHTOOL_LINK_MODE_10baseT_Half_BIT	= 0,
+	ETHTOOL_LINK_MODE_10baseT_Full_BIT	= 1,
+	ETHTOOL_LINK_MODE_100baseT_Half_BIT	= 2,
+	ETHTOOL_LINK_MODE_100baseT_Full_BIT	= 3,
+	ETHTOOL_LINK_MODE_1000baseT_Half_BIT	= 4,
+	ETHTOOL_LINK_MODE_1000baseT_Full_BIT	= 5,
+	ETHTOOL_LINK_MODE_Autoneg_BIT		= 6,
+	ETHTOOL_LINK_MODE_TP_BIT		= 7,
+	ETHTOOL_LINK_MODE_AUI_BIT		= 8,
+	ETHTOOL_LINK_MODE_MII_BIT		= 9,
+	ETHTOOL_LINK_MODE_FIBRE_BIT		= 10,
+	ETHTOOL_LINK_MODE_BNC_BIT		= 11,
+	ETHTOOL_LINK_MODE_10000baseT_Full_BIT	= 12,
+	ETHTOOL_LINK_MODE_Pause_BIT		= 13,
+	ETHTOOL_LINK_MODE_Asym_Pause_BIT	= 14,
+	ETHTOOL_LINK_MODE_2500baseX_Full_BIT	= 15,
+	ETHTOOL_LINK_MODE_Backplane_BIT		= 16,
+	ETHTOOL_LINK_MODE_1000baseKX_Full_BIT	= 17,
+	ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT	= 18,
+	ETHTOOL_LINK_MODE_10000baseKR_Full_BIT	= 19,
+	ETHTOOL_LINK_MODE_10000baseR_FEC_BIT	= 20,
+	ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT = 21,
+	ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT	= 22,
+	ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT	= 23,
+	ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT	= 24,
+	ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT	= 25,
+	ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT	= 26,
+	ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT	= 27,
+	ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT	= 28,
+	ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT	= 29,
+	ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT	= 30,
+
+	/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
+	 * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
+	 * macro for bits > 31. The only way to use indices > 31 is to
+	 * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
+	 */
+
+	__ETHTOOL_LINK_MODE_LAST
+	  = ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+};
+
+#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name)	\
+	(1UL << (ETHTOOL_LINK_MODE_ ## base_name ## _BIT))
+
+/* DEPRECATED macros. Please migrate to
+ * ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API. Please do NOT
+ * define any new SUPPORTED_* macro for bits > 31.
+ */
+#define SUPPORTED_10baseT_Half		__ETHTOOL_LINK_MODE_LEGACY_MASK(10baseT_Half)
+#define SUPPORTED_10baseT_Full		__ETHTOOL_LINK_MODE_LEGACY_MASK(10baseT_Full)
+#define SUPPORTED_100baseT_Half		__ETHTOOL_LINK_MODE_LEGACY_MASK(100baseT_Half)
+#define SUPPORTED_100baseT_Full		__ETHTOOL_LINK_MODE_LEGACY_MASK(100baseT_Full)
+#define SUPPORTED_1000baseT_Half	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseT_Half)
+#define SUPPORTED_1000baseT_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseT_Full)
+#define SUPPORTED_Autoneg		__ETHTOOL_LINK_MODE_LEGACY_MASK(Autoneg)
+#define SUPPORTED_TP			__ETHTOOL_LINK_MODE_LEGACY_MASK(TP)
+#define SUPPORTED_AUI			__ETHTOOL_LINK_MODE_LEGACY_MASK(AUI)
+#define SUPPORTED_MII			__ETHTOOL_LINK_MODE_LEGACY_MASK(MII)
+#define SUPPORTED_FIBRE			__ETHTOOL_LINK_MODE_LEGACY_MASK(FIBRE)
+#define SUPPORTED_BNC			__ETHTOOL_LINK_MODE_LEGACY_MASK(BNC)
+#define SUPPORTED_10000baseT_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseT_Full)
+#define SUPPORTED_Pause			__ETHTOOL_LINK_MODE_LEGACY_MASK(Pause)
+#define SUPPORTED_Asym_Pause		__ETHTOOL_LINK_MODE_LEGACY_MASK(Asym_Pause)
+#define SUPPORTED_2500baseX_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(2500baseX_Full)
+#define SUPPORTED_Backplane		__ETHTOOL_LINK_MODE_LEGACY_MASK(Backplane)
+#define SUPPORTED_1000baseKX_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseKX_Full)
+#define SUPPORTED_10000baseKX4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseKX4_Full)
+#define SUPPORTED_10000baseKR_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseKR_Full)
+#define SUPPORTED_10000baseR_FEC	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseR_FEC)
+#define SUPPORTED_20000baseMLD2_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(20000baseMLD2_Full)
+#define SUPPORTED_20000baseKR2_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(20000baseKR2_Full)
+#define SUPPORTED_40000baseKR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseKR4_Full)
+#define SUPPORTED_40000baseCR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseCR4_Full)
+#define SUPPORTED_40000baseSR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseSR4_Full)
+#define SUPPORTED_40000baseLR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseLR4_Full)
+#define SUPPORTED_56000baseKR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseKR4_Full)
+#define SUPPORTED_56000baseCR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseCR4_Full)
+#define SUPPORTED_56000baseSR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseSR4_Full)
+#define SUPPORTED_56000baseLR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseLR4_Full)
+/* Please do not define any new SUPPORTED_* macro for bits > 31, see
+ * notice above.
+ */
+
+/*
+ * DEPRECATED macros. Please migrate to
+ * ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API. Please do NOT
+ * define any new ADERTISE_* macro for bits > 31.
+ */
+#define ADVERTISED_10baseT_Half		__ETHTOOL_LINK_MODE_LEGACY_MASK(10baseT_Half)
+#define ADVERTISED_10baseT_Full		__ETHTOOL_LINK_MODE_LEGACY_MASK(10baseT_Full)
+#define ADVERTISED_100baseT_Half	__ETHTOOL_LINK_MODE_LEGACY_MASK(100baseT_Half)
+#define ADVERTISED_100baseT_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(100baseT_Full)
+#define ADVERTISED_1000baseT_Half	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseT_Half)
+#define ADVERTISED_1000baseT_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseT_Full)
+#define ADVERTISED_Autoneg		__ETHTOOL_LINK_MODE_LEGACY_MASK(Autoneg)
+#define ADVERTISED_TP			__ETHTOOL_LINK_MODE_LEGACY_MASK(TP)
+#define ADVERTISED_AUI			__ETHTOOL_LINK_MODE_LEGACY_MASK(AUI)
+#define ADVERTISED_MII			__ETHTOOL_LINK_MODE_LEGACY_MASK(MII)
+#define ADVERTISED_FIBRE		__ETHTOOL_LINK_MODE_LEGACY_MASK(FIBRE)
+#define ADVERTISED_BNC			__ETHTOOL_LINK_MODE_LEGACY_MASK(BNC)
+#define ADVERTISED_10000baseT_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseT_Full)
+#define ADVERTISED_Pause		__ETHTOOL_LINK_MODE_LEGACY_MASK(Pause)
+#define ADVERTISED_Asym_Pause		__ETHTOOL_LINK_MODE_LEGACY_MASK(Asym_Pause)
+#define ADVERTISED_2500baseX_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(2500baseX_Full)
+#define ADVERTISED_Backplane		__ETHTOOL_LINK_MODE_LEGACY_MASK(Backplane)
+#define ADVERTISED_1000baseKX_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(1000baseKX_Full)
+#define ADVERTISED_10000baseKX4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseKX4_Full)
+#define ADVERTISED_10000baseKR_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseKR_Full)
+#define ADVERTISED_10000baseR_FEC	__ETHTOOL_LINK_MODE_LEGACY_MASK(10000baseR_FEC)
+#define ADVERTISED_20000baseMLD2_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(20000baseMLD2_Full)
+#define ADVERTISED_20000baseKR2_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(20000baseKR2_Full)
+#define ADVERTISED_40000baseKR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseKR4_Full)
+#define ADVERTISED_40000baseCR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseCR4_Full)
+#define ADVERTISED_40000baseSR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseSR4_Full)
+#define ADVERTISED_40000baseLR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(40000baseLR4_Full)
+#define ADVERTISED_56000baseKR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseKR4_Full)
+#define ADVERTISED_56000baseCR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseCR4_Full)
+#define ADVERTISED_56000baseSR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseSR4_Full)
+#define ADVERTISED_56000baseLR4_Full	__ETHTOOL_LINK_MODE_LEGACY_MASK(56000baseLR4_Full)
+/* Please do not define any new ADVERTISED_* macro for bits > 31, see
+ * notice above.
+ */
 
 /* The following are all involved in forcing a particular link
  * mode for the device for setting things.  When getting the
@@ -1264,23 +1450,44 @@ enum ethtool_sfeatures_retval_bits {
  * it was forced up into this mode or autonegotiated.
  */
 
-/* The forced speed, 10Mb, 100Mb, gigabit, [2.5|10|20|40|56]GbE. */
+/* The forced speed, in units of 1Mb. All values 0 to INT_MAX are legal. */
 #define SPEED_10		10
 #define SPEED_100		100
 #define SPEED_1000		1000
 #define SPEED_2500		2500
+#define SPEED_5000		5000
 #define SPEED_10000		10000
 #define SPEED_20000		20000
+#define SPEED_25000		25000
 #define SPEED_40000		40000
+#define SPEED_50000		50000
 #define SPEED_56000		56000
+#define SPEED_100000		100000
 
 #define SPEED_UNKNOWN		-1
 
+static __inline__ int ethtool_validate_speed(__u32 speed)
+{
+	return speed <= INT_MAX || speed == SPEED_UNKNOWN;
+}
+
 /* Duplex, half or full. */
 #define DUPLEX_HALF		0x00
 #define DUPLEX_FULL		0x01
 #define DUPLEX_UNKNOWN		0xff
 
+static __inline__ int ethtool_validate_duplex(__u8 duplex)
+{
+	switch (duplex) {
+	case DUPLEX_HALF:
+	case DUPLEX_FULL:
+	case DUPLEX_UNKNOWN:
+		return 1;
+	}
+
+	return 0;
+}
+
 /* Which connector port. */
 #define PORT_TP			0x00
 #define PORT_AUI		0x01
@@ -1324,15 +1531,17 @@ enum ethtool_sfeatures_retval_bits {
 #define	UDP_V4_FLOW	0x02	/* hash or spec (udp_ip4_spec) */
 #define	SCTP_V4_FLOW	0x03	/* hash or spec (sctp_ip4_spec) */
 #define	AH_ESP_V4_FLOW	0x04	/* hash only */
-#define	TCP_V6_FLOW	0x05	/* hash only */
-#define	UDP_V6_FLOW	0x06	/* hash only */
-#define	SCTP_V6_FLOW	0x07	/* hash only */
+#define	TCP_V6_FLOW	0x05	/* hash or spec (tcp_ip6_spec; nfc only) */
+#define	UDP_V6_FLOW	0x06	/* hash or spec (udp_ip6_spec; nfc only) */
+#define	SCTP_V6_FLOW	0x07	/* hash or spec (sctp_ip6_spec; nfc only) */
 #define	AH_ESP_V6_FLOW	0x08	/* hash only */
 #define	AH_V4_FLOW	0x09	/* hash or spec (ah_ip4_spec) */
 #define	ESP_V4_FLOW	0x0a	/* hash or spec (esp_ip4_spec) */
-#define	AH_V6_FLOW	0x0b	/* hash only */
-#define	ESP_V6_FLOW	0x0c	/* hash only */
-#define	IP_USER_FLOW	0x0d	/* spec only (usr_ip4_spec) */
+#define	AH_V6_FLOW	0x0b	/* hash or spec (ah_ip6_spec; nfc only) */
+#define	ESP_V6_FLOW	0x0c	/* hash or spec (esp_ip6_spec; nfc only) */
+#define	IPV4_USER_FLOW	0x0d	/* spec only (usr_ip4_spec) */
+#define	IP_USER_FLOW	IPV4_USER_FLOW
+#define	IPV6_USER_FLOW	0x0e	/* spec only (usr_ip6_spec; nfc only) */
 #define	IPV4_FLOW	0x10	/* hash only */
 #define	IPV6_FLOW	0x11	/* hash only */
 #define	ETHER_FLOW	0x12	/* spec only (ether_spec) */
@@ -1398,4 +1607,123 @@ enum ethtool_reset_flags {
 };
 #define ETH_RESET_SHARED_SHIFT	16
 
+
+/**
+ * struct ethtool_link_settings - link control and status
+ *
+ * IMPORTANT, Backward compatibility notice: When implementing new
+ *	user-space tools, please first try %ETHTOOL_GLINKSETTINGS, and
+ *	if it succeeds use %ETHTOOL_SLINKSETTINGS to change link
+ *	settings; do not use %ETHTOOL_SSET if %ETHTOOL_GLINKSETTINGS
+ *	succeeded: stick to %ETHTOOL_GLINKSETTINGS/%SLINKSETTINGS in
+ *	that case.  Conversely, if %ETHTOOL_GLINKSETTINGS fails, use
+ *	%ETHTOOL_GSET to query and %ETHTOOL_SSET to change link
+ *	settings; do not use %ETHTOOL_SLINKSETTINGS if
+ *	%ETHTOOL_GLINKSETTINGS failed: stick to
+ *	%ETHTOOL_GSET/%ETHTOOL_SSET in that case.
+ *
+ * @cmd: Command number = %ETHTOOL_GLINKSETTINGS or %ETHTOOL_SLINKSETTINGS
+ * @speed: Link speed (Mbps)
+ * @duplex: Duplex mode; one of %DUPLEX_*
+ * @port: Physical connector type; one of %PORT_*
+ * @phy_address: MDIO address of PHY (transceiver); 0 or 255 if not
+ *	applicable.  For clause 45 PHYs this is the PRTAD.
+ * @autoneg: Enable/disable autonegotiation and auto-detection;
+ *	either %AUTONEG_DISABLE or %AUTONEG_ENABLE
+ * @mdio_support: Bitmask of %ETH_MDIO_SUPPORTS_* flags for the MDIO
+ *	protocols supported by the interface; 0 if unknown.
+ *	Read-only.
+ * @eth_tp_mdix: Ethernet twisted-pair MDI(-X) status; one of
+ *	%ETH_TP_MDI_*.  If the status is unknown or not applicable, the
+ *	value will be %ETH_TP_MDI_INVALID.  Read-only.
+ * @eth_tp_mdix_ctrl: Ethernet twisted pair MDI(-X) control; one of
+ *	%ETH_TP_MDI_*.  If MDI(-X) control is not implemented, reads
+ *	yield %ETH_TP_MDI_INVALID and writes may be ignored or rejected.
+ *	When written successfully, the link should be renegotiated if
+ *	necessary.
+ * @link_mode_masks_nwords: Number of 32-bit words for each of the
+ *	supported, advertising, lp_advertising link mode bitmaps. For
+ *	%ETHTOOL_GLINKSETTINGS: on entry, number of words passed by user
+ *	(>= 0); on return, if handshake in progress, negative if
+ *	request size unsupported by kernel: absolute value indicates
+ *	kernel recommended size and cmd field is 0, as well as all the
+ *	other fields; otherwise (handshake completed), strictly
+ *	positive to indicate size used by kernel and cmd field is
+ *	%ETHTOOL_GLINKSETTINGS, all other fields populated by driver. For
+ *	%ETHTOOL_SLINKSETTINGS: must be valid on entry, ie. a positive
+ *	value returned previously by %ETHTOOL_GLINKSETTINGS, otherwise
+ *	refused. For drivers: ignore this field (use kernel's
+ *	__ETHTOOL_LINK_MODE_MASK_NBITS instead), any change to it will
+ *	be overwritten by kernel.
+ * @supported: Bitmap with each bit meaning given by
+ *	%ethtool_link_mode_bit_indices for the link modes, physical
+ *	connectors and other link features for which the interface
+ *	supports autonegotiation or auto-detection.  Read-only.
+ * @advertising: Bitmap with each bit meaning given by
+ *	%ethtool_link_mode_bit_indices for the link modes, physical
+ *	connectors and other link features that are advertised through
+ *	autonegotiation or enabled for auto-detection.
+ * @lp_advertising: Bitmap with each bit meaning given by
+ *	%ethtool_link_mode_bit_indices for the link modes, and other
+ *	link features that the link partner advertised through
+ *	autonegotiation; 0 if unknown or not applicable.  Read-only.
+ *
+ * If autonegotiation is disabled, the speed and @duplex represent the
+ * fixed link mode and are writable if the driver supports multiple
+ * link modes.  If it is enabled then they are read-only; if the link
+ * is up they represent the negotiated link mode; if the link is down,
+ * the speed is 0, %SPEED_UNKNOWN or the highest enabled speed and
+ * @duplex is %DUPLEX_UNKNOWN or the best enabled duplex mode.
+ *
+ * Some hardware interfaces may have multiple PHYs and/or physical
+ * connectors fitted or do not allow the driver to detect which are
+ * fitted.  For these interfaces @port and/or @phy_address may be
+ * writable, possibly dependent on @autoneg being %AUTONEG_DISABLE.
+ * Otherwise, attempts to write different values may be ignored or
+ * rejected.
+ *
+ * Deprecated %ethtool_cmd fields transceiver, maxtxpkt and maxrxpkt
+ * are not available in %ethtool_link_settings. Until all drivers are
+ * converted to ignore them or to the new %ethtool_link_settings API,
+ * for both queries and changes, users should always try
+ * %ETHTOOL_GLINKSETTINGS first, and if it fails with -ENOTSUPP stick
+ * only to %ETHTOOL_GSET and %ETHTOOL_SSET consistently. If it
+ * succeeds, then users should stick to %ETHTOOL_GLINKSETTINGS and
+ * %ETHTOOL_SLINKSETTINGS (which would support drivers implementing
+ * either %ethtool_cmd or %ethtool_link_settings).
+ *
+ * Users should assume that all fields not marked read-only are
+ * writable and subject to validation by the driver.  They should use
+ * %ETHTOOL_GLINKSETTINGS to get the current values before making specific
+ * changes and then applying them with %ETHTOOL_SLINKSETTINGS.
+ *
+ * Drivers that implement %get_link_ksettings and/or
+ * %set_link_ksettings should ignore the @cmd
+ * and @link_mode_masks_nwords fields (any change to them overwritten
+ * by kernel), and rely only on kernel's internal
+ * %__ETHTOOL_LINK_MODE_MASK_NBITS and
+ * %ethtool_link_mode_mask_t. Drivers that implement
+ * %set_link_ksettings() should validate all fields other than @cmd
+ * and @link_mode_masks_nwords that are not described as read-only or
+ * deprecated, and must ignore all fields described as read-only.
+ */
+struct ethtool_link_settings {
+	__u32	cmd;
+	__u32	speed;
+	__u8	duplex;
+	__u8	port;
+	__u8	phy_address;
+	__u8	autoneg;
+	__u8	mdio_support;
+	__u8	eth_tp_mdix;
+	__u8	eth_tp_mdix_ctrl;
+	__s8	link_mode_masks_nwords;
+	__u32	reserved[8];
+	__u32	link_mode_masks[0];
+	/* layout of link_mode_masks fields:
+	 * __u32 map_supported[link_mode_masks_nwords];
+	 * __u32 map_advertising[link_mode_masks_nwords];
+	 * __u32 map_lp_advertising[link_mode_masks_nwords];
+	 */
+};
 #endif /* _LINUX_ETHTOOL_H */
-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls

[David Decotigny]

From: David Decotigny <decot@googlers.com>

More info with kernel SHA1: 8d3f2806f8fbd9b22 "Merge branch
'ethtool-ksettings'".


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c      | 682 +++++++++++++++++++++++++++++++++++++++++++--------------
 internal.h     |  67 ++++++
 test-cmdline.c |  12 +
 3 files changed, 602 insertions(+), 159 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 47f0259..761252f 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -47,42 +47,6 @@
 #define MAX_ADDR_LEN	32
 #endif
 
-#define ALL_ADVERTISED_MODES			\
-	(ADVERTISED_10baseT_Half |		\
-	 ADVERTISED_10baseT_Full |		\
-	 ADVERTISED_100baseT_Half |		\
-	 ADVERTISED_100baseT_Full |		\
-	 ADVERTISED_1000baseT_Half |		\
-	 ADVERTISED_1000baseT_Full |		\
-	 ADVERTISED_1000baseKX_Full|		\
-	 ADVERTISED_2500baseX_Full |		\
-	 ADVERTISED_10000baseT_Full |		\
-	 ADVERTISED_10000baseKX4_Full |		\
-	 ADVERTISED_10000baseKR_Full |		\
-	 ADVERTISED_10000baseR_FEC |		\
-	 ADVERTISED_20000baseMLD2_Full |	\
-	 ADVERTISED_20000baseKR2_Full |		\
-	 ADVERTISED_40000baseKR4_Full |		\
-	 ADVERTISED_40000baseCR4_Full |		\
-	 ADVERTISED_40000baseSR4_Full |		\
-	 ADVERTISED_40000baseLR4_Full |		\
-	 ADVERTISED_56000baseKR4_Full |		\
-	 ADVERTISED_56000baseCR4_Full |		\
-	 ADVERTISED_56000baseSR4_Full |		\
-	 ADVERTISED_56000baseLR4_Full)
-
-#define ALL_ADVERTISED_FLAGS			\
-	(ADVERTISED_Autoneg |			\
-	 ADVERTISED_TP |			\
-	 ADVERTISED_AUI |			\
-	 ADVERTISED_MII |			\
-	 ADVERTISED_FIBRE |			\
-	 ADVERTISED_BNC |			\
-	 ADVERTISED_Pause |			\
-	 ADVERTISED_Asym_Pause |		\
-	 ADVERTISED_Backplane |			\
-	 ALL_ADVERTISED_MODES)
-
 #ifndef HAVE_NETIF_MSG
 enum {
 	NETIF_MSG_DRV		= 0x0001,
@@ -294,6 +258,45 @@ static void get_mac_addr(char *src, unsigned char *dest)
 	}
 }
 
+static int parse_hex_u32_bitmap(const char *s,
+				unsigned int nbits, u32 *result)
+{
+	const unsigned nwords = __KERNEL_DIV_ROUND_UP(nbits, 32);
+	size_t slen = strlen(s);
+	size_t i;
+
+	/* ignore optional '0x' prefix */
+	if ((slen > 2) && (
+		    (0 == memcmp(s, "0x", 2)
+		     || (0 == memcmp(s, "0X", 2))))) {
+		slen -= 2;
+		s += 2;
+	}
+
+	if (slen > 8*nwords)  /* up to 2 digits per byte */
+		return -1;
+
+	memset(result, 0, 4*nwords);
+	for (i = 0 ; i < slen ; ++i) {
+		const unsigned shift = (slen - 1 - i)*4;
+		u32 *dest = &result[shift / 32];
+		u32 nibble;
+
+		if ('a' <= s[i] && s[i] <= 'f')
+			nibble = 0xa + (s[i] - 'a');
+		else if ('A' <= s[i] && s[i] <= 'F')
+			nibble = 0xa + (s[i] - 'A');
+		else if ('0' <= s[i] && s[i] <= '9')
+			nibble = (s[i] - '0');
+		else
+			return -1;
+
+		*dest |= (nibble << (shift % 32));
+	}
+
+	return 0;
+}
+
 static void parse_generic_cmdline(struct cmd_context *ctx,
 				  int *changed,
 				  struct cmdline_info *info,
@@ -473,64 +476,157 @@ static int do_version(struct cmd_context *ctx)
 	return 0;
 }
 
-static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
-			   int link_mode_only);
+/* link mode routines */
+
+static __ETHTOOL_DECLARE_LINK_MODE_MASK(all_advertised_modes);
+static __ETHTOOL_DECLARE_LINK_MODE_MASK(all_advertised_flags);
 
-static void dump_supported(struct ethtool_cmd *ep)
+static void init_global_link_mode_masks()
 {
-	u32 mask = ep->supported;
+	static const enum ethtool_link_mode_bit_indices
+		all_advertised_modes_bits[] = {
+		ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+		ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseR_FEC_BIT,
+		ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
+		ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+	};
+	static const enum ethtool_link_mode_bit_indices
+		additional_advertised_flags_bits[] = {
+		ETHTOOL_LINK_MODE_Autoneg_BIT,
+		ETHTOOL_LINK_MODE_TP_BIT,
+		ETHTOOL_LINK_MODE_AUI_BIT,
+		ETHTOOL_LINK_MODE_MII_BIT,
+		ETHTOOL_LINK_MODE_FIBRE_BIT,
+		ETHTOOL_LINK_MODE_BNC_BIT,
+		ETHTOOL_LINK_MODE_Pause_BIT,
+		ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+		ETHTOOL_LINK_MODE_Backplane_BIT,
+	};
+	unsigned i;
+
+	ethtool_link_mode_zero(all_advertised_modes);
+	ethtool_link_mode_zero(all_advertised_flags);
+	for (i = 0 ; i < ARRAY_SIZE(all_advertised_modes_bits) ; ++i) {
+		ethtool_link_mode_set_bit(all_advertised_modes_bits[i],
+					  all_advertised_modes);
+		ethtool_link_mode_set_bit(all_advertised_modes_bits[i],
+					  all_advertised_flags);
+	}
+
+	for (i = 0 ; i < ARRAY_SIZE(additional_advertised_flags_bits) ; ++i) {
+		ethtool_link_mode_set_bit(
+			additional_advertised_flags_bits[i],
+			all_advertised_flags);
+	}
+}
+
+static void dump_link_caps(const char *prefix, const char *an_prefix,
+			   const u32 *mask, int link_mode_only);
 
+static void dump_supported(const struct ethtool_link_usettings *link_usettings)
+{
 	fprintf(stdout, "	Supported ports: [ ");
-	if (mask & SUPPORTED_TP)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_TP_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "TP ");
-	if (mask & SUPPORTED_AUI)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_AUI_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "AUI ");
-	if (mask & SUPPORTED_BNC)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_BNC_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "BNC ");
-	if (mask & SUPPORTED_MII)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_MII_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "MII ");
-	if (mask & SUPPORTED_FIBRE)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_FIBRE_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "FIBRE ");
-	if (mask & SUPPORTED_Backplane)
+	if (ethtool_link_mode_test_bit(
+		    ETHTOOL_LINK_MODE_Backplane_BIT,
+		    link_usettings->link_modes.supported))
 		fprintf(stdout, "Backplane ");
 	fprintf(stdout, "]\n");
 
-	dump_link_caps("Supported", "Supports", mask, 0);
+	dump_link_caps("Supported", "Supports",
+		       link_usettings->link_modes.supported, 0);
 }
 
 /* Print link capability flags (supported, advertised or lp_advertised).
  * Assumes that the corresponding SUPPORTED and ADVERTISED flags are equal.
  */
-static void
-dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
-	       int link_mode_only)
+static void dump_link_caps(const char *prefix, const char *an_prefix,
+			   const u32 *mask, int link_mode_only)
 {
 	static const struct {
 		int same_line; /* print on same line as previous */
-		u32 value;
+		unsigned bit_indice;
 		const char *name;
 	} mode_defs[] = {
-		{ 0, ADVERTISED_10baseT_Half,       "10baseT/Half" },
-		{ 1, ADVERTISED_10baseT_Full,       "10baseT/Full" },
-		{ 0, ADVERTISED_100baseT_Half,      "100baseT/Half" },
-		{ 1, ADVERTISED_100baseT_Full,      "100baseT/Full" },
-		{ 0, ADVERTISED_1000baseT_Half,     "1000baseT/Half" },
-		{ 1, ADVERTISED_1000baseT_Full,     "1000baseT/Full" },
-		{ 0, ADVERTISED_1000baseKX_Full,    "1000baseKX/Full" },
-		{ 0, ADVERTISED_2500baseX_Full,     "2500baseX/Full" },
-		{ 0, ADVERTISED_10000baseT_Full,    "10000baseT/Full" },
-		{ 0, ADVERTISED_10000baseKX4_Full,  "10000baseKX4/Full" },
-		{ 0, ADVERTISED_10000baseKR_Full,   "10000baseKR/Full" },
-		{ 0, ADVERTISED_20000baseMLD2_Full, "20000baseMLD2/Full" },
-		{ 0, ADVERTISED_20000baseKR2_Full,  "20000baseKR2/Full" },
-		{ 0, ADVERTISED_40000baseKR4_Full,  "40000baseKR4/Full" },
-		{ 0, ADVERTISED_40000baseCR4_Full,  "40000baseCR4/Full" },
-		{ 0, ADVERTISED_40000baseSR4_Full,  "40000baseSR4/Full" },
-		{ 0, ADVERTISED_40000baseLR4_Full,  "40000baseLR4/Full" },
-		{ 0, ADVERTISED_56000baseKR4_Full,  "56000baseKR4/Full" },
-		{ 0, ADVERTISED_56000baseCR4_Full,  "56000baseCR4/Full" },
-		{ 0, ADVERTISED_56000baseSR4_Full,  "56000baseSR4/Full" },
-		{ 0, ADVERTISED_56000baseLR4_Full,  "56000baseLR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+		  "10baseT/Half" },
+		{ 1, ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+		  "10baseT/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+		  "100baseT/Half" },
+		{ 1, ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+		  "100baseT/Full" },
+		{ 0, ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+		  "1000baseT/Half" },
+		{ 1, ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+		  "1000baseT/Full" },
+		{ 0, ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+		  "1000baseKX/Full" },
+		{ 0, ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+		  "2500baseX/Full" },
+		{ 0, ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+		  "10000baseT/Full" },
+		{ 0, ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+		  "10000baseKX4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+		  "10000baseKR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
+		  "20000baseMLD2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT,
+		  "20000baseKR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+		  "40000baseKR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+		  "40000baseCR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+		  "40000baseSR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+		  "40000baseLR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT,
+		  "56000baseKR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT,
+		  "56000baseCR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT,
+		  "56000baseSR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+		  "56000baseLR4/Full" },
 	};
 	int indent;
 	int did1, new_line_pend, i;
@@ -547,7 +643,8 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
 	for (i = 0; i < ARRAY_SIZE(mode_defs); i++) {
 		if (did1 && !mode_defs[i].same_line)
 			new_line_pend = 1;
-		if (mask & mode_defs[i].value) {
+		if (ethtool_link_mode_test_bit(mode_defs[i].bit_indice,
+					       mask)) {
 			if (new_line_pend) {
 				fprintf(stdout, "\n");
 				fprintf(stdout, "	%*s", indent, "");
@@ -558,51 +655,57 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
 		}
 	}
 	if (did1 == 0)
-		 fprintf(stdout, "Not reported");
+		fprintf(stdout, "Not reported");
 	fprintf(stdout, "\n");
 
 	if (!link_mode_only) {
 		fprintf(stdout, "	%s pause frame use: ", prefix);
-		if (mask & ADVERTISED_Pause) {
+		if (ethtool_link_mode_test_bit(
+			    ETHTOOL_LINK_MODE_Pause_BIT, mask)) {
 			fprintf(stdout, "Symmetric");
-			if (mask & ADVERTISED_Asym_Pause)
+			if (ethtool_link_mode_test_bit(
+				    ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask))
 				fprintf(stdout, " Receive-only");
 			fprintf(stdout, "\n");
 		} else {
-			if (mask & ADVERTISED_Asym_Pause)
+			if (ethtool_link_mode_test_bit(
+				    ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask))
 				fprintf(stdout, "Transmit-only\n");
 			else
 				fprintf(stdout, "No\n");
 		}
 
 		fprintf(stdout, "	%s auto-negotiation: ", an_prefix);
-		if (mask & ADVERTISED_Autoneg)
+		if (ethtool_link_mode_test_bit(
+			    ETHTOOL_LINK_MODE_Autoneg_BIT, mask))
 			fprintf(stdout, "Yes\n");
 		else
 			fprintf(stdout, "No\n");
 	}
 }
 
-static int dump_ecmd(struct ethtool_cmd *ep)
+static int
+dump_link_usettings(const struct ethtool_link_usettings *link_usettings)
 {
-	u32 speed;
-
-	dump_supported(ep);
-	dump_link_caps("Advertised", "Advertised", ep->advertising, 0);
-	if (ep->lp_advertising)
+	dump_supported(link_usettings);
+	dump_link_caps("Advertised", "Advertised",
+		       link_usettings->link_modes.advertising, 0);
+	if (!ethtool_link_mode_is_empty(
+		    link_usettings->link_modes.lp_advertising))
 		dump_link_caps("Link partner advertised",
-			       "Link partner advertised", ep->lp_advertising,
-			       0);
+			       "Link partner advertised",
+			       link_usettings->link_modes.lp_advertising, 0);
 
 	fprintf(stdout, "	Speed: ");
-	speed = ethtool_cmd_speed(ep);
-	if (speed == 0 || speed == (u16)(-1) || speed == (u32)(-1))
+	if (link_usettings->base.speed == 0
+	    || link_usettings->base.speed == (u16)(-1)
+	    || link_usettings->base.speed == (u32)(-1))
 		fprintf(stdout, "Unknown!\n");
 	else
-		fprintf(stdout, "%uMb/s\n", speed);
+		fprintf(stdout, "%uMb/s\n", link_usettings->base.speed);
 
 	fprintf(stdout, "	Duplex: ");
-	switch (ep->duplex) {
+	switch (link_usettings->base.duplex) {
 	case DUPLEX_HALF:
 		fprintf(stdout, "Half\n");
 		break;
@@ -610,12 +713,12 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 		fprintf(stdout, "Full\n");
 		break;
 	default:
-		fprintf(stdout, "Unknown! (%i)\n", ep->duplex);
+		fprintf(stdout, "Unknown! (%i)\n", link_usettings->base.duplex);
 		break;
 	};
 
 	fprintf(stdout, "	Port: ");
-	switch (ep->port) {
+	switch (link_usettings->base.port) {
 	case PORT_TP:
 		fprintf(stdout, "Twisted Pair\n");
 		break;
@@ -641,13 +744,13 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 		fprintf(stdout, "Other\n");
 		break;
 	default:
-		fprintf(stdout, "Unknown! (%i)\n", ep->port);
+		fprintf(stdout, "Unknown! (%i)\n", link_usettings->base.port);
 		break;
 	};
 
-	fprintf(stdout, "	PHYAD: %d\n", ep->phy_address);
+	fprintf(stdout, "	PHYAD: %d\n", link_usettings->base.phy_address);
 	fprintf(stdout, "	Transceiver: ");
-	switch (ep->transceiver) {
+	switch (link_usettings->deprecated.transceiver) {
 	case XCVR_INTERNAL:
 		fprintf(stdout, "internal\n");
 		break;
@@ -660,17 +763,17 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	};
 
 	fprintf(stdout, "	Auto-negotiation: %s\n",
-		(ep->autoneg == AUTONEG_DISABLE) ?
+		(link_usettings->base.autoneg == AUTONEG_DISABLE) ?
 		"off" : "on");
 
-	if (ep->port == PORT_TP) {
+	if (link_usettings->base.port == PORT_TP) {
 		fprintf(stdout, "	MDI-X: ");
-		if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI) {
+		if (link_usettings->base.eth_tp_mdix_ctrl == ETH_TP_MDI) {
 			fprintf(stdout, "off (forced)\n");
-		} else if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
+		} else if (link_usettings->base.eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
 			fprintf(stdout, "on (forced)\n");
 		} else {
-			switch (ep->eth_tp_mdix) {
+			switch (link_usettings->base.eth_tp_mdix) {
 			case ETH_TP_MDI:
 				fprintf(stdout, "off");
 				break;
@@ -681,7 +784,8 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 				fprintf(stdout, "Unknown");
 				break;
 			}
-			if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			if (link_usettings->base.eth_tp_mdix_ctrl
+			    == ETH_TP_MDI_AUTO)
 				fprintf(stdout, " (auto)");
 			fprintf(stdout, "\n");
 		}
@@ -1369,6 +1473,7 @@ static int dump_rxfhash(int fhash, u64 val)
 
 static void dump_eeecmd(struct ethtool_eee *ep)
 {
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(link_mode);
 
 	fprintf(stdout, "	EEE status: ");
 	if (!ep->supported) {
@@ -1390,9 +1495,16 @@ static void dump_eeecmd(struct ethtool_eee *ep)
 	else
 		fprintf(stdout, " disabled\n");
 
-	dump_link_caps("Supported EEE", "", ep->supported, 1);
-	dump_link_caps("Advertised EEE", "", ep->advertised, 1);
-	dump_link_caps("Link partner advertised EEE", "", ep->lp_advertised, 1);
+	ethtool_link_mode_zero(link_mode);
+
+	link_mode[0] = ep->supported;
+	dump_link_caps("Supported EEE", "", link_mode, 1);
+
+	link_mode[0] = ep->advertised;
+	dump_link_caps("Advertised EEE", "", link_mode, 1);
+
+	link_mode[0] = ep->lp_advertised;
+	dump_link_caps("Link partner advertised EEE", "", link_mode, 1);
 }
 
 #define N_SOTS 7
@@ -2248,10 +2360,219 @@ static int do_sfeatures(struct cmd_context *ctx)
 	return 0;
 }
 
-static int do_gset(struct cmd_context *ctx)
+static struct ethtool_link_usettings *
+__do_ioctl_glinksettings(struct cmd_context * ctx)
+{
+	int err;
+	struct {
+		struct ethtool_link_settings req;
+		__u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
+	} ecmd;
+	struct ethtool_link_usettings *link_usettings;
+	unsigned u32_offs;
+
+	/* handshake with kernel to determine number of words for link
+	 * mode bitmaps */
+	memset(&ecmd, 0, sizeof(ecmd));
+	ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
+	err = send_ioctl(ctx, &ecmd);
+	if (err < 0)
+		return NULL;
+
+	if (ecmd.req.link_mode_masks_nwords >= 0 || ecmd.req.cmd)
+		return NULL;
+
+	/* got the real ecmd.req.link_mode_masks_nwords,
+	   now send the real request */
+	ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
+	ecmd.req.link_mode_masks_nwords = -ecmd.req.link_mode_masks_nwords;
+	err = send_ioctl(ctx, &ecmd);
+	if (err < 0)
+		return NULL;
+
+	if (ecmd.req.link_mode_masks_nwords <= 0 || !ecmd.req.cmd)
+		return NULL;
+
+	/* Convert to usettings struct */
+	link_usettings = malloc(sizeof(*link_usettings));
+	if (NULL == link_usettings)
+		return NULL;
+
+	memset(link_usettings, 0, sizeof(*link_usettings));
+	/* keep transceiver 0 */
+	memcpy(&link_usettings->base, &ecmd.req, sizeof(link_usettings->base));
+	/* remember that ETHTOOL_GLINKSETTINGS was used */
+	link_usettings->base.cmd = ETHTOOL_GLINKSETTINGS;
+
+	/* copy link mode bitmaps */
+	u32_offs = 0;
+	memcpy(link_usettings->link_modes.supported,
+	       &ecmd.link_mode_data[u32_offs],
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	u32_offs += ecmd.req.link_mode_masks_nwords;
+	memcpy(link_usettings->link_modes.advertising,
+	       &ecmd.link_mode_data[u32_offs],
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	u32_offs += ecmd.req.link_mode_masks_nwords;
+	memcpy(link_usettings->link_modes.lp_advertising,
+	       &ecmd.link_mode_data[u32_offs],
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	return link_usettings;
+}
+
+static int
+__do_ioctl_slinksettings(struct cmd_context * ctx,
+			 const struct ethtool_link_usettings *link_usettings)
+{
+	struct {
+		struct ethtool_link_settings req;
+		__u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
+	} ecmd;
+	unsigned u32_offs;
+
+	/* refuse to send ETHTOOL_SLINKSETTINGS ioctl if
+	 * link_usettings was retrieved with ETHTOOL_GSET */
+	if (ETHTOOL_GLINKSETTINGS != link_usettings->base.cmd)
+		return -1;
+
+	/* refuse to send ETHTOOL_SLINKSETTINGS ioctl if deprecated fields
+	 * were set */
+	if (link_usettings->deprecated.transceiver)
+		return -1;
+
+	if (link_usettings->base.link_mode_masks_nwords <= 0)
+		return -1;
+
+	memcpy(&ecmd.req, &link_usettings->base, sizeof(ecmd.req));
+	ecmd.req.cmd = ETHTOOL_SLINKSETTINGS;
+
+	/* copy link mode bitmaps */
+	u32_offs = 0;
+	memcpy(&ecmd.link_mode_data[u32_offs],
+	       link_usettings->link_modes.supported,
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	u32_offs += ecmd.req.link_mode_masks_nwords;
+	memcpy(&ecmd.link_mode_data[u32_offs],
+	       link_usettings->link_modes.advertising,
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	u32_offs += ecmd.req.link_mode_masks_nwords;
+	memcpy(&ecmd.link_mode_data[u32_offs],
+	       link_usettings->link_modes.lp_advertising,
+	       4*ecmd.req.link_mode_masks_nwords);
+
+	return send_ioctl(ctx, &ecmd);
+}
+
+static struct ethtool_link_usettings *
+__do_ioctl_gset(struct cmd_context * ctx)
 {
 	int err;
 	struct ethtool_cmd ecmd;
+	struct ethtool_link_usettings *link_usettings;
+
+	memset(&ecmd, 0, sizeof(ecmd));
+	ecmd.cmd = ETHTOOL_GSET;
+	err = send_ioctl(ctx, &ecmd);
+	if (err < 0)
+		return NULL;
+
+	link_usettings = malloc(sizeof(*link_usettings));
+	if (NULL == link_usettings)
+		return NULL;
+
+	memset(link_usettings, 0, sizeof(*link_usettings));
+	/* remember that ETHTOOL_GSET was used */
+	link_usettings->base.cmd = ETHTOOL_GSET;
+
+	link_usettings->base.link_mode_masks_nwords = 1;
+	link_usettings->link_modes.supported[0] = ecmd.supported;
+	link_usettings->link_modes.advertising[0] = ecmd.advertising;
+	link_usettings->link_modes.lp_advertising[0] = ecmd.lp_advertising;
+	link_usettings->base.speed = ethtool_cmd_speed(&ecmd);
+	link_usettings->base.duplex = ecmd.duplex;
+	link_usettings->base.port = ecmd.port;
+	link_usettings->base.phy_address = ecmd.phy_address;
+	link_usettings->deprecated.transceiver = ecmd.transceiver;
+	link_usettings->base.autoneg = ecmd.autoneg;
+	link_usettings->base.mdio_support = ecmd.mdio_support;
+	/* ignored (fully deprecated): maxrxpkt, maxtxpkt */
+	link_usettings->base.eth_tp_mdix = ecmd.eth_tp_mdix;
+	link_usettings->base.eth_tp_mdix_ctrl = ecmd.eth_tp_mdix_ctrl;
+	link_usettings->deprecated.transceiver = ecmd.transceiver;
+
+	return link_usettings;
+}
+
+static bool ethtool_link_mode_is_backward_compatible(
+	const u32 *mask)
+{
+	unsigned int i;
+
+	for (i = 1 ; i < __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 ; ++i)
+		if (mask[i])
+			return false;
+
+	return true;
+}
+
+static int
+__do_ioctl_sset(struct cmd_context *ctx,
+		const struct ethtool_link_usettings *link_usettings)
+{
+	struct ethtool_cmd ecmd;
+
+	/* refuse to send ETHTOOL_SSET ioctl if link_usettings was
+	 * retrived with ETHTOOL_GLINKSETTINGS */
+	if (ETHTOOL_GSET != link_usettings->base.cmd)
+		return -1;
+
+	if (link_usettings->base.link_mode_masks_nwords <= 0)
+		return -1;
+
+	/* refuse to sset if any bit > 31 is set */
+	if (!ethtool_link_mode_is_backward_compatible(
+		    link_usettings->link_modes.supported))
+		return -1;
+	if (!ethtool_link_mode_is_backward_compatible(
+		    link_usettings->link_modes.advertising))
+		return -1;
+	if (!ethtool_link_mode_is_backward_compatible(
+		    link_usettings->link_modes.lp_advertising))
+		return -1;
+
+	memset(&ecmd, 0, sizeof(ecmd));
+	ecmd.cmd = ETHTOOL_SSET;
+
+	ecmd.supported = link_usettings->link_modes.supported[0];
+	ecmd.advertising = link_usettings->link_modes.advertising[0];
+	ecmd.lp_advertising = link_usettings->link_modes.lp_advertising[0];
+	ethtool_cmd_speed_set(&ecmd, link_usettings->base.speed);
+	ecmd.duplex = link_usettings->base.duplex;
+	ecmd.port = link_usettings->base.port;
+	ecmd.phy_address = link_usettings->base.phy_address;
+	ecmd.transceiver = link_usettings->deprecated.transceiver;
+	ecmd.autoneg = link_usettings->base.autoneg;
+	ecmd.mdio_support = link_usettings->base.mdio_support;
+	/* ignored (fully deprecated): maxrxpkt, maxtxpkt */
+	ecmd.eth_tp_mdix = link_usettings->base.eth_tp_mdix;
+	ecmd.eth_tp_mdix_ctrl = link_usettings->base.eth_tp_mdix_ctrl;
+	return send_ioctl(ctx, &ecmd);
+}
+
+static void
+__free_link_usettings(struct ethtool_link_usettings *link_usettings) {
+	free(link_usettings);
+}
+
+static int do_gset(struct cmd_context *ctx)
+{
+	int err;
+	struct ethtool_link_usettings *link_usettings;
 	struct ethtool_wolinfo wolinfo;
 	struct ethtool_value edata;
 	int allfail = 1;
@@ -2261,10 +2582,12 @@ static int do_gset(struct cmd_context *ctx)
 
 	fprintf(stdout, "Settings for %s:\n", ctx->devname);
 
-	ecmd.cmd = ETHTOOL_GSET;
-	err = send_ioctl(ctx, &ecmd);
-	if (err == 0) {
-		err = dump_ecmd(&ecmd);
+	link_usettings = __do_ioctl_glinksettings(ctx);
+	if (NULL == link_usettings)
+		link_usettings = __do_ioctl_gset(ctx);
+	if (NULL != link_usettings) {
+		err = dump_link_usettings(link_usettings);
+		__free_link_usettings(link_usettings);
 		if (err)
 			return err;
 		allfail = 0;
@@ -2323,8 +2646,10 @@ static int do_sset(struct cmd_context *ctx)
 	int autoneg_wanted = -1;
 	int phyad_wanted = -1;
 	int xcvr_wanted = -1;
-	int full_advertising_wanted = -1;
-	int advertising_wanted = -1;
+	u32 *full_advertising_wanted = NULL;
+	u32 *advertising_wanted = NULL;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(_mask_full_advertising_wanted);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(_mask_advertising_wanted);
 	int gset_changed = 0; /* did anything in GSET change? */
 	u32 wol_wanted = 0;
 	int wol_change = 0;
@@ -2338,7 +2663,7 @@ static int do_sset(struct cmd_context *ctx)
 	int argc = ctx->argc;
 	char **argp = ctx->argp;
 	int i;
-	int err;
+	int err = 0;
 
 	for (i = 0; i < ARRAY_SIZE(flags_msglvl); i++)
 		flag_to_cmdline_info(flags_msglvl[i].name,
@@ -2412,7 +2737,12 @@ static int do_sset(struct cmd_context *ctx)
 			i += 1;
 			if (i >= argc)
 				exit_bad_args();
-			full_advertising_wanted = get_int(argp[i], 16);
+			if (0 != parse_hex_u32_bitmap(
+				    argp[i],
+				    __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS,
+				    _mask_full_advertising_wanted))
+				exit_bad_args();
+			full_advertising_wanted = _mask_full_advertising_wanted;
 		} else if (!strcmp(argp[i], "phyad")) {
 			gset_changed = 1;
 			i += 1;
@@ -2469,75 +2799,88 @@ static int do_sset(struct cmd_context *ctx)
 		}
 	}
 
-	if (full_advertising_wanted < 0) {
+	if (NULL == full_advertising_wanted) {
 		/* User didn't supply a full advertisement bitfield:
 		 * construct one from the specified speed and duplex.
 		 */
+		int adv_bit = -1;
+
 		if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
-			advertising_wanted = ADVERTISED_10baseT_Half;
+			adv_bit = ETHTOOL_LINK_MODE_10baseT_Half_BIT;
 		else if (speed_wanted == SPEED_10 &&
 			 duplex_wanted == DUPLEX_FULL)
-			advertising_wanted = ADVERTISED_10baseT_Full;
+			adv_bit = ETHTOOL_LINK_MODE_10baseT_Full_BIT;
 		else if (speed_wanted == SPEED_100 &&
 			 duplex_wanted == DUPLEX_HALF)
-			advertising_wanted = ADVERTISED_100baseT_Half;
+			adv_bit = ETHTOOL_LINK_MODE_100baseT_Half_BIT;
 		else if (speed_wanted == SPEED_100 &&
 			 duplex_wanted == DUPLEX_FULL)
-			advertising_wanted = ADVERTISED_100baseT_Full;
+			adv_bit = ETHTOOL_LINK_MODE_100baseT_Full_BIT;
 		else if (speed_wanted == SPEED_1000 &&
 			 duplex_wanted == DUPLEX_HALF)
-			advertising_wanted = ADVERTISED_1000baseT_Half;
+			adv_bit = ETHTOOL_LINK_MODE_1000baseT_Half_BIT;
 		else if (speed_wanted == SPEED_1000 &&
 			 duplex_wanted == DUPLEX_FULL)
-			advertising_wanted = ADVERTISED_1000baseT_Full;
+			adv_bit = ETHTOOL_LINK_MODE_1000baseT_Full_BIT;
 		else if (speed_wanted == SPEED_2500 &&
 			 duplex_wanted == DUPLEX_FULL)
-			advertising_wanted = ADVERTISED_2500baseX_Full;
+			adv_bit = ETHTOOL_LINK_MODE_2500baseX_Full_BIT;
 		else if (speed_wanted == SPEED_10000 &&
 			 duplex_wanted == DUPLEX_FULL)
-			advertising_wanted = ADVERTISED_10000baseT_Full;
-		else
-			/* auto negotiate without forcing,
-			 * all supported speed will be assigned below
-			 */
-			advertising_wanted = 0;
+			adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
+
+		if (adv_bit >= 0) {
+			advertising_wanted = _mask_advertising_wanted;
+			ethtool_link_mode_set_bit(
+				adv_bit, advertising_wanted);
+		}
+		/* otherwise: auto negotiate without forcing,
+		 * all supported speed will be assigned below
+		 */
 	}
 
 	if (gset_changed) {
-		struct ethtool_cmd ecmd;
+		struct ethtool_link_usettings *link_usettings;
 
-		ecmd.cmd = ETHTOOL_GSET;
-		err = send_ioctl(ctx, &ecmd);
-		if (err < 0) {
+		link_usettings = __do_ioctl_glinksettings(ctx);
+		if (NULL == link_usettings)
+			link_usettings = __do_ioctl_gset(ctx);
+		if (NULL == link_usettings) {
 			perror("Cannot get current device settings");
+			err = -1;
 		} else {
 			/* Change everything the user specified. */
 			if (speed_wanted != -1)
-				ethtool_cmd_speed_set(&ecmd, speed_wanted);
+				link_usettings->base.speed = speed_wanted;
 			if (duplex_wanted != -1)
-				ecmd.duplex = duplex_wanted;
+				link_usettings->base.duplex = duplex_wanted;
 			if (port_wanted != -1)
-				ecmd.port = port_wanted;
+				link_usettings->base.port = port_wanted;
 			if (mdix_wanted != -1) {
 				/* check driver supports MDI-X */
-				if (ecmd.eth_tp_mdix_ctrl != ETH_TP_MDI_INVALID)
-					ecmd.eth_tp_mdix_ctrl = mdix_wanted;
+				if (link_usettings->base.eth_tp_mdix_ctrl
+				    != ETH_TP_MDI_INVALID)
+					link_usettings->base.eth_tp_mdix_ctrl
+						= mdix_wanted;
 				else
-					fprintf(stderr, "setting MDI not supported\n");
+					fprintf(stderr,
+						"setting MDI not supported\n");
 			}
 			if (autoneg_wanted != -1)
-				ecmd.autoneg = autoneg_wanted;
+				link_usettings->base.autoneg = autoneg_wanted;
 			if (phyad_wanted != -1)
-				ecmd.phy_address = phyad_wanted;
+				link_usettings->base.phy_address = phyad_wanted;
 			if (xcvr_wanted != -1)
-				ecmd.transceiver = xcvr_wanted;
+				link_usettings->deprecated.transceiver
+					= xcvr_wanted;
 			/* XXX If the user specified speed or duplex
 			 * then we should mask the advertised modes
 			 * accordingly.  For now, warn that we aren't
 			 * doing that.
 			 */
 			if ((speed_wanted != -1 || duplex_wanted != -1) &&
-			    ecmd.autoneg && advertising_wanted == 0) {
+			    link_usettings->base.autoneg &&
+			    NULL == advertising_wanted) {
 				fprintf(stderr, "Cannot advertise");
 				if (speed_wanted >= 0)
 					fprintf(stderr, " speed %d",
@@ -2549,37 +2892,56 @@ static int do_sset(struct cmd_context *ctx)
 				fprintf(stderr,	"\n");
 			}
 			if (autoneg_wanted == AUTONEG_ENABLE &&
-			    advertising_wanted == 0) {
+			    NULL == advertising_wanted) {
+				unsigned int i;
+
 				/* Auto negotiation enabled, but with
 				 * unspecified speed and duplex: enable all
 				 * supported speeds and duplexes.
 				 */
-				ecmd.advertising =
-					(ecmd.advertising &
-					 ~ALL_ADVERTISED_MODES) |
-					(ALL_ADVERTISED_MODES & ecmd.supported);
+				ethtool_link_mode_for_each_u32(i)
+				{
+					u32 sup = link_usettings->link_modes.supported[i];
+					u32 *adv = link_usettings->link_modes.advertising + i;
+					*adv = ((*adv & ~all_advertised_modes[i])
+						| (sup & all_advertised_modes[i]));
+				}
 
 				/* If driver supports unknown flags, we cannot
 				 * be sure that we enable all link modes.
 				 */
-				if ((ecmd.supported & ALL_ADVERTISED_FLAGS) !=
-				    ecmd.supported) {
-					fprintf(stderr, "Driver supports one "
-					        "or more unknown flags\n");
+				ethtool_link_mode_for_each_u32(i)
+				{
+					u32 sup = link_usettings->link_modes.supported[i];
+					if ((sup & all_advertised_flags[i]) != sup) {
+						fprintf(stderr, "Driver supports one "
+							"or more unknown flags\n");
+						break;
+					}
 				}
-			} else if (advertising_wanted > 0) {
+			} else if (NULL != advertising_wanted) {
+				unsigned int i;
+
 				/* Enable all requested modes */
-				ecmd.advertising =
-					(ecmd.advertising &
-					 ~ALL_ADVERTISED_MODES) |
-					advertising_wanted;
-			} else if (full_advertising_wanted > 0) {
-				ecmd.advertising = full_advertising_wanted;
+				ethtool_link_mode_for_each_u32(i)
+				{
+					u32 *adv = link_usettings->link_modes.advertising + i;
+					*adv = ((*adv & all_advertised_modes[i])
+						| advertising_wanted[i]);
+				}
+			} else if (NULL != full_advertising_wanted) {
+				ethtool_link_mode_copy(
+					link_usettings->link_modes.advertising,
+					full_advertising_wanted);
 			}
 
 			/* Try to perform the update. */
-			ecmd.cmd = ETHTOOL_SSET;
-			err = send_ioctl(ctx, &ecmd);
+			if (ETHTOOL_GLINKSETTINGS == link_usettings->base.cmd) {
+				err = __do_ioctl_slinksettings(ctx, link_usettings);
+			} else {
+				err = __do_ioctl_sset(ctx, link_usettings);
+			}
+			__free_link_usettings(link_usettings);
 			if (err < 0)
 				perror("Cannot set new settings");
 		}
@@ -4200,6 +4562,8 @@ int main(int argc, char **argp)
 	struct cmd_context ctx;
 	int k;
 
+	init_global_link_mode_masks();
+
 	/* Skip command name */
 	argp++;
 	argc--;
diff --git a/internal.h b/internal.h
index 1c64306..806a70d 100644
--- a/internal.h
+++ b/internal.h
@@ -12,6 +12,7 @@
 #ifdef HAVE_CONFIG_H
 #include "ethtool-config.h"
 #endif
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -116,6 +117,72 @@ static inline int test_bit(unsigned int nr, const unsigned long *addr)
 				 ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE |	\
 				 ETH_FLAG_RXHASH)
 
+/* internal API for link mode bitmap interaction with kernel. */
+
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32	\
+	(SCHAR_MAX)
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS	\
+	(32*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32)
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES	\
+	(4*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32)
+#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)			\
+	u32 name[__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32]
+
+struct ethtool_link_usettings {
+	struct {
+		__u8 transceiver;
+	} deprecated;
+	struct ethtool_link_settings base;
+	struct {
+		__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+		__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
+		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+	} link_modes;
+};
+
+#define ethtool_link_mode_for_each_u32(index)				\
+	for ((index) = 0 ;						\
+	     (index) < __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 ;	\
+	     ++(index))
+
+static inline void ethtool_link_mode_zero(u32 *dst)
+{
+	memset(dst, 0, __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES);
+}
+
+static inline bool ethtool_link_mode_is_empty(const u32 *mask)
+{
+	unsigned int i;
+
+	ethtool_link_mode_for_each_u32(i)
+	{
+		if (0 != mask[i])
+			return false;
+	}
+
+	return true;
+}
+
+static inline void ethtool_link_mode_copy(u32 *dst, const u32 *src)
+{
+	memcpy(dst, src, __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES);
+}
+
+static inline int ethtool_link_mode_test_bit(unsigned int nr, const u32 *mask)
+{
+	if (nr >= __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
+		return !!0;
+	return !!(mask[nr / 32] & (1 << (nr % 32)));
+}
+
+static inline int ethtool_link_mode_set_bit(unsigned int nr, u32 *mask)
+{
+       if (nr >= __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
+               return -1;
+       mask[nr / 32] |= (1 << (nr % 32));
+       return 0;
+}
+
 /* Context for sub-commands */
 struct cmd_context {
 	const char *devname;	/* net device name */
diff --git a/test-cmdline.c b/test-cmdline.c
index 2fd7cbb..2f5737c 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -37,7 +37,19 @@ static struct test_case {
 	{ 1, "--change devname autoneg foo" },
 	{ 1, "-s devname autoneg" },
 	{ 0, "--change devname advertise 0x1" },
+	{ 0, "--change devname advertise 0xf" },
+	{ 0, "--change devname advertise 1" },
+	{ 0, "--change devname advertise f" },
+	{ 0, "--change devname advertise 01" },
+	{ 0, "--change devname advertise 0f" },
+	{ 0, "--change devname advertise 0xfffffffffffffffffffffffffffffffff" },
+	{ 0, "--change devname advertise fffffffffffffffffffffffffffffffff" },
+	{ 0, "--change devname advertise 0x0000fffffffffffffffffffffffffffff" },
+	{ 0, "--change devname advertise 0000fffffffffffffffffffffffffffff" },
+	{ 1, "-s devname advertise" },
+	{ 1, "-s devname advertise 0x" },
 	{ 1, "-s devname advertise foo" },
+	{ 1, "-s devname advertise 0xfoo" },
 	{ 1, "--change devname advertise" },
 	{ 0, "-s devname phyad 1" },
 	{ 1, "--change devname phyad foo" },
-- 
2.7.0.rc3.207.g0ac5344

Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 8379 bytes --]

On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
[...]
> +static int parse_hex_u32_bitmap(const char *s,
> +				unsigned int nbits, u32 *result)
> +{
> +	const unsigned nwords = __KERNEL_DIV_ROUND_UP(nbits, 32);
> +	size_t slen = strlen(s);
> +	size_t i;
> +
> +	/* ignore optional '0x' prefix */
> +	if ((slen > 2) && (
> +		    (0 == memcmp(s, "0x", 2)
> +		     || (0 == memcmp(s, "0X", 2))))) {

memcmp() is a really poor tool for comparing strings.  You should use
strncasecmp() here.

> +		slen -= 2;
> +		s += 2;
> +	}
> +
> +	if (slen > 8*nwords)  /* up to 2 digits per byte */

The '*' operator should have spaces around it.  Please run
checkpatch.pl over your ethtool patches to catch style errors like this
(there are many others in this one).

[...]
> +static void dump_link_caps(const char *prefix, const char *an_prefix,
> +			   const u32 *mask, int link_mode_only)
>  {
>  	static const struct {
>  		int same_line; /* print on same line as previous */
> -		u32 value;
> +		unsigned bit_indice;

bit_index

[...]
> @@ -558,51 +655,57 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
>  		}
>  	}
>  	if (did1 == 0)
> -		 fprintf(stdout, "Not reported");
> +		fprintf(stdout, "Not reported");

Good catch, but whitespace fixes belong in another patch.

[...]

> @@ -2248,10 +2360,219 @@ static int do_sfeatures(struct cmd_context *ctx)
>  	return 0;
>  }
>  
> -static int do_gset(struct cmd_context *ctx)
> +static struct ethtool_link_usettings *
> +__do_ioctl_glinksettings(struct cmd_context * ctx)
> +{
> +	int err;
> +	struct {
> +		struct ethtool_link_settings req;
> +		__u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
> +	} ecmd;
> +	struct ethtool_link_usettings *link_usettings;
> +	unsigned u32_offs;
> +
> +	/* handshake with kernel to determine number of words for link
> +	 * mode bitmaps */
> +	memset(&ecmd, 0, sizeof(ecmd));
> +	ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
> +	err = send_ioctl(ctx, &ecmd);
> +	if (err < 0)
> +		return NULL;
> +
> +	if (ecmd.req.link_mode_masks_nwords >= 0 || ecmd.req.cmd)
> +		return NULL;

Oops, I missed the kernel side of this.  Clearing the cmd field is a
very strange thing to do and inconsistent with every other ethtool
command, so I've just sent a patch to change the kernel side of that.
You'll need to change this to match.

(I also think the negative size is a bit weird, but don't feel so
strongly about it.)

[...]
> +	link_usettings = malloc(sizeof(*link_usettings));
> +	if (NULL == link_usettings)

Comparison is the wrong way round.

> +		return NULL;
> +
> +	memset(link_usettings, 0, sizeof(*link_usettings));

Could use calloc() instead of malloc() + memset().

> +	/* keep transceiver 0 */
> +	memcpy(&link_usettings->base, &ecmd.req, sizeof(link_usettings->base));
> +	/* remember that ETHTOOL_GLINKSETTINGS was used */
> +	link_usettings->base.cmd = ETHTOOL_GLINKSETTINGS;

This is redundant as we know that ecmd.req.cmd ==
ETHTOOL_GLINKSETTINGS.

[...]
> +static struct ethtool_link_usettings *
> +__do_ioctl_gset(struct cmd_context * ctx)
>  {
[...]
> +	link_usettings->base.link_mode_masks_nwords = 1;
> +	link_usettings->link_modes.supported[0] = ecmd.supported;
> +	link_usettings->link_modes.advertising[0] = ecmd.advertising;
> +	link_usettings->link_modes.lp_advertising[0] = ecmd.lp_advertising;
> +	link_usettings->base.speed = ethtool_cmd_speed(&ecmd);
> +	link_usettings->base.duplex = ecmd.duplex;
> +	link_usettings->base.port = ecmd.port;
> +	link_usettings->base.phy_address = ecmd.phy_address;
> +	link_usettings->deprecated.transceiver = ecmd.transceiver;
> +	link_usettings->base.autoneg = ecmd.autoneg;
> +	link_usettings->base.mdio_support = ecmd.mdio_support;
> +	/* ignored (fully deprecated): maxrxpkt, maxtxpkt */
> +	link_usettings->base.eth_tp_mdix = ecmd.eth_tp_mdix;
> +	link_usettings->base.eth_tp_mdix_ctrl = ecmd.eth_tp_mdix_ctrl;
> +	link_usettings->deprecated.transceiver = ecmd.transceiver;

Duplicate assignment.

> +	return link_usettings;
> +}
> +
> +static bool ethtool_link_mode_is_backward_compatible(
> +	const u32 *mask)

Don't wrap function parameter lists or statements that are already
under 80 characters.

[...]
> +static void
> +__free_link_usettings(struct ethtool_link_usettings *link_usettings) {
> +	free(link_usettings);
> +}

Is this function ever likely to do more than just free()?  If not,
don't bother abstracting that.

[...]
> @@ -2469,75 +2799,88 @@ static int do_sset(struct cmd_context *ctx)
>  		}
>  	}
>  
> -	if (full_advertising_wanted < 0) {
> +	if (NULL == full_advertising_wanted) {
>  		/* User didn't supply a full advertisement bitfield:
>  		 * construct one from the specified speed and duplex.
>  		 */
> +		int adv_bit = -1;
> +
>  		if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
> -			advertising_wanted = ADVERTISED_10baseT_Half;
> +			adv_bit = ETHTOOL_LINK_MODE_10baseT_Half_BIT;
>  		else if (speed_wanted == SPEED_10 &&
>  			 duplex_wanted == DUPLEX_FULL)
> -			advertising_wanted = ADVERTISED_10baseT_Full;
> +			adv_bit = ETHTOOL_LINK_MODE_10baseT_Full_BIT;
>  		else if (speed_wanted == SPEED_100 &&
>  			 duplex_wanted == DUPLEX_HALF)
> -			advertising_wanted = ADVERTISED_100baseT_Half;
> +			adv_bit = ETHTOOL_LINK_MODE_100baseT_Half_BIT;
>  		else if (speed_wanted == SPEED_100 &&
>  			 duplex_wanted == DUPLEX_FULL)
> -			advertising_wanted = ADVERTISED_100baseT_Full;
> +			adv_bit = ETHTOOL_LINK_MODE_100baseT_Full_BIT;
>  		else if (speed_wanted == SPEED_1000 &&
>  			 duplex_wanted == DUPLEX_HALF)
> -			advertising_wanted = ADVERTISED_1000baseT_Half;
> +			adv_bit = ETHTOOL_LINK_MODE_1000baseT_Half_BIT;
>  		else if (speed_wanted == SPEED_1000 &&
>  			 duplex_wanted == DUPLEX_FULL)
> -			advertising_wanted = ADVERTISED_1000baseT_Full;
> +			adv_bit = ETHTOOL_LINK_MODE_1000baseT_Full_BIT;
>  		else if (speed_wanted == SPEED_2500 &&
>  			 duplex_wanted == DUPLEX_FULL)
> -			advertising_wanted = ADVERTISED_2500baseX_Full;
> +			adv_bit = ETHTOOL_LINK_MODE_2500baseX_Full_BIT;
>  		else if (speed_wanted == SPEED_10000 &&
>  			 duplex_wanted == DUPLEX_FULL)
> -			advertising_wanted = ADVERTISED_10000baseT_Full;
> -		else
> -			/* auto negotiate without forcing,
> -			 * all supported speed will be assigned below
> -			 */
> -			advertising_wanted = 0;
> +			adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
> +
> +		if (adv_bit >= 0) {
> +			advertising_wanted = _mask_advertising_wanted;

If I'm not mistaken, _mask_advertising_wanted is uninitialised at this
point so you need to clear it before setting the one bit.

> +			ethtool_link_mode_set_bit(
> +				adv_bit, advertising_wanted);
[...]
> @@ -2549,37 +2892,56 @@ static int do_sset(struct cmd_context *ctx)
>  				fprintf(stderr,	"\n");
>  			}
>  			if (autoneg_wanted == AUTONEG_ENABLE &&
> -			    advertising_wanted == 0) {
> +			    NULL == advertising_wanted) {
> +				unsigned int i;
> +
>  				/* Auto negotiation enabled, but with
>  				 * unspecified speed and duplex: enable all
>  				 * supported speeds and duplexes.
>  				 */
> -				ecmd.advertising =
> -					(ecmd.advertising &
> -					 ~ALL_ADVERTISED_MODES) |
> -					(ALL_ADVERTISED_MODES & ecmd.supported);
> +				ethtool_link_mode_for_each_u32(i)
> +				{

Brace belongs on the previous line (everywhere you use
ethtool_link_mode_for_each_u32()).

[...]
> -			} else if (advertising_wanted > 0) {
> +			} else if (NULL != advertising_wanted) {
> +				unsigned int i;
> +
>  				/* Enable all requested modes */
> -				ecmd.advertising =
> -					(ecmd.advertising &
> -					 ~ALL_ADVERTISED_MODES) |
> -					advertising_wanted;
> -			} else if (full_advertising_wanted > 0) {
> -				ecmd.advertising = full_advertising_wanted;
> +				ethtool_link_mode_for_each_u32(i)
> +				{
> +					u32 *adv = link_usettings->link_modes.advertising + i;
> +					*adv = ((*adv & all_advertised_modes[i])
[...]

Should be ~all_advertised_modes[i] rather than all_advertised_modes[i].

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls

[David Laight]

> > +	/* ignore optional '0x' prefix */
> > +	if ((slen > 2) && (

Unnecessary ().

> > +		    (0 == memcmp(s, "0x", 2)
> > +		     || (0 == memcmp(s, "0X", 2))))) {

A-about-F comparisons.

> memcmp() is a really poor tool for comparing strings.  You should use
> strncasecmp() here.

Even that is overkill, why not just:
	if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X'))

	David

Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls

[David Decotigny]

Just sent v5 of the series: now only this patch left. Applied all your
suggestions.

Many thanks for the 2 bugs you caught, path was not covered by my
tests. v5 was tested on a 10G nic with: ethtool -s eth1 msglvl 0x15
speed 10000 duplex full

On Sun, Mar 13, 2016 at 6:32 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
> [...]
>> +static int parse_hex_u32_bitmap(const char *s,
>> +                             unsigned int nbits, u32 *result)
>> +{
>> +     const unsigned nwords = __KERNEL_DIV_ROUND_UP(nbits, 32);
>> +     size_t slen = strlen(s);
>> +     size_t i;
>> +
>> +     /* ignore optional '0x' prefix */
>> +     if ((slen > 2) && (
>> +                 (0 == memcmp(s, "0x", 2)
>> +                  || (0 == memcmp(s, "0X", 2))))) {
>
> memcmp() is a really poor tool for comparing strings.  You should use
> strncasecmp() here.

done.

>
>> +             slen -= 2;
>> +             s += 2;
>> +     }
>> +
>> +     if (slen > 8*nwords)  /* up to 2 digits per byte */
>
> The '*' operator should have spaces around it.  Please run
> checkpatch.pl over your ethtool patches to catch style errors like this
> (there are many others in this one).

both done. a few new lines over 80 chars that I prefer to keep,
otherwise line break would make code harder to read imho.

>
> [...]
>> +static void dump_link_caps(const char *prefix, const char *an_prefix,
>> +                        const u32 *mask, int link_mode_only)
>>  {
>>       static const struct {
>>               int same_line; /* print on same line as previous */
>> -             u32 value;
>> +             unsigned bit_indice;
>
> bit_index

done.

>
> [...]
>> @@ -558,51 +655,57 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
>>               }
>>       }
>>       if (did1 == 0)
>> -              fprintf(stdout, "Not reported");
>> +             fprintf(stdout, "Not reported");
>
> Good catch, but whitespace fixes belong in another patch.

removed for now.

>
> [...]
>
>> @@ -2248,10 +2360,219 @@ static int do_sfeatures(struct cmd_context *ctx)
>>       return 0;
>>  }
>>
>> -static int do_gset(struct cmd_context *ctx)
>> +static struct ethtool_link_usettings *
>> +__do_ioctl_glinksettings(struct cmd_context * ctx)
>> +{
>> +     int err;
>> +     struct {
>> +             struct ethtool_link_settings req;
>> +             __u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
>> +     } ecmd;
>> +     struct ethtool_link_usettings *link_usettings;
>> +     unsigned u32_offs;
>> +
>> +     /* handshake with kernel to determine number of words for link
>> +      * mode bitmaps */
>> +     memset(&ecmd, 0, sizeof(ecmd));
>> +     ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
>> +     err = send_ioctl(ctx, &ecmd);
>> +     if (err < 0)
>> +             return NULL;
>> +
>> +     if (ecmd.req.link_mode_masks_nwords >= 0 || ecmd.req.cmd)
>> +             return NULL;
>
> Oops, I missed the kernel side of this.  Clearing the cmd field is a
> very strange thing to do and inconsistent with every other ethtool
> command, so I've just sent a patch to change the kernel side of that.
> You'll need to change this to match.
>
> (I also think the negative size is a bit weird, but don't feel so
> strongly about it.)

updated based on your patch. sent a suggestion
http://marc.info/?l=linux-netdev&m=145806871332416 , looks like it was
not added properly to the thread, because I don't see it as a reply to
your patch in patchwork. sorry about this.

>
> [...]
>> +     link_usettings = malloc(sizeof(*link_usettings));
>> +     if (NULL == link_usettings)
>
> Comparison is the wrong way round.

done.

>
>> +             return NULL;
>> +
>> +     memset(link_usettings, 0, sizeof(*link_usettings));
>
> Could use calloc() instead of malloc() + memset().

done.

>
>> +     /* keep transceiver 0 */
>> +     memcpy(&link_usettings->base, &ecmd.req, sizeof(link_usettings->base));
>> +     /* remember that ETHTOOL_GLINKSETTINGS was used */
>> +     link_usettings->base.cmd = ETHTOOL_GLINKSETTINGS;
>
> This is redundant as we know that ecmd.req.cmd ==
> ETHTOOL_GLINKSETTINGS.

done.

>
> [...]
>> +static struct ethtool_link_usettings *
>> +__do_ioctl_gset(struct cmd_context * ctx)
>>  {
> [...]
>> +     link_usettings->base.link_mode_masks_nwords = 1;
>> +     link_usettings->link_modes.supported[0] = ecmd.supported;
>> +     link_usettings->link_modes.advertising[0] = ecmd.advertising;
>> +     link_usettings->link_modes.lp_advertising[0] = ecmd.lp_advertising;
>> +     link_usettings->base.speed = ethtool_cmd_speed(&ecmd);
>> +     link_usettings->base.duplex = ecmd.duplex;
>> +     link_usettings->base.port = ecmd.port;
>> +     link_usettings->base.phy_address = ecmd.phy_address;
>> +     link_usettings->deprecated.transceiver = ecmd.transceiver;
>> +     link_usettings->base.autoneg = ecmd.autoneg;
>> +     link_usettings->base.mdio_support = ecmd.mdio_support;
>> +     /* ignored (fully deprecated): maxrxpkt, maxtxpkt */
>> +     link_usettings->base.eth_tp_mdix = ecmd.eth_tp_mdix;
>> +     link_usettings->base.eth_tp_mdix_ctrl = ecmd.eth_tp_mdix_ctrl;
>> +     link_usettings->deprecated.transceiver = ecmd.transceiver;
>
> Duplicate assignment.

done.

>
>> +     return link_usettings;
>> +}
>> +
>> +static bool ethtool_link_mode_is_backward_compatible(
>> +     const u32 *mask)
>
> Don't wrap function parameter lists or statements that are already
> under 80 characters.

done.

>
> [...]
>> +static void
>> +__free_link_usettings(struct ethtool_link_usettings *link_usettings) {
>> +     free(link_usettings);
>> +}
>
> Is this function ever likely to do more than just free()?  If not,
> don't bother abstracting that.

free() is likely to stay good enough. updated.

>
> [...]
>> @@ -2469,75 +2799,88 @@ static int do_sset(struct cmd_context *ctx)
>>               }
>>       }
>>
>> -     if (full_advertising_wanted < 0) {
>> +     if (NULL == full_advertising_wanted) {
>>               /* User didn't supply a full advertisement bitfield:
>>                * construct one from the specified speed and duplex.
>>                */
>> +             int adv_bit = -1;
>> +
>>               if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
>> -                     advertising_wanted = ADVERTISED_10baseT_Half;
>> +                     adv_bit = ETHTOOL_LINK_MODE_10baseT_Half_BIT;
>>               else if (speed_wanted == SPEED_10 &&
>>                        duplex_wanted == DUPLEX_FULL)
>> -                     advertising_wanted = ADVERTISED_10baseT_Full;
>> +                     adv_bit = ETHTOOL_LINK_MODE_10baseT_Full_BIT;
>>               else if (speed_wanted == SPEED_100 &&
>>                        duplex_wanted == DUPLEX_HALF)
>> -                     advertising_wanted = ADVERTISED_100baseT_Half;
>> +                     adv_bit = ETHTOOL_LINK_MODE_100baseT_Half_BIT;
>>               else if (speed_wanted == SPEED_100 &&
>>                        duplex_wanted == DUPLEX_FULL)
>> -                     advertising_wanted = ADVERTISED_100baseT_Full;
>> +                     adv_bit = ETHTOOL_LINK_MODE_100baseT_Full_BIT;
>>               else if (speed_wanted == SPEED_1000 &&
>>                        duplex_wanted == DUPLEX_HALF)
>> -                     advertising_wanted = ADVERTISED_1000baseT_Half;
>> +                     adv_bit = ETHTOOL_LINK_MODE_1000baseT_Half_BIT;
>>               else if (speed_wanted == SPEED_1000 &&
>>                        duplex_wanted == DUPLEX_FULL)
>> -                     advertising_wanted = ADVERTISED_1000baseT_Full;
>> +                     adv_bit = ETHTOOL_LINK_MODE_1000baseT_Full_BIT;
>>               else if (speed_wanted == SPEED_2500 &&
>>                        duplex_wanted == DUPLEX_FULL)
>> -                     advertising_wanted = ADVERTISED_2500baseX_Full;
>> +                     adv_bit = ETHTOOL_LINK_MODE_2500baseX_Full_BIT;
>>               else if (speed_wanted == SPEED_10000 &&
>>                        duplex_wanted == DUPLEX_FULL)
>> -                     advertising_wanted = ADVERTISED_10000baseT_Full;
>> -             else
>> -                     /* auto negotiate without forcing,
>> -                      * all supported speed will be assigned below
>> -                      */
>> -                     advertising_wanted = 0;
>> +                     adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
>> +
>> +             if (adv_bit >= 0) {
>> +                     advertising_wanted = _mask_advertising_wanted;
>
> If I'm not mistaken, _mask_advertising_wanted is uninitialised at this
> point so you need to clear it before setting the one bit.

great catch! thanks.

>
>> +                     ethtool_link_mode_set_bit(
>> +                             adv_bit, advertising_wanted);
> [...]
>> @@ -2549,37 +2892,56 @@ static int do_sset(struct cmd_context *ctx)
>>                               fprintf(stderr, "\n");
>>                       }
>>                       if (autoneg_wanted == AUTONEG_ENABLE &&
>> -                         advertising_wanted == 0) {
>> +                         NULL == advertising_wanted) {
>> +                             unsigned int i;
>> +
>>                               /* Auto negotiation enabled, but with
>>                                * unspecified speed and duplex: enable all
>>                                * supported speeds and duplexes.
>>                                */
>> -                             ecmd.advertising =
>> -                                     (ecmd.advertising &
>> -                                      ~ALL_ADVERTISED_MODES) |
>> -                                     (ALL_ADVERTISED_MODES & ecmd.supported);
>> +                             ethtool_link_mode_for_each_u32(i)
>> +                             {
>
> Brace belongs on the previous line (everywhere you use
> ethtool_link_mode_for_each_u32()).

done.

>
> [...]
>> -                     } else if (advertising_wanted > 0) {
>> +                     } else if (NULL != advertising_wanted) {
>> +                             unsigned int i;
>> +
>>                               /* Enable all requested modes */
>> -                             ecmd.advertising =
>> -                                     (ecmd.advertising &
>> -                                      ~ALL_ADVERTISED_MODES) |
>> -                                     advertising_wanted;
>> -                     } else if (full_advertising_wanted > 0) {
>> -                             ecmd.advertising = full_advertising_wanted;
>> +                             ethtool_link_mode_for_each_u32(i)
>> +                             {
>> +                                     u32 *adv = link_usettings->link_modes.advertising + i;
>> +                                     *adv = ((*adv & all_advertised_modes[i])
> [...]
>
> Should be ~all_advertised_modes[i] rather than all_advertised_modes[i].

aouch! thanks.

>
> Ben.
>
> --
> Ben Hutchings
> If at first you don't succeed, you're doing about average.

Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls

[David Decotigny]

Please note that v5 patch depends on Ben Hutchings'
http://patchwork.ozlabs.org/patch/596879/ to make any sense.

On Tue, Mar 15, 2016 at 4:42 PM, David Decotigny <ddecotig@gmail.com> wrote:
> Just sent v5 of the series: now only this patch left. Applied all your
> suggestions.
>
> Many thanks for the 2 bugs you caught, path was not covered by my
> tests. v5 was tested on a 10G nic with: ethtool -s eth1 msglvl 0x15
> speed 10000 duplex full
>
> On Sun, Mar 13, 2016 at 6:32 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
>> [...]
>>> +static int parse_hex_u32_bitmap(const char *s,
>>> +                             unsigned int nbits, u32 *result)
>>> +{
>>> +     const unsigned nwords = __KERNEL_DIV_ROUND_UP(nbits, 32);
>>> +     size_t slen = strlen(s);
>>> +     size_t i;
>>> +
>>> +     /* ignore optional '0x' prefix */
>>> +     if ((slen > 2) && (
>>> +                 (0 == memcmp(s, "0x", 2)
>>> +                  || (0 == memcmp(s, "0X", 2))))) {
>>
>> memcmp() is a really poor tool for comparing strings.  You should use
>> strncasecmp() here.
>
> done.
>
>>
>>> +             slen -= 2;
>>> +             s += 2;
>>> +     }
>>> +
>>> +     if (slen > 8*nwords)  /* up to 2 digits per byte */
>>
>> The '*' operator should have spaces around it.  Please run
>> checkpatch.pl over your ethtool patches to catch style errors like this
>> (there are many others in this one).
>
> both done. a few new lines over 80 chars that I prefer to keep,
> otherwise line break would make code harder to read imho.
>
>>
>> [...]
>>> +static void dump_link_caps(const char *prefix, const char *an_prefix,
>>> +                        const u32 *mask, int link_mode_only)
>>>  {
>>>       static const struct {
>>>               int same_line; /* print on same line as previous */
>>> -             u32 value;
>>> +             unsigned bit_indice;
>>
>> bit_index
>
> done.
>
>>
>> [...]
>>> @@ -558,51 +655,57 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
>>>               }
>>>       }
>>>       if (did1 == 0)
>>> -              fprintf(stdout, "Not reported");
>>> +             fprintf(stdout, "Not reported");
>>
>> Good catch, but whitespace fixes belong in another patch.
>
> removed for now.
>
>>
>> [...]
>>
>>> @@ -2248,10 +2360,219 @@ static int do_sfeatures(struct cmd_context *ctx)
>>>       return 0;
>>>  }
>>>
>>> -static int do_gset(struct cmd_context *ctx)
>>> +static struct ethtool_link_usettings *
>>> +__do_ioctl_glinksettings(struct cmd_context * ctx)
>>> +{
>>> +     int err;
>>> +     struct {
>>> +             struct ethtool_link_settings req;
>>> +             __u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
>>> +     } ecmd;
>>> +     struct ethtool_link_usettings *link_usettings;
>>> +     unsigned u32_offs;
>>> +
>>> +     /* handshake with kernel to determine number of words for link
>>> +      * mode bitmaps */
>>> +     memset(&ecmd, 0, sizeof(ecmd));
>>> +     ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
>>> +     err = send_ioctl(ctx, &ecmd);
>>> +     if (err < 0)
>>> +             return NULL;
>>> +
>>> +     if (ecmd.req.link_mode_masks_nwords >= 0 || ecmd.req.cmd)
>>> +             return NULL;
>>
>> Oops, I missed the kernel side of this.  Clearing the cmd field is a
>> very strange thing to do and inconsistent with every other ethtool
>> command, so I've just sent a patch to change the kernel side of that.
>> You'll need to change this to match.
>>
>> (I also think the negative size is a bit weird, but don't feel so
>> strongly about it.)
>
> updated based on your patch. sent a suggestion
> http://marc.info/?l=linux-netdev&m=145806871332416 , looks like it was
> not added properly to the thread, because I don't see it as a reply to
> your patch in patchwork. sorry about this.
>
>>
>> [...]
>>> +     link_usettings = malloc(sizeof(*link_usettings));
>>> +     if (NULL == link_usettings)
>>
>> Comparison is the wrong way round.
>
> done.
>
>>
>>> +             return NULL;
>>> +
>>> +     memset(link_usettings, 0, sizeof(*link_usettings));
>>
>> Could use calloc() instead of malloc() + memset().
>
> done.
>
>>
>>> +     /* keep transceiver 0 */
>>> +     memcpy(&link_usettings->base, &ecmd.req, sizeof(link_usettings->base));
>>> +     /* remember that ETHTOOL_GLINKSETTINGS was used */
>>> +     link_usettings->base.cmd = ETHTOOL_GLINKSETTINGS;
>>
>> This is redundant as we know that ecmd.req.cmd ==
>> ETHTOOL_GLINKSETTINGS.
>
> done.
>
>>
>> [...]
>>> +static struct ethtool_link_usettings *
>>> +__do_ioctl_gset(struct cmd_context * ctx)
>>>  {
>> [...]
>>> +     link_usettings->base.link_mode_masks_nwords = 1;
>>> +     link_usettings->link_modes.supported[0] = ecmd.supported;
>>> +     link_usettings->link_modes.advertising[0] = ecmd.advertising;
>>> +     link_usettings->link_modes.lp_advertising[0] = ecmd.lp_advertising;
>>> +     link_usettings->base.speed = ethtool_cmd_speed(&ecmd);
>>> +     link_usettings->base.duplex = ecmd.duplex;
>>> +     link_usettings->base.port = ecmd.port;
>>> +     link_usettings->base.phy_address = ecmd.phy_address;
>>> +     link_usettings->deprecated.transceiver = ecmd.transceiver;
>>> +     link_usettings->base.autoneg = ecmd.autoneg;
>>> +     link_usettings->base.mdio_support = ecmd.mdio_support;
>>> +     /* ignored (fully deprecated): maxrxpkt, maxtxpkt */
>>> +     link_usettings->base.eth_tp_mdix = ecmd.eth_tp_mdix;
>>> +     link_usettings->base.eth_tp_mdix_ctrl = ecmd.eth_tp_mdix_ctrl;
>>> +     link_usettings->deprecated.transceiver = ecmd.transceiver;
>>
>> Duplicate assignment.
>
> done.
>
>>
>>> +     return link_usettings;
>>> +}
>>> +
>>> +static bool ethtool_link_mode_is_backward_compatible(
>>> +     const u32 *mask)
>>
>> Don't wrap function parameter lists or statements that are already
>> under 80 characters.
>
> done.
>
>>
>> [...]
>>> +static void
>>> +__free_link_usettings(struct ethtool_link_usettings *link_usettings) {
>>> +     free(link_usettings);
>>> +}
>>
>> Is this function ever likely to do more than just free()?  If not,
>> don't bother abstracting that.
>
> free() is likely to stay good enough. updated.
>
>>
>> [...]
>>> @@ -2469,75 +2799,88 @@ static int do_sset(struct cmd_context *ctx)
>>>               }
>>>       }
>>>
>>> -     if (full_advertising_wanted < 0) {
>>> +     if (NULL == full_advertising_wanted) {
>>>               /* User didn't supply a full advertisement bitfield:
>>>                * construct one from the specified speed and duplex.
>>>                */
>>> +             int adv_bit = -1;
>>> +
>>>               if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
>>> -                     advertising_wanted = ADVERTISED_10baseT_Half;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_10baseT_Half_BIT;
>>>               else if (speed_wanted == SPEED_10 &&
>>>                        duplex_wanted == DUPLEX_FULL)
>>> -                     advertising_wanted = ADVERTISED_10baseT_Full;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_10baseT_Full_BIT;
>>>               else if (speed_wanted == SPEED_100 &&
>>>                        duplex_wanted == DUPLEX_HALF)
>>> -                     advertising_wanted = ADVERTISED_100baseT_Half;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_100baseT_Half_BIT;
>>>               else if (speed_wanted == SPEED_100 &&
>>>                        duplex_wanted == DUPLEX_FULL)
>>> -                     advertising_wanted = ADVERTISED_100baseT_Full;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_100baseT_Full_BIT;
>>>               else if (speed_wanted == SPEED_1000 &&
>>>                        duplex_wanted == DUPLEX_HALF)
>>> -                     advertising_wanted = ADVERTISED_1000baseT_Half;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_1000baseT_Half_BIT;
>>>               else if (speed_wanted == SPEED_1000 &&
>>>                        duplex_wanted == DUPLEX_FULL)
>>> -                     advertising_wanted = ADVERTISED_1000baseT_Full;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_1000baseT_Full_BIT;
>>>               else if (speed_wanted == SPEED_2500 &&
>>>                        duplex_wanted == DUPLEX_FULL)
>>> -                     advertising_wanted = ADVERTISED_2500baseX_Full;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_2500baseX_Full_BIT;
>>>               else if (speed_wanted == SPEED_10000 &&
>>>                        duplex_wanted == DUPLEX_FULL)
>>> -                     advertising_wanted = ADVERTISED_10000baseT_Full;
>>> -             else
>>> -                     /* auto negotiate without forcing,
>>> -                      * all supported speed will be assigned below
>>> -                      */
>>> -                     advertising_wanted = 0;
>>> +                     adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
>>> +
>>> +             if (adv_bit >= 0) {
>>> +                     advertising_wanted = _mask_advertising_wanted;
>>
>> If I'm not mistaken, _mask_advertising_wanted is uninitialised at this
>> point so you need to clear it before setting the one bit.
>
> great catch! thanks.
>
>>
>>> +                     ethtool_link_mode_set_bit(
>>> +                             adv_bit, advertising_wanted);
>> [...]
>>> @@ -2549,37 +2892,56 @@ static int do_sset(struct cmd_context *ctx)
>>>                               fprintf(stderr, "\n");
>>>                       }
>>>                       if (autoneg_wanted == AUTONEG_ENABLE &&
>>> -                         advertising_wanted == 0) {
>>> +                         NULL == advertising_wanted) {
>>> +                             unsigned int i;
>>> +
>>>                               /* Auto negotiation enabled, but with
>>>                                * unspecified speed and duplex: enable all
>>>                                * supported speeds and duplexes.
>>>                                */
>>> -                             ecmd.advertising =
>>> -                                     (ecmd.advertising &
>>> -                                      ~ALL_ADVERTISED_MODES) |
>>> -                                     (ALL_ADVERTISED_MODES & ecmd.supported);
>>> +                             ethtool_link_mode_for_each_u32(i)
>>> +                             {
>>
>> Brace belongs on the previous line (everywhere you use
>> ethtool_link_mode_for_each_u32()).
>
> done.
>
>>
>> [...]
>>> -                     } else if (advertising_wanted > 0) {
>>> +                     } else if (NULL != advertising_wanted) {
>>> +                             unsigned int i;
>>> +
>>>                               /* Enable all requested modes */
>>> -                             ecmd.advertising =
>>> -                                     (ecmd.advertising &
>>> -                                      ~ALL_ADVERTISED_MODES) |
>>> -                                     advertising_wanted;
>>> -                     } else if (full_advertising_wanted > 0) {
>>> -                             ecmd.advertising = full_advertising_wanted;
>>> +                             ethtool_link_mode_for_each_u32(i)
>>> +                             {
>>> +                                     u32 *adv = link_usettings->link_modes.advertising + i;
>>> +                                     *adv = ((*adv & all_advertised_modes[i])
>> [...]
>>
>> Should be ~all_advertised_modes[i] rather than all_advertised_modes[i].
>
> aouch! thanks.
>
>>
>> Ben.
>>
>> --
>> Ben Hutchings
>> If at first you don't succeed, you're doing about average.

[ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets

[David Decotigny]

From: David Decotigny <decot@googlers.com>


Signed-off-by: David Decotigny <decot@googlers.com>
---
 ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 761252f..f9336e3 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4615,6 +4615,9 @@ opt_found:
 		/* Open control socket. */
 		ctx.fd = socket(AF_INET, SOCK_DGRAM, 0);
 		if (ctx.fd < 0) {
+			ctx.fd = socket(AF_UNIX, SOCK_DGRAM, 0);
+		}
+		if (ctx.fd < 0) {
 			perror("Cannot get control socket");
 			return 70;
 		}
-- 
2.7.0.rc3.207.g0ac5344

Re: [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
> From: David Decotigny <decot@googlers.com>
> 
> 
> Signed-off-by: David Decotigny <decot@googlers.com>
> ---
>  ethtool.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 761252f..f9336e3 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -4615,6 +4615,9 @@ opt_found:
>  		/* Open control socket. */
>  		ctx.fd = socket(AF_INET, SOCK_DGRAM, 0);
>  		if (ctx.fd < 0) {
> +			ctx.fd = socket(AF_UNIX, SOCK_DGRAM, 0);
> +		}

You still haven't answered whether this is a real problem on Linux.

Ben.

> +		if (ctx.fd < 0) {
>  			perror("Cannot get control socket");
>  			return 70;
>  		}
-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets

[David Decotigny]

will send a v5 shortly without this patch: question below.

about this patch: this is in prevision of a world where INET can be
compiled out. So it is not something that matters today with current
kernels.

Now, as you mentioned in another patch, the only socket that survives
various reasonable CONFIG_* gymnastics is netlink. So even though
non-IPv4 kernels with IP support is not feasible today, I believe
there is some logic to using netlink sockets for ethtool purposes,
instead of IPv4 or IPv6 sockets. Shall I propose a patch to add
ethtool support on AF_NETLINK sockets, and update the tool to try
AF_INET first for backward compatibility reasons, then fallback to
AF_NETLINK?

On Sun, Mar 13, 2016 at 10:24 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
>> From: David Decotigny <decot@googlers.com>
>>
>>
>> Signed-off-by: David Decotigny <decot@googlers.com>
>> ---
>>  ethtool.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/ethtool.c b/ethtool.c
>> index 761252f..f9336e3 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -4615,6 +4615,9 @@ opt_found:
>>               /* Open control socket. */
>>               ctx.fd = socket(AF_INET, SOCK_DGRAM, 0);
>>               if (ctx.fd < 0) {
>> +                     ctx.fd = socket(AF_UNIX, SOCK_DGRAM, 0);
>> +             }
>
> You still haven't answered whether this is a real problem on Linux.
>
> Ben.
>
>> +             if (ctx.fd < 0) {
>>                       perror("Cannot get control socket");
>>                       return 70;
>>               }
> --
> Ben Hutchings
> If at first you don't succeed, you're doing about average.

Re: [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Tue, 2016-03-15 at 16:19 -0700, David Decotigny wrote:
> will send a v5 shortly without this patch: question below.
> 
> about this patch: this is in prevision of a world where INET can be
> compiled out. So it is not something that matters today with current
> kernels.

OK.

> Now, as you mentioned in another patch, the only socket that survives
> various reasonable CONFIG_* gymnastics is netlink. So even though
> non-IPv4 kernels with IP support is not feasible today, I believe
> there is some logic to using netlink sockets for ethtool purposes,
> instead of IPv4 or IPv6 sockets. Shall I propose a patch to add
> ethtool support on AF_NETLINK sockets, and update the tool to try
> AF_INET first for backward compatibility reasons, then fallback to
> AF_NETLINK?

Yes, please do that.

Ben.

-- 
Ben Hutchings
Hoare's Law of Large Problems:
        Inside every large problem is a small problem struggling to get out.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Fri, 2016-03-11 at 09:58 -0800, David Decotigny wrote:
> From: David Decotigny <decot@googlers.com>
> 
> This adds support for the new ETHTOOL_xLINKSETTINGS ioctls. This also
> fixes a few compilation warnings as well as a heap corruption bug.
[...]

Patches 1-7: applied, thanks.

Patch 8: this breaks builds from tarballs as you didn't list the new
header as a source in Makefile.am.

Patch 9: I applied an equivalent change already.

Patch 10: still to be reviewed, but I will probably do so later today.

Patch 11: you need to answer my question about this.

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Sun, 2016-03-13 at 17:30 +0000, Ben Hutchings wrote:
[...]
> Patch 11: you need to answer my question about this.

[This is about why/whether there should be a fallback from creating an
AF_INET socket to issue SIOCETHTOOL on.]

I looked a little further, and found:
- The only address/protocol family that is always present in the kernel
  (assuming CONFIG_NET=y) is netlink
- Netlink sockets don't support ioctl at all

So there seems to be no right answer to 'which family of socket should
I create to perform device ioctls'.  There really ought to be!

Ben.

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls

[David Decotigny]

From: David Decotigny <decot@googlers.com>

This adds support for the new ETHTOOL_xLINKSETTINGS ioctls. This also
fixes a few compilation warnings as well as a heap corruption bug.

History:
  v4
    review Ben Hutchings:
      using AF_UNIX instead of INET6 in the absence of v4 sockets
      use stdbool.h
      do_seeprom always fails when offset/length out of bounds
      sync to latest ethtool.h + kernel.h from net-next
      __SANE_USERSPACE_TYPES__ always defined
      cosmetic updates for var == const tests
      cosmetic updates for associativity in tests
  v3
    TRUE/FALSE obvious-ification
  v2
    added do_seeprom patch
    added netdev@ as recipient
  v1
    initial submission

############################################
# Patch Set Summary:

David Decotigny (7):
  ethtool.c: don't ignore fread() return value
  ethtool.c: fix dump_regs heap corruption
  ethtool.c: do_seeprom checks for params & stdin sanity
  kernel-copy.h: import kernel.h from net-next and use it
  ethtool-copy.h: sync with net-next
  ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls
  ethtool.c: support absence of v4 sockets

Maciej Żenczykowski (4):
  internal.h: change to new sane kernel headers on 64-bit archs
  marvell.c: fix strict alias warnings
  test-common.c: fix test_realloc(NULL, ...)
  test-features.c: add braces around array initialization

 ethtool-copy.h  | 478 ++++++++++++++++++++++++++++++------
 ethtool.c       | 751 ++++++++++++++++++++++++++++++++++++++++++--------------
 internal.h      |  77 +++++-
 kernel-copy.h   |  14 ++
 marvell.c       |  21 +-
 test-cmdline.c  |  12 +
 test-common.c   |   2 +-
 test-features.c |   2 +-
 8 files changed, 1086 insertions(+), 271 deletions(-)
 create mode 100644 kernel-copy.h

-- 
2.7.0.rc3.207.g0ac5344

[ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings

[David Decotigny]

From: Maciej Żenczykowski <maze@google.com>

Addresses the following warnings:
  marvell.c:426:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  marvell.c:427:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  marvell.c:428:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]

Note: code appears endian-dependent, not fixed by this commit.


Signed-off-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: David Decotigny <decot@googlers.com>
---
 marvell.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/marvell.c b/marvell.c
index e583c82..af21188 100644
--- a/marvell.c
+++ b/marvell.c
@@ -381,7 +381,8 @@ static void dump_prefetch(const char *name, const void *r)
 
 int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 {
-	const u32 *r = (const u32 *) regs->data;
+	const u16 *r16 = (const u16 *) regs->data;
+	const u32 *r32 = (const u32 *) regs->data;
 	int dual;
 
 	dump_pci(regs->data + 0x1c00);
@@ -390,15 +391,15 @@ int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 
 	printf("\nBus Management Unit\n");
 	printf("-------------------\n");
-	printf("CSR Receive Queue 1              0x%08X\n", r[24]);
-	printf("CSR Sync Queue 1                 0x%08X\n", r[26]);
-	printf("CSR Async Queue 1                0x%08X\n", r[27]);
+	printf("CSR Receive Queue 1              0x%08X\n", r32[24]);
+	printf("CSR Sync Queue 1                 0x%08X\n", r32[26]);
+	printf("CSR Async Queue 1                0x%08X\n", r32[27]);
 
 	dual = (regs->data[0x11e] & 2) != 0;
 	if (dual) {
-		printf("CSR Receive Queue 2              0x%08X\n", r[25]);
-		printf("CSR Async Queue 2                0x%08X\n", r[29]);
-		printf("CSR Sync Queue 2                 0x%08X\n", r[28]);
+		printf("CSR Receive Queue 2              0x%08X\n", r32[25]);
+		printf("CSR Async Queue 2                0x%08X\n", r32[29]);
+		printf("CSR Sync Queue 2                 0x%08X\n", r32[28]);
 	}
 
 	dump_mac(regs->data);
@@ -423,9 +424,9 @@ int sky2_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 	dump_timer("TX status", regs->data + 0xec0);
 	dump_timer("ISR", regs->data + 0xed0);
 
-	printf("\nGMAC control             0x%04X\n", *(u32 *)(regs->data + 0xf00));
-	printf("GPHY control             0x%04X\n", *(u32 *)(regs->data + 0xf04));
-	printf("LINK control             0x%02hX\n", *(u16 *)(regs->data + 0xf10));
+	printf("\nGMAC control             0x%04X\n", r32[0xf00 >> 2]);
+	printf("GPHY control             0x%04X\n", r32[0xf04 >> 2]);
+	printf("LINK control             0x%02hX\n", r16[0xf10 >> 1]);
 
 	dump_gmac("GMAC 1", regs->data + 0x2800);
 	dump_gmac_fifo("Rx GMAC 1", regs->data + 0xc40);
-- 
2.7.0.rc3.207.g0ac5344