* [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls @ 2016-03-08 3:34 David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs David Decotigny ` (10 more replies) 0 siblings, 11 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 02/11] ethtool.c: don't ignore fread() return value David Decotigny ` (9 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, Maciej Żenczykowski, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 02/11] ethtool.c: don't ignore fread() return value 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 03/11] ethtool.c: fix dump_regs heap corruption David Decotigny ` (8 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 03/11] ethtool.c: fix dump_regs heap corruption 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 02/11] ethtool.c: don't ignore fread() return value David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 04/11] ethtool.c: do_seeprom checks for params & stdin sanity David Decotigny ` (7 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 04/11] ethtool.c: do_seeprom checks for params & stdin sanity 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (2 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 03/11] ethtool.c: fix dump_regs heap corruption David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings David Decotigny ` (6 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (3 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 04/11] ethtool.c: do_seeprom checks for params & stdin sanity David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 06/11] test-common.c: fix test_realloc(NULL, ...) David Decotigny ` (5 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, Maciej Żenczykowski, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 06/11] test-common.c: fix test_realloc(NULL, ...) 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (4 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 07/11] test-features.c: add braces around array initialization David Decotigny ` (4 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, Maciej Żenczykowski, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 07/11] test-features.c: add braces around array initialization 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (5 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 06/11] test-common.c: fix test_realloc(NULL, ...) David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 08/11] kernel-copy.h: import kernel.h from net-next and use it David Decotigny ` (3 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, Maciej Żenczykowski, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 08/11] kernel-copy.h: import kernel.h from net-next and use it 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (6 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 07/11] test-features.c: add braces around array initialization David Decotigny @ 2016-03-08 3:34 ` David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 09/11] ethtool-copy.h: sync with net-next David Decotigny ` (2 subsequent siblings) 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:34 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 09/11] ethtool-copy.h: sync with net-next 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (7 preceding siblings ...) 2016-03-08 3:34 ` [ethtool PATCH v4 08/11] kernel-copy.h: import kernel.h from net-next and use it David Decotigny @ 2016-03-08 3:35 ` David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets David Decotigny 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:35 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (8 preceding siblings ...) 2016-03-08 3:35 ` [ethtool PATCH v4 09/11] ethtool-copy.h: sync with net-next David Decotigny @ 2016-03-08 3:35 ` David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets David Decotigny 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:35 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny ` (9 preceding siblings ...) 2016-03-08 3:35 ` [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls David Decotigny @ 2016-03-08 3:35 ` David Decotigny 10 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-08 3:35 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls @ 2016-03-11 17:58 David Decotigny 2016-03-11 17:58 ` [ethtool PATCH v4 10/11] ethtool.c: add support for " David Decotigny 0 siblings, 1 reply; 17+ messages in thread From: David Decotigny @ 2016-03-11 17:58 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-11 17:58 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny @ 2016-03-11 17:58 ` David Decotigny 2016-03-14 1:32 ` Ben Hutchings 0 siblings, 1 reply; 17+ messages in thread From: David Decotigny @ 2016-03-11 17:58 UTC (permalink / raw) To: netdev Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati, Joe Perches, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-11 17:58 ` [ethtool PATCH v4 10/11] ethtool.c: add support for " David Decotigny @ 2016-03-14 1:32 ` Ben Hutchings 2016-03-14 16:43 ` David Laight 2016-03-15 23:42 ` David Decotigny 0 siblings, 2 replies; 17+ messages in thread From: Ben Hutchings @ 2016-03-14 1:32 UTC (permalink / raw) To: David Decotigny, netdev Cc: Jeff Garzik, David Miller, Vidya Sagar Ravipati, Joe Perches, David Decotigny [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-14 1:32 ` Ben Hutchings @ 2016-03-14 16:43 ` David Laight 2016-03-15 23:42 ` David Decotigny 1 sibling, 0 replies; 17+ messages in thread From: David Laight @ 2016-03-14 16:43 UTC (permalink / raw) To: 'Ben Hutchings', David Decotigny, netdev@vger.kernel.org Cc: Jeff Garzik, David Miller, Vidya Sagar Ravipati, Joe Perches, David Decotigny > > + /* 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-14 1:32 ` Ben Hutchings 2016-03-14 16:43 ` David Laight @ 2016-03-15 23:42 ` David Decotigny 2016-03-15 23:47 ` David Decotigny 1 sibling, 1 reply; 17+ messages in thread From: David Decotigny @ 2016-03-15 23:42 UTC (permalink / raw) To: Ben Hutchings Cc: netdev@vger.kernel.org, Jeff Garzik, David Miller, Vidya Sagar Ravipati, Joe Perches 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls 2016-03-15 23:42 ` David Decotigny @ 2016-03-15 23:47 ` David Decotigny 0 siblings, 0 replies; 17+ messages in thread From: David Decotigny @ 2016-03-15 23:47 UTC (permalink / raw) To: Ben Hutchings Cc: netdev@vger.kernel.org, Jeff Garzik, David Miller, Vidya Sagar Ravipati, Joe Perches 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-03-15 23:48 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-08 3:34 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 01/11] internal.h: change to new sane kernel headers on 64-bit archs David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 02/11] ethtool.c: don't ignore fread() return value David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 03/11] ethtool.c: fix dump_regs heap corruption David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 04/11] ethtool.c: do_seeprom checks for params & stdin sanity David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 05/11] marvell.c: fix strict alias warnings David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 06/11] test-common.c: fix test_realloc(NULL, ...) David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 07/11] test-features.c: add braces around array initialization David Decotigny 2016-03-08 3:34 ` [ethtool PATCH v4 08/11] kernel-copy.h: import kernel.h from net-next and use it David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 09/11] ethtool-copy.h: sync with net-next David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 10/11] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-08 3:35 ` [ethtool PATCH v4 11/11] ethtool.c: support absence of v4 sockets David Decotigny -- strict thread matches above, loose matches on Subject: below -- 2016-03-11 17:58 [ethtool PATCH v4 00/11] add support for new ETHTOOL_xLINKSETTINGS ioctls David Decotigny 2016-03-11 17:58 ` [ethtool PATCH v4 10/11] ethtool.c: add support for " David Decotigny 2016-03-14 1:32 ` Ben Hutchings 2016-03-14 16:43 ` David Laight 2016-03-15 23:42 ` David Decotigny 2016-03-15 23:47 ` David Decotigny
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).