Re: [PATCH net 1/2] mlxsw: spectrum_acl_bloom_filter: Expand chunk_key_offsets[chunk_index]

From: Paolo Abeni
Date: Thu Mar 13 2025 - 09:53:48 EST


On 3/12/25 2:20 PM, Ido Schimmel wrote:
> 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++) {

Possibly the compiler is inferring chunck count can be greater then
MLXSW_BLOOM_KEY_CHUNKS?

something alike:

chunk_index = min_t(0, MLXSW_BLOOM_KEY_CHUNKS - chunk_count, u8);

Could possibly please it?

/P