UAPI: net/ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings

PATCH Applied Gustavo A. R. Silva 6 participants 13 messages Oct 29, 2024 – Dec 12, 2024 3 messages not in the archive

Subsystems (9)

Networking Network drivers Traffic control (tc) Networking ethtool +5 more subsystems

lore

Share & tools

lore

Gustavo A. R. Silva

author

2024-10-29 21:55 UTC #0 lore raw

v2 cover UAPI: net/ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings

Small patch series aimed at fixing thousands of -Wflex-array-member-not-at-end warnings by creating a new tagged struct within a flexible structure. We then use this new struct type to fix problematic middle-flex-array declarations in multiple composite structs, as well as to update the type of some variables in various functions.

Changes in v2:
- Update changelog text in patch 2/2 to better reflect the changes made. (Jakub)
- Adjust variable declarations to follow the reverse xmas tree convention. (Jakub)

v1:
- Link: https://lore.kernel.org/linux-hardening/[email protected]

Gustavo A. R. Silva (2):
UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings
net: ethtool: Avoid thousands of -Wflex-array-member-not-at-end
warnings

 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  6 ++--
 .../ethernet/chelsio/cxgb4/cxgb4_ethtool.c    |  4 +--
 .../ethernet/chelsio/cxgb4vf/cxgb4vf_main.c   |  2 +-
 .../net/ethernet/cisco/enic/enic_ethtool.c    |  2 +-
 .../net/ethernet/qlogic/qede/qede_ethtool.c   |  4 +--
 include/linux/ethtool.h                       |  2 +-
 include/uapi/linux/ethtool.h                  | 33 ++++++++++---------
 net/ethtool/ioctl.c                           |  2 +-
 net/ethtool/linkinfo.c                        |  8 ++---
 net/ethtool/linkmodes.c                       | 18 ++++++----
 10 files changed, 44 insertions(+), 37 deletions(-)

Signature · 1 line

2.43.0

Gustavo A. R. Silva

author

2024-10-29 21:55 UTC #1 lore raw

PATCH v2 1/2 UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.

This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].

[1] https://git.kernel.org/linus/d88cabfd9abc

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- None.

v1:
- Link: https://lore.kernel.org/linux-hardening/e9ccb0cd7e490bfa270a7c20979e16ff84ac91e2.1729536776.git.gustavoars@kernel.org/

 include/uapi/linux/ethtool.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

include/uapi/linux/ethtool.h +18 ~~-15~~

index c405ed63acfa..fc1f54b065f9 100644
@@ -2511,21 +2511,24 @@ enum ethtool_reset_flags {
  *	autonegotiation; 0 if unknown or not applicable.  Read-only.
  */
 struct ethtool_link_settings {
-	__u32	cmd;
-	__u32	speed;
-	__u8	duplex;
-	__u8	port;
-	__u8	phy_address;
-	__u8	autoneg;
-	__u8	mdio_support;
-	__u8	eth_tp_mdix;
-	__u8	eth_tp_mdix_ctrl;
-	__s8	link_mode_masks_nwords;
-	__u8	transceiver;
-	__u8	master_slave_cfg;
-	__u8	master_slave_state;
-	__u8	rate_matching;
-	__u32	reserved[7];
+	/* New members MUST be added within the __struct_group() macro below. */
+	__struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
+		__u32	cmd;
+		__u32	speed;
+		__u8	duplex;
+		__u8	port;
+		__u8	phy_address;
+		__u8	autoneg;
+		__u8	mdio_support;
+		__u8	eth_tp_mdix;
+		__u8	eth_tp_mdix_ctrl;
+		__s8	link_mode_masks_nwords;
+		__u8	transceiver;
+		__u8	master_slave_cfg;
+		__u8	master_slave_state;
+		__u8	rate_matching;
+		__u32	reserved[7];
+	);
 	__u32	link_mode_masks[];
 	/* layout of link_mode_masks fields:
 	 * __u32 map_supported[link_mode_masks_nwords];

Signature · 1 line

2.43.0

Jakub Kicinski

maintainer

2024-11-09 18:02 UTC #2 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 9 lines · On Tue, 29 Oct 2024 15:55:35 -0600 Gustavo A. R. Silva wrote:

Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.
This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].

Possibly a very noob question, but I'm updating a C++ library with new headers and I think this makes it no longer compile.

$ cat > /tmp/t.cpp<<EOF
extern "C" {
#include "include/uapi/linux/ethtool.h"
}
int func() { return 0; }
EOF

$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
In file included from /usr/include/linux/posix_types.h:5,
                 from /usr/include/linux/types.h:9,
                 from ../linux/include/uapi/linux/ethtool.h:18,
                 from /tmp/t.cpp:2:
../linux/include/uapi/linux/ethtool.h:2515:24: error: ‘struct ethtool_link_settings::<unnamed union>::ethtool_link_settings_hdr’ invalid; an anonymous union may only have public non-static data members [-fpermissive]
 2515 |         __struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~

I don't know much about C++, tho, so quite possibly missing something obvious.

Jakub Kicinski

maintainer

2024-11-09 18:49 UTC #3 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 1 line · On Sat, 9 Nov 2024 10:02:13 -0800 Jakub Kicinski wrote:

$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2

gcc version 14.2.1 20240912 (Red Hat 14.2.1-3) (GCC)

Gustavo A. R. Silva

2024-11-11 22:23 UTC #4 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

In reply to Jakub Kicinski

Quoted 31 lines · Jakub Kicinski

Quoted 9 lines · On Tue, 29 Oct 2024 15:55:35 -0600 Gustavo A. R. Silva wrote:
Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.
This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].
Possibly a very noob question, but I'm updating a C++ library with new headers and I think this makes it no longer compile.
$ cat > /tmp/t.cpp<<EOF
extern "C" {
#include "include/uapi/linux/ethtool.h"
}
int func() { return 0; }
EOF
$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
In file included from /usr/include/linux/posix_types.h:5,
                  from /usr/include/linux/types.h:9,
                  from ../linux/include/uapi/linux/ethtool.h:18,
                  from /tmp/t.cpp:2:
../linux/include/uapi/linux/ethtool.h:2515:24: error: ‘struct ethtool_link_settings::<unnamed union>::ethtool_link_settings_hdr’ invalid; an anonymous union may only have public non-static data members [-fpermissive]
  2515 |         __struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~

Quoted 2 lines

I don't know much about C++, tho, so quite possibly missing something obvious.

We are in the same situation here.

It seems C++ considers it ambiguous to define a struct with a tag such as struct TAG { MEMBERS } ATTRS NAME; within an anonymous union.

Let me look into this further...
--
Gustavo

Gustavo A. R. Silva

2024-11-13 01:08 UTC #5 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 24 lines · Gustavo A. R. Silva

Quoted 21 lines · Jakub Kicinski
Quoted 9 lines · On Tue, 29 Oct 2024 15:55:35 -0600 Gustavo A. R. Silva wrote:
Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.
This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].
Possibly a very noob question, but I'm updating a C++ library with new headers and I think this makes it no longer compile.
$ cat > /tmp/t.cpp<<EOF
extern "C" {
#include "include/uapi/linux/ethtool.h"
}
int func() { return 0; }
EOF

Quoted 10 lines

$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
In file included from /usr/include/linux/posix_types.h:5,
                  from /usr/include/linux/types.h:9,
                  from ../linux/include/uapi/linux/ethtool.h:18,
                  from /tmp/t.cpp:2:
../linux/include/uapi/linux/ethtool.h:2515:24: error: ‘struct ethtool_link_settings::<unnamed union>::ethtool_link_settings_hdr’ invalid; an anonymous union may only have public non-static data members [-fpermissive]
  2515 |         __struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~

This seems to work with Clang:

$ clang++-18 -fms-extensions /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2

However, -fms-extensions doesn't seem to work for this case with GCC:

https://godbolt.org/z/1shsPhz3s

-Gustavo

Quoted 12 lines

Quoted 2 lines
I don't know much about C++, tho, so quite possibly missing something obvious.
We are in the same situation here.
It seems C++ considers it ambiguous to define a struct with a tag such as struct TAG { MEMBERS } ATTRS NAME; within an anonymous union.
Let me look into this further...
Signature · 1 line
Gustavo

Kees Cook

2024-11-15 20:38 UTC #6 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 41 lines · Gustavo A. R. Silva

Quoted 24 lines · Gustavo A. R. Silva
Quoted 21 lines · Jakub Kicinski
Quoted 9 lines · On Tue, 29 Oct 2024 15:55:35 -0600 Gustavo A. R. Silva wrote:
Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.
This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].
Possibly a very noob question, but I'm updating a C++ library with new headers and I think this makes it no longer compile.
$ cat > /tmp/t.cpp<<EOF
extern "C" {
#include "include/uapi/linux/ethtool.h"
}
int func() { return 0; }
EOF
Quoted 13 lines
Quoted 13 lines
$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
In file included from /usr/include/linux/posix_types.h:5,
                  from /usr/include/linux/types.h:9,
                  from ../linux/include/uapi/linux/ethtool.h:18,
                  from /tmp/t.cpp:2:
../linux/include/uapi/linux/ethtool.h:2515:24: error: ‘struct
ethtool_link_settings::<unnamed union>::ethtool_link_settings_hdr’
invalid; an anonymous union may only have public non-static data
members [-fpermissive]
  2515 |         __struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~

Quoted 7 lines

This seems to work with Clang:
$ clang++-18 -fms-extensions /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
However, -fms-extensions doesn't seem to work for this case with GCC:
https://godbolt.org/z/1shsPhz3s

Hm, we can't break UAPI even for C++, so even if we had compiler options that would make it work, it's really unfriendly to userspace to make all the projects there suddenly start needing to use it.

I think this means we just can't use tagged struct groups in UAPI. :(

I have what I think is a much simpler solution. Sending it now...

-Kees

Signature · 1 line

Kees Cook

Nick Desaulniers

2024-12-05 16:49 UTC #7 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 50 lines · Kees Cook

Quoted 41 lines · Gustavo A. R. Silva
Quoted 24 lines · Gustavo A. R. Silva
Quoted 21 lines · Jakub Kicinski
Quoted 9 lines · On Tue, 29 Oct 2024 15:55:35 -0600 Gustavo A. R. Silva wrote:
Use the __struct_group() helper to create a new tagged struct ethtool_link_settings_hdr. This structure groups together all the members of the flexible struct ethtool_link_settings except the flexible array. As a result, the array is effectively separated from the rest of the members without modifying the memory layout of the flexible structure.
This new tagged struct will be used to fix problematic declarations of middle-flex-arrays in composite structs[1].
Possibly a very noob question, but I'm updating a C++ library with new headers and I think this makes it no longer compile.
$ cat > /tmp/t.cpp<<EOF
extern "C" {
#include "include/uapi/linux/ethtool.h"
}
int func() { return 0; }
EOF
Quoted 13 lines
Quoted 13 lines
$ g++ /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
In file included from /usr/include/linux/posix_types.h:5,
                  from /usr/include/linux/types.h:9,
                  from ../linux/include/uapi/linux/ethtool.h:18,
                  from /tmp/t.cpp:2:
../linux/include/uapi/linux/ethtool.h:2515:24: error: ‘struct
ethtool_link_settings::<unnamed union>::ethtool_link_settings_hdr’
invalid; an anonymous union may only have public non-static data
members [-fpermissive]
  2515 |         __struct_group(ethtool_link_settings_hdr, hdr, /* no attrs */,
       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 7 lines
This seems to work with Clang:
$ clang++-18 -fms-extensions /tmp/t.cpp -I../linux -o /dev/null -c -W -Wall -O2
However, -fms-extensions doesn't seem to work for this case with GCC:
https://godbolt.org/z/1shsPhz3s

Quoted 7 lines

Hm, we can't break UAPI even for C++, so even if we had compiler options that would make it work, it's really unfriendly to userspace to make all the projects there suddenly start needing to use it.
I think this means we just can't use tagged struct groups in UAPI. :(
I have what I think is a much simpler solution. Sending it now...

What was the follow up?

cferris@ is reporting something similar for linux/uapi/linux/pkt_cls.h in the structure tc_u32_sel definition, found while updating the kernel headers in Android. Maybe that file/definition needs the same follow up?

~Nick

A message that is not in the archive (sent off-list or not yet received): CANtHk4mnjE5aATk2r8uOsyLKm+7-tbEv5AaXVWGP_unhLNEvsg@mail.gmail.com

Jakub Kicinski

maintainer

2024-12-09 21:10 UTC #9 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 4 lines · On Mon, 9 Dec 2024 12:59:40 -0800 Christopher Ferris wrote:

It looks like the way this was fixed in the ethtool.h uapi header was to revert the usage of __struct_group. Should something similar happen for pkt_cls.h? Or would it be easier to simply remove the usage of the TAG in the _struct_group macro?

Just to state it explicitly - are you running into a compilation issue with existing user space after updating pkt_cls.h?

A message that is not in the archive (sent off-list or not yet received): CANtHk4kM-9BDCm69+z3hS58uCrjCmma0aQ+nOqFUROaFhLAkDg@mail.gmail.com

Gustavo A. R. Silva

2024-12-09 21:39 UTC #11 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 5 lines · Christopher Ferris

Yes, when compiling Android, we have a C++ file that includes the pkt_cls.h directly to get access to some of the structures from that file. It currently gets the "types cannot be declared in an anonymous union" error due to the TAG part of the __struct_group usage not being empty in that file.

(sigh) this should be reverted:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9c60712d71ff07197b2982899b9db28ed548ded

--
Gustavo

Quoted 15 lines

Christopher
Quoted 9 lines · Jakub Kicinski
Quoted 4 lines · On Mon, 9 Dec 2024 12:59:40 -0800 Christopher Ferris wrote:
It looks like the way this was fixed in the ethtool.h uapi header was to revert the usage of __struct_group. Should something similar happen for pkt_cls.h? Or would it be easier to simply remove the usage of the TAG in the _struct_group macro?
Just to state it explicitly - are you running into a compilation issue with existing user space after updating pkt_cls.h?

A message that is not in the archive (sent off-list or not yet received): CANtHk4nyP8HyYMobB76z9LpbA_jD=fLkWtyK9w_aMkzP8iB7Cg@mail.gmail.com

Kees Cook

2024-12-12 17:29 UTC #13 lore raw

Re: [PATCH v2 1/2][next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

Quoted 3 lines · Christopher Ferris

By the way, there are some places where __struct_group is used in other uapi headers, the only difference is that the TAG field of the macro is left empty. That compiles fine when used in C++ code.

Does this fix the C++ inclusion for you?

include/uapi/linux/pkt_cls.h +20 ~~-13~~

index 2c32080416b5..aeff841c528d 100644
@@ -245,20 +245,27 @@ struct tc_u32_key {
 	int		offmask;
 };
 
+#define tc_u32_sel_hdr_members			\
+	unsigned char		flags;		\
+	unsigned char		offshift;	\
+	unsigned char		nkeys;		\
+	__be16			offmask;	\
+	__u16			off;		\
+	short			offoff;		\
+	short			hoff;
+
+struct tc_u32_sel_hdr {
+	tc_u32_sel_hdr_members
+};
+
 struct tc_u32_sel {
-	/* New members MUST be added within the __struct_group() macro below. */
-	__struct_group(tc_u32_sel_hdr, hdr, /* no attrs */,
-		unsigned char		flags;
-		unsigned char		offshift;
-		unsigned char		nkeys;
-
-		__be16			offmask;
-		__u16			off;
-		short			offoff;
-
-		short			hoff;
-		__be32			hmask;
-	);
+	/* Open-coded struct_group() to avoid C++ errors. */
+	union {
+		struct tc_u32_sel_hdr hdr;
+		struct {
+			tc_u32_sel_hdr_members
+		};
+	};
 	struct tc_u32_key	keys[];
 };

Signature · 1 line

Kees Cook

Gustavo A. R. Silva

author

2024-10-29 21:58 UTC #14 lore raw

PATCH v2 2/2 net: ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings

In reply to Gustavo A. R. Silva

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally.

Change the type of the middle struct member currently causing trouble from struct ethtool_link_settings to struct ethtool_link_settings_hdr.

Additionally, update the type of some variables in various functions that don't access the flexible-array member, changing them to the newly created struct ethtool_link_settings_hdr. These changes are needed because the type of the conflicting middle members changed. So, those instances that expect the type to be struct ethtool_link_settings should be adjusted to the newly created type struct ethtool_link_settings_hdr.

Also, adjust variable declarations to follow the reverse xmas tree convention.

Fix 3338 of the following -Wflex-array-member-not-at-end warnings:

include/linux/ethtool.h:214:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Update changelog text in patch to better reflect the changes made. (Jakub)
- Adjust variable declarations to follow the reverse xmas tree convention. (Jakub)

v1:
- Link: https://lore.kernel.org/linux-hardening/f4f8ca5cd7f039bcab816194342c7b6101e891fe.1729536776.git.gustavoars@kernel.org/

 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c  |  6 +++---
 .../net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c |  4 ++--
 .../ethernet/chelsio/cxgb4vf/cxgb4vf_main.c    |  2 +-
 drivers/net/ethernet/cisco/enic/enic_ethtool.c |  2 +-
 .../net/ethernet/qlogic/qede/qede_ethtool.c    |  4 ++--
 include/linux/ethtool.h                        |  2 +-
 net/ethtool/ioctl.c                            |  2 +-
 net/ethtool/linkinfo.c                         |  8 ++++----
 net/ethtool/linkmodes.c                        | 18 +++++++++++-------
 9 files changed, 26 insertions(+), 22 deletions(-)

drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +3 -3

index f71cc8188b4e..e0ebe69110bf 100644
@@ -2781,7 +2781,7 @@ u32 bnxt_fw_to_ethtool_speed(u16 fw_link_speed)
 static void bnxt_get_default_speeds(struct ethtool_link_ksettings *lk_ksettings,
 				    struct bnxt_link_info *link_info)
 {
-	struct ethtool_link_settings *base = &lk_ksettings->base;
+	struct ethtool_link_settings_hdr *base = &lk_ksettings->base;
 
 	if (link_info->link_state == BNXT_LINK_STATE_UP) {
 		base->speed = bnxt_fw_to_ethtool_speed(link_info->link_speed);

@@ -2800,7 +2800,7 @@ static void bnxt_get_default_speeds(struct ethtool_link_ksettings *lk_ksettings,
 static int bnxt_get_link_ksettings(struct net_device *dev,
 				   struct ethtool_link_ksettings *lk_ksettings)
 {
-	struct ethtool_link_settings *base = &lk_ksettings->base;
+	struct ethtool_link_settings_hdr *base = &lk_ksettings->base;
 	enum ethtool_link_mode_bit_indices link_mode;
 	struct bnxt *bp = netdev_priv(dev);
 	struct bnxt_link_info *link_info;

@@ -3023,9 +3023,9 @@ u16 bnxt_get_fw_auto_link_speeds(const unsigned long *mode)
 static int bnxt_set_link_ksettings(struct net_device *dev,
 			   const struct ethtool_link_ksettings *lk_ksettings)
 {
+	const struct ethtool_link_settings_hdr *base = &lk_ksettings->base;
 	struct bnxt *bp = netdev_priv(dev);
 	struct bnxt_link_info *link_info = &bp->link_info;
-	const struct ethtool_link_settings *base = &lk_ksettings->base;
 	bool set_pause = false;
 	u32 speed, lanes = 0;
 	int rc = 0;

drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c +2 -2

index 7f3f5afa864f..45d28a65347e 100644
@@ -662,8 +662,8 @@ static unsigned int lmm_to_fw_caps(const unsigned long *link_mode_mask)
 static int get_link_ksettings(struct net_device *dev,
 			      struct ethtool_link_ksettings *link_ksettings)
 {
+	struct ethtool_link_settings_hdr *base = &link_ksettings->base;
 	struct port_info *pi = netdev_priv(dev);
-	struct ethtool_link_settings *base = &link_ksettings->base;
 
 	/* For the nonce, the Firmware doesn't send up Port State changes
 	 * when the Virtual Interface attached to the Port is down.  So

@@ -717,9 +717,9 @@ static int get_link_ksettings(struct net_device *dev,
 static int set_link_ksettings(struct net_device *dev,
 			    const struct ethtool_link_ksettings *link_ksettings)
 {
+	const struct ethtool_link_settings_hdr *base = &link_ksettings->base;
 	struct port_info *pi = netdev_priv(dev);
 	struct link_config *lc = &pi->link_cfg;
-	const struct ethtool_link_settings *base = &link_ksettings->base;
 	struct link_config old_lc;
 	unsigned int fw_caps;
 	int ret = 0;

drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c +1 -1

index 2fbe0f059a0b..61d08547e3f9 100644
@@ -1436,8 +1436,8 @@ static void fw_caps_to_lmm(enum fw_port_type port_type,
 static int cxgb4vf_get_link_ksettings(struct net_device *dev,
 				  struct ethtool_link_ksettings *link_ksettings)
 {
+	struct ethtool_link_settings_hdr *base = &link_ksettings->base;
 	struct port_info *pi = netdev_priv(dev);
-	struct ethtool_link_settings *base = &link_ksettings->base;
 
 	/* For the nonce, the Firmware doesn't send up Port State changes
 	 * when the Virtual Interface attached to the Port is down.  So

drivers/net/ethernet/cisco/enic/enic_ethtool.c +1 -1

index f7986f2b6a17..4fe85780a950 100644
@@ -129,8 +129,8 @@ static void enic_intr_coal_set_rx(struct enic *enic, u32 timer)
 static int enic_get_ksettings(struct net_device *netdev,
 			      struct ethtool_link_ksettings *ecmd)
 {
+	struct ethtool_link_settings_hdr *base = &ecmd->base;
 	struct enic *enic = netdev_priv(netdev);
-	struct ethtool_link_settings *base = &ecmd->base;
 
 	ethtool_link_ksettings_add_link_mode(ecmd, supported,
 					     10000baseT_Full);

drivers/net/ethernet/qlogic/qede/qede_ethtool.c +2 -2

index 97b059be1041..24ff154285ac 100644
@@ -508,7 +508,7 @@ static int qede_get_link_ksettings(struct net_device *dev,
 				   struct ethtool_link_ksettings *cmd)
 {
 	typeof(cmd->link_modes) *link_modes = &cmd->link_modes;
-	struct ethtool_link_settings *base = &cmd->base;
+	struct ethtool_link_settings_hdr *base = &cmd->base;
 	struct qede_dev *edev = netdev_priv(dev);
 	struct qed_link_output current_link;

@@ -541,7 +541,7 @@ static int qede_get_link_ksettings(struct net_device *dev,
 static int qede_set_link_ksettings(struct net_device *dev,
 				   const struct ethtool_link_ksettings *cmd)
 {
-	const struct ethtool_link_settings *base = &cmd->base;
+	const struct ethtool_link_settings_hdr *base = &cmd->base;
 	const struct ethtool_forced_speed_map *map;
 	struct qede_dev *edev = netdev_priv(dev);
 	struct qed_link_output current_link;

include/linux/ethtool.h +1 -1

index 12f6dc567598..1199e308c8dd 100644
@@ -211,7 +211,7 @@ void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id);
  * fields, but they are allowed to overwrite them (will be ignored).
  */
 struct ethtool_link_ksettings {
-	struct ethtool_link_settings base;
+	struct ethtool_link_settings_hdr base;
 	struct {
 		__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
 		__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);

net/ethtool/ioctl.c +1 -1

index 5cc131cdb1bc..7da94e26ced6 100644
@@ -425,7 +425,7 @@ convert_link_ksettings_to_legacy_settings(
 
 /* layout of the struct passed from/to userland */
 struct ethtool_link_usettings {
-	struct ethtool_link_settings base;
+	struct ethtool_link_settings_hdr base;
 	struct {
 		__u32 supported[__ETHTOOL_LINK_MODE_MASK_NU32];
 		__u32 advertising[__ETHTOOL_LINK_MODE_MASK_NU32];

net/ethtool/linkinfo.c +4 -4

index 30b8ce275159..2d5bc57160be 100644
@@ -8,9 +8,9 @@ struct linkinfo_req_info {
 };
 
 struct linkinfo_reply_data {
-	struct ethnl_reply_data		base;
-	struct ethtool_link_ksettings	ksettings;
-	struct ethtool_link_settings	*lsettings;
+	struct ethnl_reply_data			base;
+	struct ethtool_link_ksettings		ksettings;
+	struct ethtool_link_settings_hdr	*lsettings;
 };
 
 #define LINKINFO_REPDATA(__reply_base) \

@@ -98,7 +98,7 @@ static int
 ethnl_set_linkinfo(struct ethnl_req_info *req_info, struct genl_info *info)
 {
 	struct ethtool_link_ksettings ksettings = {};
-	struct ethtool_link_settings *lsettings;
+	struct ethtool_link_settings_hdr *lsettings;
 	struct net_device *dev = req_info->dev;
 	struct nlattr **tb = info->attrs;
 	bool mod = false;

net/ethtool/linkmodes.c +11 -7

index 259cd9ef1f2a..17e49cf89f03 100644
@@ -11,10 +11,10 @@ struct linkmodes_req_info {
 };
 
 struct linkmodes_reply_data {
-	struct ethnl_reply_data		base;
-	struct ethtool_link_ksettings	ksettings;
-	struct ethtool_link_settings	*lsettings;
-	bool				peer_empty;
+	struct ethnl_reply_data			base;
+	struct ethtool_link_ksettings		ksettings;
+	struct ethtool_link_settings_hdr	*lsettings;
+	bool					peer_empty;
 };
 
 #define LINKMODES_REPDATA(__reply_base) \

@@ -62,10 +62,12 @@ static int linkmodes_reply_size(const struct ethnl_req_info *req_base,
 {
 	const struct linkmodes_reply_data *data = LINKMODES_REPDATA(reply_base);
 	const struct ethtool_link_ksettings *ksettings = &data->ksettings;
-	const struct ethtool_link_settings *lsettings = &ksettings->base;
 	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+	const struct ethtool_link_settings_hdr *lsettings;
 	int len, ret;
 
+	lsettings = &ksettings->base;
+
 	len = nla_total_size(sizeof(u8)) /* LINKMODES_AUTONEG */
 		+ nla_total_size(sizeof(u32)) /* LINKMODES_SPEED */
 		+ nla_total_size(sizeof(u32)) /* LINKMODES_LANES */

@@ -103,10 +105,12 @@ static int linkmodes_fill_reply(struct sk_buff *skb,
 {
 	const struct linkmodes_reply_data *data = LINKMODES_REPDATA(reply_base);
 	const struct ethtool_link_ksettings *ksettings = &data->ksettings;
-	const struct ethtool_link_settings *lsettings = &ksettings->base;
 	bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+	const struct ethtool_link_settings_hdr *lsettings;
 	int ret;
 
+	lsettings = &ksettings->base;
+
 	if (nla_put_u8(skb, ETHTOOL_A_LINKMODES_AUTONEG, lsettings->autoneg))
 		return -EMSGSIZE;

@@ -237,7 +241,7 @@ static int ethnl_update_linkmodes(struct genl_info *info, struct nlattr **tb,
 				  struct ethtool_link_ksettings *ksettings,
 				  bool *mod, const struct net_device *dev)
 {
-	struct ethtool_link_settings *lsettings = &ksettings->base;
+	struct ethtool_link_settings_hdr *lsettings = &ksettings->base;
 	bool req_speed, req_lanes, req_duplex;
 	const struct nlattr *master_slave_cfg, *lanes_cfg;
 	int ret;

Signature · 1 line

2.43.0

[email protected]

CI Applied

2024-11-03 19:20 UTC #15 lore raw

In reply to Gustavo A. R. Silva

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 6 lines · On Tue, 29 Oct 2024 15:55:02 -0600 you wrote:

Small patch series aimed at fixing thousands of -Wflex-array-member-not-at-end warnings by creating a new tagged struct within a flexible structure. We then use this new struct type to fix problematic middle-flex-array declarations in multiple composite structs, as well as to update the type of some variables in various functions.

Quoted 7 lines

Changes in v2:
- Update changelog text in patch 2/2 to better reflect the changes made. (Jakub)
- Adjust variable declarations to follow the reverse xmas tree convention. (Jakub)
[...]

Here is the summary with links:
- [v2,1/2,next] UAPI: ethtool: Use __struct_group() in struct ethtool_link_settings

    https://git.kernel.org/netdev/net-next/c/43d3487035e9
  - [v2,2/2,next] net: ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings
    https://git.kernel.org/netdev/net-next/c/3bd9b9abdf15

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html