[PATCH ethtool v2 00/13] ethtool: clean up and fix

[PATCH ethtool v2 00/13] ethtool: clean up and fix

[Jesse Brandeburg]

This series is an update to the ethtool application code, and was
triggered by running some static analysis tools and doing some general
refactor on the ethtool code to make it clearer.

The tools run were:
$ scan-build make
$ cppcheck
$ make CFLAGS+='-fsanitize=address,undefined' LDFLAGS+='-lubsan -lasan'

The big change in this series is a refactor of all the various bit
shifts from (1 << foo) to BIT(foo).  The goal was to make the code more
readable and maintainable, while fixing a few small bugs and hopefully
preventing more in the future from people forgetting to add 1UL to a 1
being used in a bitshift.

It includes a uapi sync/update to match a patch that was sent to the kernel
separately in the following link:
Link: https://lore.kernel.org/netdev/20221207231728.2331166-1-jesse.brandeburg@intel.com/

v2: first external version, updated commit message for 7/13
v1: internal version

Jesse Brandeburg (13):
  ethtool: convert boilerplate licenses to SPDX
  ethtool: fix trivial issue in allocation
  ethtool: disallow passing null to find_option
  ethtool: commonize power related strings
  ethtool: fix extra warnings
  ethtool: fix uninitialized local variable use
  ethtool: avoid null pointer dereference
  ethtool: fix runtime errors found by sanitizers
  ethtool: merge uapi changes to implement BIT and friends
  ethtool: refactor bit shifts to use BIT and BIT_ULL
  ethtool: fix missing free of memory after failure
  ethtool: fix leak of memory after realloc
  ethtool: fix bug and use standard string parsing

 amd8111e.c                   | 198 ++++++++---------
 cmis.c                       |  10 +-
 de2104x.c                    | 410 +++++++++++++++++------------------
 ethtool.c                    |  38 ++--
 fsl_enetc.c                  |   2 -
 internal.h                   |  26 +--
 json_print.c                 |   6 +-
 json_print.h                 |   6 +-
 natsemi.c                    | 358 +++++++++++++++---------------
 netlink/bitset.c             |   6 +-
 netlink/features.c           |   4 +-
 netlink/monitor.c            |   4 +-
 netlink/msgbuff.c            |  39 ++--
 netlink/parser.c             |  13 +-
 netlink/permaddr.c           |   2 +-
 netlink/settings.c           |  10 +-
 netlink/stats.c              |   2 +-
 qsfp.c                       |  21 +-
 qsfp.h                       | 353 +++++++++++++++---------------
 realtek.c                    |  48 ++--
 rxclass.c                    |   4 +-
 sfc.c                        |   7 +-
 sff-common.c                 |   6 +-
 sff-common.h                 |   9 +-
 sfpdiag.c                    |  67 +++---
 sfpid.c                      | 151 +++++++------
 stmmac.c                     |   5 +-
 test-cmdline.c               |   5 +-
 test-common.c                |  14 +-
 test-features.c              |   5 +-
 tse.c                        |   7 +-
 uapi/linux/ethtool.h         | 112 ++++++----
 uapi/linux/ethtool_netlink.h |   6 +-
 33 files changed, 961 insertions(+), 993 deletions(-)


base-commit: 3acf7eee7ade666289f98311befe334bb57d3765
-- 
2.31.1

[PATCH ethtool v2 01/13] ethtool: convert boilerplate licenses to SPDX

[Jesse Brandeburg]

Used scancode (ScanCode-Toolkit) to find some licenses that have
old boilerplate style.

In the interests of enabling better automated code License scanning,
convert these to SPDX as the Linux kernel source has done.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 json_print.c    | 6 +-----
 json_print.h    | 6 +-----
 qsfp.c          | 6 +-----
 qsfp.h          | 7 +------
 sfc.c           | 5 +----
 sff-common.c    | 6 +-----
 sff-common.h    | 6 +-----
 sfpid.c         | 5 +----
 stmmac.c        | 5 +----
 test-cmdline.c  | 5 +----
 test-common.c   | 5 +----
 test-features.c | 5 +----
 tse.c           | 5 +----
 13 files changed, 13 insertions(+), 59 deletions(-)

diff --git a/json_print.c b/json_print.c
index 4f62767bdbc9..ac19765d53b3 100644
--- a/json_print.c
+++ b/json_print.c
@@ -1,11 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * json_print.c		"print regular or json output, based on json_writer".
  *
- *             This program is free software; you can redistribute it and/or
- *             modify it under the terms of the GNU General Public License
- *             as published by the Free Software Foundation; either version
- *             2 of the License, or (at your option) any later version.
- *
  * Authors:    Julien Fortin, <julien@cumulusnetworks.com>
  */
 
diff --git a/json_print.h b/json_print.h
index df15314bafe2..18e9beb251fe 100644
--- a/json_print.h
+++ b/json_print.h
@@ -1,11 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * json_print.h		"print regular or json output, based on json_writer".
  *
- *             This program is free software; you can redistribute it and/or
- *             modify it under the terms of the GNU General Public License
- *             as published by the Free Software Foundation; either version
- *             2 of the License, or (at your option) any later version.
- *
  * Authors:    Julien Fortin, <julien@cumulusnetworks.com>
  */
 
diff --git a/qsfp.c b/qsfp.c
index 1fe5de1a863f..fb94202757d3 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * qsfp.c: Implements SFF-8636 based QSFP+/QSFP28 Diagnostics Memory map.
  *
@@ -5,11 +6,6 @@
  * Aurelien Guillaume <aurelien@iwi.me> (C) 2012
  * Copyright (C) 2014 Cumulus networks Inc.
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Freeoftware Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
  *  Vidya Ravipati <vidya@cumulusnetworks.com>
  *   This implementation is loosely based on current SFP parser
  *   and SFF-8636 spec Rev 2.7 (ftp://ftp.seagate.com/pub/sff/SFF-8636.PDF)
diff --git a/qsfp.h b/qsfp.h
index aabf09fdc623..7960bf26fb07 100644
--- a/qsfp.h
+++ b/qsfp.h
@@ -1,13 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * SFF 8636 standards based QSFP EEPROM Field Definitions
  *
  * Vidya Ravipati <vidya@cumulusnetworks.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
  */
 
 #ifndef QSFP_H__
diff --git a/sfc.c b/sfc.c
index 340800ee0fa0..a33077b4f263 100644
--- a/sfc.c
+++ b/sfc.c
@@ -1,10 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Support for Solarflare Solarstorm network controllers and boards
  * Copyright 2010-2012 Solarflare Communications Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <stdio.h>
diff --git a/sff-common.c b/sff-common.c
index e951cf15c1d6..94dc0643d3ec 100644
--- a/sff-common.c
+++ b/sff-common.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * sff-common.c: Implements SFF-8024 Rev 4.0 i.e. Specifcation
  * of pluggable I/O configuration
@@ -9,11 +10,6 @@
  * Aurelien Guillaume <aurelien@iwi.me> (C) 2012
  * Copyright (C) 2014 Cumulus networks Inc.
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Freeoftware Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
  *  Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
  *   This implementation is loosely based on current SFP parser
  *   and SFF-8024 Rev 4.0 spec (ftp://ftp.seagate.com/pub/sff/SFF-8024.PDF)
diff --git a/sff-common.h b/sff-common.h
index dd12dda7bbce..2f58f91ab7ff 100644
--- a/sff-common.h
+++ b/sff-common.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * sff-common.h: Implements SFF-8024 Rev 4.0 i.e. Specifcation
  * of pluggable I/O configuration
@@ -9,11 +10,6 @@
  * Aurelien Guillaume <aurelien@iwi.me> (C) 2012
  * Copyright (C) 2014 Cumulus networks Inc.
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Freeoftware Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
  *  Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
  *   This implementation is loosely based on current SFP parser
  *   and SFF-8024 specs (ftp://ftp.seagate.com/pub/sff/SFF-8024.PDF)
diff --git a/sfpid.c b/sfpid.c
index 1bc45c183770..b701e292518d 100644
--- a/sfpid.c
+++ b/sfpid.c
@@ -1,10 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Support for Solarflare Solarstorm network controllers and boards
  * Copyright 2010 Solarflare Communications Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <stdio.h>
diff --git a/stmmac.c b/stmmac.c
index 58471200cd80..772d4470a61e 100644
--- a/stmmac.c
+++ b/stmmac.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Support for the Synopsys MAC 10/100/1000 on-chip Ethernet controllers
  *
  * Copyright (C) 2007-2009  STMicroelectronics Ltd
  *
  * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <stdio.h>
diff --git a/test-cmdline.c b/test-cmdline.c
index cb803ed1a93d..a708f645d748 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -1,10 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Test cases for ethtool command-line parsing
  * Copyright 2011 Solarflare Communications Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <stdio.h>
diff --git a/test-common.c b/test-common.c
index 1dab0ce9dd10..e4dac3298577 100644
--- a/test-common.c
+++ b/test-common.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Common test functions for ethtool
  * Copyright 2011 Solarflare Communications Inc.
  *
  * Partly derived from kernel <linux/list.h>.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <assert.h>
diff --git a/test-features.c b/test-features.c
index b9f80f073d1f..a1f7c8a58569 100644
--- a/test-features.c
+++ b/test-features.c
@@ -1,10 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Test cases for ethtool features
  * Copyright 2012 Solarflare Communications Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <errno.h>
diff --git a/tse.c b/tse.c
index fb00d218ab8a..8fd2d2304ea8 100644
--- a/tse.c
+++ b/tse.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /****************************************************************************
  * Support for the Altera Triple Speed Ethernet 10/100/1000 Controller
  *
  * Copyright (C) 2014 Altera Corporation
  *
  * Author: Vince Bridgers <vbridgers2013@gmail.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation, incorporated herein by reference.
  */
 
 #include <stdio.h>
-- 
2.31.1

Re: [PATCH ethtool v2 01/13] ethtool: convert boilerplate licenses to SPDX

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:10PM -0800, Jesse Brandeburg wrote:
> Used scancode (ScanCode-Toolkit) to find some licenses that have
> old boilerplate style.
> 
> In the interests of enabling better automated code License scanning,
> convert these to SPDX as the Linux kernel source has done.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Acked-by: Michal Kubecek <mkubecek@suse.cz>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 02/13] ethtool: fix trivial issue in allocation

[Jesse Brandeburg]

Fix the following warning by changing the type being multiplied by to
the type being assigned to.

Description: Result of 'calloc' is converted to a pointer of type
'unsigned long', which is incompatible with sizeof operand type 'long'
File: /home/jbrandeb/git/ethtool/rxclass.c
Line: 527

Fixes: 5a3279e43f2b ("rxclass: Replace global rmgr with automatic variable/parameter")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 rxclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rxclass.c b/rxclass.c
index 6cf81fdafc85..ebdd97960e5b 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -524,7 +524,7 @@ static int rmgr_init(struct cmd_context *ctx, struct rmgr_ctrl *rmgr)
 	}
 
 	/* initialize bitmap for storage of valid locations */
-	rmgr->slot = calloc(1, BITS_TO_LONGS(rmgr->size) * sizeof(long));
+	rmgr->slot = calloc(1, BITS_TO_LONGS(rmgr->size) * sizeof(unsigned long));
 	if (!rmgr->slot) {
 		perror("rmgr: Cannot allocate memory for RX class rules");
 		return -1;
-- 
2.31.1

Re: [PATCH ethtool v2 02/13] ethtool: fix trivial issue in allocation

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:11PM -0800, Jesse Brandeburg wrote:
> Fix the following warning by changing the type being multiplied by to
> the type being assigned to.
> 
> Description: Result of 'calloc' is converted to a pointer of type
> 'unsigned long', which is incompatible with sizeof operand type 'long'
> File: /home/jbrandeb/git/ethtool/rxclass.c
> Line: 527
> 
> Fixes: 5a3279e43f2b ("rxclass: Replace global rmgr with automatic variable/parameter")
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  rxclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rxclass.c b/rxclass.c
> index 6cf81fdafc85..ebdd97960e5b 100644
> --- a/rxclass.c
> +++ b/rxclass.c
> @@ -524,7 +524,7 @@ static int rmgr_init(struct cmd_context *ctx, struct rmgr_ctrl *rmgr)
>  	}
>  
>  	/* initialize bitmap for storage of valid locations */
> -	rmgr->slot = calloc(1, BITS_TO_LONGS(rmgr->size) * sizeof(long));
> +	rmgr->slot = calloc(1, BITS_TO_LONGS(rmgr->size) * sizeof(unsigned long));

While at it, maybe we should take the cleanup one step further and use
sizeof(*rmgr->slot) or sizeof(rmgr->slot[0]) instead. And perhaps it
would also make sense to follow the logic of calloc() arguments and use

	calloc(BITS_TO_LONGS(rmgr->size), sizeof(rmgr->slot[0]))

Michal


>  	if (!rmgr->slot) {
>  		perror("rmgr: Cannot allocate memory for RX class rules");
>  		return -1;
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 03/13] ethtool: disallow passing null to find_option

[Jesse Brandeburg]

After testing with this code in the debugger, it is technically possible
to pass a NULL argument to ethtool which then prods it to call strncmp
with a NULL value, which triggers this warning:

Description: Null pointer passed to 1st parameter expecting 'nonnull'
File: /git/ethtool/ethtool.c
Line: 6129

Since segfaults are bad, let's just add a check for NULL when parsing
the initial arguments. The other cases of a NULL option are seemingly
handled.

Fixes: 127f80691f96 ("Move argument parsing to sub-command functions")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ethtool.c b/ethtool.c
index 3207e49137c4..a72577b32601 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6389,7 +6389,7 @@ int main(int argc, char **argp)
 	 * name to get settings for (which we don't expect to begin
 	 * with '-').
 	 */
-	if (argc == 0)
+	if (argc == 0 || *argp == NULL)
 		exit_bad_args();
 
 	k = find_option(*argp);
-- 
2.31.1

Re: [PATCH ethtool v2 03/13] ethtool: disallow passing null to find_option

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:12PM -0800, Jesse Brandeburg wrote:
> After testing with this code in the debugger, it is technically possible
> to pass a NULL argument to ethtool which then prods it to call strncmp
> with a NULL value, which triggers this warning:
> 
> Description: Null pointer passed to 1st parameter expecting 'nonnull'
> File: /git/ethtool/ethtool.c
> Line: 6129
> 
> Since segfaults are bad, let's just add a check for NULL when parsing
> the initial arguments. The other cases of a NULL option are seemingly
> handled.
> 
> Fixes: 127f80691f96 ("Move argument parsing to sub-command functions")
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Learning about (kernel) mainline commit dcd46d897adb ("exec: Force
single empty string when argv is empty"), I'm not opposed to mistrusting
kernel provided argc and argv[] but rather than hunting all places where
we might possibly hit a null pointer in argv[] (mind that we may iterate
over argv[] up to three times), I would simply check the whole array at
the beginning of main() with something like

	k = 0;
	while (k <= argc && argv[k])
		k++;
	if (k != argc) {
		fprintf(stderr, "ethtool: inconsistent command line (OS bug?)\n");
		return 1;
	}

and be done with it.

Michal

> ---
>  ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 3207e49137c4..a72577b32601 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6389,7 +6389,7 @@ int main(int argc, char **argp)
>  	 * name to get settings for (which we don't expect to begin
>  	 * with '-').
>  	 */
> -	if (argc == 0)
> +	if (argc == 0 || *argp == NULL)
>  		exit_bad_args();
>  
>  	k = find_option(*argp);
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 04/13] ethtool: commonize power related strings

[Jesse Brandeburg]

When looking into the implementation of the qsfp.h file, I found three
pieces of code all doing the same thing and using similar, but bespoke
strings.

Just make one set of strings for all three places to use. I made an
effort to see if there was any size change due to making this change but
I see no difference.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 cmis.c       | 10 ++--------
 qsfp.c       | 15 +++++++--------
 sff-common.h |  3 +++
 sfpdiag.c    |  9 ++-------
 4 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/cmis.c b/cmis.c
index d0b62728e998..40ff5e541737 100644
--- a/cmis.c
+++ b/cmis.c
@@ -727,16 +727,10 @@ cmis_show_dom_chan_lvl_rx_power_bank(const struct cmis_memory_map *map,
 
 	for (i = 0; i < CMIS_CHANNELS_PER_BANK; i++) {
 		int chan = bank * CMIS_CHANNELS_PER_BANK + i;
-		char *rx_power_str;
 		char fmt_str[80];
 
-		if (!sd->rx_power_type)
-			rx_power_str = "Receiver signal OMA";
-		else
-			rx_power_str = "Rcvr signal avg optical power";
-
-		snprintf(fmt_str, 80, "%s (Channel %d)", rx_power_str,
-			 chan + 1);
+		snprintf(fmt_str, 80, "%s (Channel %d)", sd->rx_power_type ?
+			 rx_power_average : rx_power_oma, chan + 1);
 		PRINT_xX_PWR(fmt_str, sd->scd[chan].rx_power);
 	}
 }
diff --git a/qsfp.c b/qsfp.c
index fb94202757d3..a79da29de950 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -798,7 +798,6 @@ out:
 static void sff8636_show_dom(const struct sff8636_memory_map *map)
 {
 	struct sff_diags sd = {0};
-	char *rx_power_string = NULL;
 	char power_string[MAX_DESC_SIZE];
 	int i;
 
@@ -846,14 +845,14 @@ static void sff8636_show_dom(const struct sff8636_memory_map *map)
 		PRINT_xX_PWR(power_string, sd.scd[i].tx_power);
 	}
 
-	if (!sd.rx_power_type)
-		rx_power_string = "Receiver signal OMA";
-	else
-		rx_power_string = "Rcvr signal avg optical power";
-
 	for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
-		snprintf(power_string, MAX_DESC_SIZE, "%s(Channel %d)",
-					rx_power_string, i+1);
+		int chars;
+
+		chars = snprintf(power_string, MAX_DESC_SIZE,
+				 sd.rx_power_type ?
+				 rx_power_average : rx_power_oma);
+		snprintf(power_string + chars, MAX_DESC_SIZE - chars,
+			 "(Channel %d)", i + 1);
 		PRINT_xX_PWR(power_string, sd.scd[i].rx_power);
 	}
 
diff --git a/sff-common.h b/sff-common.h
index 2f58f91ab7ff..4fc78cf9ee50 100644
--- a/sff-common.h
+++ b/sff-common.h
@@ -188,6 +188,9 @@ struct sff_diags {
 	struct sff_channel_diags scd[MAX_CHANNEL_NUM];
 };
 
+static const char rx_power_oma[] = "Receiver Signal OMA";
+static const char rx_power_average[] = "Receiver Signal average optical power";
+
 double convert_mw_to_dbm(double mw);
 void sff_show_value_with_unit(const __u8 *id, unsigned int reg,
 			      const char *name, unsigned int mult,
diff --git a/sfpdiag.c b/sfpdiag.c
index 1fa8b7ba8fec..502b6e3bf11e 100644
--- a/sfpdiag.c
+++ b/sfpdiag.c
@@ -242,7 +242,6 @@ static void sff8472_parse_eeprom(const __u8 *id, struct sff_diags *sd)
 void sff8472_show_all(const __u8 *id)
 {
 	struct sff_diags sd = {0};
-	char *rx_power_string = NULL;
 	int i;
 
 	sff8472_parse_eeprom(id, &sd);
@@ -256,12 +255,8 @@ void sff8472_show_all(const __u8 *id)
 	PRINT_BIAS("Laser bias current", sd.bias_cur[MCURR]);
 	PRINT_xX_PWR("Laser output power", sd.tx_power[MCURR]);
 
-	if (!sd.rx_power_type)
-		rx_power_string = "Receiver signal OMA";
-	else
-		rx_power_string = "Receiver signal average optical power";
-
-	PRINT_xX_PWR(rx_power_string, sd.rx_power[MCURR]);
+	PRINT_xX_PWR(sd.rx_power_type ? rx_power_average : rx_power_oma,
+		     sd.rx_power[MCURR]);
 
 	PRINT_TEMP("Module temperature", sd.sfp_temp[MCURR]);
 	PRINT_VCC("Module voltage", sd.sfp_voltage[MCURR]);
-- 
2.31.1

Re: [PATCH ethtool v2 04/13] ethtool: commonize power related strings

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:13PM -0800, Jesse Brandeburg wrote:
> When looking into the implementation of the qsfp.h file, I found three
> pieces of code all doing the same thing and using similar, but bespoke
> strings.
> 
> Just make one set of strings for all three places to use. I made an
> effort to see if there was any size change due to making this change but
> I see no difference.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Acked-by: Michal Kubecek <mkubecek@suse.cz>

Perhaps you might go one step further and replace the whole repeating

  sd->rx_power_type ? rx_power_average : rx_power_oma

pattern with an inline helper.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 05/13] ethtool: fix extra warnings

[Jesse Brandeburg]

'$ scan-build make' reports

netlink/permaddr.c:44:2: warning: Value stored to 'ifinfo' is never read [deadcode.DeadStores]
        ifinfo = mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));
        ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So just remove the extra assignment which is never used.

Fixes: 7f3585b22a4b ("netlink: add handler for permaddr (-P)")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 netlink/permaddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/netlink/permaddr.c b/netlink/permaddr.c
index 006eac6c0094..dccb0c6cfdb7 100644
--- a/netlink/permaddr.c
+++ b/netlink/permaddr.c
@@ -41,7 +41,7 @@ static int permaddr_prep_request(struct nl_socket *nlsk)
 	nlhdr->nlmsg_type = RTM_GETLINK;
 	nlhdr->nlmsg_flags = nlm_flags;
 	msgbuff->nlhdr = nlhdr;
-	ifinfo = mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));
+	mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));
 
 	if (devname) {
 		uint16_t type = IFLA_IFNAME;
-- 
2.31.1

Re: [PATCH ethtool v2 05/13] ethtool: fix extra warnings

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:14PM -0800, Jesse Brandeburg wrote:
> '$ scan-build make' reports
> 
> netlink/permaddr.c:44:2: warning: Value stored to 'ifinfo' is never read [deadcode.DeadStores]
>         ifinfo = mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));
>         ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> So just remove the extra assignment which is never used.
> 
> Fixes: 7f3585b22a4b ("netlink: add handler for permaddr (-P)")
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  netlink/permaddr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/netlink/permaddr.c b/netlink/permaddr.c
> index 006eac6c0094..dccb0c6cfdb7 100644
> --- a/netlink/permaddr.c
> +++ b/netlink/permaddr.c
> @@ -41,7 +41,7 @@ static int permaddr_prep_request(struct nl_socket *nlsk)
>  	nlhdr->nlmsg_type = RTM_GETLINK;
>  	nlhdr->nlmsg_flags = nlm_flags;
>  	msgbuff->nlhdr = nlhdr;
> -	ifinfo = mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));
> +	mnl_nlmsg_put_extra_header(nlhdr, sizeof(*ifinfo));

Good point. But looking at the code, the variable isn't in fact used
at all, except of the two occurences of sizeof(*ifinfo). So we can
just as well replace them with sizeof(struct ifinfomsg) and drop the
variable.

Michal

>  
>  	if (devname) {
>  		uint16_t type = IFLA_IFNAME;
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 06/13] ethtool: fix uninitialized local variable use

[Jesse Brandeburg]

'$ scan-build make' reports:

netlink/parser.c:252:25: warning: The left operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult]
        cm = (uint32_t)(meters * 100 + 0.5);
                        ~~~~~~ ^

This is a little more complicated than it seems, but for some
unexplained reason, parse_float always returns integers but was declared
to return a float. This is confusing at best. In the case of the error
above, parse_float could conceivably return without initializing it's
output variable, and because the function return variable was declared
as float but downgraded to an int via assignment (-Wconversion anyone?)
upon the return, the return value could be ignored.

To fix the bug above, declare an initial value for meters, and make sure
that parse_float() always returns an integer value.

It would probably be even more ideal if parse_float always initialized
it's output variables before even checking for input errors, but that's
for some future day.

CC: Andrew Lunn <andrew@lunn.ch>
Fixes: 9561db9b76f4 ("Add cable test TDR support")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 netlink/parser.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/netlink/parser.c b/netlink/parser.c
index f982f229a040..70f451008eb4 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -54,8 +54,7 @@ static bool __prefix_0x(const char *p)
 	return p[0] == '0' && (p[1] == 'x' || p[1] == 'X');
 }
 
-static float parse_float(const char *arg, float *result, float min,
-			 float max)
+static int parse_float(const char *arg, float *result, float min, float max)
 {
 	char *endptr;
 	float val;
@@ -237,7 +236,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
 			 struct nl_msg_buff *msgbuff, void *dest)
 {
 	const char *arg = *nlctx->argp;
-	float meters;
+	float meters = 0;
 	uint32_t cm;
 	int ret;
 
-- 
2.31.1

Re: [PATCH ethtool v2 06/13] ethtool: fix uninitialized local variable use

[Andrew Lunn]

On Wed, Dec 07, 2022 at 05:11:15PM -0800, Jesse Brandeburg wrote:
> '$ scan-build make' reports:
> 
> netlink/parser.c:252:25: warning: The left operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult]
>         cm = (uint32_t)(meters * 100 + 0.5);
>                         ~~~~~~ ^
> 
> This is a little more complicated than it seems, but for some
> unexplained reason, parse_float always returns integers but was declared
> to return a float.

Looks like i got that wrong. Duh!

> This is confusing at best. In the case of the error
> above, parse_float could conceivably return without initializing it's
> output variable, and because the function return variable was declared
> as float but downgraded to an int via assignment (-Wconversion anyone?)
> upon the return, the return value could be ignored.
> 
> To fix the bug above, declare an initial value for meters, and make sure
> that parse_float() always returns an integer value.
> 
> It would probably be even more ideal if parse_float always initialized
> it's output variables before even checking for input errors, but that's
> for some future day.

It is also following the pattern of other parse_foo functions. None of
them set the result in case of errors.

> 
> CC: Andrew Lunn <andrew@lunn.ch>
> Fixes: 9561db9b76f4 ("Add cable test TDR support")
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  netlink/parser.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/netlink/parser.c b/netlink/parser.c
> index f982f229a040..70f451008eb4 100644
> --- a/netlink/parser.c
> +++ b/netlink/parser.c
> @@ -54,8 +54,7 @@ static bool __prefix_0x(const char *p)
>  	return p[0] == '0' && (p[1] == 'x' || p[1] == 'X');
>  }
>  
> -static float parse_float(const char *arg, float *result, float min,
> -			 float max)
> +static int parse_float(const char *arg, float *result, float min, float max)
>  {
>  	char *endptr;
>  	float val;
> @@ -237,7 +236,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
>  			 struct nl_msg_buff *msgbuff, void *dest)
>  {
>  	const char *arg = *nlctx->argp;
> -	float meters;
> +	float meters = 0;
>  	uint32_t cm;
>  	int ret;

Should this actually be 0.0? Otherwise if -Wconversion gets turned on,
you have an int being converted to a float?

Anyway:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH ethtool v2 07/13] ethtool: avoid null pointer dereference

[Jesse Brandeburg]

'$ scan-build make' reports:

Description: Array access (from variable 'arg') results in a null
pointer dereference
File: /git/ethtool/netlink/parser.c
Line: 782

Description: Dereference of null pointer (loaded from variable 'p')
File: /git/ethtool/netlink/parser.c
Line: 794

Both of these bugs are prevented by checking the input in
nl_parse_char_bitset(), which is called from nl_sset() via the kernel
callback, specifically for the parsing of the wake-on-lan options (-s
wol). None of the other functions in this file seem to have the issue of
deferencing data without checking for validity first. This could
"technically" allow nlctxt->argp to be NULL, and scan-build is limited
in it's ability to parse for bugs only at file scope in this case.
This particular bug should be unlikely to happen because the kernel
builds/parses the netlink structure before handing it to the
application. However in the interests of being able to run this
scan-build tool regularly, I'm still sending the initial version of this
patch as I tried several other ways to fix the bug with an earlier check
for NULL in nl_sset, but it won't prevent the scan-build error due to
the file scope problem.

CC: Michal Kubecek <mkubecek@suse.cz>
Fixes: 81a30f416ec7 ("netlink: add bitset command line parser handlers")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
v2: updated commit message with more nuance after feedback from ethtool
maintainer. I'd be open to fixing this a different way but this seemed
the most straight-forward with the smallest amount of code changed.
v1: original version
---
 netlink/parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/netlink/parser.c b/netlink/parser.c
index 70f451008eb4..c573a9598a9f 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -874,7 +874,7 @@ int nl_parse_bitset(struct nl_context *nlctx, uint16_t type, const void *data,
  * optionally followed by '/' and another numeric value (mask, unless no_mask
  * is set), or a string consisting of characters corresponding to bit indices.
  * The @data parameter points to struct char_bitset_parser_data. Generates
- * biset nested attribute. Fails if type is zero or if @dest is not null.
+ * bitset nested attribute. Fails if type is zero or if @dest is not null.
  */
 int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
 			 const void *data, struct nl_msg_buff *msgbuff,
@@ -882,7 +882,7 @@ int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
 {
 	const struct char_bitset_parser_data *parser_data = data;
 
-	if (!type || dest) {
+	if (!type || dest || !*nlctx->argp) {
 		fprintf(stderr, "ethtool (%s): internal error parsing '%s'\n",
 			nlctx->cmd, nlctx->param);
 		return -EFAULT;
-- 
2.31.1

Re: [PATCH ethtool v2 07/13] ethtool: avoid null pointer dereference

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:16PM -0800, Jesse Brandeburg wrote:
> '$ scan-build make' reports:
> 
> Description: Array access (from variable 'arg') results in a null
> pointer dereference
> File: /git/ethtool/netlink/parser.c
> Line: 782
> 
> Description: Dereference of null pointer (loaded from variable 'p')
> File: /git/ethtool/netlink/parser.c
> Line: 794
> 
> Both of these bugs are prevented by checking the input in
> nl_parse_char_bitset(), which is called from nl_sset() via the kernel
> callback, specifically for the parsing of the wake-on-lan options (-s
> wol). None of the other functions in this file seem to have the issue of
> deferencing data without checking for validity first. This could
> "technically" allow nlctxt->argp to be NULL, and scan-build is limited
> in it's ability to parse for bugs only at file scope in this case.
> This particular bug should be unlikely to happen because the kernel
> builds/parses the netlink structure before handing it to the

Again: this has nothing to do with netlink, this is command line parser,
nlctx->argp is a member of argv[] array. And as execve() (which is the
only syscall in the exec* family, the rest are wrappers) does not pass
argc, only argv[], argc is actually determined by kernel so for it to be
actually null, you would need a serious bug in kernel first.

Even if we want to be safe against buggy kernel passing garbage as
command line arguments, I still believe we should do that earlier, in
the general code, not deep in a specific helper function. Also, you only
check for null but that does not catch an invalid pointer in argv[]
which, unlike a null pointer, could do an actual harm. And I don't see
how that could be checked, I'm afraid we have to trust kernel.

Michal

> application. However in the interests of being able to run this
> scan-build tool regularly, I'm still sending the initial version of this
> patch as I tried several other ways to fix the bug with an earlier check
> for NULL in nl_sset, but it won't prevent the scan-build error due to
> the file scope problem.
> 
> CC: Michal Kubecek <mkubecek@suse.cz>
> Fixes: 81a30f416ec7 ("netlink: add bitset command line parser handlers")
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> v2: updated commit message with more nuance after feedback from ethtool
> maintainer. I'd be open to fixing this a different way but this seemed
> the most straight-forward with the smallest amount of code changed.
> v1: original version
> ---
>  netlink/parser.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/netlink/parser.c b/netlink/parser.c
> index 70f451008eb4..c573a9598a9f 100644
> --- a/netlink/parser.c
> +++ b/netlink/parser.c
> @@ -874,7 +874,7 @@ int nl_parse_bitset(struct nl_context *nlctx, uint16_t type, const void *data,
>   * optionally followed by '/' and another numeric value (mask, unless no_mask
>   * is set), or a string consisting of characters corresponding to bit indices.
>   * The @data parameter points to struct char_bitset_parser_data. Generates
> - * biset nested attribute. Fails if type is zero or if @dest is not null.
> + * bitset nested attribute. Fails if type is zero or if @dest is not null.
>   */
>  int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
>  			 const void *data, struct nl_msg_buff *msgbuff,
> @@ -882,7 +882,7 @@ int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
>  {
>  	const struct char_bitset_parser_data *parser_data = data;
>  
> -	if (!type || dest) {
> +	if (!type || dest || !*nlctx->argp) {
>  		fprintf(stderr, "ethtool (%s): internal error parsing '%s'\n",
>  			nlctx->cmd, nlctx->param);
>  		return -EFAULT;
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool v2 07/13] ethtool: avoid null pointer dereference

[Jesse Brandeburg]

On 12/7/2022 10:23 PM, Michal Kubecek wrote:
> On Wed, Dec 07, 2022 at 05:11:16PM -0800, Jesse Brandeburg wrote:
>> '$ scan-build make' reports:
>>
>> Description: Array access (from variable 'arg') results in a null
>> pointer dereference
>> File: /git/ethtool/netlink/parser.c
>> Line: 782
>>
>> Description: Dereference of null pointer (loaded from variable 'p')
>> File: /git/ethtool/netlink/parser.c
>> Line: 794
>>
>> Both of these bugs are prevented by checking the input in
>> nl_parse_char_bitset(), which is called from nl_sset() via the kernel
>> callback, specifically for the parsing of the wake-on-lan options (-s
>> wol). None of the other functions in this file seem to have the issue of
>> deferencing data without checking for validity first. This could
>> "technically" allow nlctxt->argp to be NULL, and scan-build is limited
>> in it's ability to parse for bugs only at file scope in this case.
>> This particular bug should be unlikely to happen because the kernel
>> builds/parses the netlink structure before handing it to the
> 
> Again: this has nothing to do with netlink, this is command line parser,
> nlctx->argp is a member of argv[] array. And as execve() (which is the
> only syscall in the exec* family, the rest are wrappers) does not pass
> argc, only argv[], argc is actually determined by kernel so for it to be
> actually null, you would need a serious bug in kernel first.

Thank you for explaining! I can drop this patch, but it's disappointing 
that one fairly cheap conditional will prevent us from being able to 
cleanly run scan-build. If you have any other suggestions please let me 
know (and see below)

I spent some time today trying to get the command line to pass a NULL 
value but I couldn't do it, and elsewhere in the code the checks for 
argc prevent the NULL value or no value from getting into the ethtool 
code parsing the commands, so in this case it's not really a false 
positive, but taken care of by other code that isn't observable to the 
scan-build virtual machine. The good news is I don't see a real issue here.

> Even if we want to be safe against buggy kernel passing garbage as
> command line arguments, I still believe we should do that earlier, in
> the general code, not deep in a specific helper function. Also, you only
> check for null but that does not catch an invalid pointer in argv[]
> which, unlike a null pointer, could do an actual harm. And I don't see
> how that could be checked, I'm afraid we have to trust kernel.

OK, let's trust the kernel, but can we still fix this issue in order to 
be able to add scan-build to a list of tools to run cleanly in automation?

some TL;DR details in case there is someone else that has a suggestion!

Here is the callchain, for reference:
This is from the command
# ethtool -s eth0 wol pumbag

#0  nl_parse_char_bitset
#1  in nl_parser at netlink/parser.c:1099
#2  in nl_sset at netlink/settings.c:1247
#3  in netlink_run_handler at netlink/netlink.c:493
#4  in main at ethtool.c:6425

and in the #0 frame above, *nlctx->argp = "pumbag"
in the callchain above, scan-build doesn't like us de-referencing argp 
because it doesn't have proof it's not null.

Further I tried putting the check in every element of the stack frame 
above and they all fail the scan-build check still, probably because the 
pointer is advanced to the "pumbag" argument later in the code.

Anyway, I'm still working on the v3 of the series.

Re: [PATCH ethtool v2 07/13] ethtool: avoid null pointer dereference

[Michal Kubecek]

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

On Fri, Dec 09, 2022 at 09:36:54AM -0800, Jesse Brandeburg wrote:
> Here is the callchain, for reference:
> This is from the command
> # ethtool -s eth0 wol pumbag
> 
> #0  nl_parse_char_bitset
> #1  in nl_parser at netlink/parser.c:1099
> #2  in nl_sset at netlink/settings.c:1247
> #3  in netlink_run_handler at netlink/netlink.c:493
> #4  in main at ethtool.c:6425
> 
> and in the #0 frame above, *nlctx->argp = "pumbag"
> in the callchain above, scan-build doesn't like us de-referencing argp
> because it doesn't have proof it's not null.
> 
> Further I tried putting the check in every element of the stack frame above
> and they all fail the scan-build check still, probably because the pointer
> is advanced to the "pumbag" argument later in the code.

This should be guaranteed by min_argc beeing set to 1 in the relevant
member of sset_params[] array in netlink/setings.c. The dispatcher code
in nl_parser() checks that there are at least ->min_argc parameters left
before calling specific ->handler(). So as long as argc and argv[] are
consistent, we should be safe.

But I suppose we cannot expect the static checker to be smart enough to
see through that logic.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 08/13] ethtool: fix runtime errors found by sanitizers

[Jesse Brandeburg]

The sanitizers[1] found a couple of things, but this change addresses
some bit shifts that cannot be contained by the target type.

The mistake is that the code is using unsigned int a = (1 << N) all over
the place, but the appropriate way to code this is unsigned int an
assignment of (1UL << N) especially if N can ever be 31.

Fix the most egregious of these problems by changing "1" to "1UL", as
per it would be if we had used the BIT() macro.

[1] make CFLAGS+='-fsanitize=address,undefined' \
         LDFLAGS+='-lubsan -lasan'

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 amd8111e.c         | 2 +-
 internal.h         | 4 ++--
 netlink/features.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/amd8111e.c b/amd8111e.c
index 175516bd2904..5a2fc2082e55 100644
--- a/amd8111e.c
+++ b/amd8111e.c
@@ -75,7 +75,7 @@ typedef enum {
 }CMD3_BITS;
 typedef enum {
 
-	INTR			= (1 << 31),
+	INTR			= (1UL << 31),
 	PCSINT			= (1 << 28),
 	LCINT			= (1 << 27),
 	APINT5			= (1 << 26),
diff --git a/internal.h b/internal.h
index dd7d6ac70ad4..6e79374bcfd5 100644
--- a/internal.h
+++ b/internal.h
@@ -205,14 +205,14 @@ 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)));
+	return !!(mask[nr / 32] & (1UL << (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));
+	mask[nr / 32] |= (1UL << (nr % 32));
 	return 0;
 }
 
diff --git a/netlink/features.c b/netlink/features.c
index a4dae8fac4dc..f6ba47f21a12 100644
--- a/netlink/features.c
+++ b/netlink/features.c
@@ -57,7 +57,7 @@ static int prepare_feature_results(const struct nlattr *const *tb,
 
 static bool feature_on(const uint32_t *bitmap, unsigned int idx)
 {
-	return bitmap[idx / 32] & (1 << (idx % 32));
+	return bitmap[idx / 32] & (1UL << (idx % 32));
 }
 
 static void dump_feature(const struct feature_results *results,
@@ -302,7 +302,7 @@ static void set_sf_req_mask(struct nl_context *nlctx, unsigned int idx)
 {
 	struct sfeatures_context *sfctx = nlctx->cmd_private;
 
-	sfctx->req_mask[idx / 32] |= (1 << (idx % 32));
+	sfctx->req_mask[idx / 32] |= (1UL << (idx % 32));
 }
 
 static int fill_legacy_flag(struct nl_context *nlctx, const char *flag_name,
-- 
2.31.1

Re: [PATCH ethtool v2 08/13] ethtool: fix runtime errors found by sanitizers

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:17PM -0800, Jesse Brandeburg wrote:
> The sanitizers[1] found a couple of things, but this change addresses
> some bit shifts that cannot be contained by the target type.
> 
> The mistake is that the code is using unsigned int a = (1 << N) all over
> the place, but the appropriate way to code this is unsigned int an
> assignment of (1UL << N) especially if N can ever be 31.
> 
> Fix the most egregious of these problems by changing "1" to "1UL", as
> per it would be if we had used the BIT() macro.
> 
> [1] make CFLAGS+='-fsanitize=address,undefined' \
>          LDFLAGS+='-lubsan -lasan'
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  amd8111e.c         | 2 +-
>  internal.h         | 4 ++--
>  netlink/features.c | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/amd8111e.c b/amd8111e.c
> index 175516bd2904..5a2fc2082e55 100644
> --- a/amd8111e.c
> +++ b/amd8111e.c
> @@ -75,7 +75,7 @@ typedef enum {
>  }CMD3_BITS;
>  typedef enum {
>  
> -	INTR			= (1 << 31),
> +	INTR			= (1UL << 31),
>  	PCSINT			= (1 << 28),
>  	LCINT			= (1 << 27),
>  	APINT5			= (1 << 26),

While the signedness issue only directly affects only INTR value,
I would rather prefer to keep the code consistent and fix the whole enum.
Also, as you intend to introduce the BIT() macro in the series anyway,
wouldn't it be cleaner to move this patch after the UAPI update and use
BIT() instead?

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool v2 08/13] ethtool: fix runtime errors found by sanitizers

[Jesse Brandeburg]

On 12/7/2022 10:34 PM, Michal Kubecek wrote:
> On Wed, Dec 07, 2022 at 05:11:17PM -0800, Jesse Brandeburg wrote:

>> -	INTR			= (1 << 31),
>> +	INTR			= (1UL << 31),
>>   	PCSINT			= (1 << 28),
>>   	LCINT			= (1 << 27),
>>   	APINT5			= (1 << 26),
> 
> While the signedness issue only directly affects only INTR value,
> I would rather prefer to keep the code consistent and fix the whole enum.
> Also, as you intend to introduce the BIT() macro in the series anyway,
> wouldn't it be cleaner to move this patch after the UAPI update and use
> BIT() instead?

I had done it this way to separate the "most minimal fix" from the 
"refactor", as I figure that is a clearer way to delineate changes. 
Also, this specifically addresses the issues found by the undefined 
behavior sanitizer.

I'll do it whichever way you like, but you're correct, later in this 
series I fix up all the BIT() usages. Maybe we can just leave this patch 
as is, knowing the full fix comes during the refactor in 10/13 ?

Re: [PATCH ethtool v2 08/13] ethtool: fix runtime errors found by sanitizers

[Michal Kubecek]

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

On Fri, Dec 09, 2022 at 09:42:59AM -0800, Jesse Brandeburg wrote:
> On 12/7/2022 10:34 PM, Michal Kubecek wrote:
> > On Wed, Dec 07, 2022 at 05:11:17PM -0800, Jesse Brandeburg wrote:
> 
> > > -	INTR			= (1 << 31),
> > > +	INTR			= (1UL << 31),
> > >   	PCSINT			= (1 << 28),
> > >   	LCINT			= (1 << 27),
> > >   	APINT5			= (1 << 26),
> > 
> > While the signedness issue only directly affects only INTR value,
> > I would rather prefer to keep the code consistent and fix the whole enum.
> > Also, as you intend to introduce the BIT() macro in the series anyway,
> > wouldn't it be cleaner to move this patch after the UAPI update and use
> > BIT() instead?
> 
> I had done it this way to separate the "most minimal fix" from the
> "refactor", as I figure that is a clearer way to delineate changes. Also,
> this specifically addresses the issues found by the undefined behavior
> sanitizer.
> 
> I'll do it whichever way you like, but you're correct, later in this series
> I fix up all the BIT() usages. Maybe we can just leave this patch as is,
> knowing the full fix comes during the refactor in 10/13 ?

As we end up with BIT() everywhere anyway, I'm OK with either option,
leaving this patch as it is or dropping it. When I was writing that
comment, I had seen 09/13 (introduction of BIT()) but not 10/13
(refactoring everything to use it).

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool v2 08/13] ethtool: fix runtime errors found by sanitizers

[Jesse Brandeburg]

On 12/9/2022 10:09 AM, Michal Kubecek wrote:
>> I'll do it whichever way you like, but you're correct, later in this series
>> I fix up all the BIT() usages. Maybe we can just leave this patch as is,
>> knowing the full fix comes during the refactor in 10/13 ?
> 
> As we end up with BIT() everywhere anyway, I'm OK with either option,
> leaving this patch as it is or dropping it. When I was writing that
> comment, I had seen 09/13 (introduction of BIT()) but not 10/13
> (refactoring everything to use it).

Ok, thanks. Also Jakub pointed out to me there is a UAPI compliant 
_BITUL()/_BITULL() function in include/uapi/linux/const.h, which I'll 
switch 9 and 10 to using. Wish that function had been a tad more 
discoverable.

[PATCH ethtool v2 09/13] ethtool: merge uapi changes to implement BIT and friends

[Jesse Brandeburg]

I was looking into some errors reported by the runtime sanitizers and
found a couple of places where (1 << 31) was being used. This is a shift
of a bit into the sign-bit of an integer. This is undefined behavior for
the C-specification, and can be easily fixed with using (1UL << 31)
instead. A better way to do this is to use the BIT() macro, which
already has the 1UL in it (see future patch in series).

Convert and sync with the same changes made upstream to the uapi file,
to implement ethtool use BIT() and friends.

This required an unfortunate bit of extra fussing around declaring "same
definition" versions of the BIT* macros so that the UAPI file will
compile both under the kernel and in user-space (here).

A local declaration of BIT() had to be moved out of fsl_enetc.c when
the implementation was moved to a header.

NOTE:
This change will be followed by a larger conversion patch, but *this*
commit only has the UAPI file changes and the initial implementation to
keep the work separate from the application only changes.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 fsl_enetc.c                  |   2 -
 uapi/linux/ethtool.h         | 112 +++++++++++++++++++++--------------
 uapi/linux/ethtool_netlink.h |   6 +-
 3 files changed, 70 insertions(+), 50 deletions(-)

diff --git a/fsl_enetc.c b/fsl_enetc.c
index 1180a664f777..9dbeae951bad 100644
--- a/fsl_enetc.c
+++ b/fsl_enetc.c
@@ -5,8 +5,6 @@
 #include <stdio.h>
 #include "internal.h"
 
-#define BIT(x)			(1U << (x))
-
 enum enetc_bdr_type {TX, RX};
 #define ENETC_SIMR		0
 #define ENETC_SIPMAR0		0x80
diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
index 944711cfa6f6..8773143e4737 100644
--- a/uapi/linux/ethtool.h
+++ b/uapi/linux/ethtool.h
@@ -9,6 +9,7 @@
  *                                christopher.leech@intel.com,
  *                                scott.feldman@intel.com)
  * Portions Copyright (C) Sun Microsystems 2008
+ * Portions Copyright (C) 2022 Intel Corporation
  */
 
 #ifndef _LINUX_ETHTOOL_H
@@ -20,6 +21,27 @@
 
 #include <limits.h> /* for INT_MAX */
 
+/* BIT() and BIT_ULL() are defined in include/linux/bits.h but we need a
+ * local version to clean up this file and not break simultaneous
+ * kernel/userspace where userspace doesn't have the BIT and BIT_ULL
+ * defined. To avoid compiler issues we use the exact same definitions here
+ * of the macros as defined in the file noted below, so that we don't get
+ * 'duplicate define' or 'redefinition' errors.
+ */
+/* include/uapi/linux/const.h */
+#define __AC(X,Y)	(X##Y)
+#define _AC(X,Y)	__AC(X,Y)
+#define _AT(T,X)	((T)(X))
+#define _UL(x)		(_AC(x, UL))
+#define _ULL(x)		(_AC(x, ULL))
+/* include/vdso/linux/const.h */
+#define UL(x)		(_UL(x))
+#define ULL(x)		(_ULL(x))
+/* include/vdso/bits.h */
+#define BIT(nr)		(UL(1) << (nr))
+/* include/linux/bits.h */
+#define BIT_ULL(nr)	(ULL(1) << (nr))
+
 /* All structures exposed to userland should be defined such that they
  * have the same layout for 32-bit and 64-bit userland.
  */
@@ -789,10 +811,10 @@ struct ethtool_sset_info {
  */
 
 enum ethtool_test_flags {
-	ETH_TEST_FL_OFFLINE	= (1 << 0),
-	ETH_TEST_FL_FAILED	= (1 << 1),
-	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
-	ETH_TEST_FL_EXTERNAL_LB_DONE	= (1 << 3),
+	ETH_TEST_FL_OFFLINE		= BIT(0),
+	ETH_TEST_FL_FAILED		= BIT(1),
+	ETH_TEST_FL_EXTERNAL_LB		= BIT(2),
+	ETH_TEST_FL_EXTERNAL_LB_DONE	= BIT(3),
 };
 
 /**
@@ -862,11 +884,11 @@ struct ethtool_perm_addr {
  * flag differs from the read-only value.
  */
 enum ethtool_flags {
-	ETH_FLAG_TXVLAN		= (1 << 7),	/* TX VLAN offload enabled */
-	ETH_FLAG_RXVLAN		= (1 << 8),	/* RX VLAN offload enabled */
-	ETH_FLAG_LRO		= (1 << 15),	/* LRO is enabled */
-	ETH_FLAG_NTUPLE		= (1 << 27),	/* N-tuple filters enabled */
-	ETH_FLAG_RXHASH		= (1 << 28),
+	ETH_FLAG_TXVLAN		= BIT(7),	/* TX VLAN offload enabled */
+	ETH_FLAG_RXVLAN		= BIT(8),	/* RX VLAN offload enabled */
+	ETH_FLAG_LRO		= BIT(15),	/* LRO is enabled */
+	ETH_FLAG_NTUPLE		= BIT(27),	/* N-tuple filters enabled */
+	ETH_FLAG_RXHASH		= BIT(28),
 };
 
 /* The following structures are for supporting RX network flow
@@ -1354,7 +1376,7 @@ struct ethtool_sfeatures {
  * The bits in the 'tx_types' and 'rx_filters' fields correspond to
  * 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.
+ * then BIT(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
@@ -1402,9 +1424,9 @@ enum ethtool_sfeatures_retval_bits {
 	ETHTOOL_F_COMPAT__BIT,
 };
 
-#define ETHTOOL_F_UNSUPPORTED   (1 << ETHTOOL_F_UNSUPPORTED__BIT)
-#define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
-#define ETHTOOL_F_COMPAT        (1 << ETHTOOL_F_COMPAT__BIT)
+#define ETHTOOL_F_UNSUPPORTED   BIT(ETHTOOL_F_UNSUPPORTED__BIT)
+#define ETHTOOL_F_WISH          BIT(ETHTOOL_F_WISH__BIT)
+#define ETHTOOL_F_COMPAT        BIT(ETHTOOL_F_COMPAT__BIT)
 
 #define MAX_NUM_QUEUE		4096
 
@@ -1481,12 +1503,12 @@ enum ethtool_fec_config_bits {
 	ETHTOOL_FEC_LLRS_BIT,
 };
 
-#define ETHTOOL_FEC_NONE		(1 << ETHTOOL_FEC_NONE_BIT)
-#define ETHTOOL_FEC_AUTO		(1 << ETHTOOL_FEC_AUTO_BIT)
-#define ETHTOOL_FEC_OFF			(1 << ETHTOOL_FEC_OFF_BIT)
-#define ETHTOOL_FEC_RS			(1 << ETHTOOL_FEC_RS_BIT)
-#define ETHTOOL_FEC_BASER		(1 << ETHTOOL_FEC_BASER_BIT)
-#define ETHTOOL_FEC_LLRS		(1 << ETHTOOL_FEC_LLRS_BIT)
+#define ETHTOOL_FEC_NONE		BIT(ETHTOOL_FEC_NONE_BIT)
+#define ETHTOOL_FEC_AUTO		BIT(ETHTOOL_FEC_AUTO_BIT)
+#define ETHTOOL_FEC_OFF			BIT(ETHTOOL_FEC_OFF_BIT)
+#define ETHTOOL_FEC_RS			BIT(ETHTOOL_FEC_RS_BIT)
+#define ETHTOOL_FEC_BASER		BIT(ETHTOOL_FEC_BASER_BIT)
+#define ETHTOOL_FEC_LLRS		BIT(ETHTOOL_FEC_LLRS_BIT)
 
 /* CMDs currently supported */
 #define ETHTOOL_GSET		0x00000001 /* DEPRECATED, Get settings.
@@ -1695,7 +1717,7 @@ enum ethtool_link_mode_bit_indices {
 };
 
 #define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name)	\
-	(1UL << (ETHTOOL_LINK_MODE_ ## base_name ## _BIT))
+	BIT_ULL((ETHTOOL_LINK_MODE_ ## base_name ## _BIT))
 
 /* DEPRECATED macros. Please migrate to
  * ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API. Please do NOT
@@ -1868,14 +1890,14 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define ETH_TP_MDI_AUTO		0x03 /*                  control: auto-select */
 
 /* Wake-On-Lan options. */
-#define WAKE_PHY		(1 << 0)
-#define WAKE_UCAST		(1 << 1)
-#define WAKE_MCAST		(1 << 2)
-#define WAKE_BCAST		(1 << 3)
-#define WAKE_ARP		(1 << 4)
-#define WAKE_MAGIC		(1 << 5)
-#define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
-#define WAKE_FILTER		(1 << 7)
+#define WAKE_PHY		BIT(0)
+#define WAKE_UCAST		BIT(1)
+#define WAKE_MCAST		BIT(2)
+#define WAKE_BCAST		BIT(3)
+#define WAKE_ARP		BIT(4)
+#define WAKE_MAGIC		BIT(5)
+#define WAKE_MAGICSECURE	BIT(6) /* only meaningful if WAKE_MAGIC */
+#define WAKE_FILTER		BIT(7)
 
 #define WOL_MODE_COUNT		8
 
@@ -1905,14 +1927,14 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define	FLOW_RSS	0x20000000
 
 /* L3-L4 network traffic flow hash options */
-#define	RXH_L2DA	(1 << 1)
-#define	RXH_VLAN	(1 << 2)
-#define	RXH_L3_PROTO	(1 << 3)
-#define	RXH_IP_SRC	(1 << 4)
-#define	RXH_IP_DST	(1 << 5)
-#define	RXH_L4_B_0_1	(1 << 6) /* src port in case of TCP/UDP/SCTP */
-#define	RXH_L4_B_2_3	(1 << 7) /* dst port in case of TCP/UDP/SCTP */
-#define	RXH_DISCARD	(1 << 31)
+#define	RXH_L2DA	BIT(1)
+#define	RXH_VLAN	BIT(2)
+#define	RXH_L3_PROTO	BIT(3)
+#define	RXH_IP_SRC	BIT(4)
+#define	RXH_IP_DST	BIT(5)
+#define	RXH_L4_B_0_1	BIT(6) /* src port in case of TCP/UDP/SCTP */
+#define	RXH_L4_B_2_3	BIT(7) /* dst port in case of TCP/UDP/SCTP */
+#define	RXH_DISCARD	BIT(31)
 
 #define	RX_CLS_FLOW_DISC	0xffffffffffffffffULL
 #define RX_CLS_FLOW_WAKE	0xfffffffffffffffeULL
@@ -1949,16 +1971,16 @@ enum ethtool_reset_flags {
 	 * ETH_RESET_SHARED_SHIFT to reset a shared component of the
 	 * same type.
 	 */
-	ETH_RESET_MGMT		= 1 << 0,	/* Management processor */
-	ETH_RESET_IRQ		= 1 << 1,	/* Interrupt requester */
-	ETH_RESET_DMA		= 1 << 2,	/* DMA engine */
-	ETH_RESET_FILTER	= 1 << 3,	/* Filtering/flow direction */
-	ETH_RESET_OFFLOAD	= 1 << 4,	/* Protocol offload */
-	ETH_RESET_MAC		= 1 << 5,	/* Media access controller */
-	ETH_RESET_PHY		= 1 << 6,	/* Transceiver/PHY */
-	ETH_RESET_RAM		= 1 << 7,	/* RAM shared between
+	ETH_RESET_MGMT		= BIT(0),	/* Management processor */
+	ETH_RESET_IRQ		= BIT(1),	/* Interrupt requester */
+	ETH_RESET_DMA		= BIT(2),	/* DMA engine */
+	ETH_RESET_FILTER	= BIT(3),	/* Filtering/flow direction */
+	ETH_RESET_OFFLOAD	= BIT(4),	/* Protocol offload */
+	ETH_RESET_MAC		= BIT(5),	/* Media access controller */
+	ETH_RESET_PHY		= BIT(6),	/* Transceiver/PHY */
+	ETH_RESET_RAM		= BIT(7),	/* RAM shared between
 						 * multiple components */
-	ETH_RESET_AP		= 1 << 8,	/* Application processor */
+	ETH_RESET_AP		= BIT(8),	/* Application processor */
 
 	ETH_RESET_DEDICATED	= 0x0000ffff,	/* All components dedicated to
 						 * this interface */
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index 378ad7da74f4..8b9814e2c704 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -103,11 +103,11 @@ enum {
 /* request header */
 
 /* use compact bitsets in reply */
-#define ETHTOOL_FLAG_COMPACT_BITSETS	(1 << 0)
+#define ETHTOOL_FLAG_COMPACT_BITSETS	BIT(0)
 /* provide optional reply for SET or ACT requests */
-#define ETHTOOL_FLAG_OMIT_REPLY	(1 << 1)
+#define ETHTOOL_FLAG_OMIT_REPLY		BIT(1)
 /* request statistics, if supported by the driver */
-#define ETHTOOL_FLAG_STATS		(1 << 2)
+#define ETHTOOL_FLAG_STATS		BIT(2)
 
 #define ETHTOOL_FLAG_ALL (ETHTOOL_FLAG_COMPACT_BITSETS | \
 			  ETHTOOL_FLAG_OMIT_REPLY | \
-- 
2.31.1

Re: [PATCH ethtool v2 09/13] ethtool: merge uapi changes to implement BIT and friends

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:18PM -0800, Jesse Brandeburg wrote:
> I was looking into some errors reported by the runtime sanitizers and
> found a couple of places where (1 << 31) was being used. This is a shift
> of a bit into the sign-bit of an integer. This is undefined behavior for
> the C-specification, and can be easily fixed with using (1UL << 31)
> instead. A better way to do this is to use the BIT() macro, which
> already has the 1UL in it (see future patch in series).
> 
> Convert and sync with the same changes made upstream to the uapi file,
> to implement ethtool use BIT() and friends.

Please follow the guidelines on updating UAPI header copies in
"Submitting patches" section of

  https://www.kernel.org/pub/software/network/ethtool/devel.html

Fixing fsl_enetc.c within the UAPI update is OK (and definitely better
than trying to avoid it) but please update all UAPI header copies to the
state of the same kernel tree commit which will be indicated in the
commit message.

Michal

> This required an unfortunate bit of extra fussing around declaring "same
> definition" versions of the BIT* macros so that the UAPI file will
> compile both under the kernel and in user-space (here).
> 
> A local declaration of BIT() had to be moved out of fsl_enetc.c when
> the implementation was moved to a header.
> 
> NOTE:
> This change will be followed by a larger conversion patch, but *this*
> commit only has the UAPI file changes and the initial implementation to
> keep the work separate from the application only changes.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool v2 09/13] ethtool: merge uapi changes to implement BIT and friends

[Jesse Brandeburg]

On 12/7/2022 10:44 PM, Michal Kubecek wrote:
> On Wed, Dec 07, 2022 at 05:11:18PM -0800, Jesse Brandeburg wrote:
>> I was looking into some errors reported by the runtime sanitizers and
>> found a couple of places where (1 << 31) was being used. This is a shift
>> of a bit into the sign-bit of an integer. This is undefined behavior for
>> the C-specification, and can be easily fixed with using (1UL << 31)
>> instead. A better way to do this is to use the BIT() macro, which
>> already has the 1UL in it (see future patch in series).
>>
>> Convert and sync with the same changes made upstream to the uapi file,
>> to implement ethtool use BIT() and friends.
> 
> Please follow the guidelines on updating UAPI header copies in
> "Submitting patches" section of
> 
>    https://www.kernel.org/pub/software/network/ethtool/devel.html
> 
> Fixing fsl_enetc.c within the UAPI update is OK (and definitely better
> than trying to avoid it) but please update all UAPI header copies to the
> state of the same kernel tree commit which will be indicated in the
> commit message.

I'll respin the uapi changes in the kernel to use uapi/linux/const.h and 
wait to see how that series goes then.

[PATCH ethtool v2 10/13] ethtool: refactor bit shifts to use BIT and BIT_ULL

[Jesse Brandeburg]

A previous patch fixed bit shifts into the sign bit. Formalize this
change and enable clearer code with a conversion of all (1 << foo) to
BIT(foo). This aligns better with the kernel and makes mistakes less
likely due to forgetting the '1UL' during (1UL << foo) usage.

There were a couple of places changed with 1ULL << foo that got changed
to BIT_ULL(foo).

Most of the changes were made by a complicated regular expression that
helped me find the issues (vim style regex):
s/\(\s\+\)(\?\s*1\(U\|UL\)\?\s*<<\s*\([0-9A-Za-z_]\+\))\?/\1BIT(\3)/

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 amd8111e.c         | 198 +++++++++++-----------
 de2104x.c          | 410 ++++++++++++++++++++++-----------------------
 ethtool.c          |  18 +-
 internal.h         |  26 +--
 natsemi.c          | 358 +++++++++++++++++++--------------------
 netlink/bitset.c   |   6 +-
 netlink/features.c |   4 +-
 netlink/monitor.c  |   4 +-
 netlink/parser.c   |   4 +-
 netlink/settings.c |  10 +-
 netlink/stats.c    |   2 +-
 qsfp.h             | 346 +++++++++++++++++++-------------------
 realtek.c          |  48 +++---
 rxclass.c          |   2 +-
 sfc.c              |   2 +-
 sfpdiag.c          |  58 +++----
 sfpid.c            | 146 ++++++++--------
 tse.c              |   2 +-
 18 files changed, 822 insertions(+), 822 deletions(-)

diff --git a/amd8111e.c b/amd8111e.c
index 5a2fc2082e55..17394f9e4565 100644
--- a/amd8111e.c
+++ b/amd8111e.c
@@ -5,20 +5,20 @@
 
 typedef enum {
 	/* VAL2 */
-	RDMD0			= (1 << 16),
+	RDMD0			= BIT(16),
 	/* VAL1 */
-	TDMD3			= (1 << 11),
-	TDMD2			= (1 << 10),
-	TDMD1			= (1 << 9),
-	TDMD0			= (1 << 8),
+	TDMD3			= BIT(11),
+	TDMD2			= BIT(10),
+	TDMD1			= BIT(9),
+	TDMD0			= BIT(8),
 	/* VAL0 */
-	UINTCMD			= (1 << 6),
-	RX_FAST_SPND		= (1 << 5),
-	TX_FAST_SPND		= (1 << 4),
-	RX_SPND			= (1 << 3),
-	TX_SPND			= (1 << 2),
-	INTREN			= (1 << 1),
-	RUN			= (1 << 0),
+	UINTCMD			= BIT(6),
+	RX_FAST_SPND		= BIT(5),
+	TX_FAST_SPND		= BIT(4),
+	RX_SPND			= BIT(3),
+	TX_SPND			= BIT(2),
+	INTREN			= BIT(1),
+	RUN			= BIT(0),
 
 	CMD0_CLEAR 		= 0x000F0F7F,   /* Command style register */
 
@@ -26,27 +26,27 @@ typedef enum {
 typedef enum {
 
 	/* VAL3 */
-	CONDUIT_MODE		= (1 << 29),
+	CONDUIT_MODE		= BIT(29),
 	/* VAL2 */
-	RPA			= (1 << 19),
-	DRCVPA			= (1 << 18),
-	DRCVBC			= (1 << 17),
-	PROM			= (1 << 16),
+	RPA			= BIT(19),
+	DRCVPA			= BIT(18),
+	DRCVBC			= BIT(17),
+	PROM			= BIT(16),
 	/* VAL1 */
-	ASTRP_RCV		= (1 << 13),
-	RCV_DROP0	  	= (1 << 12),
-	EMBA			= (1 << 11),
-	DXMT2PD			= (1 << 10),
-	LTINTEN			= (1 << 9),
-	DXMTFCS			= (1 << 8),
+	ASTRP_RCV		= BIT(13),
+	RCV_DROP0		= BIT(12),
+	EMBA			= BIT(11),
+	DXMT2PD			= BIT(10),
+	LTINTEN			= BIT(9),
+	DXMTFCS			= BIT(8),
 	/* VAL0 */
-	APAD_XMT		= (1 << 6),
-	DRTY			= (1 << 5),
-	INLOOP			= (1 << 4),
-	EXLOOP			= (1 << 3),
-	REX_RTRY		= (1 << 2),
-	REX_UFLO		= (1 << 1),
-	REX_LCOL		= (1 << 0),
+	APAD_XMT		= BIT(6),
+	DRTY			= BIT(5),
+	INLOOP			= BIT(4),
+	EXLOOP			= BIT(3),
+	REX_RTRY		= BIT(2),
+	REX_UFLO		= BIT(1),
+	REX_LCOL		= BIT(0),
 
 	CMD2_CLEAR 		= 0x3F7F3F7F,   /* Command style register */
 
@@ -54,79 +54,79 @@ typedef enum {
 typedef enum {
 
 	/* VAL3 */
-	ASF_INIT_DONE_ALIAS	= (1 << 29),
+	ASF_INIT_DONE_ALIAS	= BIT(29),
 	/* VAL2 */
-	JUMBO			= (1 << 21),
-	VSIZE			= (1 << 20),
-	VLONLY			= (1 << 19),
-	VL_TAG_DEL		= (1 << 18),
+	JUMBO			= BIT(21),
+	VSIZE			= BIT(20),
+	VLONLY			= BIT(19),
+	VL_TAG_DEL		= BIT(18),
 	/* VAL1 */
-	EN_PMGR			= (1 << 14),
-	INTLEVEL		= (1 << 13),
-	FORCE_FULL_DUPLEX	= (1 << 12),
-	FORCE_LINK_STATUS	= (1 << 11),
-	APEP			= (1 << 10),
-	MPPLBA			= (1 << 9),
+	EN_PMGR			= BIT(14),
+	INTLEVEL		= BIT(13),
+	FORCE_FULL_DUPLEX	= BIT(12),
+	FORCE_LINK_STATUS	= BIT(11),
+	APEP			= BIT(10),
+	MPPLBA			= BIT(9),
 	/* VAL0 */
-	RESET_PHY_PULSE		= (1 << 2),
-	RESET_PHY		= (1 << 1),
-	PHY_RST_POL		= (1 << 0),
+	RESET_PHY_PULSE		= BIT(2),
+	RESET_PHY		= BIT(1),
+	PHY_RST_POL		= BIT(0),
 
 }CMD3_BITS;
 typedef enum {
 
-	INTR			= (1UL << 31),
-	PCSINT			= (1 << 28),
-	LCINT			= (1 << 27),
-	APINT5			= (1 << 26),
-	APINT4			= (1 << 25),
-	APINT3			= (1 << 24),
-	TINT_SUM		= (1 << 23),
-	APINT2			= (1 << 22),
-	APINT1			= (1 << 21),
-	APINT0			= (1 << 20),
-	MIIPDTINT		= (1 << 19),
-	MCCINT			= (1 << 17),
-	MREINT			= (1 << 16),
-	RINT_SUM		= (1 << 15),
-	SPNDINT			= (1 << 14),
-	MPINT			= (1 << 13),
-	SINT			= (1 << 12),
-	TINT3			= (1 << 11),
-	TINT2			= (1 << 10),
-	TINT1			= (1 << 9),
-	TINT0			= (1 << 8),
-	UINT			= (1 << 7),
-	STINT			= (1 << 4),
-	RINT0			= (1 << 0),
+	INTR			= BIT(31),
+	PCSINT			= BIT(28),
+	LCINT			= BIT(27),
+	APINT5			= BIT(26),
+	APINT4			= BIT(25),
+	APINT3			= BIT(24),
+	TINT_SUM		= BIT(23),
+	APINT2			= BIT(22),
+	APINT1			= BIT(21),
+	APINT0			= BIT(20),
+	MIIPDTINT		= BIT(19),
+	MCCINT			= BIT(17),
+	MREINT			= BIT(16),
+	RINT_SUM		= BIT(15),
+	SPNDINT			= BIT(14),
+	MPINT			= BIT(13),
+	SINT			= BIT(12),
+	TINT3			= BIT(11),
+	TINT2			= BIT(10),
+	TINT1			= BIT(9),
+	TINT0			= BIT(8),
+	UINT			= BIT(7),
+	STINT			= BIT(4),
+	RINT0			= BIT(0),
 
 }INT0_BITS;
 typedef enum {
 
 	/* VAL3 */
-	LCINTEN			= (1 << 27),
-	APINT5EN		= (1 << 26),
-	APINT4EN		= (1 << 25),
-	APINT3EN		= (1 << 24),
+	LCINTEN			= BIT(27),
+	APINT5EN		= BIT(26),
+	APINT4EN		= BIT(25),
+	APINT3EN		= BIT(24),
 	/* VAL2 */
-	APINT2EN		= (1 << 22),
-	APINT1EN		= (1 << 21),
-	APINT0EN		= (1 << 20),
-	MIIPDTINTEN		= (1 << 19),
-	MCCIINTEN		= (1 << 18),
-	MCCINTEN		= (1 << 17),
-	MREINTEN		= (1 << 16),
+	APINT2EN		= BIT(22),
+	APINT1EN		= BIT(21),
+	APINT0EN		= BIT(20),
+	MIIPDTINTEN		= BIT(19),
+	MCCIINTEN		= BIT(18),
+	MCCINTEN		= BIT(17),
+	MREINTEN		= BIT(16),
 	/* VAL1 */
-	SPNDINTEN		= (1 << 14),
-	MPINTEN			= (1 << 13),
-	TINTEN3			= (1 << 11),
-	SINTEN			= (1 << 12),
-	TINTEN2			= (1 << 10),
-	TINTEN1			= (1 << 9),
-	TINTEN0			= (1 << 8),
+	SPNDINTEN		= BIT(14),
+	MPINTEN			= BIT(13),
+	TINTEN3			= BIT(11),
+	SINTEN			= BIT(12),
+	TINTEN2			= BIT(10),
+	TINTEN1			= BIT(9),
+	TINTEN0			= BIT(8),
 	/* VAL0 */
-	STINTEN			= (1 << 4),
-	RINTEN0			= (1 << 0),
+	STINTEN			= BIT(4),
+	RINTEN0			= BIT(0),
 
 	INTEN0_CLEAR 		= 0x1F7F7F1F, /* Command style register */
 
@@ -134,17 +134,17 @@ typedef enum {
 
 typedef enum {
 
-	PMAT_DET		= (1 << 12),
-	MP_DET		        = (1 << 11),
-	LC_DET			= (1 << 10),
-	SPEED_MASK		= (1 << 9)|(1 << 8)|(1 << 7),
-	FULL_DPLX		= (1 << 6),
-	LINK_STATS		= (1 << 5),
-	AUTONEG_COMPLETE	= (1 << 4),
-	MIIPD			= (1 << 3),
-	RX_SUSPENDED		= (1 << 2),
-	TX_SUSPENDED		= (1 << 1),
-	RUNNING			= (1 << 0),
+	PMAT_DET		= BIT(12),
+	MP_DET		        = BIT(11),
+	LC_DET			= BIT(10),
+	SPEED_MASK		= BIT(9) | BIT(8) | BIT(7),
+	FULL_DPLX		= BIT(6),
+	LINK_STATS		= BIT(5),
+	AUTONEG_COMPLETE	= BIT(4),
+	MIIPD			= BIT(3),
+	RX_SUSPENDED		= BIT(2),
+	TX_SUSPENDED		= BIT(1),
+	RUNNING			= BIT(0),
 
 }STAT0_BITS;
 
diff --git a/de2104x.c b/de2104x.c
index 190422fb2249..a429281272b5 100644
--- a/de2104x.c
+++ b/de2104x.c
@@ -97,7 +97,7 @@ print_rx_missed(u32 csr8)
 {
 	fprintf(stdout,
 		"0x40: CSR8 (Missed Frames Counter)       0x%08x\n", csr8);
-	if (csr8 & (1 << 16))
+	if (csr8 & BIT(16))
 		fprintf(stdout,
 		"      Counter overflow\n");
 	else {
@@ -131,7 +131,7 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		,
 		v,
 		csr0_tap[(v >> 17) & 3],
-		v & (1 << 16) ? "Diagnostic" : "Standard",
+		v & BIT(16) ? "Diagnostic" : "Standard",
 		csr0_cache_al[(v >> 14) & 3]);
 	tmp = (v >> 8) & 0x3f;
 	if (tmp == 0)
@@ -145,10 +145,10 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Descriptor skip length %d longwords\n"
 		"      %s bus arbitration scheme\n"
 		,
-		v & (1 << 7) ? "Big" : "Little",
+		v & BIT(7) ? "Big" : "Little",
 		(v >> 2) & 0x1f,
-		v & (1 << 1) ? "Round-robin" : "RX-has-priority");
-	if (v & (1 << 0))
+		v & BIT(1) ? "Round-robin" : "RX-has-priority");
+	if (v & BIT(0))
 		fprintf(stdout, "      Software reset asserted\n");
 
 	/*
@@ -168,29 +168,29 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Link %s\n"
 		,
 		v,
-		v & (1 << 13) ? csr5_buserr[(v >> 23) & 0x7] : "",
+		v & BIT(13) ? csr5_buserr[(v >> 23) & 0x7] : "",
 		csr5_tx_state[(v >> 20) & 0x7],
 		csr5_rx_state[(v >> 17) & 0x7],
-		v & (1 << 12) ? "fail" : "OK");
-	if (v & (1 << 16))
+		v & BIT(12) ? "fail" : "OK");
+	if (v & BIT(16))
 		fprintf(stdout,
 		"      Normal interrupts: %s%s%s\n"
 		,
-		v & (1 << 0) ? "TxOK " : "",
-		v & (1 << 2) ? "TxNoBufs " : "",
-		v & (1 << 6) ? "RxOK" : "");
-	if (v & (1 << 15))
+		v & BIT(0) ? "TxOK " : "",
+		v & BIT(2) ? "TxNoBufs " : "",
+		v & BIT(6) ? "RxOK" : "");
+	if (v & BIT(15))
 		fprintf(stdout,
 		"      Abnormal intr: %s%s%s%s%s%s%s%s\n"
 		,
-		v & (1 << 1) ? "TxStop " : "",
-		v & (1 << 3) ? "TxJabber " : "",
-		v & (1 << 5) ? "TxUnder " : "",
-		v & (1 << 7) ? "RxNoBufs " : "",
-		v & (1 << 8) ? "RxStopped " : "",
-		v & (1 << 9) ? "RxTimeout " : "",
-		v & (1 << 10) ? "AUI_TP " : "",
-		v & (1 << 11) ? "FD_Short " : "");
+		v & BIT(1) ? "TxStop " : "",
+		v & BIT(3) ? "TxJabber " : "",
+		v & BIT(5) ? "TxUnder " : "",
+		v & BIT(7) ? "RxNoBufs " : "",
+		v & BIT(8) ? "RxStopped " : "",
+		v & BIT(9) ? "RxTimeout " : "",
+		v & BIT(10) ? "AUI_TP " : "",
+		v & BIT(11) ? "FD_Short " : "");
 
 	/*
 	 * CSR6
@@ -216,22 +216,22 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      %s filtering mode\n"
 		,
 		v,
-		v & (1<<17) ? "      Capture effect enabled\n" : "",
-		v & (1<<16) ? "      Back pressure enabled\n" : "",
+		v & BIT(17) ? "      Capture effect enabled\n" : "",
+		v & BIT(16) ? "      Back pressure enabled\n" : "",
 		csr6_tx_thresh[(v >> 14) & 3],
-		v & (1<<13) ? "en" : "dis",
-		v & (1<<12) ? "      Forcing collisions\n" : "",
+		v & BIT(13) ? "en" : "dis",
+		v & BIT(12) ? "      Forcing collisions\n" : "",
 		csr6_om[(v >> 10) & 3],
-		v & (1<<9) ? "Full" : "Half",
-		v & (1<<8) ? "      Flaky oscillator disable\n" : "",
-		v & (1<<7) ? "      Pass All Multicast\n" : "",
-		v & (1<<6) ? "      Promisc Mode\n" : "",
-		v & (1<<5) ? "      Start/Stop Backoff Counter\n" : "",
-		v & (1<<4) ? "      Inverse Filtering\n" : "",
-		v & (1<<3) ? "      Pass Bad Frames\n" : "",
-		v & (1<<2) ? "      Hash-only Filtering\n" : "",
-		v & (1<<1) ? "en" : "dis",
-		v & (1<<0) ? "Hash" : "Perfect");
+		v & BIT(9) ? "Full" : "Half",
+		v & BIT(8) ? "      Flaky oscillator disable\n" : "",
+		v & BIT(7) ? "      Pass All Multicast\n" : "",
+		v & BIT(6) ? "      Promisc Mode\n" : "",
+		v & BIT(5) ? "      Start/Stop Backoff Counter\n" : "",
+		v & BIT(4) ? "      Inverse Filtering\n" : "",
+		v & BIT(3) ? "      Pass Bad Frames\n" : "",
+		v & BIT(2) ? "      Hash-only Filtering\n" : "",
+		v & BIT(1) ? "en" : "dis",
+		v & BIT(0) ? "Hash" : "Perfect");
 
 	/*
 	 * CSR7
@@ -256,21 +256,21 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<16) ? "      Normal interrupt summary\n" : "",
-		v & (1<<15) ? "      Abnormal interrupt summary\n" : "",
-		v & (1<<13) ? "      System error\n" : "",
-		v & (1<<12) ? "      Link fail\n" : "",
-		v & (1<<11) ? "      Full duplex\n" : "",
-		v & (1<<10) ? "      AUI_TP pin\n" : "",
-		v & (1<<9) ? "      Receive watchdog timeout\n" : "",
-		v & (1<<8) ? "      Receive stopped\n" : "",
-		v & (1<<7) ? "      Receive buffer unavailable\n" : "",
-		v & (1<<6) ? "      Receive interrupt\n" : "",
-		v & (1<<5) ? "      Transmit underflow\n" : "",
-		v & (1<<3) ? "      Transmit jabber timeout\n" : "",
-		v & (1<<2) ? "      Transmit buffer unavailable\n" : "",
-		v & (1<<1) ? "      Transmit stopped\n" : "",
-		v & (1<<0) ? "      Transmit interrupt\n" : "");
+		v & BIT(16) ? "      Normal interrupt summary\n" : "",
+		v & BIT(15) ? "      Abnormal interrupt summary\n" : "",
+		v & BIT(13) ? "      System error\n" : "",
+		v & BIT(12) ? "      Link fail\n" : "",
+		v & BIT(11) ? "      Full duplex\n" : "",
+		v & BIT(10) ? "      AUI_TP pin\n" : "",
+		v & BIT(9) ? "      Receive watchdog timeout\n" : "",
+		v & BIT(8) ? "      Receive stopped\n" : "",
+		v & BIT(7) ? "      Receive buffer unavailable\n" : "",
+		v & BIT(6) ? "      Receive interrupt\n" : "",
+		v & BIT(5) ? "      Transmit underflow\n" : "",
+		v & BIT(3) ? "      Transmit jabber timeout\n" : "",
+		v & BIT(2) ? "      Transmit buffer unavailable\n" : "",
+		v & BIT(1) ? "      Transmit stopped\n" : "",
+		v & BIT(0) ? "      Transmit interrupt\n" : "");
 
 	/*
 	 * CSR8
@@ -307,14 +307,14 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      AUI_TP pin: %s\n"
 		,
 		v,
-		v & (1<<7) ? "      PLL sampler high\n" : "",
-		v & (1<<6) ? "      PLL sampler low\n" : "",
-		v & (1<<5) ? "      PLL self-test pass\n" : "",
-		v & (1<<4) ? "      PLL self-test done\n" : "",
-		v & (1<<3) ? "      Autopolarity state\n" : "",
-		v & (1<<2) ? "      Link fail\n" : "",
-		v & (1<<1) ? "      Network connection error\n" : "",
-		v & (1<<0) ? "AUI" : "TP");
+		v & BIT(7) ? "      PLL sampler high\n" : "",
+		v & BIT(6) ? "      PLL sampler low\n" : "",
+		v & BIT(5) ? "      PLL self-test pass\n" : "",
+		v & BIT(4) ? "      PLL self-test done\n" : "",
+		v & BIT(3) ? "      Autopolarity state\n" : "",
+		v & BIT(2) ? "      Link fail\n" : "",
+		v & BIT(1) ? "      Network connection error\n" : "",
+		v & BIT(0) ? "AUI" : "TP");
 
 	/*
 	 * CSR13
@@ -337,22 +337,22 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<15) ? "      Enable pins 5, 6, 7\n" : "",
-		v & (1<<14) ? "      Enable pins 2, 4\n" : "",
-		v & (1<<13) ? "      Enable pins 1, 3\n" : "",
-		v & (1<<12) ? "      Input enable\n" : "",
-		v & (1<<11) ? 1 : 0,
-		v & (1<<10) ? 1 : 0,
-		v & (1<<9) ? 1 : 0,
-		v & (1<<8) ? 1 : 0,
-		v & (1<<7) ? "      APLL start\n" : "",
-		v & (1<<6) ? "      Serial interface input multiplexer\n" : "",
-		v & (1<<5) ? "      Encoder input multiplexer\n" : "",
-		v & (1<<4) ? "      SIA PLL external input enable\n" : "",
-		v & (1<<3) ? "AUI" : "10base-T",
-		v & (1<<2) ? "      CSR autoconfiguration\n" : "",
-		v & (1<<1) ? "      AUI_TP pin autoconfiguration\n" : "",
-		v & (1<<0) ? "      SIA reset\n" : "");
+		v & BIT(15) ? "      Enable pins 5, 6, 7\n" : "",
+		v & BIT(14) ? "      Enable pins 2, 4\n" : "",
+		v & BIT(13) ? "      Enable pins 1, 3\n" : "",
+		v & BIT(12) ? "      Input enable\n" : "",
+		v & BIT(11) ? 1 : 0,
+		v & BIT(10) ? 1 : 0,
+		v & BIT(9) ? 1 : 0,
+		v & BIT(8) ? 1 : 0,
+		v & BIT(7) ? "      APLL start\n" : "",
+		v & BIT(6) ? "      Serial interface input multiplexer\n" : "",
+		v & BIT(5) ? "      Encoder input multiplexer\n" : "",
+		v & BIT(4) ? "      SIA PLL external input enable\n" : "",
+		v & BIT(3) ? "AUI" : "10base-T",
+		v & BIT(2) ? "      CSR autoconfiguration\n" : "",
+		v & BIT(1) ? "      AUI_TP pin autoconfiguration\n" : "",
+		v & BIT(0) ? "      SIA reset\n" : "");
 
 	/*
 	 * CSR14
@@ -374,18 +374,18 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<14) ? "      Set polarity plus\n" : "",
-		v & (1<<13) ? "      Autopolarity enable\n" : "",
-		v & (1<<12) ? "      Link test enable\n" : "",
-		v & (1<<11) ? "      Heartbeat enable\n" : "",
-		v & (1<<10) ? "      Collision detect enable\n" : "",
-		v & (1<<9) ? "      Collision squelch enable\n" : "",
-		v & (1<<8) ? "      Receive squelch enable\n" : "",
+		v & BIT(14) ? "      Set polarity plus\n" : "",
+		v & BIT(13) ? "      Autopolarity enable\n" : "",
+		v & BIT(12) ? "      Link test enable\n" : "",
+		v & BIT(11) ? "      Heartbeat enable\n" : "",
+		v & BIT(10) ? "      Collision detect enable\n" : "",
+		v & BIT(9) ? "      Collision squelch enable\n" : "",
+		v & BIT(8) ? "      Receive squelch enable\n" : "",
 		csr14_tp_comp[(v >> 4) & 0x3],
-		v & (1<<3) ? "      Link pulse send enable\n" : "",
-		v & (1<<2) ? "      Driver enable\n" : "",
-		v & (1<<1) ? "      Loopback enable\n" : "",
-		v & (1<<0) ? "      Encoder enable\n" : "");
+		v & BIT(3) ? "      Link pulse send enable\n" : "",
+		v & BIT(2) ? "      Driver enable\n" : "",
+		v & BIT(1) ? "      Loopback enable\n" : "",
+		v & BIT(0) ? "      Encoder enable\n" : "");
 
 	/*
 	 * CSR15
@@ -405,16 +405,16 @@ static void de21040_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<13) ? "      Force receiver low\n" : "",
-		v & (1<<12) ? "      PLL self-test start\n" : "",
-		v & (1<<11) ? "      Force link fail\n" : "",
-		v & (1<<9) ? "      Force unsquelch\n" : "",
-		v & (1<<8) ? "      Test clock\n" : "",
-		v & (1<<5) ? "      Receive watchdog release\n" : "",
-		v & (1<<4) ? "      Receive watchdog disable\n" : "",
-		v & (1<<2) ? "      Jabber clock\n" : "",
-		v & (1<<1) ? "      Host unjab\n" : "",
-		v & (1<<0) ? "      Jabber disable\n" : "");
+		v & BIT(13) ? "      Force receiver low\n" : "",
+		v & BIT(12) ? "      PLL self-test start\n" : "",
+		v & BIT(11) ? "      Force link fail\n" : "",
+		v & BIT(9) ? "      Force unsquelch\n" : "",
+		v & BIT(8) ? "      Test clock\n" : "",
+		v & BIT(5) ? "      Receive watchdog release\n" : "",
+		v & BIT(4) ? "      Receive watchdog disable\n" : "",
+		v & BIT(2) ? "      Jabber clock\n" : "",
+		v & BIT(1) ? "      Host unjab\n" : "",
+		v & BIT(0) ? "      Jabber disable\n" : "");
 }
 
 static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
@@ -437,9 +437,9 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Cache alignment: %s\n"
 		,
 		v,
-		v & (1 << 20) ? "Big" : "Little",
+		v & BIT(20) ? "Big" : "Little",
 		csr0_tap[(v >> 17) & 3],
-		v & (1 << 16) ? "Diagnostic" : "Standard",
+		v & BIT(16) ? "Diagnostic" : "Standard",
 		csr0_cache_al[(v >> 14) & 3]);
 	tmp = (v >> 8) & 0x3f;
 	if (tmp == 0)
@@ -453,10 +453,10 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Descriptor skip length %d longwords\n"
 		"      %s bus arbitration scheme\n"
 		,
-		v & (1 << 7) ? "Big" : "Little",
+		v & BIT(7) ? "Big" : "Little",
 		(v >> 2) & 0x1f,
-		v & (1 << 1) ? "Round-robin" : "RX-has-priority");
-	if (v & (1 << 0))
+		v & BIT(1) ? "Round-robin" : "RX-has-priority");
+	if (v & BIT(0))
 		fprintf(stdout, "      Software reset asserted\n");
 
 	/*
@@ -476,30 +476,30 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Link %s\n"
 		,
 		v,
-		v & (1 << 13) ? csr5_buserr[(v >> 23) & 0x7] : "",
+		v & BIT(13) ? csr5_buserr[(v >> 23) & 0x7] : "",
 		csr5_tx_state[(v >> 20) & 0x7],
 		csr5_rx_state[(v >> 17) & 0x7],
-		v & (1 << 12) ? "fail" : "OK");
-	if (v & (1 << 16))
+		v & BIT(12) ? "fail" : "OK");
+	if (v & BIT(16))
 		fprintf(stdout,
 		"      Normal interrupts: %s%s%s%s%s\n"
 		,
-		v & (1 << 0) ? "TxOK " : "",
-		v & (1 << 2) ? "TxNoBufs " : "",
-		v & (1 << 6) ? "RxOK" : "",
-		v & (1 << 11) ? "TimerExp " : "",
-		v & (1 << 14) ? "EarlyRx " : "");
-	if (v & (1 << 15))
+		v & BIT(0) ? "TxOK " : "",
+		v & BIT(2) ? "TxNoBufs " : "",
+		v & BIT(6) ? "RxOK" : "",
+		v & BIT(11) ? "TimerExp " : "",
+		v & BIT(14) ? "EarlyRx " : "");
+	if (v & BIT(15))
 		fprintf(stdout,
 		"      Abnormal intr: %s%s%s%s%s%s%s\n"
 		,
-		v & (1 << 1) ? "TxStop " : "",
-		v & (1 << 3) ? "TxJabber " : "",
-		v & (1 << 4) ? "ANC " : "",
-		v & (1 << 5) ? "TxUnder " : "",
-		v & (1 << 7) ? "RxNoBufs " : "",
-		v & (1 << 8) ? "RxStopped " : "",
-		v & (1 << 9) ? "RxTimeout " : "");
+		v & BIT(1) ? "TxStop " : "",
+		v & BIT(3) ? "TxJabber " : "",
+		v & BIT(4) ? "ANC " : "",
+		v & BIT(5) ? "TxUnder " : "",
+		v & BIT(7) ? "RxNoBufs " : "",
+		v & BIT(8) ? "RxStopped " : "",
+		v & BIT(9) ? "RxTimeout " : "");
 
 	/*
 	 * CSR6
@@ -525,22 +525,22 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      %s filtering mode\n"
 		,
 		v,
-		v & (1<<31) ? "      Special capture effect enabled\n" : "",
-		v & (1<<17) ? "      Capture effect enabled\n" : "",
+		v & BIT(31) ? "      Special capture effect enabled\n" : "",
+		v & BIT(17) ? "      Capture effect enabled\n" : "",
 		csr6_tx_thresh[(v >> 14) & 3],
-		v & (1<<13) ? "en" : "dis",
-		v & (1<<12) ? "      Forcing collisions\n" : "",
+		v & BIT(13) ? "en" : "dis",
+		v & BIT(12) ? "      Forcing collisions\n" : "",
 		csr6_om[(v >> 10) & 3],
-		v & (1<<9) ? "Full" : "Half",
-		v & (1<<8) ? "      Flaky oscillator disable\n" : "",
-		v & (1<<7) ? "      Pass All Multicast\n" : "",
-		v & (1<<6) ? "      Promisc Mode\n" : "",
-		v & (1<<5) ? "      Start/Stop Backoff Counter\n" : "",
-		v & (1<<4) ? "      Inverse Filtering\n" : "",
-		v & (1<<3) ? "      Pass Bad Frames\n" : "",
-		v & (1<<2) ? "      Hash-only Filtering\n" : "",
-		v & (1<<1) ? "en" : "dis",
-		v & (1<<0) ? "Hash" : "Perfect");
+		v & BIT(9) ? "Full" : "Half",
+		v & BIT(8) ? "      Flaky oscillator disable\n" : "",
+		v & BIT(7) ? "      Pass All Multicast\n" : "",
+		v & BIT(6) ? "      Promisc Mode\n" : "",
+		v & BIT(5) ? "      Start/Stop Backoff Counter\n" : "",
+		v & BIT(4) ? "      Inverse Filtering\n" : "",
+		v & BIT(3) ? "      Pass Bad Frames\n" : "",
+		v & BIT(2) ? "      Hash-only Filtering\n" : "",
+		v & BIT(1) ? "en" : "dis",
+		v & BIT(0) ? "Hash" : "Perfect");
 
 	/*
 	 * CSR7
@@ -566,22 +566,22 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<16) ? "      Normal interrupt summary\n" : "",
-		v & (1<<15) ? "      Abnormal interrupt summary\n" : "",
-		v & (1<<14) ? "      Early receive interrupt\n" : "",
-		v & (1<<13) ? "      System error\n" : "",
-		v & (1<<12) ? "      Link fail\n" : "",
-		v & (1<<11) ? "      Timer expired\n" : "",
-		v & (1<<9) ? "      Receive watchdog timeout\n" : "",
-		v & (1<<8) ? "      Receive stopped\n" : "",
-		v & (1<<7) ? "      Receive buffer unavailable\n" : "",
-		v & (1<<6) ? "      Receive interrupt\n" : "",
-		v & (1<<5) ? "      Transmit underflow\n" : "",
-		v & (1<<4) ? "      Link pass\n" : "",
-		v & (1<<3) ? "      Transmit jabber timeout\n" : "",
-		v & (1<<2) ? "      Transmit buffer unavailable\n" : "",
-		v & (1<<1) ? "      Transmit stopped\n" : "",
-		v & (1<<0) ? "      Transmit interrupt\n" : "");
+		v & BIT(16) ? "      Normal interrupt summary\n" : "",
+		v & BIT(15) ? "      Abnormal interrupt summary\n" : "",
+		v & BIT(14) ? "      Early receive interrupt\n" : "",
+		v & BIT(13) ? "      System error\n" : "",
+		v & BIT(12) ? "      Link fail\n" : "",
+		v & BIT(11) ? "      Timer expired\n" : "",
+		v & BIT(9) ? "      Receive watchdog timeout\n" : "",
+		v & BIT(8) ? "      Receive stopped\n" : "",
+		v & BIT(7) ? "      Receive buffer unavailable\n" : "",
+		v & BIT(6) ? "      Receive interrupt\n" : "",
+		v & BIT(5) ? "      Transmit underflow\n" : "",
+		v & BIT(4) ? "      Link pass\n" : "",
+		v & BIT(3) ? "      Transmit jabber timeout\n" : "",
+		v & BIT(2) ? "      Transmit buffer unavailable\n" : "",
+		v & BIT(1) ? "      Transmit stopped\n" : "",
+		v & BIT(0) ? "      Transmit interrupt\n" : "");
 
 	/*
 	 * CSR8
@@ -598,20 +598,20 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Data: %d%d%d%d%d%d%d%d\n"
 		,
 		v,
-		v & (1<<15) ? "Mode " : "",
-		v & (1<<14) ? "Read " : "",
-		v & (1<<13) ? "Write " : "",
-		v & (1<<12) ? "BootROM " : "",
-		v & (1<<11) ? "SROM " : "",
-		v & (1<<10) ? "ExtReg " : "",
-		v & (1<<7) ? 1 : 0,
-		v & (1<<6) ? 1 : 0,
-		v & (1<<5) ? 1 : 0,
-		v & (1<<4) ? 1 : 0,
-		v & (1<<3) ? 1 : 0,
-		v & (1<<2) ? 1 : 0,
-		v & (1<<1) ? 1 : 0,
-		v & (1<<0) ? 1 : 0);
+		v & BIT(15) ? "Mode " : "",
+		v & BIT(14) ? "Read " : "",
+		v & BIT(13) ? "Write " : "",
+		v & BIT(12) ? "BootROM " : "",
+		v & BIT(11) ? "SROM " : "",
+		v & BIT(10) ? "ExtReg " : "",
+		v & BIT(7) ? 1 : 0,
+		v & BIT(6) ? 1 : 0,
+		v & BIT(5) ? 1 : 0,
+		v & BIT(4) ? 1 : 0,
+		v & BIT(3) ? 1 : 0,
+		v & BIT(2) ? 1 : 0,
+		v & BIT(1) ? 1 : 0,
+		v & BIT(0) ? 1 : 0);
 
 	/*
 	 * CSR10
@@ -630,7 +630,7 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"      Timer value: %u cycles\n"
 		,
 		v,
-		v & (1<<16) ? "      Continuous mode\n" : "",
+		v & BIT(16) ? "      Continuous mode\n" : "",
 		v & 0xffff);
 
 	/*
@@ -656,19 +656,19 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		,
 		v,
 		v >> 16,
-		v & (1<<15) ? "      Link partner negotiable\n" : "",
+		v & BIT(15) ? "      Link partner negotiable\n" : "",
 		csr12_nway_state[(v >> 12) & 0x7],
-		v & (1<<11) ? "      Transmit remote fault\n" : "",
-		v & (1<<10) ? "      Unstable NLP detected\n" : "",
-		v & (1<<9) ? "      Non-selected port receive activity\n" : "",
-		v & (1<<8) ? "      Selected port receive activity\n" : "",
-		v & (1<<7) ? "      PLL sampler high\n" : "",
-		v & (1<<6) ? "      PLL sampler low\n" : "",
-		v & (1<<5) ? "      PLL self-test pass\n" : "",
-		v & (1<<4) ? "      PLL self-test done\n" : "",
-		v & (1<<3) ? "      Autopolarity state\n" : "",
-		v & (1<<2) ? "      Link fail\n" : "",
-		v & (1<<1) ? "      Network connection error\n" : "");
+		v & BIT(11) ? "      Transmit remote fault\n" : "",
+		v & BIT(10) ? "      Unstable NLP detected\n" : "",
+		v & BIT(9) ? "      Non-selected port receive activity\n" : "",
+		v & BIT(8) ? "      Selected port receive activity\n" : "",
+		v & BIT(7) ? "      PLL sampler high\n" : "",
+		v & BIT(6) ? "      PLL sampler low\n" : "",
+		v & BIT(5) ? "      PLL self-test pass\n" : "",
+		v & BIT(4) ? "      PLL self-test done\n" : "",
+		v & BIT(3) ? "      Autopolarity state\n" : "",
+		v & BIT(2) ? "      Link fail\n" : "",
+		v & BIT(1) ? "      Network connection error\n" : "");
 
 	/*
 	 * CSR13
@@ -683,9 +683,9 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		,
 		v,
 		(v >> 4) & 0xfff,
-		v & (1<<3) ? "AUI/BNC port" : "10base-T port",
-		v & (1<<2) ? "      CSR autoconfiguration enabled\n" : "",
-		v & (1<<0) ? "      SIA register reset asserted\n" : "");
+		v & BIT(3) ? "AUI/BNC port" : "10base-T port",
+		v & BIT(2) ? "      CSR autoconfiguration enabled\n" : "",
+		v & BIT(0) ? "      SIA register reset asserted\n" : "");
 
 	/*
 	 * CSR14
@@ -710,21 +710,21 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<15) ? "      10base-T/AUI autosensing\n" : "",
-		v & (1<<14) ? "      Set polarity plus\n" : "",
-		v & (1<<13) ? "      Autopolarity enable\n" : "",
-		v & (1<<12) ? "      Link test enable\n" : "",
-		v & (1<<11) ? "      Heartbeat enable\n" : "",
-		v & (1<<10) ? "      Collision detect enable\n" : "",
-		v & (1<<9) ? "      Collision squelch enable\n" : "",
-		v & (1<<8) ? "      Receive squelch enable\n" : "",
-		v & (1<<7) ? "      Autonegotiation enable\n" : "",
-		v & (1<<6) ? "      Must Be One\n" : "",
+		v & BIT(15) ? "      10base-T/AUI autosensing\n" : "",
+		v & BIT(14) ? "      Set polarity plus\n" : "",
+		v & BIT(13) ? "      Autopolarity enable\n" : "",
+		v & BIT(12) ? "      Link test enable\n" : "",
+		v & BIT(11) ? "      Heartbeat enable\n" : "",
+		v & BIT(10) ? "      Collision detect enable\n" : "",
+		v & BIT(9) ? "      Collision squelch enable\n" : "",
+		v & BIT(8) ? "      Receive squelch enable\n" : "",
+		v & BIT(7) ? "      Autonegotiation enable\n" : "",
+		v & BIT(6) ? "      Must Be One\n" : "",
 		csr14_tp_comp[(v >> 4) & 0x3],
-		v & (1<<3) ? "      Link pulse send enable\n" : "",
-		v & (1<<2) ? "      Driver enable\n" : "",
-		v & (1<<1) ? "      Loopback enable\n" : "",
-		v & (1<<0) ? "      Encoder enable\n" : "");
+		v & BIT(3) ? "      Link pulse send enable\n" : "",
+		v & BIT(2) ? "      Driver enable\n" : "",
+		v & BIT(1) ? "      Loopback enable\n" : "",
+		v & BIT(0) ? "      Encoder enable\n" : "");
 
 	/*
 	 * CSR15
@@ -750,22 +750,22 @@ static void de21041_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"%s"
 		,
 		v,
-		v & (1<<15) ? "      GP LED2 on\n" : "",
-		v & (1<<14) ? "      GP LED2 enable\n" : "",
-		v & (1<<13) ? "      Force receiver low\n" : "",
-		v & (1<<12) ? "      PLL self-test start\n" : "",
-		v & (1<<11) ? "      LED stretch disable\n" : "",
-		v & (1<<10) ? "      Force link fail\n" : "",
-		v & (1<<9) ? "      Force unsquelch\n" : "",
-		v & (1<<8) ? "      Test clock\n" : "",
-		v & (1<<7) ? "      GP LED1 on\n" : "",
-		v & (1<<6) ? "      GP LED1 enable\n" : "",
-		v & (1<<5) ? "      Receive watchdog release\n" : "",
-		v & (1<<4) ? "      Receive watchdog disable\n" : "",
-		v & (1<<3) ? "AUI" : "BNC",
-		v & (1<<2) ? "      Jabber clock\n" : "",
-		v & (1<<1) ? "      Host unjab\n" : "",
-		v & (1<<0) ? "      Jabber disable\n" : "");
+		v & BIT(15) ? "      GP LED2 on\n" : "",
+		v & BIT(14) ? "      GP LED2 enable\n" : "",
+		v & BIT(13) ? "      Force receiver low\n" : "",
+		v & BIT(12) ? "      PLL self-test start\n" : "",
+		v & BIT(11) ? "      LED stretch disable\n" : "",
+		v & BIT(10) ? "      Force link fail\n" : "",
+		v & BIT(9) ? "      Force unsquelch\n" : "",
+		v & BIT(8) ? "      Test clock\n" : "",
+		v & BIT(7) ? "      GP LED1 on\n" : "",
+		v & BIT(6) ? "      GP LED1 enable\n" : "",
+		v & BIT(5) ? "      Receive watchdog release\n" : "",
+		v & BIT(4) ? "      Receive watchdog disable\n" : "",
+		v & BIT(3) ? "AUI" : "BNC",
+		v & BIT(2) ? "      Jabber clock\n" : "",
+		v & BIT(1) ? "      Host unjab\n" : "",
+		v & BIT(0) ? "      Jabber disable\n" : "");
 }
 
 int
diff --git a/ethtool.c b/ethtool.c
index a72577b32601..4776afe89e23 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -128,7 +128,7 @@ struct feature_defs {
 
 #define FEATURE_BITS_TO_BLOCKS(n_bits)		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_FIELD_FLAG(index)		(BIT((index)) % 32U)
 #define FEATURE_BIT_SET(blocks, index, field)			\
 	(FEATURE_WORD(blocks, index, field) |= FEATURE_FIELD_FLAG(index))
 #define FEATURE_BIT_CLEAR(blocks, index, field)			\
@@ -1667,7 +1667,7 @@ static int dump_tsinfo(const struct ethtool_ts_info *info)
 	fprintf(stdout, "Capabilities:\n");
 
 	for (i = 0; i < N_SOTS; i++) {
-		if (info->so_timestamping & (1 << i))
+		if (info->so_timestamping & BIT(i))
 			fprintf(stdout, "\t%s\n", so_timestamping_labels[i]);
 	}
 
@@ -1686,7 +1686,7 @@ static int dump_tsinfo(const struct ethtool_ts_info *info)
 		fprintf(stdout, "\n");
 
 	for (i = 0; i < N_TX_TYPES; i++) {
-		if (info->tx_types & (1 << i))
+		if (info->tx_types & BIT(i))
 			fprintf(stdout,	"\t%s\n", tx_type_labels[i]);
 	}
 
@@ -1698,7 +1698,7 @@ static int dump_tsinfo(const struct ethtool_ts_info *info)
 		fprintf(stdout, "\n");
 
 	for (i = 0; i < N_RX_FILTERS; i++) {
-		if (info->rx_filters & (1 << i))
+		if (info->rx_filters & BIT(i))
 			fprintf(stdout, "\t%s\n", rx_filter_labels[i]);
 	}
 
@@ -1719,7 +1719,7 @@ get_stringset(struct cmd_context *ctx, enum ethtool_stringset set_id,
 
 	sset_info.hdr.cmd = ETHTOOL_GSSET_INFO;
 	sset_info.hdr.reserved = 0;
-	sset_info.hdr.sset_mask = 1ULL << set_id;
+	sset_info.hdr.sset_mask = BIT_ULL(set_id);
 	if (send_ioctl(ctx, &sset_info) == 0) {
 		const u32 *sset_lengths = sset_info.hdr.data;
 
@@ -4029,7 +4029,7 @@ static int do_grxfh(struct cmd_context *ctx)
 	for (i = 0; i < hfuncs->len; i++)
 		printf("    %s: %s\n",
 		       (const char *)hfuncs->data + i * ETH_GSTRING_LEN,
-		       (rss->hfunc & (1 << i)) ? "on" : "off");
+		       (rss->hfunc & BIT(i)) ? "on" : "off");
 
 out:
 	free(hfuncs);
@@ -4315,7 +4315,7 @@ static int do_srxfh(struct cmd_context *ctx)
 					      i * ETH_GSTRING_LEN);
 			if (!strncmp(hfunc_name, req_hfunc_name,
 				     ETH_GSTRING_LEN))
-				req_hfunc = (u32)1 << i;
+				req_hfunc = BIT(i);
 		}
 
 		if (!req_hfunc) {
@@ -4722,7 +4722,7 @@ static int do_gprivflags(struct cmd_context *ctx)
 		printf("%-*s: %s\n",
 		       max_len,
 		       (const char *)strings->data + i * ETH_GSTRING_LEN,
-		       (flags.data & (1U << i)) ? "on" : "off");
+		       (flags.data & BIT(i)) ? "on" : "off");
 
 	rc = 0;
 
@@ -4769,7 +4769,7 @@ static int do_sprivflags(struct cmd_context *ctx)
 				   i * ETH_GSTRING_LEN);
 		cmdline[i].type = CMDL_FLAG;
 		cmdline[i].wanted_val = &wanted_flags;
-		cmdline[i].flag_val = 1U << i;
+		cmdline[i].flag_val = BIT(i);
 		cmdline[i].seen_val = &seen_flags;
 	}
 	parse_generic_cmdline(ctx, &any_changed, cmdline, strings->len);
diff --git a/internal.h b/internal.h
index 6e79374bcfd5..b5561d24c030 100644
--- a/internal.h
+++ b/internal.h
@@ -97,17 +97,17 @@ static inline u64 cpu_to_be64(u64 value)
 
 static inline void set_bit(unsigned int nr, unsigned long *addr)
 {
-	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
+	addr[nr / BITS_PER_LONG] |= BIT_ULL(nr % BITS_PER_LONG);
 }
 
 static inline void clear_bit(unsigned int nr, unsigned long *addr)
 {
-	addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
+	addr[nr / BITS_PER_LONG] &= ~BIT_ULL(nr % BITS_PER_LONG);
 }
 
 static inline int test_bit(unsigned int nr, const unsigned long *addr)
 {
-	return !!((1UL << (nr % BITS_PER_LONG)) &
+	return !!(BIT_ULL(nr % BITS_PER_LONG) &
 		  (((unsigned long *)addr)[nr / BITS_PER_LONG]));
 }
 
@@ -130,19 +130,19 @@ enum {
 
 static inline bool debug_on(unsigned long debug, unsigned int bit)
 {
-	return (debug & (1 << bit));
+	return (debug & BIT(bit));
 }
 
 /* Internal values for old-style offload flags.  Values and names
  * must not clash with the flags defined for ETHTOOL_{G,S}FLAGS.
  */
-#define ETH_FLAG_RXCSUM		(1 << 0)
-#define ETH_FLAG_TXCSUM		(1 << 1)
-#define ETH_FLAG_SG		(1 << 2)
-#define ETH_FLAG_TSO		(1 << 3)
-#define ETH_FLAG_UFO		(1 << 4)
-#define ETH_FLAG_GSO		(1 << 5)
-#define ETH_FLAG_GRO		(1 << 6)
+#define ETH_FLAG_RXCSUM		BIT(0)
+#define ETH_FLAG_TXCSUM		BIT(1)
+#define ETH_FLAG_SG		BIT(2)
+#define ETH_FLAG_TSO		BIT(3)
+#define ETH_FLAG_UFO		BIT(4)
+#define ETH_FLAG_GSO		BIT(5)
+#define ETH_FLAG_GRO		BIT(6)
 #define ETH_FLAG_INT_MASK	(ETH_FLAG_RXCSUM | ETH_FLAG_TXCSUM |	\
 				 ETH_FLAG_SG | ETH_FLAG_TSO | ETH_FLAG_UFO | \
 				 ETH_FLAG_GSO | ETH_FLAG_GRO),
@@ -205,14 +205,14 @@ 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] & (1UL << (nr % 32)));
+	return !!(mask[nr / 32] & BIT(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] |= (1UL << (nr % 32));
+	mask[nr / 32] |= BIT(nr % 32);
 	return 0;
 }
 
diff --git a/natsemi.c b/natsemi.c
index 4d9fc092b623..43d38f3b337c 100644
--- a/natsemi.c
+++ b/natsemi.c
@@ -9,155 +9,155 @@
 
 /* register indices in the ethtool_regs->data */
 #define REG_CR				0
-#define   BIT_CR_TXE			(1<<0)
-#define   BIT_CR_RXE			(1<<2)
-#define   BIT_CR_RST			(1<<8)
+#define   BIT_CR_TXE			BIT(0)
+#define   BIT_CR_RXE			BIT(2)
+#define   BIT_CR_RST			BIT(8)
 #define REG_CFG				1
-#define   BIT_CFG_BEM			(1<<0)
-#define   BIT_CFG_BROM_DIS		(1<<2)
-#define   BIT_CFG_PHY_DIS		(1<<9)
-#define   BIT_CFG_PHY_RST		(1<<10)
-#define   BIT_CFG_EXT_PHY		(1<<12)
-#define   BIT_CFG_ANEG_EN		(1<<13)
-#define   BIT_CFG_ANEG_100		(1<<14)
-#define   BIT_CFG_ANEG_FDUP		(1<<15)
-#define   BIT_CFG_PINT_ACEN		(1<<17)
+#define   BIT_CFG_BEM			BIT(0)
+#define   BIT_CFG_BROM_DIS		BIT(2)
+#define   BIT_CFG_PHY_DIS		BIT(9)
+#define   BIT_CFG_PHY_RST		BIT(10)
+#define   BIT_CFG_EXT_PHY		BIT(12)
+#define   BIT_CFG_ANEG_EN		BIT(13)
+#define   BIT_CFG_ANEG_100		BIT(14)
+#define   BIT_CFG_ANEG_FDUP		BIT(15)
+#define   BIT_CFG_PINT_ACEN		BIT(17)
 #define   BIT_CFG_PHY_CFG		(0x3f<<18)
-#define   BIT_CFG_ANEG_DN		(1<<27)
-#define   BIT_CFG_POL			(1<<28)
-#define   BIT_CFG_FDUP			(1<<29)
-#define   BIT_CFG_SPEED100		(1<<30)
-#define   BIT_CFG_LNKSTS		(1<<31)
+#define   BIT_CFG_ANEG_DN		BIT(27)
+#define   BIT_CFG_POL			BIT(28)
+#define   BIT_CFG_FDUP			BIT(29)
+#define   BIT_CFG_SPEED100		BIT(30)
+#define   BIT_CFG_LNKSTS		BIT(31)
 
 #define REG_MEAR			2
 #define REG_PTSCR			3
-#define   BIT_PTSCR_EEBIST_FAIL		(1<<0)
-#define   BIT_PTSCR_EELOAD_EN		(1<<2)
-#define   BIT_PTSCR_RBIST_RXFFAIL	(1<<3)
-#define   BIT_PTSCR_RBIST_TXFAIL	(1<<4)
-#define   BIT_PTSCR_RBIST_RXFAIL	(1<<5)
+#define   BIT_PTSCR_EEBIST_FAIL		BIT(0)
+#define   BIT_PTSCR_EELOAD_EN		BIT(2)
+#define   BIT_PTSCR_RBIST_RXFFAIL	BIT(3)
+#define   BIT_PTSCR_RBIST_TXFAIL	BIT(4)
+#define   BIT_PTSCR_RBIST_RXFAIL	BIT(5)
 #define REG_ISR				4
 #define REG_IMR				5
-#define   BIT_INTR_RXOK			(1<<0)
+#define   BIT_INTR_RXOK			BIT(0)
 #define   NAME_INTR_RXOK		"Rx Complete"
-#define   BIT_INTR_RXDESC		(1<<1)
+#define   BIT_INTR_RXDESC		BIT(1)
 #define   NAME_INTR_RXDESC		"Rx Descriptor"
-#define   BIT_INTR_RXERR		(1<<2)
+#define   BIT_INTR_RXERR		BIT(2)
 #define   NAME_INTR_RXERR		"Rx Packet Error"
-#define   BIT_INTR_RXEARLY		(1<<3)
+#define   BIT_INTR_RXEARLY		BIT(3)
 #define   NAME_INTR_RXEARLY		"Rx Early Threshold"
-#define   BIT_INTR_RXIDLE		(1<<4)
+#define   BIT_INTR_RXIDLE		BIT(4)
 #define   NAME_INTR_RXIDLE		"Rx Idle"
-#define   BIT_INTR_RXORN		(1<<5)
+#define   BIT_INTR_RXORN		BIT(5)
 #define   NAME_INTR_RXORN		"Rx Overrun"
-#define   BIT_INTR_TXOK			(1<<6)
+#define   BIT_INTR_TXOK			BIT(6)
 #define   NAME_INTR_TXOK		"Tx Packet OK"
-#define   BIT_INTR_TXDESC		(1<<7)
+#define   BIT_INTR_TXDESC		BIT(7)
 #define   NAME_INTR_TXDESC		"Tx Descriptor"
-#define   BIT_INTR_TXERR		(1<<8)
+#define   BIT_INTR_TXERR		BIT(8)
 #define   NAME_INTR_TXERR		"Tx Packet Error"
-#define   BIT_INTR_TXIDLE		(1<<9)
+#define   BIT_INTR_TXIDLE		BIT(9)
 #define   NAME_INTR_TXIDLE		"Tx Idle"
-#define   BIT_INTR_TXURN		(1<<10)
+#define   BIT_INTR_TXURN		BIT(10)
 #define   NAME_INTR_TXURN		"Tx Underrun"
-#define   BIT_INTR_MIB			(1<<11)
+#define   BIT_INTR_MIB			BIT(11)
 #define   NAME_INTR_MIB			"MIB Service"
-#define   BIT_INTR_SWI			(1<<12)
+#define   BIT_INTR_SWI			BIT(12)
 #define   NAME_INTR_SWI			"Software"
-#define   BIT_INTR_PME			(1<<13)
+#define   BIT_INTR_PME			BIT(13)
 #define   NAME_INTR_PME			"Power Management Event"
-#define   BIT_INTR_PHY			(1<<14)
+#define   BIT_INTR_PHY			BIT(14)
 #define   NAME_INTR_PHY			"Phy"
-#define   BIT_INTR_HIBERR		(1<<15)
+#define   BIT_INTR_HIBERR		BIT(15)
 #define   NAME_INTR_HIBERR		"High Bits Error"
-#define   BIT_INTR_RXSOVR		(1<<16)
+#define   BIT_INTR_RXSOVR		BIT(16)
 #define   NAME_INTR_RXSOVR		"Rx Status FIFO Overrun"
-#define   BIT_INTR_RTABT		(1<<20)
+#define   BIT_INTR_RTABT		BIT(20)
 #define   NAME_INTR_RTABT		"Received Target Abort"
-#define   BIT_INTR_RMABT		(1<<20)
+#define   BIT_INTR_RMABT		BIT(20)
 #define   NAME_INTR_RMABT		"Received Master Abort"
-#define   BIT_INTR_SSERR		(1<<20)
+#define   BIT_INTR_SSERR		BIT(20)
 #define   NAME_INTR_SSERR		"Signaled System Error"
-#define   BIT_INTR_DPERR		(1<<20)
+#define   BIT_INTR_DPERR		BIT(20)
 #define   NAME_INTR_DPERR		"Detected Parity Error"
-#define   BIT_INTR_RXRCMP		(1<<20)
+#define   BIT_INTR_RXRCMP		BIT(20)
 #define   NAME_INTR_RXRCMP		"Rx Reset Complete"
-#define   BIT_INTR_TXRCMP		(1<<20)
+#define   BIT_INTR_TXRCMP		BIT(20)
 #define   NAME_INTR_TXRCMP		"Tx Reset Complete"
 #define REG_IER				6
-#define   BIT_IER_IE			(1<<0)
+#define   BIT_IER_IE			BIT(0)
 #define REG_TXDP			8
 #define REG_TXCFG			9
 #define   BIT_TXCFG_DRTH		(0x3f<<0)
 #define   BIT_TXCFG_FLTH		(0x3f<<8)
 #define   BIT_TXCFG_MXDMA		(0x7<<20)
-#define   BIT_TXCFG_ATP			(1<<28)
-#define   BIT_TXCFG_MLB			(1<<29)
-#define   BIT_TXCFG_HBI			(1<<30)
-#define   BIT_TXCFG_CSI			(1<<31)
+#define   BIT_TXCFG_ATP			BIT(28)
+#define   BIT_TXCFG_MLB			BIT(29)
+#define   BIT_TXCFG_HBI			BIT(30)
+#define   BIT_TXCFG_CSI			BIT(31)
 #define REG_RXDP			12
 #define REG_RXCFG			13
 #define   BIT_RXCFG_DRTH		(0x1f<<1)
 #define   BIT_RXCFG_MXDMA		(0x7<<20)
-#define   BIT_RXCFG_ALP			(1<<27)
-#define   BIT_RXCFG_ATX			(1<<28)
-#define   BIT_RXCFG_ARP			(1<<30)
-#define   BIT_RXCFG_AEP			(1<<31)
+#define   BIT_RXCFG_ALP			BIT(27)
+#define   BIT_RXCFG_ATX			BIT(28)
+#define   BIT_RXCFG_ARP			BIT(30)
+#define   BIT_RXCFG_AEP			BIT(31)
 #define REG_CCSR			15
-#define   BIT_CCSR_CLKRUN_EN		(1<<0)
-#define   BIT_CCSR_PMEEN		(1<<8)
-#define   BIT_CCSR_PMESTS		(1<<15)
+#define   BIT_CCSR_CLKRUN_EN		BIT(0)
+#define   BIT_CCSR_PMEEN		BIT(8)
+#define   BIT_CCSR_PMESTS		BIT(15)
 #define REG_WCSR			16
-#define   BIT_WCSR_WKPHY		(1<<0)
-#define   BIT_WCSR_WKUCP		(1<<1)
-#define   BIT_WCSR_WKMCP		(1<<2)
-#define   BIT_WCSR_WKBCP		(1<<3)
-#define   BIT_WCSR_WKARP		(1<<4)
-#define   BIT_WCSR_WKPAT0		(1<<5)
-#define   BIT_WCSR_WKPAT1		(1<<6)
-#define   BIT_WCSR_WKPAT2		(1<<7)
-#define   BIT_WCSR_WKPAT3		(1<<8)
-#define   BIT_WCSR_WKMAG		(1<<9)
-#define   BIT_WCSR_MPSOE		(1<<10)
-#define   BIT_WCSR_SOHACK		(1<<20)
-#define   BIT_WCSR_PHYINT		(1<<22)
-#define   BIT_WCSR_UCASTR		(1<<23)
-#define   BIT_WCSR_MCASTR		(1<<24)
-#define   BIT_WCSR_BCASTR		(1<<25)
-#define   BIT_WCSR_ARPR			(1<<26)
-#define   BIT_WCSR_PATM0		(1<<27)
-#define   BIT_WCSR_PATM1		(1<<28)
-#define   BIT_WCSR_PATM2		(1<<29)
-#define   BIT_WCSR_PATM3		(1<<30)
-#define   BIT_WCSR_MPR			(1<<31)
+#define   BIT_WCSR_WKPHY		BIT(0)
+#define   BIT_WCSR_WKUCP		BIT(1)
+#define   BIT_WCSR_WKMCP		BIT(2)
+#define   BIT_WCSR_WKBCP		BIT(3)
+#define   BIT_WCSR_WKARP		BIT(4)
+#define   BIT_WCSR_WKPAT0		BIT(5)
+#define   BIT_WCSR_WKPAT1		BIT(6)
+#define   BIT_WCSR_WKPAT2		BIT(7)
+#define   BIT_WCSR_WKPAT3		BIT(8)
+#define   BIT_WCSR_WKMAG		BIT(9)
+#define   BIT_WCSR_MPSOE		BIT(10)
+#define   BIT_WCSR_SOHACK		BIT(20)
+#define   BIT_WCSR_PHYINT		BIT(22)
+#define   BIT_WCSR_UCASTR		BIT(23)
+#define   BIT_WCSR_MCASTR		BIT(24)
+#define   BIT_WCSR_BCASTR		BIT(25)
+#define   BIT_WCSR_ARPR			BIT(26)
+#define   BIT_WCSR_PATM0		BIT(27)
+#define   BIT_WCSR_PATM1		BIT(28)
+#define   BIT_WCSR_PATM2		BIT(29)
+#define   BIT_WCSR_PATM3		BIT(30)
+#define   BIT_WCSR_MPR			BIT(31)
 #define REG_PCR				17
 #define   BIT_PCR_PAUSE_CNT		(0xffff<<0)
-#define   BIT_PCR_PSNEG			(1<<21)
-#define   BIT_PCR_PS_RCVD		(1<<22)
-#define   BIT_PCR_PS_DA			(1<<29)
-#define   BIT_PCR_PSMCAST		(1<<30)
-#define   BIT_PCR_PSEN			(1<<31)
+#define   BIT_PCR_PSNEG			BIT(21)
+#define   BIT_PCR_PS_RCVD		BIT(22)
+#define   BIT_PCR_PS_DA			BIT(29)
+#define   BIT_PCR_PSMCAST		BIT(30)
+#define   BIT_PCR_PSEN			BIT(31)
 #define REG_RFCR			18
-#define   BIT_RFCR_UHEN			(1<<20)
-#define   BIT_RFCR_MHEN			(1<<21)
-#define   BIT_RFCR_AARP			(1<<22)
-#define   BIT_RFCR_APAT0		(1<<23)
-#define   BIT_RFCR_APAT1		(1<<24)
-#define   BIT_RFCR_APAT2		(1<<25)
-#define   BIT_RFCR_APAT3		(1<<26)
-#define   BIT_RFCR_APM			(1<<27)
-#define   BIT_RFCR_AAU			(1<<28)
-#define   BIT_RFCR_AAM			(1<<29)
-#define   BIT_RFCR_AAB			(1<<30)
-#define   BIT_RFCR_RFEN			(1<<31)
+#define   BIT_RFCR_UHEN			BIT(20)
+#define   BIT_RFCR_MHEN			BIT(21)
+#define   BIT_RFCR_AARP			BIT(22)
+#define   BIT_RFCR_APAT0		BIT(23)
+#define   BIT_RFCR_APAT1		BIT(24)
+#define   BIT_RFCR_APAT2		BIT(25)
+#define   BIT_RFCR_APAT3		BIT(26)
+#define   BIT_RFCR_APM			BIT(27)
+#define   BIT_RFCR_AAU			BIT(28)
+#define   BIT_RFCR_AAM			BIT(29)
+#define   BIT_RFCR_AAB			BIT(30)
+#define   BIT_RFCR_RFEN			BIT(31)
 #define REG_RFDR			19
 #define REG_BRAR			20
-#define   BIT_BRAR_AUTOINC		(1<<31)
+#define   BIT_BRAR_AUTOINC		BIT(31)
 #define REG_BRDR			21
 #define REG_SRR				22
 #define REG_MIBC			23
-#define   BIT_MIBC_WRN			(1<<0)
-#define   BIT_MIBC_FRZ			(1<<1)
+#define   BIT_MIBC_WRN			BIT(0)
+#define   BIT_MIBC_FRZ			BIT(1)
 #define REG_MIB0			24
 #define REG_MIB1			25
 #define REG_MIB2			26
@@ -166,26 +166,26 @@
 #define REG_MIB5			29
 #define REG_MIB6			30
 #define REG_BMCR			32
-#define   BIT_BMCR_FDUP			(1<<8)
-#define   BIT_BMCR_ANRST		(1<<9)
-#define   BIT_BMCR_ISOL			(1<<10)
-#define   BIT_BMCR_PDOWN		(1<<11)
-#define   BIT_BMCR_ANEN			(1<<12)
-#define   BIT_BMCR_SPEED		(1<<13)
-#define   BIT_BMCR_LOOP			(1<<14)
-#define   BIT_BMCR_RST			(1<<15)
+#define   BIT_BMCR_FDUP			BIT(8)
+#define   BIT_BMCR_ANRST		BIT(9)
+#define   BIT_BMCR_ISOL			BIT(10)
+#define   BIT_BMCR_PDOWN		BIT(11)
+#define   BIT_BMCR_ANEN			BIT(12)
+#define   BIT_BMCR_SPEED		BIT(13)
+#define   BIT_BMCR_LOOP			BIT(14)
+#define   BIT_BMCR_RST			BIT(15)
 #define REG_BMSR			33
-#define   BIT_BMSR_JABBER		(1<<1)
-#define   BIT_BMSR_LNK			(1<<2)
-#define   BIT_BMSR_ANCAP		(1<<3)
-#define   BIT_BMSR_RFAULT		(1<<4)
-#define   BIT_BMSR_ANDONE		(1<<5)
-#define   BIT_BMSR_PREAMBLE		(1<<6)
-#define   BIT_BMSR_10HCAP		(1<<11)
-#define   BIT_BMSR_10FCAP		(1<<12)
-#define   BIT_BMSR_100HCAP		(1<<13)
-#define   BIT_BMSR_100FCAP		(1<<14)
-#define   BIT_BMSR_100T4CAP		(1<<15)
+#define   BIT_BMSR_JABBER		BIT(1)
+#define   BIT_BMSR_LNK			BIT(2)
+#define   BIT_BMSR_ANCAP		BIT(3)
+#define   BIT_BMSR_RFAULT		BIT(4)
+#define   BIT_BMSR_ANDONE		BIT(5)
+#define   BIT_BMSR_PREAMBLE		BIT(6)
+#define   BIT_BMSR_10HCAP		BIT(11)
+#define   BIT_BMSR_10FCAP		BIT(12)
+#define   BIT_BMSR_100HCAP		BIT(13)
+#define   BIT_BMSR_100FCAP		BIT(14)
+#define   BIT_BMSR_100T4CAP		BIT(15)
 #define REG_PHYIDR1			34
 #define REG_PHYIDR2			35
 #define   BIT_PHYIDR2_OUILSB		(0x3f<<10)
@@ -193,81 +193,81 @@
 #define   BIT_PHYIDR2_REV		(0xf)
 #define REG_ANAR			36
 #define   BIT_ANAR_PROTO		(0x1f<<0)
-#define   BIT_ANAR_10			(1<<5)
-#define   BIT_ANAR_10_FD		(1<<6)
-#define   BIT_ANAR_TX			(1<<7)
-#define   BIT_ANAR_TXFD			(1<<8)
-#define   BIT_ANAR_T4			(1<<9)
-#define   BIT_ANAR_PAUSE		(1<<10)
-#define   BIT_ANAR_RF			(1<<13)
-#define   BIT_ANAR_NP			(1<<15)
+#define   BIT_ANAR_10			BIT(5)
+#define   BIT_ANAR_10_FD		BIT(6)
+#define   BIT_ANAR_TX			BIT(7)
+#define   BIT_ANAR_TXFD			BIT(8)
+#define   BIT_ANAR_T4			BIT(9)
+#define   BIT_ANAR_PAUSE		BIT(10)
+#define   BIT_ANAR_RF			BIT(13)
+#define   BIT_ANAR_NP			BIT(15)
 #define REG_ANLPAR			37
 #define   BIT_ANLPAR_PROTO		(0x1f<<0)
-#define   BIT_ANLPAR_10			(1<<5)
-#define   BIT_ANLPAR_10_FD		(1<<6)
-#define   BIT_ANLPAR_TX			(1<<7)
-#define   BIT_ANLPAR_TXFD		(1<<8)
-#define   BIT_ANLPAR_T4			(1<<9)
-#define   BIT_ANLPAR_PAUSE		(1<<10)
-#define   BIT_ANLPAR_RF			(1<<13)
-#define   BIT_ANLPAR_ACK		(1<<14)
-#define   BIT_ANLPAR_NP			(1<<15)
+#define   BIT_ANLPAR_10			BIT(5)
+#define   BIT_ANLPAR_10_FD		BIT(6)
+#define   BIT_ANLPAR_TX			BIT(7)
+#define   BIT_ANLPAR_TXFD		BIT(8)
+#define   BIT_ANLPAR_T4			BIT(9)
+#define   BIT_ANLPAR_PAUSE		BIT(10)
+#define   BIT_ANLPAR_RF			BIT(13)
+#define   BIT_ANLPAR_ACK		BIT(14)
+#define   BIT_ANLPAR_NP			BIT(15)
 #define REG_ANER			38
-#define   BIT_ANER_LP_AN_ENABLE		(1<<0)
-#define   BIT_ANER_PAGE_RX		(1<<1)
-#define   BIT_ANER_NP_ABLE		(1<<2)
-#define   BIT_ANER_LP_NP_ABLE		(1<<3)
-#define   BIT_ANER_PDF			(1<<4)
+#define   BIT_ANER_LP_AN_ENABLE		BIT(0)
+#define   BIT_ANER_PAGE_RX		BIT(1)
+#define   BIT_ANER_NP_ABLE		BIT(2)
+#define   BIT_ANER_LP_NP_ABLE		BIT(3)
+#define   BIT_ANER_PDF			BIT(4)
 #define REG_ANNPTR			39
 #define REG_PHYSTS			48
-#define   BIT_PHYSTS_LNK		(1<<0)
-#define   BIT_PHYSTS_SPD10		(1<<1)
-#define   BIT_PHYSTS_FDUP		(1<<2)
-#define   BIT_PHYSTS_LOOP		(1<<3)
-#define   BIT_PHYSTS_ANDONE		(1<<4)
-#define   BIT_PHYSTS_JABBER		(1<<5)
-#define   BIT_PHYSTS_RF			(1<<6)
-#define   BIT_PHYSTS_MINT		(1<<7)
-#define   BIT_PHYSTS_FC			(1<<11)
-#define   BIT_PHYSTS_POL		(1<<12)
-#define   BIT_PHYSTS_RXERR		(1<<13)
+#define   BIT_PHYSTS_LNK		BIT(0)
+#define   BIT_PHYSTS_SPD10		BIT(1)
+#define   BIT_PHYSTS_FDUP		BIT(2)
+#define   BIT_PHYSTS_LOOP		BIT(3)
+#define   BIT_PHYSTS_ANDONE		BIT(4)
+#define   BIT_PHYSTS_JABBER		BIT(5)
+#define   BIT_PHYSTS_RF			BIT(6)
+#define   BIT_PHYSTS_MINT		BIT(7)
+#define   BIT_PHYSTS_FC			BIT(11)
+#define   BIT_PHYSTS_POL		BIT(12)
+#define   BIT_PHYSTS_RXERR		BIT(13)
 #define REG_MICR			49
-#define   BIT_MICR_INTEN		(1<<1)
+#define   BIT_MICR_INTEN		BIT(1)
 #define REG_MISR			50
-#define   BIT_MISR_MSK_RHF		(1<<9)
-#define   BIT_MISR_MSK_FHF		(1<<10)
-#define   BIT_MISR_MSK_ANC		(1<<11)
-#define   BIT_MISR_MSK_RF		(1<<12)
-#define   BIT_MISR_MSK_JAB		(1<<13)
-#define   BIT_MISR_MSK_LNK		(1<<14)
-#define   BIT_MISR_MINT			(1<<15)
+#define   BIT_MISR_MSK_RHF		BIT(9)
+#define   BIT_MISR_MSK_FHF		BIT(10)
+#define   BIT_MISR_MSK_ANC		BIT(11)
+#define   BIT_MISR_MSK_RF		BIT(12)
+#define   BIT_MISR_MSK_JAB		BIT(13)
+#define   BIT_MISR_MSK_LNK		BIT(14)
+#define   BIT_MISR_MINT			BIT(15)
 #define REG_PGSEL			51
 #define REG_FCSCR			52
 #define REG_RECR			53
 #define REG_PCSR			54
-#define   BIT_PCSR_NRZI			(1<<2)
-#define   BIT_PCSR_FORCE_100		(1<<5)
-#define   BIT_PCSR_SDOPT		(1<<8)
-#define   BIT_PCSR_SDFORCE		(1<<9)
-#define   BIT_PCSR_TQM			(1<<10)
-#define   BIT_PCSR_CLK			(1<<11)
-#define   BIT_PCSR_4B5B			(1<<12)
+#define   BIT_PCSR_NRZI			BIT(2)
+#define   BIT_PCSR_FORCE_100		BIT(5)
+#define   BIT_PCSR_SDOPT		BIT(8)
+#define   BIT_PCSR_SDFORCE		BIT(9)
+#define   BIT_PCSR_TQM			BIT(10)
+#define   BIT_PCSR_CLK			BIT(11)
+#define   BIT_PCSR_4B5B			BIT(12)
 #define REG_PHYCR			57
 #define   BIT_PHYCR_PHYADDR		(0x1f<<0)
-#define   BIT_PHYCR_PAUSE_STS		(1<<7)
-#define   BIT_PHYCR_STRETCH		(1<<8)
-#define   BIT_PHYCR_BIST		(1<<9)
-#define   BIT_PHYCR_BIST_STAT		(1<<10)
-#define   BIT_PHYCR_PSR15		(1<<11)
+#define   BIT_PHYCR_PAUSE_STS		BIT(7)
+#define   BIT_PHYCR_STRETCH		BIT(8)
+#define   BIT_PHYCR_BIST		BIT(9)
+#define   BIT_PHYCR_BIST_STAT		BIT(10)
+#define   BIT_PHYCR_PSR15		BIT(11)
 #define REG_TBTSCR			58
-#define   BIT_TBTSCR_JAB		(1<<0)
-#define   BIT_TBTSCR_BEAT		(1<<1)
-#define   BIT_TBTSCR_AUTOPOL		(1<<3)
-#define   BIT_TBTSCR_POL		(1<<4)
-#define   BIT_TBTSCR_FPOL		(1<<5)
-#define   BIT_TBTSCR_FORCE_10		(1<<6)
-#define   BIT_TBTSCR_PULSE		(1<<7)
-#define   BIT_TBTSCR_LOOP		(1<<8)
+#define   BIT_TBTSCR_JAB		BIT(0)
+#define   BIT_TBTSCR_BEAT		BIT(1)
+#define   BIT_TBTSCR_AUTOPOL		BIT(3)
+#define   BIT_TBTSCR_POL		BIT(4)
+#define   BIT_TBTSCR_FPOL		BIT(5)
+#define   BIT_TBTSCR_FORCE_10		BIT(6)
+#define   BIT_TBTSCR_PULSE		BIT(7)
+#define   BIT_TBTSCR_LOOP		BIT(8)
 #define REG_PMDCSR			64
 #define REG_TSTDAT			65
 #define REG_DSPCFG			66
diff --git a/netlink/bitset.c b/netlink/bitset.c
index 10ce8e9def9a..cecadeb7bb5a 100644
--- a/netlink/bitset.c
+++ b/netlink/bitset.c
@@ -75,7 +75,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, unsigned int idx,
 
 		if (idx >= 8 * mnl_attr_get_payload_len(bits))
 			return false;
-		return bitmap[idx / 32] & (1U << (idx % 32));
+		return bitmap[idx / 32] & BIT(idx % 32);
 	}
 
 	bits = bitset_tb[ETHTOOL_A_BITSET_BITS];
@@ -227,9 +227,9 @@ int walk_bitset(const struct nlattr *bitset, const struct stringset *labels,
 		val_bm = mnl_attr_get_payload(bits);
 		mask_bm = mask ? mnl_attr_get_payload(mask) : NULL;
 		for (idx = 0; idx < count; idx++)
-			if (!mask_bm || (mask_bm[idx / 32] & (1 << (idx % 32))))
+			if (!mask_bm || (mask_bm[idx / 32] & BIT(idx % 32)))
 				cb(idx, get_string(labels, idx),
-				   val_bm[idx / 32] & (1 << (idx % 32)), data);
+				   val_bm[idx / 32] & BIT(idx % 32), data);
 		return 0;
 	}
 
diff --git a/netlink/features.c b/netlink/features.c
index f6ba47f21a12..38f6272852d7 100644
--- a/netlink/features.c
+++ b/netlink/features.c
@@ -57,7 +57,7 @@ static int prepare_feature_results(const struct nlattr *const *tb,
 
 static bool feature_on(const uint32_t *bitmap, unsigned int idx)
 {
-	return bitmap[idx / 32] & (1UL << (idx % 32));
+	return bitmap[idx / 32] & BIT(idx % 32);
 }
 
 static void dump_feature(const struct feature_results *results,
@@ -302,7 +302,7 @@ static void set_sf_req_mask(struct nl_context *nlctx, unsigned int idx)
 {
 	struct sfeatures_context *sfctx = nlctx->cmd_private;
 
-	sfctx->req_mask[idx / 32] |= (1UL << (idx % 32));
+	sfctx->req_mask[idx / 32] |= BIT(idx % 32);
 }
 
 static int fill_legacy_flag(struct nl_context *nlctx, const char *flag_name,
diff --git a/netlink/monitor.c b/netlink/monitor.c
index ace9b259d39f..3f746671acb4 100644
--- a/netlink/monitor.c
+++ b/netlink/monitor.c
@@ -87,12 +87,12 @@ static void clear_filter(struct nl_context *nlctx)
 
 static bool test_filter_cmd(const struct nl_context *nlctx, unsigned int cmd)
 {
-	return nlctx->filter_cmds[cmd / 32] & (1U << (cmd % 32));
+	return nlctx->filter_cmds[cmd / 32] & BIT(cmd % 32);
 }
 
 static void set_filter_cmd(struct nl_context *nlctx, unsigned int cmd)
 {
-	nlctx->filter_cmds[cmd / 32] |= (1U << (cmd % 32));
+	nlctx->filter_cmds[cmd / 32] |= BIT(cmd % 32);
 }
 
 static void set_filter_all(struct nl_context *nlctx)
diff --git a/netlink/parser.c b/netlink/parser.c
index c573a9598a9f..d105d64ad6ea 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -910,12 +910,12 @@ static const struct param_parser *find_parser(const struct param_parser *params,
 
 static bool __parser_bit(const uint64_t *map, unsigned int idx)
 {
-	return map[idx / 64] & (1 << (idx % 64));
+	return map[idx / 64] & BIT(idx % 64);
 }
 
 static void __parser_set(uint64_t *map, unsigned int idx)
 {
-	map[idx / 64] |= (1 << (idx % 64));
+	map[idx / 64] |= BIT(idx % 64);
 }
 
 static void __parser_set_group(const struct param_parser *params,
diff --git a/netlink/settings.c b/netlink/settings.c
index ea86e365383b..e067ccfe507c 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -275,7 +275,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 			const uint32_t *raw_data = mnl_attr_get_payload(bits);
 			char buff[14];
 
-			if (!(raw_data[idx / 32] & (1U << (idx % 32))))
+			if (!(raw_data[idx / 32] & BIT(idx % 32)))
 				continue;
 			if (!lm_class_match(idx, class))
 				continue;
@@ -782,9 +782,9 @@ void wol_modes_cb(unsigned int idx, const char *name __maybe_unused, bool val,
 
 	if (idx >= 32)
 		return;
-	wol->supported |= (1U << idx);
+	wol->supported |= BIT(idx);
 	if (val)
-		wol->wolopts |= (1U << idx);
+		wol->wolopts |= BIT(idx);
 }
 
 int wol_reply_cb(const struct nlmsghdr *nlhdr, void *data)
@@ -832,7 +832,7 @@ void msgmask_cb(unsigned int idx, const char *name __maybe_unused, bool val,
 	if (idx >= 32)
 		return;
 	if (val)
-		*msg_mask |= (1U << idx);
+		*msg_mask |= BIT(idx);
 }
 
 void msgmask_cb2(unsigned int idx __maybe_unused, const char *name,
@@ -1172,7 +1172,7 @@ static int linkmodes_reply_advert_all_cb(const struct nlmsghdr *nlhdr,
 	/* keep only "real" link modes */
 	for (i = 0; i < modes_count; i++)
 		if (!lm_class_match(i, LM_CLASS_REAL))
-			supported_modes[i / 32] &= ~((uint32_t)1 << (i % 32));
+			supported_modes[i / 32] &= ~BIT(i % 32);
 
 	req_bitset = ethnla_nest_start(req_msgbuff, ETHTOOL_A_LINKMODES_OURS);
 	if (!req_bitset)
diff --git a/netlink/stats.c b/netlink/stats.c
index 9f609a4ec550..c4605ba24838 100644
--- a/netlink/stats.c
+++ b/netlink/stats.c
@@ -244,7 +244,7 @@ static int stats_parse_all_groups(struct nl_context *nlctx, uint16_t type,
 		return -ENOMEM;
 
 	for (i = 0; i < nbits; i++)
-		bits[i / 32] |= 1U << (i % 32);
+		bits[i / 32] |= BIT(i % 32);
 
 	ret = -EMSGSIZE;
 	nest = ethnla_nest_start(msgbuff, type);
diff --git a/qsfp.h b/qsfp.h
index 7960bf26fb07..94cb796a0b13 100644
--- a/qsfp.h
+++ b/qsfp.h
@@ -29,100 +29,100 @@
 
 #define	SFF8636_STATUS_2_OFFSET	0x02
 /* Flat Memory:0- Paging, 1- Page 0 only */
-#define	 SFF8636_STATUS_PAGE_3_PRESENT		(1 << 2)
-#define	 SFF8636_STATUS_INTL_OUTPUT		(1 << 1)
-#define	 SFF8636_STATUS_DATA_NOT_READY		(1 << 0)
+#define	 SFF8636_STATUS_PAGE_3_PRESENT		BIT(2)
+#define	 SFF8636_STATUS_INTL_OUTPUT		BIT(1)
+#define	 SFF8636_STATUS_DATA_NOT_READY		BIT(0)
 
 /* Channel Status Interrupt Flags - 3-5 */
 #define	SFF8636_LOS_AW_OFFSET	0x03
-#define	 SFF8636_TX4_LOS_AW		(1 << 7)
-#define	 SFF8636_TX3_LOS_AW		(1 << 6)
-#define	 SFF8636_TX2_LOS_AW		(1 << 5)
-#define	 SFF8636_TX1_LOS_AW		(1 << 4)
-#define	 SFF8636_RX4_LOS_AW		(1 << 3)
-#define	 SFF8636_RX3_LOS_AW		(1 << 2)
-#define	 SFF8636_RX2_LOS_AW		(1 << 1)
-#define	 SFF8636_RX1_LOS_AW		(1 << 0)
+#define	 SFF8636_TX4_LOS_AW		BIT(7)
+#define	 SFF8636_TX3_LOS_AW		BIT(6)
+#define	 SFF8636_TX2_LOS_AW		BIT(5)
+#define	 SFF8636_TX1_LOS_AW		BIT(4)
+#define	 SFF8636_RX4_LOS_AW		BIT(3)
+#define	 SFF8636_RX3_LOS_AW		BIT(2)
+#define	 SFF8636_RX2_LOS_AW		BIT(1)
+#define	 SFF8636_RX1_LOS_AW		BIT(0)
 
 #define	SFF8636_FAULT_AW_OFFSET	0x04
-#define	 SFF8636_TX4_FAULT_AW	(1 << 3)
-#define	 SFF8636_TX3_FAULT_AW	(1 << 2)
-#define	 SFF8636_TX2_FAULT_AW	(1 << 1)
-#define	 SFF8636_TX1_FAULT_AW	(1 << 0)
+#define	 SFF8636_TX4_FAULT_AW	BIT(3)
+#define	 SFF8636_TX3_FAULT_AW	BIT(2)
+#define	 SFF8636_TX2_FAULT_AW	BIT(1)
+#define	 SFF8636_TX1_FAULT_AW	BIT(0)
 
 /* Module Monitor Interrupt Flags - 6-8 */
 #define	SFF8636_TEMP_AW_OFFSET	0x06
-#define	 SFF8636_TEMP_HALARM_STATUS		(1 << 7)
-#define	 SFF8636_TEMP_LALARM_STATUS		(1 << 6)
-#define	 SFF8636_TEMP_HWARN_STATUS		(1 << 5)
-#define	 SFF8636_TEMP_LWARN_STATUS		(1 << 4)
+#define	 SFF8636_TEMP_HALARM_STATUS		BIT(7)
+#define	 SFF8636_TEMP_LALARM_STATUS		BIT(6)
+#define	 SFF8636_TEMP_HWARN_STATUS		BIT(5)
+#define	 SFF8636_TEMP_LWARN_STATUS		BIT(4)
 
 #define	SFF8636_VCC_AW_OFFSET	0x07
-#define	 SFF8636_VCC_HALARM_STATUS		(1 << 7)
-#define	 SFF8636_VCC_LALARM_STATUS		(1 << 6)
-#define	 SFF8636_VCC_HWARN_STATUS		(1 << 5)
-#define	 SFF8636_VCC_LWARN_STATUS		(1 << 4)
+#define	 SFF8636_VCC_HALARM_STATUS		BIT(7)
+#define	 SFF8636_VCC_LALARM_STATUS		BIT(6)
+#define	 SFF8636_VCC_HWARN_STATUS		BIT(5)
+#define	 SFF8636_VCC_LWARN_STATUS		BIT(4)
 
 /* Channel Monitor Interrupt Flags - 9-21 */
 #define	SFF8636_RX_PWR_12_AW_OFFSET	0x09
-#define	 SFF8636_RX_PWR_1_HALARM		(1 << 7)
-#define	 SFF8636_RX_PWR_1_LALARM		(1 << 6)
-#define	 SFF8636_RX_PWR_1_HWARN			(1 << 5)
-#define	 SFF8636_RX_PWR_1_LWARN			(1 << 4)
-#define	 SFF8636_RX_PWR_2_HALARM		(1 << 3)
-#define	 SFF8636_RX_PWR_2_LALARM		(1 << 2)
-#define	 SFF8636_RX_PWR_2_HWARN			(1 << 1)
-#define	 SFF8636_RX_PWR_2_LWARN			(1 << 0)
+#define	 SFF8636_RX_PWR_1_HALARM		BIT(7)
+#define	 SFF8636_RX_PWR_1_LALARM		BIT(6)
+#define	 SFF8636_RX_PWR_1_HWARN			BIT(5)
+#define	 SFF8636_RX_PWR_1_LWARN			BIT(4)
+#define	 SFF8636_RX_PWR_2_HALARM		BIT(3)
+#define	 SFF8636_RX_PWR_2_LALARM		BIT(2)
+#define	 SFF8636_RX_PWR_2_HWARN			BIT(1)
+#define	 SFF8636_RX_PWR_2_LWARN			BIT(0)
 
 #define	SFF8636_RX_PWR_34_AW_OFFSET	0x0A
-#define	 SFF8636_RX_PWR_3_HALARM		(1 << 7)
-#define	 SFF8636_RX_PWR_3_LALARM		(1 << 6)
-#define	 SFF8636_RX_PWR_3_HWARN			(1 << 5)
-#define	 SFF8636_RX_PWR_3_LWARN			(1 << 4)
-#define	 SFF8636_RX_PWR_4_HALARM		(1 << 3)
-#define	 SFF8636_RX_PWR_4_LALARM		(1 << 2)
-#define	 SFF8636_RX_PWR_4_HWARN			(1 << 1)
-#define	 SFF8636_RX_PWR_4_LWARN			(1 << 0)
+#define	 SFF8636_RX_PWR_3_HALARM		BIT(7)
+#define	 SFF8636_RX_PWR_3_LALARM		BIT(6)
+#define	 SFF8636_RX_PWR_3_HWARN			BIT(5)
+#define	 SFF8636_RX_PWR_3_LWARN			BIT(4)
+#define	 SFF8636_RX_PWR_4_HALARM		BIT(3)
+#define	 SFF8636_RX_PWR_4_LALARM		BIT(2)
+#define	 SFF8636_RX_PWR_4_HWARN			BIT(1)
+#define	 SFF8636_RX_PWR_4_LWARN			BIT(0)
 
 #define	SFF8636_TX_BIAS_12_AW_OFFSET	0x0B
-#define	 SFF8636_TX_BIAS_1_HALARM		(1 << 7)
-#define	 SFF8636_TX_BIAS_1_LALARM		(1 << 6)
-#define	 SFF8636_TX_BIAS_1_HWARN		(1 << 5)
-#define	 SFF8636_TX_BIAS_1_LWARN		(1 << 4)
-#define	 SFF8636_TX_BIAS_2_HALARM		(1 << 3)
-#define	 SFF8636_TX_BIAS_2_LALARM		(1 << 2)
-#define	 SFF8636_TX_BIAS_2_HWARN		(1 << 1)
-#define	 SFF8636_TX_BIAS_2_LWARN		(1 << 0)
+#define	 SFF8636_TX_BIAS_1_HALARM		BIT(7)
+#define	 SFF8636_TX_BIAS_1_LALARM		BIT(6)
+#define	 SFF8636_TX_BIAS_1_HWARN		BIT(5)
+#define	 SFF8636_TX_BIAS_1_LWARN		BIT(4)
+#define	 SFF8636_TX_BIAS_2_HALARM		BIT(3)
+#define	 SFF8636_TX_BIAS_2_LALARM		BIT(2)
+#define	 SFF8636_TX_BIAS_2_HWARN		BIT(1)
+#define	 SFF8636_TX_BIAS_2_LWARN		BIT(0)
 
 #define	SFF8636_TX_BIAS_34_AW_OFFSET	0xC
-#define	 SFF8636_TX_BIAS_3_HALARM		(1 << 7)
-#define	 SFF8636_TX_BIAS_3_LALARM		(1 << 6)
-#define	 SFF8636_TX_BIAS_3_HWARN		(1 << 5)
-#define	 SFF8636_TX_BIAS_3_LWARN		(1 << 4)
-#define	 SFF8636_TX_BIAS_4_HALARM		(1 << 3)
-#define	 SFF8636_TX_BIAS_4_LALARM		(1 << 2)
-#define	 SFF8636_TX_BIAS_4_HWARN		(1 << 1)
-#define	 SFF8636_TX_BIAS_4_LWARN		(1 << 0)
+#define	 SFF8636_TX_BIAS_3_HALARM		BIT(7)
+#define	 SFF8636_TX_BIAS_3_LALARM		BIT(6)
+#define	 SFF8636_TX_BIAS_3_HWARN		BIT(5)
+#define	 SFF8636_TX_BIAS_3_LWARN		BIT(4)
+#define	 SFF8636_TX_BIAS_4_HALARM		BIT(3)
+#define	 SFF8636_TX_BIAS_4_LALARM		BIT(2)
+#define	 SFF8636_TX_BIAS_4_HWARN		BIT(1)
+#define	 SFF8636_TX_BIAS_4_LWARN		BIT(0)
 
 #define	SFF8636_TX_PWR_12_AW_OFFSET	0x0D
-#define	 SFF8636_TX_PWR_1_HALARM		(1 << 7)
-#define	 SFF8636_TX_PWR_1_LALARM		(1 << 6)
-#define	 SFF8636_TX_PWR_1_HWARN			(1 << 5)
-#define	 SFF8636_TX_PWR_1_LWARN			(1 << 4)
-#define	 SFF8636_TX_PWR_2_HALARM		(1 << 3)
-#define	 SFF8636_TX_PWR_2_LALARM		(1 << 2)
-#define	 SFF8636_TX_PWR_2_HWARN			(1 << 1)
-#define	 SFF8636_TX_PWR_2_LWARN			(1 << 0)
+#define	 SFF8636_TX_PWR_1_HALARM		BIT(7)
+#define	 SFF8636_TX_PWR_1_LALARM		BIT(6)
+#define	 SFF8636_TX_PWR_1_HWARN			BIT(5)
+#define	 SFF8636_TX_PWR_1_LWARN			BIT(4)
+#define	 SFF8636_TX_PWR_2_HALARM		BIT(3)
+#define	 SFF8636_TX_PWR_2_LALARM		BIT(2)
+#define	 SFF8636_TX_PWR_2_HWARN			BIT(1)
+#define	 SFF8636_TX_PWR_2_LWARN			BIT(0)
 
 #define	SFF8636_TX_PWR_34_AW_OFFSET	0x0E
-#define	 SFF8636_TX_PWR_3_HALARM		(1 << 7)
-#define	 SFF8636_TX_PWR_3_LALARM		(1 << 6)
-#define	 SFF8636_TX_PWR_3_HWARN			(1 << 5)
-#define	 SFF8636_TX_PWR_3_LWARN			(1 << 4)
-#define	 SFF8636_TX_PWR_4_HALARM		(1 << 3)
-#define	 SFF8636_TX_PWR_4_LALARM		(1 << 2)
-#define	 SFF8636_TX_PWR_4_HWARN			(1 << 1)
-#define	 SFF8636_TX_PWR_4_LWARN			(1 << 0)
+#define	 SFF8636_TX_PWR_3_HALARM		BIT(7)
+#define	 SFF8636_TX_PWR_3_LALARM		BIT(6)
+#define	 SFF8636_TX_PWR_3_HWARN			BIT(5)
+#define	 SFF8636_TX_PWR_3_LWARN			BIT(4)
+#define	 SFF8636_TX_PWR_4_HALARM		BIT(3)
+#define	 SFF8636_TX_PWR_4_LALARM		BIT(2)
+#define	 SFF8636_TX_PWR_4_HWARN			BIT(1)
+#define	 SFF8636_TX_PWR_4_LWARN			BIT(0)
 
 /* Module Monitoring Values - 22-33 */
 #define	SFF8636_TEMP_CURR		0x16
@@ -151,10 +151,10 @@
 
 /* Control Bytes - 86 - 99 */
 #define	SFF8636_TX_DISABLE_OFFSET	0x56
-#define	 SFF8636_TX_DISABLE_4			(1 << 3)
-#define	 SFF8636_TX_DISABLE_3			(1 << 2)
-#define	 SFF8636_TX_DISABLE_2			(1 << 1)
-#define	 SFF8636_TX_DISABLE_1			(1 << 0)
+#define	 SFF8636_TX_DISABLE_4			BIT(3)
+#define	 SFF8636_TX_DISABLE_3			BIT(2)
+#define	 SFF8636_TX_DISABLE_2			BIT(1)
+#define	 SFF8636_TX_DISABLE_1			BIT(0)
 
 #define	SFF8636_RX_RATE_SELECT_OFFSET	0x57
 #define	 SFF8636_RX_RATE_SELECT_4_MASK		(3 << 6)
@@ -174,9 +174,9 @@
 #define	SFF8636_RX_APP_SELECT_1_OFFSET	0x5B
 
 #define	SFF8636_PWR_MODE_OFFSET		0x5D
-#define	 SFF8636_HIGH_PWR_ENABLE		(1 << 2)
-#define	 SFF8636_LOW_PWR_SET			(1 << 1)
-#define	 SFF8636_PWR_OVERRIDE			(1 << 0)
+#define	 SFF8636_HIGH_PWR_ENABLE		BIT(2)
+#define	 SFF8636_LOW_PWR_SET			BIT(1)
+#define	 SFF8636_PWR_OVERRIDE			BIT(0)
 
 #define	SFF8636_TX_APP_SELECT_4_OFFSET	0x5E
 #define	SFF8636_TX_APP_SELECT_3_OFFSET	0x5F
@@ -184,32 +184,32 @@
 #define	SFF8636_TX_APP_SELECT_1_OFFSET	0x61
 
 #define	SFF8636_LOS_MASK_OFFSET		0x64
-#define	 SFF8636_TX_LOS_4_MASK			(1 << 7)
-#define	 SFF8636_TX_LOS_3_MASK			(1 << 6)
-#define	 SFF8636_TX_LOS_2_MASK			(1 << 5)
-#define	 SFF8636_TX_LOS_1_MASK			(1 << 4)
-#define	 SFF8636_RX_LOS_4_MASK			(1 << 3)
-#define	 SFF8636_RX_LOS_3_MASK			(1 << 2)
-#define	 SFF8636_RX_LOS_2_MASK			(1 << 1)
-#define	 SFF8636_RX_LOS_1_MASK			(1 << 0)
+#define	 SFF8636_TX_LOS_4_MASK			BIT(7)
+#define	 SFF8636_TX_LOS_3_MASK			BIT(6)
+#define	 SFF8636_TX_LOS_2_MASK			BIT(5)
+#define	 SFF8636_TX_LOS_1_MASK			BIT(4)
+#define	 SFF8636_RX_LOS_4_MASK			BIT(3)
+#define	 SFF8636_RX_LOS_3_MASK			BIT(2)
+#define	 SFF8636_RX_LOS_2_MASK			BIT(1)
+#define	 SFF8636_RX_LOS_1_MASK			BIT(0)
 
 #define	SFF8636_FAULT_MASK_OFFSET	0x65
-#define	 SFF8636_TX_FAULT_1_MASK		(1 << 3)
-#define	 SFF8636_TX_FAULT_2_MASK		(1 << 2)
-#define	 SFF8636_TX_FAULT_3_MASK		(1 << 1)
-#define	 SFF8636_TX_FAULT_4_MASK		(1 << 0)
+#define	 SFF8636_TX_FAULT_1_MASK		BIT(3)
+#define	 SFF8636_TX_FAULT_2_MASK		BIT(2)
+#define	 SFF8636_TX_FAULT_3_MASK		BIT(1)
+#define	 SFF8636_TX_FAULT_4_MASK		BIT(0)
 
 #define	SFF8636_TEMP_MASK_OFFSET	0x67
-#define	 SFF8636_TEMP_HALARM_MASK		(1 << 7)
-#define	 SFF8636_TEMP_LALARM_MASK		(1 << 6)
-#define	 SFF8636_TEMP_HWARN_MASK		(1 << 5)
-#define	 SFF8636_TEMP_LWARN_MASK		(1 << 4)
+#define	 SFF8636_TEMP_HALARM_MASK		BIT(7)
+#define	 SFF8636_TEMP_LALARM_MASK		BIT(6)
+#define	 SFF8636_TEMP_HWARN_MASK		BIT(5)
+#define	 SFF8636_TEMP_LWARN_MASK		BIT(4)
 
 #define	SFF8636_VCC_MASK_OFFSET		0x68
-#define	 SFF8636_VCC_HALARM_MASK		(1 << 7)
-#define	 SFF8636_VCC_LALARM_MASK		(1 << 6)
-#define	 SFF8636_VCC_HWARN_MASK			(1 << 5)
-#define	 SFF8636_VCC_LWARN_MASK			(1 << 4)
+#define	 SFF8636_VCC_HALARM_MASK		BIT(7)
+#define	 SFF8636_VCC_LALARM_MASK		BIT(6)
+#define	 SFF8636_VCC_HWARN_MASK			BIT(5)
+#define	 SFF8636_VCC_LWARN_MASK			BIT(4)
 
 /*------------------------------------------------------------------------------
  *
@@ -225,15 +225,15 @@
 #define SFF8636_EXT_ID_OFFSET		0x81
 #define	 SFF8636_EXT_ID_PWR_CLASS_MASK		0xC0
 #define	  SFF8636_EXT_ID_PWR_CLASS_1		(0 << 6)
-#define	  SFF8636_EXT_ID_PWR_CLASS_2		(1 << 6)
+#define	  SFF8636_EXT_ID_PWR_CLASS_2		BIT(6)
 #define	  SFF8636_EXT_ID_PWR_CLASS_3		(2 << 6)
 #define	  SFF8636_EXT_ID_PWR_CLASS_4		(3 << 6)
 #define	 SFF8636_EXT_ID_CLIE_MASK		0x10
-#define	  SFF8636_EXT_ID_CLIEI_CODE_PRESENT	(1 << 4)
+#define	  SFF8636_EXT_ID_CLIEI_CODE_PRESENT	BIT(4)
 #define	 SFF8636_EXT_ID_CDR_TX_MASK		0x08
-#define	  SFF8636_EXT_ID_CDR_TX_PRESENT		(1 << 3)
+#define	  SFF8636_EXT_ID_CDR_TX_PRESENT		BIT(3)
 #define	 SFF8636_EXT_ID_CDR_RX_MASK		0x04
-#define	  SFF8636_EXT_ID_CDR_RX_PRESENT		(1 << 2)
+#define	  SFF8636_EXT_ID_CDR_RX_PRESENT		BIT(2)
 #define	 SFF8636_EXT_ID_EPWR_CLASS_MASK		0x03
 #define	  SFF8636_EXT_ID_PWR_CLASS_LEGACY	0
 #define	  SFF8636_EXT_ID_PWR_CLASS_5		1
@@ -266,78 +266,78 @@
 /* Specification Compliance - 131-138 */
 /* Ethernet Compliance Codes - 131 */
 #define	SFF8636_ETHERNET_COMP_OFFSET	0x83
-#define	 SFF8636_ETHERNET_RSRVD			(1 << 7)
-#define	 SFF8636_ETHERNET_10G_LRM		(1 << 6)
-#define	 SFF8636_ETHERNET_10G_LR		(1 << 5)
-#define	 SFF8636_ETHERNET_10G_SR		(1 << 4)
-#define	 SFF8636_ETHERNET_40G_CR4		(1 << 3)
-#define	 SFF8636_ETHERNET_40G_SR4		(1 << 2)
-#define	 SFF8636_ETHERNET_40G_LR4		(1 << 1)
-#define	 SFF8636_ETHERNET_40G_ACTIVE	(1 << 0)
+#define	 SFF8636_ETHERNET_RSRVD			BIT(7)
+#define	 SFF8636_ETHERNET_10G_LRM		BIT(6)
+#define	 SFF8636_ETHERNET_10G_LR		BIT(5)
+#define	 SFF8636_ETHERNET_10G_SR		BIT(4)
+#define	 SFF8636_ETHERNET_40G_CR4		BIT(3)
+#define	 SFF8636_ETHERNET_40G_SR4		BIT(2)
+#define	 SFF8636_ETHERNET_40G_LR4		BIT(1)
+#define	 SFF8636_ETHERNET_40G_ACTIVE	BIT(0)
 
 /* SONET Compliance Codes - 132 */
 #define	SFF8636_SONET_COMP_OFFSET	0x84
-#define	 SFF8636_SONET_40G_OTN			(1 << 3)
-#define	 SFF8636_SONET_OC48_LR			(1 << 2)
-#define	 SFF8636_SONET_OC48_IR			(1 << 1)
-#define	 SFF8636_SONET_OC48_SR			(1 << 0)
+#define	 SFF8636_SONET_40G_OTN			BIT(3)
+#define	 SFF8636_SONET_OC48_LR			BIT(2)
+#define	 SFF8636_SONET_OC48_IR			BIT(1)
+#define	 SFF8636_SONET_OC48_SR			BIT(0)
 
 /* SAS/SATA Complaince Codes - 133 */
 #define	SFF8636_SAS_COMP_OFFSET		0x85
-#define	 SFF8636_SAS_12G			(1 << 6)
-#define	 SFF8636_SAS_6G				(1 << 5)
-#define	 SFF8636_SAS_3G				(1 << 4)
+#define	 SFF8636_SAS_12G			BIT(6)
+#define	 SFF8636_SAS_6G				BIT(5)
+#define	 SFF8636_SAS_3G				BIT(4)
 
 /* Gigabit Ethernet Compliance Codes - 134 */
 #define	SFF8636_GIGE_COMP_OFFSET	0x86
-#define	 SFF8636_GIGE_1000_BASE_T		(1 << 3)
-#define	 SFF8636_GIGE_1000_BASE_CX		(1 << 2)
-#define	 SFF8636_GIGE_1000_BASE_LX		(1 << 1)
-#define	 SFF8636_GIGE_1000_BASE_SX		(1 << 0)
+#define	 SFF8636_GIGE_1000_BASE_T		BIT(3)
+#define	 SFF8636_GIGE_1000_BASE_CX		BIT(2)
+#define	 SFF8636_GIGE_1000_BASE_LX		BIT(1)
+#define	 SFF8636_GIGE_1000_BASE_SX		BIT(0)
 
 /* Fibre Channel Link length/Transmitter Tech. - 135,136 */
 #define	SFF8636_FC_LEN_OFFSET		0x87
-#define	 SFF8636_FC_LEN_VERY_LONG		(1 << 7)
-#define	 SFF8636_FC_LEN_SHORT			(1 << 6)
-#define	 SFF8636_FC_LEN_INT			(1 << 5)
-#define	 SFF8636_FC_LEN_LONG			(1 << 4)
-#define	 SFF8636_FC_LEN_MED			(1 << 3)
-#define	 SFF8636_FC_TECH_LONG_LC		(1 << 1)
-#define	 SFF8636_FC_TECH_ELEC_INTER		(1 << 0)
+#define	 SFF8636_FC_LEN_VERY_LONG		BIT(7)
+#define	 SFF8636_FC_LEN_SHORT			BIT(6)
+#define	 SFF8636_FC_LEN_INT			BIT(5)
+#define	 SFF8636_FC_LEN_LONG			BIT(4)
+#define	 SFF8636_FC_LEN_MED			BIT(3)
+#define	 SFF8636_FC_TECH_LONG_LC		BIT(1)
+#define	 SFF8636_FC_TECH_ELEC_INTER		BIT(0)
 
 #define	SFF8636_FC_TECH_OFFSET		0x88
-#define	 SFF8636_FC_TECH_ELEC_INTRA		(1 << 7)
-#define	 SFF8636_FC_TECH_SHORT_WO_OFC		(1 << 6)
-#define	 SFF8636_FC_TECH_SHORT_W_OFC		(1 << 5)
-#define	 SFF8636_FC_TECH_LONG_LL		(1 << 4)
+#define	 SFF8636_FC_TECH_ELEC_INTRA		BIT(7)
+#define	 SFF8636_FC_TECH_SHORT_WO_OFC		BIT(6)
+#define	 SFF8636_FC_TECH_SHORT_W_OFC		BIT(5)
+#define	 SFF8636_FC_TECH_LONG_LL		BIT(4)
 
 /* Fibre Channel Transmitter Media - 137 */
 #define	SFF8636_FC_TRANS_MEDIA_OFFSET	0x89
 /* Twin Axial Pair */
-#define	 SFF8636_FC_TRANS_MEDIA_TW		(1 << 7)
+#define	 SFF8636_FC_TRANS_MEDIA_TW		BIT(7)
 /* Shielded Twisted Pair */
-#define	 SFF8636_FC_TRANS_MEDIA_TP		(1 << 6)
+#define	 SFF8636_FC_TRANS_MEDIA_TP		BIT(6)
 /* Miniature Coax */
-#define	 SFF8636_FC_TRANS_MEDIA_MI		(1 << 5)
+#define	 SFF8636_FC_TRANS_MEDIA_MI		BIT(5)
 /* Video Coax */
-#define	 SFF8636_FC_TRANS_MEDIA_TV		(1 << 4)
+#define	 SFF8636_FC_TRANS_MEDIA_TV		BIT(4)
 /* Multi-mode 62.5m */
-#define	 SFF8636_FC_TRANS_MEDIA_M6		(1 << 3)
+#define	 SFF8636_FC_TRANS_MEDIA_M6		BIT(3)
 /* Multi-mode 50m */
-#define	 SFF8636_FC_TRANS_MEDIA_M5		(1 << 2)
+#define	 SFF8636_FC_TRANS_MEDIA_M5		BIT(2)
 /* Multi-mode 50um */
-#define	 SFF8636_FC_TRANS_MEDIA_OM3		(1 << 1)
+#define	 SFF8636_FC_TRANS_MEDIA_OM3		BIT(1)
 /* Single Mode */
-#define	 SFF8636_FC_TRANS_MEDIA_SM		(1 << 0)
+#define	 SFF8636_FC_TRANS_MEDIA_SM		BIT(0)
 
 /* Fibre Channel Speed - 138 */
 #define	SFF8636_FC_SPEED_OFFSET		0x8A
-#define	 SFF8636_FC_SPEED_1200_MBPS		(1 << 7)
-#define	 SFF8636_FC_SPEED_800_MBPS		(1 << 6)
-#define	 SFF8636_FC_SPEED_1600_MBPS		(1 << 5)
-#define	 SFF8636_FC_SPEED_400_MBPS		(1 << 4)
-#define	 SFF8636_FC_SPEED_200_MBPS		(1 << 2)
-#define	 SFF8636_FC_SPEED_100_MBPS		(1 << 0)
+#define	 SFF8636_FC_SPEED_1200_MBPS		BIT(7)
+#define	 SFF8636_FC_SPEED_800_MBPS		BIT(6)
+#define	 SFF8636_FC_SPEED_1600_MBPS		BIT(5)
+#define	 SFF8636_FC_SPEED_400_MBPS		BIT(4)
+#define	 SFF8636_FC_SPEED_200_MBPS		BIT(2)
+#define	 SFF8636_FC_SPEED_100_MBPS		BIT(0)
 
 /* Encoding - 139 */
 /* Values are defined under SFF8024_ENCODING */
@@ -355,7 +355,7 @@
 
 /* Extended RateSelect - 141 */
 #define	SFF8636_EXT_RS_OFFSET		0x8D
-#define	 SFF8636_EXT_RS_V1			(1 << 0)
+#define	 SFF8636_EXT_RS_V1			BIT(0)
 
 /* Length (Standard SM Fiber)-km - 142 */
 #define	SFF8636_SM_LEN_OFFSET		0x8E
@@ -405,18 +405,18 @@
 /* 1550 nm VCSEL */
 #define	 SFF8636_TRANS_1550_VCSEL		(2 << 4)
 /* 1310 nm VCSEL */
-#define	 SFF8636_TRANS_1310_VCSEL		(1 << 4)
+#define	 SFF8636_TRANS_1310_VCSEL		BIT(4)
 /* 850 nm VCSEL */
 #define	 SFF8636_TRANS_850_VCSEL		(0 << 4)
 
  /* Active/No wavelength control */
-#define	 SFF8636_DEV_TECH_ACTIVE_WAVE_LEN	(1 << 3)
+#define	 SFF8636_DEV_TECH_ACTIVE_WAVE_LEN	BIT(3)
 /* Cooled transmitter */
-#define	 SFF8636_DEV_TECH_COOL_TRANS		(1 << 2)
+#define	 SFF8636_DEV_TECH_COOL_TRANS		BIT(2)
 /* APD/Pin Detector */
-#define	 SFF8636_DEV_TECH_APD_DETECTOR		(1 << 1)
+#define	 SFF8636_DEV_TECH_APD_DETECTOR		BIT(1)
 /* Transmitter tunable */
-#define	 SFF8636_DEV_TECH_TUNABLE		(1 << 0)
+#define	 SFF8636_DEV_TECH_TUNABLE		BIT(0)
 
 /* Vendor Name - 148-163 */
 #define	 SFF8636_VENDOR_NAME_START_OFFSET	0x94
@@ -424,11 +424,11 @@
 
 /* Extended Module Codes - 164 */
 #define	 SFF8636_EXT_MOD_CODE_OFFSET	0xA4
-#define	  SFF8636_EXT_MOD_INFINIBAND_EDR	(1 << 4)
-#define	  SFF8636_EXT_MOD_INFINIBAND_FDR	(1 << 3)
-#define	  SFF8636_EXT_MOD_INFINIBAND_QDR	(1 << 2)
-#define	  SFF8636_EXT_MOD_INFINIBAND_DDR	(1 << 1)
-#define	  SFF8636_EXT_MOD_INFINIBAND_SDR	(1 << 0)
+#define	  SFF8636_EXT_MOD_INFINIBAND_EDR	BIT(4)
+#define	  SFF8636_EXT_MOD_INFINIBAND_FDR	BIT(3)
+#define	  SFF8636_EXT_MOD_INFINIBAND_QDR	BIT(2)
+#define	  SFF8636_EXT_MOD_INFINIBAND_DDR	BIT(1)
+#define	  SFF8636_EXT_MOD_INFINIBAND_SDR	BIT(0)
 
 /* Vendor OUI - 165-167 */
 #define	 SFF8636_VENDOR_OUI_OFFSET		0xA5
@@ -521,31 +521,31 @@
 
 #define	 SFF8636_OPTION_2_OFFSET	0xC1
 /* Rx output amplitude */
-#define	  SFF8636_O2_RX_OUTPUT_AMP	(1 << 0)
+#define	  SFF8636_O2_RX_OUTPUT_AMP	BIT(0)
 #define	 SFF8636_OPTION_3_OFFSET	0xC2
 /* Rx Squelch Disable */
-#define	  SFF8636_O3_RX_SQL_DSBL	(1 << 3)
+#define	  SFF8636_O3_RX_SQL_DSBL	BIT(3)
 /* Rx Output Disable capable */
-#define	  SFF8636_O3_RX_OUTPUT_DSBL	(1 << 2)
+#define	  SFF8636_O3_RX_OUTPUT_DSBL	BIT(2)
 /* Tx Squelch Disable */
-#define	  SFF8636_O3_TX_SQL_DSBL	(1 << 1)
+#define	  SFF8636_O3_TX_SQL_DSBL	BIT(1)
 /* Tx Squelch Impl */
-#define	  SFF8636_O3_TX_SQL_IMPL	(1 << 0)
+#define	  SFF8636_O3_TX_SQL_IMPL	BIT(0)
 #define	 SFF8636_OPTION_4_OFFSET	0xC3
 /* Memory Page 02 present */
-#define	  SFF8636_O4_PAGE_02_PRESENT	(1 << 7)
+#define	  SFF8636_O4_PAGE_02_PRESENT	BIT(7)
 /* Memory Page 01 present */
-#define	  SFF8636_O4_PAGE_01_PRESENT	(1 << 6)
+#define	  SFF8636_O4_PAGE_01_PRESENT	BIT(6)
 /* Rate Select implemented */
-#define	  SFF8636_O4_RATE_SELECT	(1 << 5)
+#define	  SFF8636_O4_RATE_SELECT	BIT(5)
 /* Tx_DISABLE implemented */
-#define	  SFF8636_O4_TX_DISABLE		(1 << 4)
+#define	  SFF8636_O4_TX_DISABLE		BIT(4)
 /* Tx_FAULT implemented */
-#define	  SFF8636_O4_TX_FAULT		(1 << 3)
+#define	  SFF8636_O4_TX_FAULT		BIT(3)
 /* Tx Squelch implemented */
-#define	  SFF8636_O4_TX_SQUELCH		(1 << 2)
+#define	  SFF8636_O4_TX_SQUELCH		BIT(2)
 /* Tx Loss of Signal */
-#define	  SFF8636_O4_TX_LOS		(1 << 1)
+#define	  SFF8636_O4_TX_LOS		BIT(1)
 
 /* Vendor SN - 196-211 */
 #define	 SFF8636_VENDOR_SN_START_OFFSET	0xC4
@@ -564,15 +564,15 @@
 /* Diagnostic Monitoring Type - 220 */
 #define	 SFF8636_DIAG_TYPE_OFFSET	0xDC
 #define	  SFF8636_RX_PWR_TYPE_MASK	0x8
-#define	   SFF8636_RX_PWR_TYPE_AVG_PWR	(1 << 3)
+#define	   SFF8636_RX_PWR_TYPE_AVG_PWR	BIT(3)
 #define	   SFF8636_RX_PWR_TYPE_OMA	(0 << 3)
 #define	  SFF8636_TX_PWR_TYPE_MASK	0x4
-#define	   SFF8636_TX_PWR_TYPE_AVG_PWR	(1 << 2)
+#define	   SFF8636_TX_PWR_TYPE_AVG_PWR	BIT(2)
 
 /* Enhanced Options - 221 */
 #define	 SFF8636_ENH_OPTIONS_OFFSET	0xDD
-#define	  SFF8636_RATE_SELECT_EXT_SUPPORT	(1 << 3)
-#define	  SFF8636_RATE_SELECT_APP_TABLE_SUPPORT	(1 << 2)
+#define	  SFF8636_RATE_SELECT_EXT_SUPPORT	BIT(3)
+#define	  SFF8636_RATE_SELECT_APP_TABLE_SUPPORT	BIT(2)
 
 /* Check code - 223 */
 #define	 SFF8636_CC_EXT_OFFSET		0xDF
diff --git a/realtek.c b/realtek.c
index ee0c6119dafa..a2d6ebe6aaa4 100644
--- a/realtek.c
+++ b/realtek.c
@@ -227,17 +227,17 @@ print_intr_bits(u16 mask)
 {
 	fprintf(stdout,
 		"      %s%s%s%s%s%s%s%s%s%s%s\n",
-		mask & (1 << 15)	? "SERR " : "",
-		mask & (1 << 14)	? "TimeOut " : "",
-		mask & (1 << 8)		? "SWInt " : "",
-		mask & (1 << 7)		? "TxNoBuf " : "",
-		mask & (1 << 6)		? "RxFIFO " : "",
-		mask & (1 << 5)		? "LinkChg " : "",
-		mask & (1 << 4)		? "RxNoBuf " : "",
-		mask & (1 << 3)		? "TxErr " : "",
-		mask & (1 << 2)		? "TxOK " : "",
-		mask & (1 << 1)		? "RxErr " : "",
-		mask & (1 << 0)		? "RxOK " : "");
+		mask & BIT(15)	? "SERR " : "",
+		mask & BIT(14)	? "TimeOut " : "",
+		mask & BIT(8)		? "SWInt " : "",
+		mask & BIT(7)		? "TxNoBuf " : "",
+		mask & BIT(6)		? "RxFIFO " : "",
+		mask & BIT(5)		? "LinkChg " : "",
+		mask & BIT(4)		? "RxNoBuf " : "",
+		mask & BIT(3)		? "TxErr " : "",
+		mask & BIT(2)		? "TxOK " : "",
+		mask & BIT(1)		? "RxErr " : "",
+		mask & BIT(0)		? "RxOK " : "");
 }
 
 int
@@ -342,10 +342,10 @@ realtek_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 	if (v & 0xf) {
 	fprintf(stdout,
 		"      %s%s%s%s\n",
-		v & (1 << 3) ? "ERxGood " : "",
-		v & (1 << 2) ? "ERxBad " : "",
-		v & (1 << 1) ? "ERxOverWrite " : "",
-		v & (1 << 0) ? "ERxOK " : "");
+		v & BIT(3) ? "ERxGood " : "",
+		v & BIT(2) ? "ERxBad " : "",
+		v & BIT(1) ? "ERxOverWrite " : "",
+		v & BIT(0) ? "ERxOK " : "");
 	}
 
 	v = data8[0x37];
@@ -353,9 +353,9 @@ realtek_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		"0x37: Command                                       0x%02x\n"
 		"      Rx %s, Tx %s%s\n",
 		data8[0x37],
-		v & (1 << 3) ? "on" : "off",
-		v & (1 << 2) ? "on" : "off",
-		v & (1 << 4) ? ", RESET" : "");
+		v & BIT(3) ? "on" : "off",
+		v & BIT(2) ? "on" : "off",
+		v & BIT(4) ? ", RESET" : "");
 
 	if (board_type < RTL_GIGA_MAC_VER_01) {
 	fprintf(stdout,
@@ -631,17 +631,17 @@ realtek_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 	fprintf(stdout,
 		"0xE0: C+ Command                                  0x%04x\n",
 		v);
-	if (v & (1 << 9))
+	if (v & BIT(9))
 		fprintf(stdout, "      Big-endian mode\n");
-	if (v & (1 << 8))
+	if (v & BIT(8))
 		fprintf(stdout, "      Home LAN enable\n");
-	if (v & (1 << 6))
+	if (v & BIT(6))
 		fprintf(stdout, "      VLAN de-tagging\n");
-	if (v & (1 << 5))
+	if (v & BIT(5))
 		fprintf(stdout, "      RX checksumming\n");
-	if (v & (1 << 4))
+	if (v & BIT(4))
 		fprintf(stdout, "      PCI 64-bit DAC\n");
-	if (v & (1 << 3))
+	if (v & BIT(3))
 		fprintf(stdout, "      PCI Multiple RW\n");
 
 	v = data[0xe0 >> 2] >> 16;
diff --git a/rxclass.c b/rxclass.c
index ebdd97960e5b..6ceb75c40a5a 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -446,7 +446,7 @@ static int rmgr_find_empty_slot(struct rmgr_ctrl *rmgr,
 	 * If loc rolls over it should be greater than or equal to rmgr->size
 	 * and as such we know we have reached the end of the list.
 	 */
-	if (!~(rmgr->slot[slot_num] | (~1UL << rmgr->size % BITS_PER_LONG))) {
+	if (!~(rmgr->slot[slot_num] | ~BIT_ULL(rmgr->size % BITS_PER_LONG))) {
 		loc -= 1 + (loc % BITS_PER_LONG);
 		slot_num--;
 	}
diff --git a/sfc.c b/sfc.c
index a33077b4f263..50b59d72b861 100644
--- a/sfc.c
+++ b/sfc.c
@@ -3773,7 +3773,7 @@ print_field_value(const struct efx_nic_reg_field *field, const u8 *buf)
 		digit = buf[left >> 3];
 		if ((left & 7) + sig_bits > 8)
 			digit |= buf[(left >> 3) + 1] << 8;
-		digit = (digit >> (left & 7)) & ((1 << sig_bits) - 1);
+		digit = (digit >> (left & 7)) & (BIT(sig_bits) - 1);
 		printf("%x", digit);
 		sig_bits = 4; /* for all subsequent digits */
 	}
diff --git a/sfpdiag.c b/sfpdiag.c
index 502b6e3bf11e..467b593d59a5 100644
--- a/sfpdiag.c
+++ b/sfpdiag.c
@@ -22,12 +22,12 @@
 #define SFF_A0_COMP                       94
 
 /* EEPROM bit values for various registers */
-#define SFF_A0_DOM_EXTCAL                 (1 << 4)
-#define SFF_A0_DOM_INTCAL                 (1 << 5)
-#define SFF_A0_DOM_IMPL                   (1 << 6)
-#define SFF_A0_DOM_PWRT                   (1 << 3)
+#define SFF_A0_DOM_EXTCAL                 BIT(4)
+#define SFF_A0_DOM_INTCAL                 BIT(5)
+#define SFF_A0_DOM_IMPL                   BIT(6)
+#define SFF_A0_DOM_PWRT                   BIT(3)
 
-#define SFF_A0_OPTIONS_AW                 (1 << 7)
+#define SFF_A0_OPTIONS_AW                 BIT(7)
 
 /*
  * See ethtool.c comments about SFF-8472, this is the offset
@@ -92,30 +92,30 @@ static struct sff8472_aw_flags {
 	int offset;             /* A2-relative address offset */
 	__u8 value;             /* Alarm is on if (offset & value) != 0. */
 } sff8472_aw_flags[] = {
-	{ "Laser bias current high alarm",   SFF_A2_ALRM_FLG, (1 << 3) },
-	{ "Laser bias current low alarm",    SFF_A2_ALRM_FLG, (1 << 2) },
-	{ "Laser bias current high warning", SFF_A2_WARN_FLG, (1 << 3) },
-	{ "Laser bias current low warning",  SFF_A2_WARN_FLG, (1 << 2) },
-
-	{ "Laser output power high alarm",   SFF_A2_ALRM_FLG, (1 << 1) },
-	{ "Laser output power low alarm",    SFF_A2_ALRM_FLG, (1 << 0) },
-	{ "Laser output power high warning", SFF_A2_WARN_FLG, (1 << 1) },
-	{ "Laser output power low warning",  SFF_A2_WARN_FLG, (1 << 0) },
-
-	{ "Module temperature high alarm",   SFF_A2_ALRM_FLG, (1 << 7) },
-	{ "Module temperature low alarm",    SFF_A2_ALRM_FLG, (1 << 6) },
-	{ "Module temperature high warning", SFF_A2_WARN_FLG, (1 << 7) },
-	{ "Module temperature low warning",  SFF_A2_WARN_FLG, (1 << 6) },
-
-	{ "Module voltage high alarm",   SFF_A2_ALRM_FLG, (1 << 5) },
-	{ "Module voltage low alarm",    SFF_A2_ALRM_FLG, (1 << 4) },
-	{ "Module voltage high warning", SFF_A2_WARN_FLG, (1 << 5) },
-	{ "Module voltage low warning",  SFF_A2_WARN_FLG, (1 << 4) },
-
-	{ "Laser rx power high alarm",   SFF_A2_ALRM_FLG + 1, (1 << 7) },
-	{ "Laser rx power low alarm",    SFF_A2_ALRM_FLG + 1, (1 << 6) },
-	{ "Laser rx power high warning", SFF_A2_WARN_FLG + 1, (1 << 7) },
-	{ "Laser rx power low warning",  SFF_A2_WARN_FLG + 1, (1 << 6) },
+	{ "Laser bias current high alarm",   SFF_A2_ALRM_FLG, BIT(3) },
+	{ "Laser bias current low alarm",    SFF_A2_ALRM_FLG, BIT(2) },
+	{ "Laser bias current high warning", SFF_A2_WARN_FLG, BIT(3) },
+	{ "Laser bias current low warning",  SFF_A2_WARN_FLG, BIT(2) },
+
+	{ "Laser output power high alarm",   SFF_A2_ALRM_FLG, BIT(1) },
+	{ "Laser output power low alarm",    SFF_A2_ALRM_FLG, BIT(0) },
+	{ "Laser output power high warning", SFF_A2_WARN_FLG, BIT(1) },
+	{ "Laser output power low warning",  SFF_A2_WARN_FLG, BIT(0) },
+
+	{ "Module temperature high alarm",   SFF_A2_ALRM_FLG, BIT(7) },
+	{ "Module temperature low alarm",    SFF_A2_ALRM_FLG, BIT(6) },
+	{ "Module temperature high warning", SFF_A2_WARN_FLG, BIT(7) },
+	{ "Module temperature low warning",  SFF_A2_WARN_FLG, BIT(6) },
+
+	{ "Module voltage high alarm",   SFF_A2_ALRM_FLG, BIT(5) },
+	{ "Module voltage low alarm",    SFF_A2_ALRM_FLG, BIT(4) },
+	{ "Module voltage high warning", SFF_A2_WARN_FLG, BIT(5) },
+	{ "Module voltage low warning",  SFF_A2_WARN_FLG, BIT(4) },
+
+	{ "Laser rx power high alarm",   SFF_A2_ALRM_FLG + 1, BIT(7) },
+	{ "Laser rx power low alarm",    SFF_A2_ALRM_FLG + 1, BIT(6) },
+	{ "Laser rx power high warning", SFF_A2_WARN_FLG + 1, BIT(7) },
+	{ "Laser rx power low warning",  SFF_A2_WARN_FLG + 1, BIT(6) },
 
 	{ NULL, 0, 0 },
 };
diff --git a/sfpid.c b/sfpid.c
index b701e292518d..7654715fe311 100644
--- a/sfpid.c
+++ b/sfpid.c
@@ -48,128 +48,128 @@ static void sff8079_show_transceiver(const __u8 *id)
 	       id[3], id[4], id[5], id[6],
 	       id[7], id[8], id[9], id[10], id[36]);
 	/* 10G Ethernet Compliance Codes */
-	if (id[3] & (1 << 7))
+	if (id[3] & BIT(7))
 		printf("%s 10G Ethernet: 10G Base-ER" \
 		       " [SFF-8472 rev10.4 onwards]\n", pfx);
-	if (id[3] & (1 << 6))
+	if (id[3] & BIT(6))
 		printf("%s 10G Ethernet: 10G Base-LRM\n", pfx);
-	if (id[3] & (1 << 5))
+	if (id[3] & BIT(5))
 		printf("%s 10G Ethernet: 10G Base-LR\n", pfx);
-	if (id[3] & (1 << 4))
+	if (id[3] & BIT(4))
 		printf("%s 10G Ethernet: 10G Base-SR\n", pfx);
 	/* Infiniband Compliance Codes */
-	if (id[3] & (1 << 3))
+	if (id[3] & BIT(3))
 		printf("%s Infiniband: 1X SX\n", pfx);
-	if (id[3] & (1 << 2))
+	if (id[3] & BIT(2))
 		printf("%s Infiniband: 1X LX\n", pfx);
-	if (id[3] & (1 << 1))
+	if (id[3] & BIT(1))
 		printf("%s Infiniband: 1X Copper Active\n", pfx);
-	if (id[3] & (1 << 0))
+	if (id[3] & BIT(0))
 		printf("%s Infiniband: 1X Copper Passive\n", pfx);
 	/* ESCON Compliance Codes */
-	if (id[4] & (1 << 7))
+	if (id[4] & BIT(7))
 		printf("%s ESCON: ESCON MMF, 1310nm LED\n", pfx);
-	if (id[4] & (1 << 6))
+	if (id[4] & BIT(6))
 		printf("%s ESCON: ESCON SMF, 1310nm Laser\n", pfx);
 	/* SONET Compliance Codes */
-	if (id[4] & (1 << 5))
+	if (id[4] & BIT(5))
 		printf("%s SONET: OC-192, short reach\n", pfx);
-	if (id[4] & (1 << 4))
+	if (id[4] & BIT(4))
 		printf("%s SONET: SONET reach specifier bit 1\n", pfx);
-	if (id[4] & (1 << 3))
+	if (id[4] & BIT(3))
 		printf("%s SONET: SONET reach specifier bit 2\n", pfx);
-	if (id[4] & (1 << 2))
+	if (id[4] & BIT(2))
 		printf("%s SONET: OC-48, long reach\n", pfx);
-	if (id[4] & (1 << 1))
+	if (id[4] & BIT(1))
 		printf("%s SONET: OC-48, intermediate reach\n", pfx);
-	if (id[4] & (1 << 0))
+	if (id[4] & BIT(0))
 		printf("%s SONET: OC-48, short reach\n", pfx);
-	if (id[5] & (1 << 6))
+	if (id[5] & BIT(6))
 		printf("%s SONET: OC-12, single mode, long reach\n", pfx);
-	if (id[5] & (1 << 5))
+	if (id[5] & BIT(5))
 		printf("%s SONET: OC-12, single mode, inter. reach\n", pfx);
-	if (id[5] & (1 << 4))
+	if (id[5] & BIT(4))
 		printf("%s SONET: OC-12, short reach\n", pfx);
-	if (id[5] & (1 << 2))
+	if (id[5] & BIT(2))
 		printf("%s SONET: OC-3, single mode, long reach\n", pfx);
-	if (id[5] & (1 << 1))
+	if (id[5] & BIT(1))
 		printf("%s SONET: OC-3, single mode, inter. reach\n", pfx);
-	if (id[5] & (1 << 0))
+	if (id[5] & BIT(0))
 		printf("%s SONET: OC-3, short reach\n", pfx);
 	/* Ethernet Compliance Codes */
-	if (id[6] & (1 << 7))
+	if (id[6] & BIT(7))
 		printf("%s Ethernet: BASE-PX\n", pfx);
-	if (id[6] & (1 << 6))
+	if (id[6] & BIT(6))
 		printf("%s Ethernet: BASE-BX10\n", pfx);
-	if (id[6] & (1 << 5))
+	if (id[6] & BIT(5))
 		printf("%s Ethernet: 100BASE-FX\n", pfx);
-	if (id[6] & (1 << 4))
+	if (id[6] & BIT(4))
 		printf("%s Ethernet: 100BASE-LX/LX10\n", pfx);
-	if (id[6] & (1 << 3))
+	if (id[6] & BIT(3))
 		printf("%s Ethernet: 1000BASE-T\n", pfx);
-	if (id[6] & (1 << 2))
+	if (id[6] & BIT(2))
 		printf("%s Ethernet: 1000BASE-CX\n", pfx);
-	if (id[6] & (1 << 1))
+	if (id[6] & BIT(1))
 		printf("%s Ethernet: 1000BASE-LX\n", pfx);
-	if (id[6] & (1 << 0))
+	if (id[6] & BIT(0))
 		printf("%s Ethernet: 1000BASE-SX\n", pfx);
 	/* Fibre Channel link length */
-	if (id[7] & (1 << 7))
+	if (id[7] & BIT(7))
 		printf("%s FC: very long distance (V)\n", pfx);
-	if (id[7] & (1 << 6))
+	if (id[7] & BIT(6))
 		printf("%s FC: short distance (S)\n", pfx);
-	if (id[7] & (1 << 5))
+	if (id[7] & BIT(5))
 		printf("%s FC: intermediate distance (I)\n", pfx);
-	if (id[7] & (1 << 4))
+	if (id[7] & BIT(4))
 		printf("%s FC: long distance (L)\n", pfx);
-	if (id[7] & (1 << 3))
+	if (id[7] & BIT(3))
 		printf("%s FC: medium distance (M)\n", pfx);
 	/* Fibre Channel transmitter technology */
-	if (id[7] & (1 << 2))
+	if (id[7] & BIT(2))
 		printf("%s FC: Shortwave laser, linear Rx (SA)\n", pfx);
-	if (id[7] & (1 << 1))
+	if (id[7] & BIT(1))
 		printf("%s FC: Longwave laser (LC)\n", pfx);
-	if (id[7] & (1 << 0))
+	if (id[7] & BIT(0))
 		printf("%s FC: Electrical inter-enclosure (EL)\n", pfx);
-	if (id[8] & (1 << 7))
+	if (id[8] & BIT(7))
 		printf("%s FC: Electrical intra-enclosure (EL)\n", pfx);
-	if (id[8] & (1 << 6))
+	if (id[8] & BIT(6))
 		printf("%s FC: Shortwave laser w/o OFC (SN)\n", pfx);
-	if (id[8] & (1 << 5))
+	if (id[8] & BIT(5))
 		printf("%s FC: Shortwave laser with OFC (SL)\n", pfx);
-	if (id[8] & (1 << 4))
+	if (id[8] & BIT(4))
 		printf("%s FC: Longwave laser (LL)\n", pfx);
-	if (id[8] & (1 << 3))
+	if (id[8] & BIT(3))
 		printf("%s Active Cable\n", pfx);
-	if (id[8] & (1 << 2))
+	if (id[8] & BIT(2))
 		printf("%s Passive Cable\n", pfx);
-	if (id[8] & (1 << 1))
+	if (id[8] & BIT(1))
 		printf("%s FC: Copper FC-BaseT\n", pfx);
 	/* Fibre Channel transmission media */
-	if (id[9] & (1 << 7))
+	if (id[9] & BIT(7))
 		printf("%s FC: Twin Axial Pair (TW)\n", pfx);
-	if (id[9] & (1 << 6))
+	if (id[9] & BIT(6))
 		printf("%s FC: Twisted Pair (TP)\n", pfx);
-	if (id[9] & (1 << 5))
+	if (id[9] & BIT(5))
 		printf("%s FC: Miniature Coax (MI)\n", pfx);
-	if (id[9] & (1 << 4))
+	if (id[9] & BIT(4))
 		printf("%s FC: Video Coax (TV)\n", pfx);
-	if (id[9] & (1 << 3))
+	if (id[9] & BIT(3))
 		printf("%s FC: Multimode, 62.5um (M6)\n", pfx);
-	if (id[9] & (1 << 2))
+	if (id[9] & BIT(2))
 		printf("%s FC: Multimode, 50um (M5)\n", pfx);
-	if (id[9] & (1 << 0))
+	if (id[9] & BIT(0))
 		printf("%s FC: Single Mode (SM)\n", pfx);
 	/* Fibre Channel speed */
-	if (id[10] & (1 << 7))
+	if (id[10] & BIT(7))
 		printf("%s FC: 1200 MBytes/sec\n", pfx);
-	if (id[10] & (1 << 6))
+	if (id[10] & BIT(6))
 		printf("%s FC: 800 MBytes/sec\n", pfx);
-	if (id[10] & (1 << 4))
+	if (id[10] & BIT(4))
 		printf("%s FC: 400 MBytes/sec\n", pfx);
-	if (id[10] & (1 << 2))
+	if (id[10] & BIT(2))
 		printf("%s FC: 200 MBytes/sec\n", pfx);
-	if (id[10] & (1 << 0))
+	if (id[10] & BIT(0))
 		printf("%s FC: 100 MBytes/sec\n", pfx);
 	/* Extended Specification Compliance Codes from SFF-8024 */
 	if (id[36] == 0x1)
@@ -304,7 +304,7 @@ static void sff8079_show_oui(const __u8 *id)
 
 static void sff8079_show_wavelength_or_copper_compliance(const __u8 *id)
 {
-	if (id[8] & (1 << 2)) {
+	if (id[8] & BIT(2)) {
 		printf("\t%-41s : 0x%02x", "Passive Cu cmplnce.", id[60]);
 		switch (id[60]) {
 		case 0x00:
@@ -318,7 +318,7 @@ static void sff8079_show_wavelength_or_copper_compliance(const __u8 *id)
 			break;
 		}
 		printf(" [SFF-8472 rev10.4 only]\n");
-	} else if (id[8] & (1 << 3)) {
+	} else if (id[8] & BIT(3)) {
 		printf("\t%-41s : 0x%02x", "Active Cu cmplnce.", id[60]);
 		switch (id[60]) {
 		case 0x00:
@@ -371,31 +371,31 @@ static void sff8079_show_options(const __u8 *id)
 		"\tOption                                    :";
 
 	printf("\t%-41s : 0x%02x 0x%02x\n", "Option values", id[64], id[65]);
-	if (id[65] & (1 << 1))
+	if (id[65] & BIT(1))
 		printf("%s RX_LOS implemented\n", pfx);
-	if (id[65] & (1 << 2))
+	if (id[65] & BIT(2))
 		printf("%s RX_LOS implemented, inverted\n", pfx);
-	if (id[65] & (1 << 3))
+	if (id[65] & BIT(3))
 		printf("%s TX_FAULT implemented\n", pfx);
-	if (id[65] & (1 << 4))
+	if (id[65] & BIT(4))
 		printf("%s TX_DISABLE implemented\n", pfx);
-	if (id[65] & (1 << 5))
+	if (id[65] & BIT(5))
 		printf("%s RATE_SELECT implemented\n", pfx);
-	if (id[65] & (1 << 6))
+	if (id[65] & BIT(6))
 		printf("%s Tunable transmitter technology\n", pfx);
-	if (id[65] & (1 << 7))
+	if (id[65] & BIT(7))
 		printf("%s Receiver decision threshold implemented\n", pfx);
-	if (id[64] & (1 << 0))
+	if (id[64] & BIT(0))
 		printf("%s Linear receiver output implemented\n", pfx);
-	if (id[64] & (1 << 1))
+	if (id[64] & BIT(1))
 		printf("%s Power level 2 requirement\n", pfx);
-	if (id[64] & (1 << 2))
+	if (id[64] & BIT(2))
 		printf("%s Cooled transceiver implemented\n", pfx);
-	if (id[64] & (1 << 3))
+	if (id[64] & BIT(3))
 		printf("%s Retimer or CDR implemented\n", pfx);
-	if (id[64] & (1 << 4))
+	if (id[64] & BIT(4))
 		printf("%s Paging implemented\n", pfx);
-	if (id[64] & (1 << 5))
+	if (id[64] & BIT(5))
 		printf("%s Power level 3 requirement\n", pfx);
 }
 
@@ -485,7 +485,7 @@ int sff8079_show_all_nl(struct cmd_context *ctx)
 	sff8079_show_all_common(buf);
 
 	/* Finish if A2h page is not present */
-	if (!(buf[92] & (1 << 6)))
+	if (!(buf[92] & BIT(6)))
 		goto out;
 
 	/* Read A2h page */
diff --git a/tse.c b/tse.c
index 8fd2d2304ea8..b21b7c6112c2 100644
--- a/tse.c
+++ b/tse.c
@@ -17,7 +17,7 @@
 int
 bitset(u32 val, int bit)
 {
-	if (val & (1 << bit))
+	if (val & BIT(bit))
 		return 1;
 	return 0;
 }
-- 
2.31.1

[PATCH ethtool v2 11/13] ethtool: fix missing free of memory after failure

[Jesse Brandeburg]

cppcheck warns:
test-common.c:106:2: error: Common realloc mistake: 'block' nulled but not freed upon failure [memleakOnRealloc]
 block = realloc(block, sizeof(*block) + size);
 ^

Fix the issue by storing a local copy of the old pointer and using that
to free the original if the realloc fails, as the manual for realloc()
suggests.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 test-common.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/test-common.c b/test-common.c
index e4dac3298577..b472027140f6 100644
--- a/test-common.c
+++ b/test-common.c
@@ -97,15 +97,18 @@ void test_free(void *ptr)
 
 void *test_realloc(void *ptr, size_t size)
 {
-	struct list_head *block = NULL;
+	struct list_head *block = NULL, *oldblock;
 
 	if (ptr) {
 		block = (struct list_head *)ptr - 1;
 		list_del(block);
 	}
-	block = realloc(block, sizeof(*block) + size);
-	if (!block)
+	oldblock = block;
+	block = realloc(oldblock, sizeof(*oldblock) + size);
+	if (!block) {
+		free(oldblock);
 		return NULL;
+	}
 	list_add(block, &malloc_list);
 	return block + 1;
 }
-- 
2.31.1

Re: [PATCH ethtool v2 11/13] ethtool: fix missing free of memory after failure

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:20PM -0800, Jesse Brandeburg wrote:
> cppcheck warns:
> test-common.c:106:2: error: Common realloc mistake: 'block' nulled but not freed upon failure [memleakOnRealloc]
>  block = realloc(block, sizeof(*block) + size);
>  ^
> 
> Fix the issue by storing a local copy of the old pointer and using that
> to free the original if the realloc fails, as the manual for realloc()
> suggests.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Acked-by: Michal Kubecek <mkubecek@suse.cz>

> ---
>  test-common.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/test-common.c b/test-common.c
> index e4dac3298577..b472027140f6 100644
> --- a/test-common.c
> +++ b/test-common.c
> @@ -97,15 +97,18 @@ void test_free(void *ptr)
>  
>  void *test_realloc(void *ptr, size_t size)
>  {
> -	struct list_head *block = NULL;
> +	struct list_head *block = NULL, *oldblock;
>  
>  	if (ptr) {
>  		block = (struct list_head *)ptr - 1;
>  		list_del(block);
>  	}
> -	block = realloc(block, sizeof(*block) + size);
> -	if (!block)
> +	oldblock = block;
> +	block = realloc(oldblock, sizeof(*oldblock) + size);
> +	if (!block) {
> +		free(oldblock);
>  		return NULL;
> +	}
>  	list_add(block, &malloc_list);
>  	return block + 1;
>  }
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 12/13] ethtool: fix leak of memory after realloc

[Jesse Brandeburg]

cppcheck finds:
netlink/msgbuff.c:63:2: error: Memory leak: nbuff [memleak]
 return 0;
 ^

This is a pretty common problem with realloc() and just requires handling
the return code correctly which makes us refactor to reuse the structure
free/reinit code that already exists in msgbuf_done().

This fixes the code flow by doing the right thing if realloc() succeeds and
if it fails then being sure to free the original memory and replicate the
steps the original code took.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 netlink/msgbuff.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/netlink/msgbuff.c b/netlink/msgbuff.c
index 216f5b946236..a599cab06014 100644
--- a/netlink/msgbuff.c
+++ b/netlink/msgbuff.c
@@ -15,6 +15,20 @@
 
 #define MAX_MSG_SIZE (4 << 20)		/* 4 MB */
 
+/**
+ * msg_done() - destroy a message buffer
+ * @msgbuff: message buffer
+ *
+ * Free the buffer and reset size and remaining size.
+ */
+void msgbuff_done(struct nl_msg_buff *msgbuff)
+{
+	free(msgbuff->buff);
+	msgbuff->buff = NULL;
+	msgbuff->size = 0;
+	msgbuff->left = 0;
+}
+
 /**
  * msgbuff_realloc() - reallocate buffer if needed
  * @msgbuff:  message buffer
@@ -43,19 +57,16 @@ int msgbuff_realloc(struct nl_msg_buff *msgbuff, unsigned int new_size)
 	if (new_size > MAX_MSG_SIZE)
 		return -EMSGSIZE;
 	nbuff = realloc(msgbuff->buff, new_size);
-	if (!nbuff) {
-		msgbuff->buff = NULL;
-		msgbuff->size = 0;
-		msgbuff->left = 0;
-		return -ENOMEM;
-	}
-	if (nbuff != msgbuff->buff) {
+	if (nbuff) {
 		if (new_size > old_size)
 			memset(nbuff + old_size, '\0', new_size - old_size);
 		msgbuff->nlhdr = (struct nlmsghdr *)(nbuff + nlhdr_off);
 		msgbuff->genlhdr = (struct genlmsghdr *)(nbuff + genlhdr_off);
 		msgbuff->payload = nbuff + payload_off;
 		msgbuff->buff = nbuff;
+	} else {
+		msgbuff_done(msgbuff);
+		return -ENOMEM;
 	}
 	msgbuff->size = new_size;
 	msgbuff->left += (new_size - old_size);
@@ -240,17 +251,3 @@ void msgbuff_init(struct nl_msg_buff *msgbuff)
 {
 	memset(msgbuff, '\0', sizeof(*msgbuff));
 }
-
-/**
- * msg_done() - destroy a message buffer
- * @msgbuff: message buffer
- *
- * Free the buffer and reset size and remaining size.
- */
-void msgbuff_done(struct nl_msg_buff *msgbuff)
-{
-	free(msgbuff->buff);
-	msgbuff->buff = NULL;
-	msgbuff->size = 0;
-	msgbuff->left = 0;
-}
-- 
2.31.1

Re: [PATCH ethtool v2 12/13] ethtool: fix leak of memory after realloc

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:21PM -0800, Jesse Brandeburg wrote:
> cppcheck finds:
> netlink/msgbuff.c:63:2: error: Memory leak: nbuff [memleak]
>  return 0;
>  ^

This reported error is misleading, the commit message should make it
clear that the leak it addresses is not of nbuff but of the original
msgbuff->buff (on failure).

> This is a pretty common problem with realloc() and just requires handling
> the return code correctly which makes us refactor to reuse the structure
> free/reinit code that already exists in msgbuf_done().
> 
> This fixes the code flow by doing the right thing if realloc() succeeds and
> if it fails then being sure to free the original memory and replicate the
> steps the original code took.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  netlink/msgbuff.c | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/netlink/msgbuff.c b/netlink/msgbuff.c
> index 216f5b946236..a599cab06014 100644
> --- a/netlink/msgbuff.c
> +++ b/netlink/msgbuff.c
[...]
> @@ -43,19 +57,16 @@ int msgbuff_realloc(struct nl_msg_buff *msgbuff, unsigned int new_size)
>  	if (new_size > MAX_MSG_SIZE)
>  		return -EMSGSIZE;
>  	nbuff = realloc(msgbuff->buff, new_size);
> -	if (!nbuff) {
> -		msgbuff->buff = NULL;
> -		msgbuff->size = 0;
> -		msgbuff->left = 0;
> -		return -ENOMEM;
> -	}
> -	if (nbuff != msgbuff->buff) {
> +	if (nbuff) {
>  		if (new_size > old_size)
>  			memset(nbuff + old_size, '\0', new_size - old_size);
>  		msgbuff->nlhdr = (struct nlmsghdr *)(nbuff + nlhdr_off);
>  		msgbuff->genlhdr = (struct genlmsghdr *)(nbuff + genlhdr_off);
>  		msgbuff->payload = nbuff + payload_off;
>  		msgbuff->buff = nbuff;
> +	} else {
> +		msgbuff_done(msgbuff);
> +		return -ENOMEM;
>  	}
>  	msgbuff->size = new_size;
>  	msgbuff->left += (new_size - old_size);

If nbuff is null (reallocation failed), we bail out anyway so there is
no point putting part of the followin code inside an if and leaving the
rest outside. Also, while it makes sense to zero the new part of the
buffer even if the buffer is not moved, there is no reason to
reinitialize all the pointers. How about this:

	if (!nbuff) {
		msgbuff_done(msgbuff);
		return -ENOMEM;
	}
	if (new_size > old_size)
		memset(nbuff + old_size, '\0', new_size - old_size);
	if (nbuff != msgbuff->buff) {
		msgbuff->nlhdr = (struct nlmsghdr *)(nbuff + nlhdr_off);
		msgbuff->genlhdr = (struct genlmsghdr *)(nbuff + genlhdr_off);
		msgbuff->payload = nbuff + payload_off;
		msgbuff->buff = nbuff;
	}
	msgbuff->size = new_size;
	msgbuff->left += (new_size - old_size);

Or we could forget about the "if (nbuff != msgbuff->buff)" test and
assign the four pointers to values they already have in case the block
stays in place. The code would look a bit tidier and the difference
would be negligible.

And looking at the code again, the "new_size > old_size" check can be
omitted too as we already have

	if (new_size <= old_size)
		return 0;

before the realloc() call. This is a relic from a development version
where realloc() was also used to shrink the buffer if new requested size
was smaller than current.


Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 13/13] ethtool: fix bug and use standard string parsing

[Jesse Brandeburg]

The parse_reset function was open-coding string parsing routines and can
be converted to using standard routines. It was trying to be tricky and
parse partial strings to make things like "mgmt" and "mgmt-shared"
strings mostly use the same code. This is better done with strncmp().

This also fixes an array overflow bug where it's possible for the for
loop to access past the end of the input array, which is bad.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 ethtool.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 4776afe89e23..d294b5f8d92a 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5280,17 +5280,21 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
 static __u32 parse_reset(char *val, __u32 bitset, char *arg, __u32 *data)
 {
 	__u32 bitval = 0;
-	int i;
+	int vallen = strlen(val);
+	int strret;
 
 	/* Check for component match */
-	for (i = 0; val[i] != '\0'; i++)
-		if (arg[i] != val[i])
-			return 0;
+	strret = strncmp(arg, val, vallen);
+	if (strret < 0)
+		return 0;
 
-	/* Check if component has -shared specified or not */
-	if (arg[i] == '\0')
+	/* if perfect match to val
+	 * else
+	 * Check if component has -shared specified or not
+	 */
+	if (strret == 0)
 		bitval = bitset;
-	else if (!strcmp(arg+i, "-shared"))
+	else if (!strcmp(arg + vallen, "-shared"))
 		bitval = bitset << ETH_RESET_SHARED_SHIFT;
 
 	if (bitval) {
-- 
2.31.1

Re: [PATCH ethtool v2 13/13] ethtool: fix bug and use standard string parsing

[Michal Kubecek]

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

On Wed, Dec 07, 2022 at 05:11:22PM -0800, Jesse Brandeburg wrote:
> The parse_reset function was open-coding string parsing routines and can
> be converted to using standard routines. It was trying to be tricky and
> parse partial strings to make things like "mgmt" and "mgmt-shared"
> strings mostly use the same code. This is better done with strncmp().
> 
> This also fixes an array overflow bug where it's possible for the for
> loop to access past the end of the input array, which is bad.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  ethtool.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 4776afe89e23..d294b5f8d92a 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -5280,17 +5280,21 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
>  static __u32 parse_reset(char *val, __u32 bitset, char *arg, __u32 *data)
>  {
>  	__u32 bitval = 0;
> -	int i;
> +	int vallen = strlen(val);
> +	int strret;
>  
>  	/* Check for component match */
> -	for (i = 0; val[i] != '\0'; i++)
> -		if (arg[i] != val[i])
> -			return 0;
> +	strret = strncmp(arg, val, vallen);
> +	if (strret < 0)
> +		return 0;

Shouldn't this be "if (strret)"? Unless I missed something, this would
lead to e.g. "zzzz-shared" being matched for "mgmt-shared" (with usual
locale) because you would get strret > 0 here and strcmp() == 0 below.

>  
> -	/* Check if component has -shared specified or not */
> -	if (arg[i] == '\0')
> +	/* if perfect match to val
> +	 * else
> +	 * Check if component has -shared specified or not
> +	 */
> +	if (strret == 0)
>  		bitval = bitset;

And this should be "if (!arg[vallen])", I believe.

Michal

> -	else if (!strcmp(arg+i, "-shared"))
> +	else if (!strcmp(arg + vallen, "-shared"))
>  		bitval = bitset << ETH_RESET_SHARED_SHIFT;
>  
>  	if (bitval) {
> -- 
> 2.31.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]