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>]

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?


  .../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,