[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z9GKE-mP3qbmK9cL@shredder>
Date: Wed, 12 Mar 2025 15:20:19 +0200
From: Ido Schimmel <idosch@...dia.com>
To: WangYuli <wangyuli@...ontech.com>
Cc: andrew+netdev@...n.ch, chenlinxuan@...ontech.com, czj2441@....com,
davem@...emloft.net, edumazet@...gle.com, guanwentao@...ontech.com,
kuba@...nel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, niecheng1@...ontech.com, pabeni@...hat.com,
petrm@...dia.com, zhanjun@...ontech.com
Subject: Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand
chunk_key_offsets[chunk_index]
On Tue, Mar 11, 2025 at 10:17:00PM +0800, WangYuli wrote:
> 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.
I'm not sure why the fix suppresses the warning when the warning is
about the destination buffer and the fix is about the source. Can you
check if the below helps? It removes the parameterization from
__mlxsw_sp_acl_bf_key_encode() and instead splits it to two variants.
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..3e1e4be72da2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
@@ -110,7 +110,6 @@ static const u16 mlxsw_sp2_acl_bf_crc16_tab[256] = {
* +-----------+----------+-----------------------------------+
*/
-#define MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES 0
#define MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES 18
#define MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES 20
@@ -229,10 +228,9 @@ static u16 mlxsw_sp2_acl_bf_crc(const u8 *buffer, size_t len)
}
static void
-__mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
- struct mlxsw_sp_acl_atcam_entry *aentry,
- char *output, u8 *len, u8 max_chunks, u8 pad_bytes,
- u8 key_offset, u8 chunk_key_len, u8 chunk_len)
+mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
+ struct mlxsw_sp_acl_atcam_entry *aentry,
+ char *output, u8 *len)
{
struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
u8 chunk_index, chunk_count, block_count;
@@ -243,30 +241,17 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
chunk_count = 1 + ((block_count - 1) >> 2);
erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
(aregion->region->id << 4));
- for (chunk_index = max_chunks - chunk_count; chunk_index < max_chunks;
- chunk_index++) {
- memset(chunk, 0, pad_bytes);
- memcpy(chunk + pad_bytes, &erp_region_id,
+ for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
+ chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
+ memset(chunk, 0, MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES);
+ memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES, &erp_region_id,
sizeof(erp_region_id));
- memcpy(chunk + key_offset,
+ memcpy(chunk + MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
&aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
- chunk_key_len);
- chunk += chunk_len;
+ MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES);
+ chunk += MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
}
- *len = chunk_count * chunk_len;
-}
-
-static void
-mlxsw_sp2_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
- struct mlxsw_sp_acl_atcam_entry *aentry,
- char *output, u8 *len)
-{
- __mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
- MLXSW_BLOOM_KEY_CHUNKS,
- MLXSW_SP2_BLOOM_CHUNK_PAD_BYTES,
- MLXSW_SP2_BLOOM_CHUNK_KEY_OFFSET,
- MLXSW_SP2_BLOOM_CHUNK_KEY_BYTES,
- MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES);
+ *len = chunk_count * MLXSW_SP2_BLOOM_KEY_CHUNK_BYTES;
}
static unsigned int
@@ -375,15 +360,24 @@ mlxsw_sp4_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
char *output, u8 *len)
{
struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
- u8 block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
- u8 chunk_count = 1 + ((block_count - 1) >> 2);
-
- __mlxsw_sp_acl_bf_key_encode(aregion, aentry, output, len,
- MLXSW_BLOOM_KEY_CHUNKS,
- MLXSW_SP4_BLOOM_CHUNK_PAD_BYTES,
- MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
- MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES,
- MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES);
+ u8 chunk_index, chunk_count, block_count;
+ char *chunk = output;
+ __be16 erp_region_id;
+
+ block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
+ chunk_count = 1 + ((block_count - 1) >> 2);
+ erp_region_id = cpu_to_be16(aentry->ht_key.erp_id |
+ (aregion->region->id << 4));
+ for (chunk_index = MLXSW_BLOOM_KEY_CHUNKS - chunk_count;
+ chunk_index < MLXSW_BLOOM_KEY_CHUNKS; chunk_index++) {
+ memcpy(chunk, &erp_region_id, sizeof(erp_region_id));
+ memcpy(chunk + MLXSW_SP4_BLOOM_CHUNK_KEY_OFFSET,
+ &aentry->ht_key.enc_key[chunk_key_offsets[chunk_index]],
+ MLXSW_SP4_BLOOM_CHUNK_KEY_BYTES);
+ chunk += MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
+ }
+ *len = chunk_count * MLXSW_SP4_BLOOM_KEY_CHUNK_BYTES;
+
mlxsw_sp4_bf_key_shift_chunks(chunk_count, output);
}
>
> 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.
>
> Co-developed-by: Zijian Chen <czj2441@....com>
> Signed-off-by: Zijian Chen <czj2441@....com>
> Co-developed-by: Wentao Guan <guanwentao@...ontech.com>
> Signed-off-by: Wentao Guan <guanwentao@...ontech.com>
> Signed-off-by: WangYuli <wangyuli@...ontech.com>
> ---
> .../mlxsw/spectrum_acl_bloom_filter.c | 39 ++++++++++++-------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> 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..96105bab680b 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c
> @@ -203,17 +203,6 @@ static const u8 mlxsw_sp4_acl_bf_crc6_tab[256] = {
> 0x2f, 0x02, 0x18, 0x35, 0x2c, 0x01, 0x1b, 0x36,
> };
>
> -/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
> - * and we need to populate it with 4 key blocks copied from the entry encoded
> - * key. The original keys layout is same for Spectrum-{2,3,4}.
> - * Since the encoded key contains a 2 bytes padding, key block 11 starts at
> - * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
> - * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
> - * 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 u16 mlxsw_sp2_acl_bf_crc16_byte(u16 crc, u8 c)
> {
> return (crc << 8) ^ mlxsw_sp2_acl_bf_crc16_tab[(crc >> 8) ^ c];
> @@ -237,6 +226,7 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> struct mlxsw_afk_key_info *key_info = aregion->region->key_info;
> u8 chunk_index, chunk_count, block_count;
> char *chunk = output;
> + char *enc_key_src_ptr;
> __be16 erp_region_id;
>
> block_count = mlxsw_afk_key_info_blocks_count_get(key_info);
> @@ -248,9 +238,30 @@ __mlxsw_sp_acl_bf_key_encode(struct mlxsw_sp_acl_atcam_region *aregion,
> 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);
> +/* Each chunk contains 4 key blocks. Chunk 2 uses key blocks 11-8,
> + * and we need to populate it with 4 key blocks copied from the entry encoded
> + * key. The original keys layout is same for Spectrum-{2,3,4}.
> + * Since the encoded key contains a 2 bytes padding, key block 11 starts at
> + * offset 2. block 7 that is used in chunk 1 starts at offset 20 as 4 key blocks
> + * take 18 bytes. See 'MLXSW_SP2_AFK_BLOCK_LAYOUT' for more details.
> + * This array defines key offsets for easy access when copying key blocks from
> + * entry key to Bloom filter chunk.
> + *
> + * Define the acceptable values for chunk_index to prevent LLVM from failing
> + * compilation in certain circumstances.
> + */
> + switch (chunk_index) {
> + case 0:
> + enc_key_src_ptr = &aentry->ht_key.enc_key[2];
> + break;
> + case 1:
> + enc_key_src_ptr = &aentry->ht_key.enc_key[20];
> + break;
> + case 2:
> + enc_key_src_ptr = &aentry->ht_key.enc_key[38];
> + break;
> + }
> + memcpy(chunk + key_offset, enc_key_src_ptr, chunk_key_len);
> chunk += chunk_len;
> }
> *len = chunk_count * chunk_len;
> --
> 2.47.2
>
Powered by blists - more mailing lists