Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV
flat view
From: Tariq Toukan <tariqt@nvidia.com>
Date: 2026-10-06 06:49:56
Also in:
stable
On 05/10/2026 21:28, Jérémy Jean wrote:
On 2026-10-05 15:50, Jérémy Jean wrote:quoted
Hello Tariq, On 2026-10-05 15:06, Tariq Toukan wrote:quoted
On 30/09/2026 17:45, Jérémy Jean wrote:quoted
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 <redacted> --- .../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)-quoted
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émyIt 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.
quoted hunk ↗ jump to hunk
.../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,