[PATCH][next] net/smc: Use static_assert() to check struct sizes

[PATCH][next] net/smc: Use static_assert() to check struct sizes

[Gustavo A. R. Silva]

Commit 9748dbc9f265 ("net/smc: Avoid -Wflex-array-member-not-at-end
warnings") introduced tagged `struct smc_clc_v2_extension_fixed` and
`struct smc_clc_smcd_v2_extension_fixed`. We want to ensure that when
new members need to be added to the flexible structures, they are
always included within these tagged structs.

So, we use `static_assert()` to ensure that the memory layout for
both the flexible structure and the tagged struct is the same after
any changes.

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 net/smc/smc_clc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 467effb50cd6..5625fda2960b 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -145,6 +145,8 @@ struct smc_clc_v2_extension {
 	);
 	u8 user_eids[][SMC_MAX_EID_LEN];
 };
+static_assert(offsetof(struct smc_clc_v2_extension, user_eids) == sizeof(struct smc_clc_v2_extension_fixed),
+	      "struct member likely outside of struct_group_tagged()");
 
 struct smc_clc_msg_proposal_prefix {	/* prefix part of clc proposal message*/
 	__be32 outgoing_subnet;	/* subnet mask */
@@ -169,6 +171,8 @@ struct smc_clc_smcd_v2_extension {
 	);
 	struct smc_clc_smcd_gid_chid gidchid[];
 };
+static_assert(offsetof(struct smc_clc_smcd_v2_extension, gidchid) == sizeof(struct smc_clc_smcd_v2_extension_fixed),
+	      "struct member likely outside of struct_group_tagged()");
 
 struct smc_clc_msg_proposal {	/* clc proposal message sent by Linux */
 	struct smc_clc_msg_hdr hdr;
-- 
2.34.1

Re: [PATCH][next] net/smc: Use static_assert() to check struct sizes

[Jan Karcher]

On 09/08/2024 00:07, Gustavo A. R. Silva wrote:
> Commit 9748dbc9f265 ("net/smc: Avoid -Wflex-array-member-not-at-end
> warnings") introduced tagged `struct smc_clc_v2_extension_fixed` and
> `struct smc_clc_smcd_v2_extension_fixed`. We want to ensure that when
> new members need to be added to the flexible structures, they are
> always included within these tagged structs.
> 
> So, we use `static_assert()` to ensure that the memory layout for
> both the flexible structure and the tagged struct is the same after
> any changes.

Hi Gustavo,

good catch. By reviewing it, it makes sense to me. Please let me give it 
a run with our test-suite before adding a r-b.

Thanks
- J

> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   net/smc/smc_clc.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 467effb50cd6..5625fda2960b 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -145,6 +145,8 @@ struct smc_clc_v2_extension {
>   	);
>   	u8 user_eids[][SMC_MAX_EID_LEN];
>   };
> +static_assert(offsetof(struct smc_clc_v2_extension, user_eids) == sizeof(struct smc_clc_v2_extension_fixed),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   struct smc_clc_msg_proposal_prefix {	/* prefix part of clc proposal message*/
>   	__be32 outgoing_subnet;	/* subnet mask */
> @@ -169,6 +171,8 @@ struct smc_clc_smcd_v2_extension {
>   	);
>   	struct smc_clc_smcd_gid_chid gidchid[];
>   };
> +static_assert(offsetof(struct smc_clc_smcd_v2_extension, gidchid) == sizeof(struct smc_clc_smcd_v2_extension_fixed),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   struct smc_clc_msg_proposal {	/* clc proposal message sent by Linux */
>   	struct smc_clc_msg_hdr hdr;

Re: [PATCH][next] net/smc: Use static_assert() to check struct sizes

[Jan Karcher]

On 09/08/2024 00:07, Gustavo A. R. Silva wrote:
> Commit 9748dbc9f265 ("net/smc: Avoid -Wflex-array-member-not-at-end
> warnings") introduced tagged `struct smc_clc_v2_extension_fixed` and
> `struct smc_clc_smcd_v2_extension_fixed`. We want to ensure that when
> new members need to be added to the flexible structures, they are
> always included within these tagged structs.
> 
> So, we use `static_assert()` to ensure that the memory layout for
> both the flexible structure and the tagged struct is the same after
> any changes.

Read up what the macro does. I like it.
Compile tested on s390.

Reviewed-by: Jan Karcher <jaka@linux.ibm.com>

> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   net/smc/smc_clc.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 467effb50cd6..5625fda2960b 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -145,6 +145,8 @@ struct smc_clc_v2_extension {
>   	);
>   	u8 user_eids[][SMC_MAX_EID_LEN];
>   };
> +static_assert(offsetof(struct smc_clc_v2_extension, user_eids) == sizeof(struct smc_clc_v2_extension_fixed),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   struct smc_clc_msg_proposal_prefix {	/* prefix part of clc proposal message*/
>   	__be32 outgoing_subnet;	/* subnet mask */
> @@ -169,6 +171,8 @@ struct smc_clc_smcd_v2_extension {
>   	);
>   	struct smc_clc_smcd_gid_chid gidchid[];
>   };
> +static_assert(offsetof(struct smc_clc_smcd_v2_extension, gidchid) == sizeof(struct smc_clc_smcd_v2_extension_fixed),
> +	      "struct member likely outside of struct_group_tagged()");
>   
>   struct smc_clc_msg_proposal {	/* clc proposal message sent by Linux */
>   	struct smc_clc_msg_hdr hdr;

Re: [PATCH][next] net/smc: Use static_assert() to check struct sizes

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 8 Aug 2024 16:07:54 -0600 you wrote:
> Commit 9748dbc9f265 ("net/smc: Avoid -Wflex-array-member-not-at-end
> warnings") introduced tagged `struct smc_clc_v2_extension_fixed` and
> `struct smc_clc_smcd_v2_extension_fixed`. We want to ensure that when
> new members need to be added to the flexible structures, they are
> always included within these tagged structs.
> 
> So, we use `static_assert()` to ensure that the memory layout for
> both the flexible structure and the tagged struct is the same after
> any changes.
> 
> [...]

Here is the summary with links:
  - [next] net/smc: Use static_assert() to check struct sizes
    https://git.kernel.org/netdev/net-next/c/0a3e6939d4b3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html