diff mbox series

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

Message ID e9ccb0cd7e490bfa270a7c20979e16ff84ac91e2.1729536776.git.gustavoars@kernel.org (mailing list archive)
State New
Headers show
Series UAPI: net/ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 10 this patch: 10
netdev/build_tools success Errors and warnings before: 157 (+1) this patch: 157 (+1)
netdev/cc_maintainers fail 1 maintainers not CCed: kory.maincent@bootlin.com
netdev/build_clang success Errors and warnings before: 55 this patch: 55
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1685 this patch: 1685
netdev/checkpatch warning CHECK: Alignment should match open parenthesis
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-10-22--00-00 (tests: 765)

Commit Message

Gustavo A. R. Silva Oct. 21, 2024, 7:01 p.m. UTC
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 <gustavoars@kernel.org>
---
 include/uapi/linux/ethtool.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

Comments

Andrew Lunn Oct. 21, 2024, 8:11 p.m. UTC | #1
>  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[];

Dumb C question. What are the padding rules for a union, compared to
base types? Do we know for sure the compiler is not going pad this
structure differently because of the union?

It is however nicely constructed. The 12 __u8 making 3 32bit words, so
we have a total of 12 32bit words, or 6 64bit words, before the
link_mode_masks[], so i don't think padding is technically an issue,
but it would be nice to know the C standard guarantees this.

	Andrew
diff mbox series

Patch

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index c405ed63acfa..fc1f54b065f9 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -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];