Re: [PATCH net] net/mlx5e: Skip IPsec flow modify when MAC address is unchanged

[Paolo Abeni]

On 5/18/26 1:27 PM, Simon Horman wrote:
> From: 'Simon Horman' <horms@xxxxxxxxxx>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> net/mlx5e: Skip IPsec flow modify when MAC address is unchanged
> 
> When a netdev event fires for an IPsec SA, skip calling
> mlx5e_accel_ipsec_fs_modify() if the MAC address has not changed and
> the rule is not in drop mode. This avoids redundant hardware updates.
> 
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>> index a52e12c3c95a2..f567cd801adb3 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>> @@ -674,19 +674,26 @@ static void mlx5e_ipsec_handle_netdev_event(struct work_struct *_work)
>>  	struct mlx5e_ipsec_sa_entry *sa_entry = work->sa_entry;
>>  	struct mlx5e_ipsec_netevent_data *data = work->data;
>>  	struct mlx5_accel_esp_xfrm_attrs *attrs;
> [ ... ]
>> +
>> +	if (ether_addr_equal(mac, data->addr) && !attrs->drop)
>> +		return;
>> +
>> +	ether_addr_copy(mac, data->addr);
>>  	attrs->drop = false;
>>  	mlx5e_accel_ipsec_fs_modify(sa_entry);
> 
> If mlx5e_accel_ipsec_fs_modify() fails (for example, due to a memory
> allocation failure), does this early return prevent the hardware state
> from recovering?
> 
> Since attrs->drop and the mac address are updated before calling
> mlx5e_accel_ipsec_fs_modify(), a failure in the hardware update leaves the
> software state out of sync with the hardware.
> 
> Prior to this patch, a subsequent netdev event for the same neighbor would
> unconditionally retry the hardware update. With this patch, the early return
> sees the optimistically updated software state, matches it, and skips the
> retry, potentially leaving the hardware permanently stuck in a dropped or
> failed state.

@Tariq, @Jianbo: I tend to think that the above qualifies as
pre-existing issue: when mlx5e_accel_ipsec_fs_modify() failed the S/W
and the H/W ended-up in out-of-sync state for a potentially unlimited
time even before this patch. WDYT?

/P