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

[Tariq Toukan <tariqt@nvidia.com>]

On 05/10/2026 21:28, Jérémy Jean wrote:
> On 2026-10-05 15:50, Jérémy Jean wrote:
>> 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
> 
> It seems to me that a delete only patch would be okay then? (see
> patch below).
> The removal of the check in mlx5e_ipsec_set_esn_ops() makes
> mlx5e_ipsec_set_iv() the only callable function. Then it seems
> one can simply remove lines. Do you confirm?
> 
> 

 From a quick look, it seems we can do even more by totally removing the 
set_iv_op function pointer.

>   .../mellanox/mlx5/core/en_accel/ipsec.c       |  5 -----
>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 22 -------------------
>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  2 --
>   3 files changed, 29 deletions(-)
> 
> 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 db260e3..43ef5b3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> @@ -659,11 +659,6 @@ static void mlx5e_ipsec_set_esn_ops(struct 
> mlx5e_ipsec_sa_entry *sa_entry)
>              x->xso.dir != XFRM_DEV_OFFLOAD_OUT)
>                  return;
> 
> -       if (x->props.flags & XFRM_STATE_ESN) {
> -               sa_entry->set_iv_op = mlx5e_ipsec_set_iv_esn;
> -               return;
> -       }
> -
>          sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
>   }
> 
> 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 6056106..9ffe094 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
> @@ -150,28 +150,6 @@ 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));
> -       iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
> -       skb_store_bits(skb, iv_offset, &seqno, 8);
> -}
> -
>   void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
>                          struct xfrm_offload *xo)
>   {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.h
> index 45b0d19..a3d92c9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
> @@ -53,8 +53,6 @@ struct mlx5e_accel_tx_ipsec_state {
> 
>   #ifdef CONFIG_MLX5_EN_IPSEC
> 
> -void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
> -                           struct xfrm_offload *xo);
>   void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
>                          struct xfrm_offload *xo);
>   bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,