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

[Jianbo Liu <jianbol@nvidia.com>]

On 5/19/2026 4:09 PM, Paolo Abeni wrote:
> On 5/18/26 1:27 PM, Simon Horman wrote:
>> From: 'Simon Horman' <horms@kernel.org>
>>
>> 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?

I agree. Please drop this patch for now. I will submit a new one.

Thanks,
Jianbo