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-06 08:49, Tariq Toukan wrote:
> 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.

Indeed, thanks for the suggestion.
Below is a diff to do this.
Does that you look right to you?

  .../mellanox/mlx5/core/en_accel/ipsec.c       | 18 -----------
  .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 --
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 31 +++----------------
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  4 ---
  4 files changed, 4 insertions(+), 51 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..de2e740 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -651,22 +651,6 @@ static void mlx5e_ipsec_modify_state(struct 
work_struct *_work)
  	mlx5_accel_esp_modify_xfrm(sa_entry, attrs);
  }

-static void mlx5e_ipsec_set_esn_ops(struct mlx5e_ipsec_sa_entry 
*sa_entry)
-{
-	struct xfrm_state *x = sa_entry->x;
-
-	if (x->xso.type != XFRM_DEV_OFFLOAD_CRYPTO ||
-	    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;
-}
-
  static void mlx5e_ipsec_handle_netdev_event(struct work_struct *_work)
  {
  	struct mlx5e_ipsec_work *work =
@@ -859,8 +843,6 @@ static int mlx5e_xfrm_add_state(struct net_device 
*dev,
  	if (err)
  		goto err_add_rule;

-	mlx5e_ipsec_set_esn_ops(sa_entry);
-
  	if (sa_entry->dwork)
  		queue_delayed_work(ipsec->wq, &sa_entry->dwork->dwork,
  				   MLX5_IPSEC_RESCHED);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
index abcbd38..172fce9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
@@ -278,8 +278,6 @@ struct mlx5e_ipsec_sa_entry {
  	struct net_device *dev;
  	struct mlx5e_ipsec *ipsec;
  	struct mlx5_accel_esp_xfrm_attrs attrs;
-	void (*set_iv_op)(struct sk_buff *skb, struct xfrm_state *x,
-			  struct xfrm_offload *xo);
  	u32 ipsec_obj_id;
  	u32 enc_key_id;
  	struct mlx5e_ipsec_rule ipsec_rule;
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..4a9c334 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,30 +150,7 @@ 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)
+static void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_offload 
*xo)
  {
  	int iv_offset;
  	__be64 seqno;
@@ -264,7 +241,6 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device 
*netdev,
  {
  	struct mlx5e_priv *priv = netdev_priv(netdev);
  	struct xfrm_offload *xo = xfrm_offload(skb);
-	struct mlx5e_ipsec_sa_entry *sa_entry;
  	struct xfrm_state *x;
  	struct sec_path *sp;

@@ -293,8 +269,9 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device 
*netdev,
  			goto drop;
  		}

-	sa_entry = (struct mlx5e_ipsec_sa_entry *)x->xso.offload_handle;
-	sa_entry->set_iv_op(skb, x, xo);
+	if (x->xso.type == XFRM_DEV_OFFLOAD_CRYPTO &&
+	    x->xso.dir == XFRM_DEV_OFFLOAD_OUT)
+		mlx5e_ipsec_set_iv(skb, xo);
  	mlx5e_ipsec_set_state(priv, skb, x, xo, ipsec_st);

  	return true;
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..9a7e032 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,10 +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,
  			       struct sk_buff *skb,
  			       struct mlx5e_accel_tx_ipsec_state *ipsec_st);