diff mbox series

[net,v2] mlxsw: spectrum_acl_bloom_filter: Workaround for some LLVM versions

Message ID CBD5806B120ADEEE+20250318054803.462085-1-wangyuli@uniontech.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v2] mlxsw: spectrum_acl_bloom_filter: Workaround for some LLVM versions | expand

Commit Message

WangYuli March 18, 2025, 5:48 a.m. UTC
This is a workaround to mitigate a compiler anomaly.

During LLVM toolchain compilation of this driver on s390x architecture, an
unreasonable __write_overflow_field warning occurs.

Contextually, chunk_index is restricted to 0, 1 or 2. By expanding these
possibilities, the compile warning is suppressed.

Fix follow error with clang-19 when -Werror:
  In file included from drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c:5:
  In file included from ./include/linux/gfp.h:7:
  In file included from ./include/linux/mmzone.h:8:
  In file included from ./include/linux/spinlock.h:63:
  In file included from ./include/linux/lockdep.h:14:
  In file included from ./include/linux/smp.h:13:
  In file included from ./include/linux/cpumask.h:12:
  In file included from ./include/linux/bitmap.h:13:
  In file included from ./include/linux/string.h:392:
  ./include/linux/fortify-string.h:571:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
    571 |                         __write_overflow_field(p_size_field, size);
        |                         ^
  1 error generated.

According to the testing, we can be fairly certain that this is a clang
compiler bug, impacting only clang-19 and below. Clang versions 20 and
21 do not exhibit this behavior.

Link: https://lore.kernel.org/all/484364B641C901CD+20250311141025.1624528-1-wangyuli@uniontech.com/
Fixes: 7585cacdb978 ("mlxsw: spectrum_acl: Add Bloom filter handling")
Co-developed-by: Zijian Chen <czj2441@163.com>
Signed-off-by: Zijian Chen <czj2441@163.com>
Co-developed-by: Wentao Guan <guanwentao@uniontech.com>
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Co-developed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: WangYuli <wangyuli@uniontech.com>
---
Changelog:
 *v1->v2:
    1. Combine patch #1 and #2 from v1.
    2. Take the more simplified change by Ido Schimmel.
    3. Append more Co-developed-by, Signed-off-by, Tested-by,
Suggested-by and Fixes tags.
    4. Retain reverse Christmas tree order for variables.
---
 .../mlxsw/spectrum_acl_bloom_filter.c         | 26 +++++++++++++++----
 1 file changed, 21 insertions(+), 5 deletions(-)

Comments

Ido Schimmel March 18, 2025, 9:09 a.m. UTC | #1
On Tue, Mar 18, 2025 at 01:48:03PM +0800, WangYuli wrote:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> index a54eedb69a3f..fc8a8cf64ec8 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -212,7 +212,22 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
>   * This array defines key offsets for easy access when copying key blocks from
>   * entry key to Bloom filter chunk.
>   */
> -static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
> +static char *
> +mlxsw_sp_acl_bf_enc_key_get(struct mlxsw_sp_acl_atcam_entry *aentry,
> +			    u8 chunk_index)
> +{
> +		switch (chunk_index) {

There's an extra indentation level here. It's not present in the patch I
shared.

[1] https://lore.kernel.org/all/Z9M1A8lOuXE4UkyR@shredder/

> +		case 0:
> +			return &aentry->ht_key.enc_key[2];
> +		case 1:
> +			return &aentry->ht_key.enc_key[20];
> +		case 2:
> +			return &aentry->ht_key.enc_key[38];
> +		default:
> +			WARN_ON_ONCE(1);
> +			return &aentry->ht_key.enc_key[0];
> +		}
> +}
>  
>  static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
>  {
> @@ -235,9 +250,10 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
>  {
>  	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> -	u8 chunk_index, chunk_count, block_count;
> +	u8 chunk_index, chunk_count;
>  	char *chunk = output;
>  	__be16 erp_region_id;
> +	u32 block_count;
>  
>  	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
>  	chunk_count = 1 + ((block_count - 1) >> 2);
> @@ -245,12 +261,12 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
>  				   (aregion->region->id << 4));
>  	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
>  	     chunk_index++) {
> +		char *enc_key;

Missing blank line here between variable declaration and code (see [1]).

>  		memset(chunk, 0, pad_bytes);
>  		memcpy(chunk + pad_bytes, &erp_region_id,
>  		       sizeof(erp_region_id));
> -		memcpy(chunk + key_offset,
> -		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
> -		       chunk_key_len);
> +		enc_key = mlxsw_sp_acl_bf_enc_key_get(aentry, chunk_index);
> +		memcpy(chunk + key_offset, enc_key, chunk_key_len);
>  		chunk += chunk_len;
>  	}
>  	*len = chunk_count * chunk_len;
> -- 
> 2.49.0
>
WangYuli March 18, 2025, 10:27 a.m. UTC | #2
Apologies, a minor formatting error.

Will send a patch v3.

Thanks,

--

WangYuli
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
index a54eedb69a3f..fc8a8cf64ec8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -212,7 +212,22 @@  static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
  * This array defines key offsets for easy access when copying key blocks from
  * entry key to Bloom filter chunk.
  */
-static const u8 chunk_key_offsets[MLXSW_BLOOM_KEY_CHUNKS] = {2, 20, 38};
+static char *
+mlxsw_sp_acl_bf_enc_key_get(struct mlxsw_sp_acl_atcam_entry *aentry,
+			    u8 chunk_index)
+{
+		switch (chunk_index) {
+		case 0:
+			return &aentry->ht_key.enc_key[2];
+		case 1:
+			return &aentry->ht_key.enc_key[20];
+		case 2:
+			return &aentry->ht_key.enc_key[38];
+		default:
+			WARN_ON_ONCE(1);
+			return &aentry->ht_key.enc_key[0];
+		}
+}
 
 static u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
 {
@@ -235,9 +250,10 @@  __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 			     u8 key_offset, u8 chunk_key_len, u8 chunk_len)
 {
 	struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
-	u8 chunk_index, chunk_count, block_count;
+	u8 chunk_index, chunk_count;
 	char *chunk = output;
 	__be16 erp_region_id;
+	u32 block_count;
 
 	block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
 	chunk_count = 1 + ((block_count - 1) >> 2);
@@ -245,12 +261,12 @@  __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
 				   (aregion->region->id << 4));
 	for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
 	     chunk_index++) {
+		char *enc_key;
 		memset(chunk, 0, pad_bytes);
 		memcpy(chunk + pad_bytes, &erp_region_id,
 		       sizeof(erp_region_id));
-		memcpy(chunk + key_offset,
-		       &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
-		       chunk_key_len);
+		enc_key = mlxsw_sp_acl_bf_enc_key_get(aentry, chunk_index);
+		memcpy(chunk + key_offset, enc_key, chunk_key_len);
 		chunk += chunk_len;
 	}
 	*len = chunk_count * chunk_len;