Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

["Jérémy Jean" <jeremy.jean@oss.cyber.gouv.fr>]

Hello Tariq,

On 2026-10-05 15:06, Tariq Toukan wrote:
> On 30/09/2026 17:45, Jérémy Jean wrote:
>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
>> on the SA's current output counter. The metadata already contains the
>> high word for the first packet, so this can produce the wrong IV.
>> 
>> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
>> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
>> for the IV instead of (H, 0). This can create a situation where GCM
>> reuses a nonce.
>> 
>> Remove the high-word adjustment and use xo->seq directly to generate
>> the IV.
>> 
>> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> ---
>>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 
>> +-----------
>>   1 file changed, 1 insertion(+), 11 deletions(-)
>> 
>> diff --git 
>> a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c 
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> index 6056106edcc6..4aa9f9c52f57 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
>> *skb,
>>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state 
>> *x,
>>   			    struct xfrm_offload *xo)
>>   {
>> -	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
>> -	__u32 oseq = replay_esn->oseq;
>>   	int iv_offset;
>>   	__be64 seqno;
>> -	u32 seq_hi;
>> -
>> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
>> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
>> skb_shinfo(skb)->gso_segs))) {
>> -		seq_hi = xo->seq.hi - 1;
>> -	} else {
>> -		seq_hi = xo->seq.hi;
>> -	}
>>     	/* Place the SN in the IV field */
>> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>>   	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
>>   	skb_store_bits(skb, iv_offset, &seqno, 8);
>>   }
> 
> Thanks for your patch.
> 
> Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
> mlx5e_ipsec_set_iv()?
> 
> I wouldn't keep both copies then..

Good point: after a quick check, indeed you are probably right.
I will look to simplify this in a new version of the patch series.

Jérémy