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

[Paolo Abeni <pabeni@redhat.com>]

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