[PATCH net-next V4 00/14] net/mlx5e: Add support for HW-GRO to PSP

[PATCH net-next V4 00/14] net/mlx5e: Add support for HW-GRO to PSP

[Tariq Toukan]

Hi,

Ingress PSP packets cannot be merged by the HW-GRO HW state machine
because they are not decapsulated and the current HW-GRO state machine
does not understand PSP.

This series by Cosmin decapsulates PSP packets in steering, which allows
the now-decapsulated PSP packets (== TCP) to go through HW-GRO and be
aggregated. The SPI and PSP version from the PSP header are handed off
to the driver in the CQE metadata fields. They are used to terminate the
HW GRO session on mismatch, and are required to construct the skb
extension which is used higher up in the stack.

Some preparatory work needed to happen to allow that:
- All accel protocol markers were moved away from ft_metadata into
  flow_tag
- Mutual exclusion between TC and accel protocols was added. Trying to
  configure both IPsec and TC at the same time for example is now
  detected at config time instead of doing weird things at runtime.

kperf tests on a pair of CX7 NICs with 200Gbps link speed:

Streams   Gbps no HW-GRO   Gbps HW-GRO   Speedup
-------   --------------   -----------   -------
      1               28            58     2.07x
      2               67           102     1.52x
      4              136           180     1.32x
      8              175           183     1.05x
        
Regards,
Tariq

Some internal Sashiko findings, plus Cosmin's comments:

> +        cfg.wait_hw_stats_settle()
> +        after = cfg.netnl.qstats_get({"ifindex": cfg.ifindex}, dump=True)[0]
Could this lead to flaky test failures on multiqueue NICs?
By hardcoding the netlink dump array access to index [0], the test statically
verifies statistics for the first queue only. Since the test sends traffic over
an ephemeral random port, Receive Side Scaling (RSS) will hash this 4-tuple and
could deliver the flow to any active RX queue. If the flow lands on a queue
other than queue 0, will the test fail to observe the hardware GRO packet
increments?

[CR] The premise is wrong. [0] is not queue 0, it's the result for the
requested dev.

> @@ -1757,11 +1755,10 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
>       rx_tables = &rx_fs->tables;
>       ft_crypto = &rx_tables->ft_crypto;
>
> -     /* Set bit[31 - 30] macsec marker - 0x01 */
>       /* Set bit[15-0] fs id */
>       MLX5_SET(set_action_in, action, action_type, MLX5_ACTION_TYPE_SET);
>       MLX5_SET(set_action_in, action, field, MLX5_ACTION_IN_FIELD_METADATA_REG_B);
> -     MLX5_SET(set_action_in, action, data, macsec_fs_set_rx_fs_id(fs_id));
> +     MLX5_SET(set_action_in, action, data, fs_id);
Does removing the BIT(30) marker from the MACsec fs_id metadata cause a
regression by exposing RoCE MACsec to an aliasing issue?
By removing BIT(30) from the fs_id in this patch, the fs_id written to
METADATA_REG_B becomes a small integer (e.g., 1, 2).
Later in mlx5_macsec_fs_add_roce_rule_rx(), the hardware rule
ft_macsec_op_check enforces MACsec on RoCE traffic by matching
metadata_reg_c_5 exactly against this same fs_id value.
Could any unencrypted packet that has its REG_B set to this small integer
by another NIC feature (such as TC rules setting packet marks, or PSP IPsec
decryption) falsely pass the MACsec check and bypass encryption enforcement?

[CR] Preexisting, TC rules could set the fsid to the the previous value even before.
'PSP IPsec' decryption doesn't make sense.

> @@ -1757,11 +1755,10 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
>  	rx_tables = &rx_fs->tables;
>  	ft_crypto = &rx_tables->ft_crypto;
>
> -	/* Set bit[31 - 30] macsec marker - 0x01 */
>  	/* Set bit[15-0] fs id */
>  	MLX5_SET(set_action_in, action, action_type, MLX5_ACTION_TYPE_SET);
>  	MLX5_SET(set_action_in, action, field, MLX5_ACTION_IN_FIELD_METADATA_REG_B);
> -	MLX5_SET(set_action_in, action, data, macsec_fs_set_rx_fs_id(fs_id));
> +	MLX5_SET(set_action_in, action, data, fs_id);
Does removing the BIT(30) marker allow metadata collisions in the hardware
RoCE datapath?
If an administrator configures a benign TC rule that sets a mark equal to
the MACsec fs_id (which is just a 16-bit integer), could an attacker send
unauthenticated RoCE traffic that matches the TC mark, falsely setting the
metadata and bypassing the MACsec hardware authentication checks?

[CR] This series makes TC and MACsec mutually exclusive.

> @@ -2160,8 +2167,8 @@ static int mlx5_macsec_fs_add_roce_rule_rx(struct mlx5_macsec_fs *macsec_fs, u32
>
>  	spec->match_criteria_enable |= MLX5_MATCH_MISC_PARAMETERS_2;
>  	MLX5_SET_TO_ONES(fte_match_param, spec->match_criteria, misc_parameters_2.metadata_reg_c_5);
> -	MLX5_SET(fte_match_param, spec->match_value, misc_parameters_2.metadata_reg_c_5,
> -		 macsec_fs_set_rx_fs_id(fs_id));
> +	MLX5_SET(fte_match_param, spec->match_value,
> +		 misc_parameters_2.metadata_reg_c_5, fs_id);
By matching on the bare fs_id without the BIT(30) marker in
metadata_reg_c_5, is this RoCE datapath rule now susceptible to the TC
mark collision issue?

[CR] See above.

> @@ -1778,6 +1775,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
>  	/* Rx crypto table with SCI rule */
>  	macsec_fs_rx_setup_fte(spec, &flow_act, attrs, true);
>
> +	spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
> +	spec->flow_context.flow_tag =
> +		FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
> +			   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
Can adding FLOW_CONTEXT_HAS_TAG to the shared MACsec crypto table corrupt
RoCE Queue Pair Numbers (QPN)?
This unconditionally adds a flow tag to the shared ft_crypto table, which
processes both Ethernet and RoCE MACsec traffic. When FLOW_CONTEXT_HAS_TAG
is set, the hardware writes the flow tag into the sop_drop_qpn field of
the CQE.
Will this cause the mlx5_ib driver to read the MACsec tag (value 2) instead
of the actual destination QPN for RDMA traffic, breaking RoCE delivery?

[CR] The flow_tag is only given for Eth CQEs. For RoCE/RDMA, that CQE
field is the QPN, and therefore mlx5_ib is unaffected.

> @@ -1803,6 +1805,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
>
>  		macsec_fs_rx_setup_fte(spec, &flow_act, attrs, false);
>
> +		spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
> +		spec->flow_context.flow_tag =
> +			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
> +				   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
Is it safe to set the flow tag on the shared crypto table here as well?
This risks corrupting the destination QPN for RoCE traffic as mentioned
above.

[CR] Same as above, QPN isn't corrupted, those CQEs don't carry flow_tag.

> @@ -1717,7 +1717,7 @@ void mlx5e_macsec_offload_handle_rx_skb(struct net_device *netdev,
>       if (!macsec)
>               return;
>
> -     fs_id = MLX5_MACSEC_RX_METADAT_HANDLE(macsec_meta_data);
> +     fs_id = MLX5_MACSEC_RX_METADATA_HANDLE(macsec_meta_data);
>
>       rcu_read_lock();
>       sc_xarray_element = xa_load(&macsec->sc_xarray, fs_id);
This is a pre-existing issue, but can this xa_load() race with an immediate
kfree() during deletion, causing a use-after-free?
In the deletion path, mlx5e_macsec_del_rxsc() calls xa_erase() and then
immediately frees the element without an RCU grace period:
mlx5e_macsec_del_rxsc() {
        ...
        xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
        dst_release(&rx_sc->md_dst->dst);
        kfree(rx_sc->sc_xarray_element);
        ...
}
Since mlx5e_macsec_offload_handle_rx_skb() retrieves sc_xarray_element
locklessly under rcu_read_lock() via xa_load(), would a concurrent deletion
result in dereferencing freed memory?

[CR] Preexisting.

> +static inline int mlx5e_accel_block_tc_offload(struct mlx5_core_dev *mdev)
> +{
> +     int ret = 0;
> +
> +     mutex_lock(&mdev->offload_block.lock);
> +     if (mdev->offload_block.num_accel)
> +             ret = -EBUSY;
> +     else
> +             mdev->offload_block.num_tc++;
This isn't a bug, but does the new naming here swap the semantics of the
variables?
In the original logic, num_block_tc counted the number of blockers on TC
(which are IPsec/accel rules). By renaming it to num_tc,
mlx5e_accel_block_tc_offload() now increments num_tc when an accel rule
is added.
Would it be clearer to keep "block" in the names, e.g., num_block_tc, so
it doesn't look like num_tc is tracking the number of TC rules?

[CR] Intentional, "offload_block.num_{tc, accel}" sounds better.

> +static inline int mlx5e_accel_block_tc_offload(struct mlx5_core_dev *mdev)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&mdev->offload_block.lock);
> +	if (mdev->offload_block.num_accel)
> +		ret = -EBUSY;
> +	else
> +		mdev->offload_block.num_tc++;
Are the semantic meanings of the num_tc and num_accel variables inverted?
When an Accel rule is added here in mlx5e_accel_block_tc_offload(), it
increments num_tc.

[CR] Same thing, it's an intentional rename.

> @@ -384,6 +429,215 @@ static void fill_transportlayer(void *buf, int seq_offset, int ack_offset,
[ ... ]
> +static char psp_scratch[L2_HLEN_MAX + IP_MAXPACKET + PSP_ENCAP_LEN];
[ ... ]
> +/* Encapsulates & encrypts @pkt with PSP transport mode into psp_scratch.
> + * Returns the scratch buffer and updates *@lenp.
> + */
> +static char *psp_encapsulate(const char *pkt, int *lenp)
> +{
[ ... ]
> +	memcpy(psp_scratch, pkt, len);
> +
> +	if (proto == PF_INET) {
> +		struct iphdr *iph = (struct iphdr *)(psp_scratch + ETH_HLEN);
Does this code violate strict aliasing rules?
Since psp_scratch is declared as a character array, casting it to an
incompatible structure pointer like struct iphdr * (and later struct ipv6hdr *
and struct udphdr *) violates C11 strict aliasing rules.
Because the tools/ directory assumes standard -fstrict-aliasing optimizations
are active, could this cause the compiler to incorrectly reorder or optimize
away memory writes to these headers, potentially resulting in malformed
packets and spurious test failures?

[CR] Maybe, but there are already 20+ similar things in the file.

V4:
- Removed dead mlx5_esw_lock() (Daniel).
- Rebased on top of f225a7317c18 ("selftests: drv-net: split gro.py into one test per coalescing mode") (Daniel).
- Dropped the gro_lib.py extraction, made obsolete by above change.
- Fixed some typos (Aleksandr).
- Added CONFIG_INET_PSP=y to the test config (Daniel).

V3:
https://lore.kernel.org/netdev/20260903085215.3691657-1-tariqt@nvidia.com/
- Cleared fs->decap_enabled on config down (Daniel).
- Made decap support optional (don't fail device reconfig on errors).
- Used bitfield ops for accel protos & psp ver (Daniel).
- Renamed psp_responder off -> len (Jakub).
- Dedicated HW GRO PSP test (Jakub).
- Extended HW GRO test coverage (Jakub).

V2:
https://lore.kernel.org/netdev/20260804083535.2946459-1-tariqt@nvidia.com/
- Use XFail in patch 13 (Jakub).

V1:
https://lore.kernel.org/all/20260730091756.2543777-1-tariqt@nvidia.com/

Cosmin Ratiu (14):
  net/mlx5e: Generalize TC <-> IPsec mutual exclusion
  net/mlx5e: ipsec: Block TC offload when IPsec is enabled
  net/mlx5e: psp: Block TC offload when PSP is enabled
  net/mlx5e: macsec: Block TC offload when MACsec is enabled
  net/mlx5e: psp: Move RX marker from ft_metadata to flow_tag
  net/mlx5e: ipsec: Move RX marker from ft_metadata to flow_tag
  net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag
  net/mlx5e: psp: Handle HW-decapsulated RX PSP packets
  net/mlx5e: psp: Add an rx_decap steering table
  net/mlx5e: shampo: Flush session on PSP mismatch
  net/mlx5e: psp: Dynamically reconfigure based on SHAMPO mode
  selftests: drv-net: psp: Extract shared helpers into psp_lib.py
  selftests: net: gro: Add PSP encapsulation and encryption
  selftests: drv-net: Add PSP HW GRO conformance tests

 .../net/ethernet/mellanox/mlx5/core/en/fs.h   |   1 +
 .../mellanox/mlx5/core/en_accel/en_accel.h    |  27 ++
 .../mellanox/mlx5/core/en_accel/flow_tag.h    |  49 +++
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    |  72 ++--
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |   8 +-
 .../mellanox/mlx5/core/en_accel/macsec.c      |  34 +-
 .../mellanox/mlx5/core/en_accel/macsec.h      |   4 +-
 .../mellanox/mlx5/core/en_accel/psp.c         | 316 ++++++++++++++--
 .../mellanox/mlx5/core/en_accel/psp.h         |   2 +
 .../mellanox/mlx5/core/en_accel/psp_rxtx.c    |  21 +-
 .../mellanox/mlx5/core/en_accel/psp_rxtx.h    |  44 ++-
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  10 +-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  32 +-
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   |  46 ++-
 .../net/ethernet/mellanox/mlx5/core/en_tc.h   |   7 +-
 .../net/ethernet/mellanox/mlx5/core/eswitch.c |  12 -
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |   1 -
 .../mellanox/mlx5/core/lib/macsec_fs.c        |  21 +-
 .../mellanox/mlx5/core/lib/macsec_fs.h        |   9 +-
 .../net/ethernet/mellanox/mlx5/core/main.c    |   3 +
 include/linux/mlx5/driver.h                   |   7 +-
 tools/testing/selftests/drivers/net/Makefile  |   1 +
 .../testing/selftests/drivers/net/gro_lib.py  |  15 +-
 .../testing/selftests/drivers/net/hw/Makefile |  17 +
 tools/testing/selftests/drivers/net/hw/config |   1 +
 .../selftests/drivers/net/hw/psp_gro.py       | 157 ++++++++
 tools/testing/selftests/drivers/net/psp.py    |  77 ++--
 .../testing/selftests/drivers/net/psp_lib.py  |  57 +++
 tools/testing/selftests/net/lib/Makefile      |  16 +
 tools/testing/selftests/net/lib/gro.c         | 337 +++++++++++++++++-
 30 files changed, 1198 insertions(+), 206 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
 create mode 100755 tools/testing/selftests/drivers/net/hw/psp_gro.py
 create mode 100644 tools/testing/selftests/drivers/net/psp_lib.py


base-commit: bc6fe9d301d59fe5cb236d646abe792bffae5fa6
-- 
2.44.0

[PATCH net-next V4 01/14] net/mlx5e: Generalize TC <-> IPsec mutual exclusion

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

There is a mechanism to mutually exclude TC offload and IPsec offload
from the same interface (commit [1]) due to ordering issues between SW
and HW paths.

TC offload makes use of flow_tag to carry the tc mark (even when 0).
Upcoming changes to accel protocols will make use of flow_tag to carry
the protocol marker. As the flow_tag cannot be partially modified by a
steering rule, the last rule setting the flow_tag will overwrite any
previous ones.

This means that TC offload cannot be active at the same time with any of
the currently implemented accel protocols (IPsec, MACsec, PSP).

Generalize the TC <-> IPsec mutual exclusion mechanism to be usable by
more accel protocols:
- move the existing mlx5e_ipsec_{,un}block_tc_offload functions to
  en_accel.h, rename them to mlx5e_accel_{,un}block_tc_offload.
- rename the mdev counter from num_block_ipsec to num_accel.
- rename is_tc_ipsec_order_check_needed -> is_tc_accel_check_needed
  and make it not bail out when IPsec isn't configured.
- replace the conditional use of the esw write lock as a protection for
  incrementing the counter with a new mutex. The esw might not be
  available, and it wasn't used correctly on the decrement path anyway,
  allowing races to happen. Using a dedicated mutex for these counters
  makes it clear and avoids races.
- add underflow warnings for the two counters to catch future
  miscounting bugs.
- move counters and the new lock into a dedicated struct in
  mlx5_core_dev named 'offload_block'. Now offload_block.num_tc means
  the number of tc blocks due to accel rules and ofload_block.num_accel
  means the number of accel blocks due to tc rules.

[1] commit c8e350e62fc5 ("net/mlx5e: Make TC and IPsec offloads mutually
exclusive on a netdev")

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/en_accel.h    | 22 ++++++++
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 54 +++----------------
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   | 46 +++++++++-------
 .../net/ethernet/mellanox/mlx5/core/eswitch.c | 12 -----
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  1 -
 .../net/ethernet/mellanox/mlx5/core/main.c    |  3 ++
 include/linux/mlx5/driver.h                   |  7 ++-
 7 files changed, 64 insertions(+), 81 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
index 3f212e46fc2f..8a2ea7616440 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
@@ -218,6 +218,28 @@ static inline void mlx5e_accel_tx_finish(struct mlx5e_txqsq *sq,
 #endif
 }
 
+static inline int mlx5e_accel_block_tc_offload(struct mlx5_core_dev *mdev)
+{
+	int ret = 0;
+
+	mutex_lock(&mdev->offload_block.lock);
+	if (mdev->offload_block.num_accel)
+		ret = -EBUSY;
+	else
+		mdev->offload_block.num_tc++;
+	mutex_unlock(&mdev->offload_block.lock);
+
+	return ret;
+}
+
+static inline void mlx5e_accel_unblock_tc_offload(struct mlx5_core_dev *mdev)
+{
+	mutex_lock(&mdev->offload_block.lock);
+	if (!WARN_ON_ONCE(!mdev->offload_block.num_tc))
+		mdev->offload_block.num_tc--;
+	mutex_unlock(&mdev->offload_block.lock);
+}
+
 static inline int mlx5e_accel_init_rx(struct mlx5e_priv *priv)
 {
 	return mlx5e_ktls_init_rx(priv);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index 329608c59313..74e0aa5b6133 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -4,6 +4,7 @@
 #include <linux/netdevice.h>
 #include "en.h"
 #include "en/fs.h"
+#include "en_accel/en_accel.h"
 #include "eswitch.h"
 #include "ipsec.h"
 #include "fs_core.h"
@@ -2574,53 +2575,12 @@ void mlx5e_accel_ipsec_fs_read_stats(struct mlx5e_priv *priv, void *ipsec_stats)
 	}
 }
 
-#ifdef CONFIG_MLX5_ESWITCH
-static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev)
-{
-	struct mlx5_eswitch *esw = mdev->priv.eswitch;
-	int err = 0;
-
-	if (esw) {
-		err = mlx5_esw_lock(esw);
-		if (err)
-			return err;
-	}
-
-	if (mdev->num_block_ipsec) {
-		err = -EBUSY;
-		goto unlock;
-	}
-
-	mdev->num_block_tc++;
-
-unlock:
-	if (esw)
-		mlx5_esw_unlock(esw);
-
-	return err;
-}
-#else
-static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev)
-{
-	if (mdev->num_block_ipsec)
-		return -EBUSY;
-
-	mdev->num_block_tc++;
-	return 0;
-}
-#endif
-
-static void mlx5e_ipsec_unblock_tc_offload(struct mlx5_core_dev *mdev)
-{
-	mdev->num_block_tc--;
-}
-
 int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 {
 	int err;
 
 	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) {
-		err = mlx5e_ipsec_block_tc_offload(sa_entry->ipsec->mdev);
+		err = mlx5e_accel_block_tc_offload(sa_entry->ipsec->mdev);
 		if (err)
 			return err;
 	}
@@ -2637,7 +2597,7 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 
 err_out:
 	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET)
-		mlx5e_ipsec_unblock_tc_offload(sa_entry->ipsec->mdev);
+		mlx5e_accel_unblock_tc_offload(sa_entry->ipsec->mdev);
 	return err;
 }
 
@@ -2652,7 +2612,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 		mlx5_packet_reformat_dealloc(mdev, ipsec_rule->pkt_reformat);
 
 	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET)
-		mlx5e_ipsec_unblock_tc_offload(mdev);
+		mlx5e_accel_unblock_tc_offload(mdev);
 
 	if (sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_OUT) {
 		tx_ft_put(sa_entry->ipsec, sa_entry->attrs.type);
@@ -2686,7 +2646,7 @@ int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry)
 {
 	int err;
 
-	err = mlx5e_ipsec_block_tc_offload(pol_entry->ipsec->mdev);
+	err = mlx5e_accel_block_tc_offload(pol_entry->ipsec->mdev);
 	if (err)
 		return err;
 
@@ -2701,7 +2661,7 @@ int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry)
 	return 0;
 
 err_out:
-	mlx5e_ipsec_unblock_tc_offload(pol_entry->ipsec->mdev);
+	mlx5e_accel_unblock_tc_offload(pol_entry->ipsec->mdev);
 	return err;
 }
 
@@ -2712,7 +2672,7 @@ void mlx5e_accel_ipsec_fs_del_pol(struct mlx5e_ipsec_pol_entry *pol_entry)
 
 	mlx5_del_flow_rules(ipsec_rule->rule);
 
-	mlx5e_ipsec_unblock_tc_offload(pol_entry->ipsec->mdev);
+	mlx5e_accel_unblock_tc_offload(pol_entry->ipsec->mdev);
 
 	if (pol_entry->attrs.dir == XFRM_DEV_OFFLOAD_IN) {
 		rx_ft_put_policy(pol_entry->ipsec,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index b290beb4369a..05c703795df4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4813,14 +4813,14 @@ static bool is_flow_rule_duplicate_allowed(struct net_device *dev,
 	return netif_is_lag_port(dev) && rpriv && rpriv->rep->vport != MLX5_VPORT_UPLINK;
 }
 
-/* As IPsec and TC order is not aligned between software and hardware-offload,
- * either IPsec offload or TC offload, not both, is allowed for a specific interface.
+/* TC offload and accel protocols can overwrite each other's flow_tag with
+ * steering rules and they cannot simultaneously operate on the same interface.
+ * Additionally, as IPsec and TC order is not aligned between software and
+ * hardware-offload, only one is allowed for a specific interface.
  */
-static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx5e_priv *priv)
+static bool is_tc_accel_check_needed(struct net_device *filter,
+				     struct mlx5e_priv *priv)
 {
-	if (!IS_ENABLED(CONFIG_MLX5_EN_IPSEC))
-		return false;
-
 	if (filter != priv->netdev)
 		return false;
 
@@ -4830,27 +4830,35 @@ static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx
 	return true;
 }
 
-static int mlx5e_tc_block_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
+static int mlx5e_tc_block_accel_offload(struct net_device *filter,
+					struct mlx5e_priv *priv)
 {
 	struct mlx5_core_dev *mdev = priv->mdev;
+	int ret = 0;
 
-	if (!is_tc_ipsec_order_check_needed(filter, priv))
+	if (!is_tc_accel_check_needed(filter, priv))
 		return 0;
 
-	if (mdev->num_block_tc)
-		return -EBUSY;
-
-	mdev->num_block_ipsec++;
+	mutex_lock(&mdev->offload_block.lock);
+	if (mdev->offload_block.num_tc)
+		ret = -EBUSY;
+	else
+		mdev->offload_block.num_accel++;
+	mutex_unlock(&mdev->offload_block.lock);
 
-	return 0;
+	return ret;
 }
 
-static void mlx5e_tc_unblock_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
+static void mlx5e_tc_unblock_accel_offload(struct net_device *filter,
+					   struct mlx5e_priv *priv)
 {
-	if (!is_tc_ipsec_order_check_needed(filter, priv))
+	if (!is_tc_accel_check_needed(filter, priv))
 		return;
 
-	priv->mdev->num_block_ipsec--;
+	mutex_lock(&priv->mdev->offload_block.lock);
+	if (!WARN_ON_ONCE(!priv->mdev->offload_block.num_accel))
+		priv->mdev->offload_block.num_accel--;
+	mutex_unlock(&priv->mdev->offload_block.lock);
 }
 
 int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv,
@@ -4865,7 +4873,7 @@ int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv,
 	if (!mlx5_esw_hold(priv->mdev))
 		return -EBUSY;
 
-	err = mlx5e_tc_block_ipsec_offload(dev, priv);
+	err = mlx5e_tc_block_accel_offload(dev, priv);
 	if (err)
 		goto esw_release;
 
@@ -4914,7 +4922,7 @@ int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv,
 err_free:
 	mlx5e_flow_put(priv, flow);
 out:
-	mlx5e_tc_unblock_ipsec_offload(dev, priv);
+	mlx5e_tc_unblock_accel_offload(dev, priv);
 	mlx5_esw_put(priv->mdev);
 esw_release:
 	mlx5_esw_release(priv->mdev);
@@ -4957,7 +4965,7 @@ int mlx5e_delete_flower(struct net_device *dev, struct mlx5e_priv *priv,
 	trace_mlx5e_delete_flower(f);
 	mlx5e_flow_put(priv, flow);
 
-	mlx5e_tc_unblock_ipsec_offload(dev, priv);
+	mlx5e_tc_unblock_accel_offload(dev, priv);
 	mlx5_esw_put(priv->mdev);
 	return 0;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index b6e2c153b4f7..8b36a96522ab 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -3022,18 +3022,6 @@ int mlx5_esw_try_lock(struct mlx5_eswitch *esw)
 	return esw->mode;
 }
 
-int mlx5_esw_lock(struct mlx5_eswitch *esw)
-{
-	down_write(&esw->mode_lock);
-
-	if (esw->eswitch_operation_in_progress) {
-		up_write(&esw->mode_lock);
-		return -EBUSY;
-	}
-
-	return 0;
-}
-
 /**
  * mlx5_esw_unlock() - Release write lock on esw mode lock
  * @esw: eswitch device.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index c655f6e8da1c..33355a97226a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -946,7 +946,6 @@ void mlx5_esw_release(struct mlx5_core_dev *dev);
 void mlx5_esw_get(struct mlx5_core_dev *dev);
 void mlx5_esw_put(struct mlx5_core_dev *dev);
 int mlx5_esw_try_lock(struct mlx5_eswitch *esw);
-int mlx5_esw_lock(struct mlx5_eswitch *esw);
 void mlx5_esw_unlock(struct mlx5_eswitch *esw);
 
 void esw_vport_change_handle_locked(struct mlx5_vport *vport);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 5f28d906c35b..46b34c80c458 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1810,6 +1810,7 @@ int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx)
 	lockdep_register_key(&dev->lock_key);
 	mutex_init(&dev->intf_state_mutex);
 	lockdep_set_class(&dev->intf_state_mutex, &dev->lock_key);
+	mutex_init(&dev->offload_block.lock);
 	mutex_init(&dev->mlx5e_res.uplink_netdev_lock);
 	mutex_init(&dev->wc_state_lock);
 
@@ -1901,6 +1902,7 @@ int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx)
 	mutex_destroy(&priv->alloc_mutex);
 	mutex_destroy(&priv->bfregs.wc_head.lock);
 	mutex_destroy(&priv->bfregs.reg_head.lock);
+	mutex_destroy(&dev->offload_block.lock);
 	mutex_destroy(&dev->intf_state_mutex);
 	lockdep_unregister_key(&dev->lock_key);
 	return err;
@@ -1928,6 +1930,7 @@ void mlx5_mdev_uninit(struct mlx5_core_dev *dev)
 	mutex_destroy(&priv->bfregs.reg_head.lock);
 	mutex_destroy(&dev->wc_state_lock);
 	mutex_destroy(&dev->mlx5e_res.uplink_netdev_lock);
+	mutex_destroy(&dev->offload_block.lock);
 	mutex_destroy(&dev->intf_state_mutex);
 	lockdep_unregister_key(&dev->lock_key);
 }
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 83d0a83bbfbc..3bdb0c5a87ac 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -788,8 +788,11 @@ struct mlx5_core_dev {
 	u32                      vsc_addr;
 	struct mlx5_hv_vhca	*hv_vhca;
 	struct mlx5_hwmon	*hwmon;
-	u64			num_block_tc;
-	u64			num_block_ipsec;
+	struct {
+		struct mutex lock;
+		u64 num_tc;
+		u64 num_accel;
+	} offload_block;
 #ifdef CONFIG_MLX5_MACSEC
 	struct mlx5_macsec_fs *macsec_fs;
 	/* MACsec notifier chain to sync MACsec core and IB database */
-- 
2.44.0

[PATCH net-next V4 02/14] net/mlx5e: ipsec: Block TC offload when IPsec is enabled

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Currently the mutual exclusion mechanism is only used in packet offload
mode. But with the upcoming changes to flow_tag, all RX IPsec flows need
to block TC offload, so do that with the help of a small helper.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index 74e0aa5b6133..f236672d3a2a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -2575,11 +2575,18 @@ void mlx5e_accel_ipsec_fs_read_stats(struct mlx5e_priv *priv, void *ipsec_stats)
 	}
 }
 
+static bool accel_ipsec_should_block_tc(struct mlx5e_ipsec_sa_entry *sa_entry)
+{
+	return sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET ||
+		sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_IN;
+}
+
 int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 {
+	bool block_tc = accel_ipsec_should_block_tc(sa_entry);
 	int err;
 
-	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) {
+	if (block_tc) {
 		err = mlx5e_accel_block_tc_offload(sa_entry->ipsec->mdev);
 		if (err)
 			return err;
@@ -2596,7 +2603,7 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 	return 0;
 
 err_out:
-	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET)
+	if (block_tc)
 		mlx5e_accel_unblock_tc_offload(sa_entry->ipsec->mdev);
 	return err;
 }
@@ -2611,7 +2618,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 	if (ipsec_rule->pkt_reformat)
 		mlx5_packet_reformat_dealloc(mdev, ipsec_rule->pkt_reformat);
 
-	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET)
+	if (accel_ipsec_should_block_tc(sa_entry))
 		mlx5e_accel_unblock_tc_offload(mdev);
 
 	if (sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_OUT) {
-- 
2.44.0

[PATCH net-next V4 03/14] net/mlx5e: psp: Block TC offload when PSP is enabled

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Make use of the mutual exclusion mechanism from the previous patch to
make sure only PSP or TC offload is active on a device. Later in the
series, the PSP protocol marker will move to the flow_tag, which is also
used by TC offload.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_accel/psp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
index 73b232379263..6cc4b9d54f6e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
@@ -6,6 +6,7 @@
 #include "mlx5_core.h"
 #include "psp.h"
 #include "lib/crypto.h"
+#include "en_accel/en_accel.h"
 #include "en_accel/psp.h"
 #include "fs_core.h"
 
@@ -526,6 +527,7 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 static void accel_psp_fs_rx_destroy(struct mlx5e_psp_fs *fs)
 {
 	struct mlx5_ttc_table *ttc = mlx5e_fs_get_ttc(fs->fs, false);
+	bool tc_blocked = fs->rx.ft;
 	int i;
 
 	/* disconnect */
@@ -535,6 +537,8 @@ static void accel_psp_fs_rx_destroy(struct mlx5e_psp_fs *fs)
 	}
 	accel_psp_fs_rx_check_ft_destroy(&fs->check);
 	accel_psp_fs_rx_ft_destroy(&fs->rx);
+	if (tc_blocked)
+		mlx5e_accel_unblock_tc_offload(fs->mdev);
 }
 
 static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
@@ -543,10 +547,16 @@ static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
 	struct mlx5_ttc_table *ttc = mlx5e_fs_get_ttc(fs->fs, false);
 	int i, err;
 
+	err = mlx5e_accel_block_tc_offload(fs->mdev);
+	if (err) {
+		NL_SET_ERR_MSG(extack, "TC offload active, cannot enable PSP");
+		return err;
+	}
+
 	err = accel_psp_fs_rx_ft_create(fs, &fs->rx);
 	if (err) {
 		NL_SET_ERR_MSG(extack, "Failed creating RX steering table");
-		return err;
+		goto err_unblock_tc;
 	}
 
 	err = accel_psp_fs_rx_check_ft_create(fs, &fs->check);
@@ -583,6 +593,8 @@ static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
 	accel_psp_fs_rx_check_ft_destroy(&fs->check);
 err_ft:
 	accel_psp_fs_rx_ft_destroy(&fs->rx);
+err_unblock_tc:
+	mlx5e_accel_unblock_tc_offload(fs->mdev);
 	return err;
 }
 
-- 
2.44.0

[PATCH net-next V4 04/14] net/mlx5e: macsec: Block TC offload when MACsec is enabled

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

The MACsec protocol marker will soon move to flow_tag and that will make
TC offload unusable at the same time as MACsec on the same device.

This patch makes use of the mutual exclusion mechanism to make sure that
TC and MACsec cannot be both active at the same time.

One extra bit of logic is in macsec_upd_secy_hw_address(), where
existing macsec rules are drained then readded. During the two loops
it's possible a mistimed TC filter add to throw a wrench into things and
prevent the 2nd loop from adding anything, since accel rules are now
blocked. Fix that by keeping a best-effort TC block across the entire
operation.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/macsec.c      | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
index daff53ba7d09..a15a0aff292f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
@@ -10,6 +10,7 @@
 #include "en.h"
 #include "lib/aso.h"
 #include "lib/crypto.h"
+#include "en_accel/en_accel.h"
 #include "en_accel/macsec.h"
 
 #define MLX5_MACSEC_EPN_SCOPE_MID 0x80000000L
@@ -324,6 +325,8 @@ static void mlx5e_macsec_cleanup_sa_fs(struct mlx5e_macsec *macsec,
 	mlx5_macsec_fs_del_rule(macsec->mdev->macsec_fs, sa->macsec_rule, action, netdev,
 				fs_id);
 	sa->macsec_rule = NULL;
+	if (!is_tx)
+		mlx5e_accel_unblock_tc_offload(macsec->mdev);
 }
 
 static void mlx5e_macsec_cleanup_sa(struct mlx5e_macsec *macsec,
@@ -343,6 +346,7 @@ static int mlx5e_macsec_init_sa_fs(struct macsec_context *ctx,
 	const struct macsec_tx_sc *tx_sc = &ctx->secy->tx_sc;
 	struct mlx5_macsec_rule_attrs rule_attrs;
 	union mlx5_macsec_rule *macsec_rule;
+	int err = 0;
 
 	if (is_tx && tx_sc->encoding_sa != sa->assoc_num)
 		return 0;
@@ -353,13 +357,26 @@ static int mlx5e_macsec_init_sa_fs(struct macsec_context *ctx,
 	rule_attrs.action = (is_tx) ? MLX5_ACCEL_MACSEC_ACTION_ENCRYPT :
 				      MLX5_ACCEL_MACSEC_ACTION_DECRYPT;
 
+	if (!is_tx) {
+		err = mlx5e_accel_block_tc_offload(priv->mdev);
+		if (err)
+			return err;
+	}
+
 	macsec_rule = mlx5_macsec_fs_add_rule(macsec_fs, ctx, &rule_attrs, fs_id);
-	if (!macsec_rule)
-		return -ENOMEM;
+	if (!macsec_rule) {
+		err = -ENOMEM;
+		goto out_unblock_tc;
+	}
 
 	sa->macsec_rule = macsec_rule;
 
 	return 0;
+
+out_unblock_tc:
+	if (!is_tx)
+		mlx5e_accel_unblock_tc_offload(priv->mdev);
+	return err;
 }
 
 static int mlx5e_macsec_init_sa(struct macsec_context *ctx,
@@ -1137,7 +1154,12 @@ static int macsec_upd_secy_hw_address(struct macsec_context *ctx,
 	struct mlx5e_macsec_sa *rx_sa;
 	struct list_head *list;
 	int i, err = 0;
+	bool block_tc;
 
+	/* Best-effort TC block across the operation, to prevent a mistimed TC
+	 * filter add from preventing the 2nd loop from happening.
+	 */
+	block_tc = mlx5e_accel_block_tc_offload(priv->mdev) == 0;
 
 	list = &macsec_device->macsec_rx_sc_list_head;
 	list_for_each_entry_safe(rx_sc, tmp, list, rx_sc_list_element) {
@@ -1168,6 +1190,8 @@ static int macsec_upd_secy_hw_address(struct macsec_context *ctx,
 
 	memcpy(macsec_device->dev_addr, dev->dev_addr, dev->addr_len);
 out:
+	if (block_tc)
+		mlx5e_accel_unblock_tc_offload(priv->mdev);
 	return err;
 }
 
-- 
2.44.0

[PATCH net-next V4 05/14] net/mlx5e: psp: Move RX marker from ft_metadata to flow_tag

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Move PSP RX marker from ft_metadata (set via modify_hdr action) to
flow_tag (set via flow_context). This frees ft_metadata for storing
SPI for future decapsulated PSP packets.

All mlx5e accel protos have to move to avoid misinterpreting packets
with high bits of ft_metadata as IPsec or Macsec. This patch is the
first step, defining a new header, bit layout and macros.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/flow_tag.h    | 41 +++++++++++++++++++
 .../mellanox/mlx5/core/en_accel/psp.c         | 36 ++++------------
 .../mellanox/mlx5/core/en_accel/psp_rxtx.h    | 10 ++---
 3 files changed, 55 insertions(+), 32 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
new file mode 100644
index 000000000000..b6fcd4b63031
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+
+#ifndef __MLX5E_FLOW_TAG_H__
+#define __MLX5E_FLOW_TAG_H__
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/mlx5/device.h>
+
+/* Unified accel flow_tag layout in CQE sop_drop_qpn [23:0]:
+ *
+ *   [23:21] = protocol ID (3 bits):
+ *              0 = none (default)
+ *              3 = PSP (HW decrypted, PSP header present)
+ *              1,2,4-7 = reserved
+ *   [20:16] = reserved
+ *   [15:0]  = used by other subsystems (e.g. TC).
+ */
+#define MLX5E_ACCEL_FLOW_TAG_MASK		GENMASK(23, 0)
+#define MLX5E_ACCEL_FLOW_TAG_PROTO_MASK		GENMASK(23, 21)
+
+enum {
+	MLX5E_ACCEL_FLOW_TAG_PROTO_NONE		= 0,
+	MLX5E_ACCEL_FLOW_TAG_PROTO_PSP		= 3,
+};
+
+static inline u32 mlx5e_accel_flow_tag(struct mlx5_cqe64 *cqe)
+{
+	return be32_to_cpu(cqe->sop_drop_qpn) & MLX5E_ACCEL_FLOW_TAG_MASK;
+}
+
+static inline u32 mlx5e_accel_flow_tag_proto(struct mlx5_cqe64 *cqe)
+{
+	return FIELD_GET(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+			 mlx5e_accel_flow_tag(cqe));
+}
+
+#endif /* __MLX5E_FLOW_TAG_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
index 6cc4b9d54f6e..20d7f14fb4ac 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
@@ -8,6 +8,7 @@
 #include "lib/crypto.h"
 #include "en_accel/en_accel.h"
 #include "en_accel/psp.h"
+#include "en_accel/psp_rxtx.h"
 #include "fs_core.h"
 
 enum accel_fs_psp_type {
@@ -42,7 +43,6 @@ struct mlx5e_psp_rx_decrypt_table {
 	struct mlx5_flow_table *ft;
 	struct mlx5_flow_group *miss_group;
 	struct mlx5_flow_handle *miss_rule;
-	struct mlx5_modify_hdr *rx_modify_hdr;
 	struct mlx5_flow_handle *rule;
 };
 
@@ -408,10 +408,6 @@ accel_psp_fs_rx_decrypt_ft_destroy(struct mlx5e_psp_fs *fs,
 				   struct mlx5e_psp_rx_decrypt_table *decrypt)
 {
 	accel_psp_fs_del_flow_rule(&decrypt->rule);
-	if (decrypt->rx_modify_hdr) {
-		mlx5_modify_header_dealloc(fs->mdev, decrypt->rx_modify_hdr);
-		decrypt->rx_modify_hdr = NULL;
-	}
 	accel_psp_fs_del_flow_rule(&decrypt->miss_rule);
 	accel_psp_fs_destroy_flow_group(&decrypt->miss_group);
 	accel_psp_fs_destroy_ft(&decrypt->ft);
@@ -431,8 +427,6 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 				  struct mlx5e_psp_rx_decrypt_table *decrypt,
 				  struct mlx5_flow_destination *default_dest)
 {
-	u8 action[MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto)] = {};
-	struct mlx5_modify_hdr *modify_hdr = NULL;
 	struct mlx5_flow_table_attr ft_attr = {};
 	struct mlx5_flow_destination dest = {};
 	struct mlx5_core_dev *mdev = fs->mdev;
@@ -482,28 +476,16 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 
 	/* Add PSP RX decrypt rule */
 	setup_fte_udp_psp(spec, PSP_DEFAULT_UDP_PORT);
-	flow_act.crypto.type = MLX5_FLOW_CONTEXT_ENCRYPT_DECRYPT_TYPE_PSP;
-	/* Set bit[31, 30] PSP marker */
-#define MLX5E_PSP_MARKER_BIT (BIT(30) | BIT(31))
-	MLX5_SET(set_action_in, action, action_type, MLX5_ACTION_TYPE_SET);
-	MLX5_SET(set_action_in, action, field, MLX5_ACTION_IN_FIELD_METADATA_REG_B);
-	MLX5_SET(set_action_in, action, data, MLX5E_PSP_MARKER_BIT);
-	MLX5_SET(set_action_in, action, offset, 0);
-	MLX5_SET(set_action_in, action, length, 32);
-
-	modify_hdr = mlx5_modify_header_alloc(mdev, MLX5_FLOW_NAMESPACE_KERNEL, 1, action);
-	if (IS_ERR(modify_hdr)) {
-		err = PTR_ERR(modify_hdr);
-		mlx5_core_err(mdev, "fail to alloc psp set modify_header_id err=%d\n", err);
-		modify_hdr = NULL;
-		goto out_err;
-	}
-	decrypt->rx_modify_hdr = modify_hdr;
 
+	/* Set PSP marker via flow_tag */
+	spec->flow_context.flags = FLOW_CONTEXT_HAS_TAG;
+	spec->flow_context.flow_tag =
+		FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+			   MLX5E_ACCEL_FLOW_TAG_PROTO_PSP);
+
+	flow_act.crypto.type = MLX5_FLOW_CONTEXT_ENCRYPT_DECRYPT_TYPE_PSP;
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
-			  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |
-			  MLX5_FLOW_CONTEXT_ACTION_MOD_HDR;
-	flow_act.modify_hdr = modify_hdr;
+			  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT;
 	dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
 	dest.ft = fs->check.ft;
 	rule = mlx5_add_flow_rules(decrypt->ft, spec, &flow_act, &dest, 1);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
index 2b080c39cc37..a26faf7cfc27 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
@@ -9,10 +9,7 @@
 #include <net/psp.h>
 #include "en.h"
 #include "en/txrx.h"
-
-/* Bit30: PSP marker, Bit22-0: PSP obj id */
-#define MLX5_PSP_METADATA_MARKER(metadata)  ((((metadata) >> 30) & 0x3) == 0x3)
-#define MLX5_PSP_METADATA_HANDLE(metadata)  ((metadata) & GENMASK(22, 0))
+#include "en_accel/flow_tag.h"
 
 struct mlx5e_accel_tx_psp_state {
 	u32 tailen;
@@ -82,7 +79,10 @@ static inline unsigned int mlx5e_psp_tx_ids_len(struct mlx5e_accel_tx_psp_state
 
 static inline bool mlx5e_psp_is_rx_flow(struct mlx5_cqe64 *cqe)
 {
-	return MLX5_PSP_METADATA_MARKER(be32_to_cpu(cqe->ft_metadata));
+	u32 proto = mlx5e_accel_flow_tag_proto(cqe);
+
+	return proto == MLX5E_ACCEL_FLOW_TAG_PROTO_PSP;
+
 }
 
 bool mlx5e_psp_offload_handle_rx_skb(struct net_device *netdev, struct sk_buff *skb,
-- 
2.44.0

[PATCH net-next V4 06/14] net/mlx5e: ipsec: Move RX marker from ft_metadata to flow_tag

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

IPsec used BIT(31) in cqe.ft_metadata as a marker for ingress
IPsec-decrypted packets. This will conflict with PSP which stores SPI
in ft_metadata - an SPI with bit 31 set would falsely trigger
mlx5_ipsec_is_rx_flow().

Define a new marker for IPsec in flow_tag and use it in steering rules.
The obj_id stays in ft_metadata bits[23:0].
Remove the unused MARKER and SYNDROM (sic) macros.

This is only done for IPsec NIC RX flows. The esw path uses a different
mechanism and is unaffected: RX handling in mlx5e_rep_tc_receive() makes
use of a few bits in ft_metadata to detect and hand off IPsec packets to
mlx5e_ipsec_offload_handle_rx_skb().

An additional complication is that mlx5e_cqe_regb_chain() used the high
order bits of ft_metadata to differentiate between TC chains and
protocol markers. With the IPSec marker moving to flow_tag, this would
falsely trigger mlx5e_cqe_regb_chain() to believe the chain is set, when
in fact, ft_metadata is something completely different. Solve that by
requiring that the new flow_tag proto is NONE, since only those packets
can carry a chain id in ft_metadata.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en_accel/flow_tag.h   |  4 +++-
 .../ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c   | 11 +++++++++--
 .../ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h |  8 ++++----
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.h       |  7 +++++--
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
index b6fcd4b63031..cf30096d6a37 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
@@ -14,8 +14,9 @@
  *
  *   [23:21] = protocol ID (3 bits):
  *              0 = none (default)
+ *              1 = IPsec
  *              3 = PSP (HW decrypted, PSP header present)
- *              1,2,4-7 = reserved
+ *              2,4-7 = reserved
  *   [20:16] = reserved
  *   [15:0]  = used by other subsystems (e.g. TC).
  */
@@ -24,6 +25,7 @@
 
 enum {
 	MLX5E_ACCEL_FLOW_TAG_PROTO_NONE		= 0,
+	MLX5E_ACCEL_FLOW_TAG_PROTO_IPSEC	= 1,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_PSP		= 3,
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index f236672d3a2a..5565c385258c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -5,6 +5,7 @@
 #include "en.h"
 #include "en/fs.h"
 #include "en_accel/en_accel.h"
+#include "en_accel/flow_tag.h"
 #include "eswitch.h"
 #include "ipsec.h"
 #include "fs_core.h"
@@ -190,7 +191,7 @@ static void ipsec_rx_rule_add_match_obj(struct mlx5e_ipsec_sa_entry *sa_entry,
 				 misc_parameters_2.metadata_reg_c_2);
 		MLX5_SET(fte_match_param, spec->match_value,
 			 misc_parameters_2.metadata_reg_c_2,
-			 sa_entry->ipsec_obj_id | BIT(31));
+			 sa_entry->ipsec_obj_id);
 
 		spec->match_criteria_enable |= MLX5_MATCH_MISC_PARAMETERS_2;
 	}
@@ -2066,7 +2067,7 @@ static int rx_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 	if (!attrs->drop) {
 		if (rx != ipsec->rx_esw)
 			err = setup_modify_header(ipsec, attrs->type,
-						  sa_entry->ipsec_obj_id | BIT(31),
+						  sa_entry->ipsec_obj_id,
 						  XFRM_DEV_OFFLOAD_IN, &flow_act);
 		else
 			err = mlx5_esw_ipsec_rx_setup_modify_header(sa_entry, &flow_act);
@@ -2099,6 +2100,12 @@ static int rx_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
 		flow_act.action |= MLX5_FLOW_CONTEXT_ACTION_DROP;
 	else
 		flow_act.action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	if (!attrs->drop && rx != ipsec->rx_esw) {
+		spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
+		spec->flow_context.flow_tag =
+			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+				   MLX5E_ACCEL_FLOW_TAG_PROTO_IPSEC);
+	}
 	dest[0].type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
 	dest[0].ft = rx->ft.status;
 	dest[1].type = MLX5_FLOW_DESTINATION_TYPE_COUNTER;
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 45b0d19e735c..57aabd5e61ec 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
@@ -38,10 +38,9 @@
 #include <net/xfrm.h>
 #include "en.h"
 #include "en/txrx.h"
+#include "en_accel/flow_tag.h"
 
-/* Bit31: IPsec marker, Bit30: reserved, Bit29-24: IPsec syndrome, Bit23-0: IPsec obj id */
-#define MLX5_IPSEC_METADATA_MARKER(metadata)  ((((metadata) >> 30) & 0x3) == 0x2)
-#define MLX5_IPSEC_METADATA_SYNDROM(metadata) (((metadata) >> 24) & GENMASK(5, 0))
+/* IPsec obj id in ft_metadata bits[23:0] */
 #define MLX5_IPSEC_METADATA_HANDLE(metadata)  ((metadata) & GENMASK(23, 0))
 
 struct mlx5e_accel_tx_ipsec_state {
@@ -74,7 +73,8 @@ static inline unsigned int mlx5e_ipsec_tx_ids_len(struct mlx5e_accel_tx_ipsec_st
 
 static inline bool mlx5_ipsec_is_rx_flow(struct mlx5_cqe64 *cqe)
 {
-	return MLX5_IPSEC_METADATA_MARKER(be32_to_cpu(cqe->ft_metadata));
+	return mlx5e_accel_flow_tag_proto(cqe) ==
+		MLX5E_ACCEL_FLOW_TAG_PROTO_IPSEC;
 }
 
 static inline bool mlx5e_ipsec_eseg_meta(struct mlx5_wqe_eth_seg *eseg)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
index e1b8cb78369f..53b6043e33d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.h
@@ -36,6 +36,7 @@
 #include <net/pkt_cls.h>
 #include "en.h"
 #include "eswitch.h"
+#include "en_accel/flow_tag.h"
 #include "en/tc_ct.h"
 #include "en/tc_tun.h"
 #include "en/tc/int_port.h"
@@ -370,11 +371,13 @@ struct mlx5e_tc_table *mlx5e_tc_table_alloc(void);
 void mlx5e_tc_table_free(struct mlx5e_tc_table *tc);
 static inline bool mlx5e_cqe_regb_chain(struct mlx5_cqe64 *cqe)
 {
-	u32 chain, reg_b;
+	u32 flow_tag, reg_b, chain;
 
+	flow_tag = mlx5e_accel_flow_tag_proto(cqe);
 	reg_b = be32_to_cpu(cqe->ft_metadata);
 
-	if (reg_b >> (MLX5E_TC_TABLE_CHAIN_TAG_BITS + ESW_ZONE_ID_BITS))
+	if (flow_tag != MLX5E_ACCEL_FLOW_TAG_PROTO_NONE ||
+	    (reg_b >> (MLX5E_TC_TABLE_CHAIN_TAG_BITS + ESW_ZONE_ID_BITS)))
 		return false;
 
 	chain = reg_b & MLX5E_TC_TABLE_CHAIN_TAG_MASK;
-- 
2.44.0

[PATCH net-next V4 07/14] net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

MACsec uses BIT(30) in cqe.ft_metadata as a marker to identify
MACsec-decrypted packets. This will conflict with PSP SPI values with
bit 30 set, which would falsely trigger mlx5e_macsec_is_rx_flow().

Define a new marker for MACsec in flow_tag and use it in steering rules.
The fs_id stays in ft_metadata bits[15:0].
Correct typos of MLX5_MACSEC_RX_METADAT_HANDLE and
MLX5_MACEC_RX_FS_ID_MAX while touching this.
Delete the METADATA_MARKER macro.

Set flow_tag on both with-SCI and without-SCI crypto rules.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/flow_tag.h    |  4 +++-
 .../mellanox/mlx5/core/en_accel/macsec.c      |  6 +++---
 .../mellanox/mlx5/core/en_accel/macsec.h      |  4 +++-
 .../mellanox/mlx5/core/lib/macsec_fs.c        | 21 ++++++++++++-------
 .../mellanox/mlx5/core/lib/macsec_fs.h        |  9 ++++----
 5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
index cf30096d6a37..1b6b0b3de738 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
@@ -15,8 +15,9 @@
  *   [23:21] = protocol ID (3 bits):
  *              0 = none (default)
  *              1 = IPsec
+ *              2 = MACsec
  *              3 = PSP (HW decrypted, PSP header present)
- *              2,4-7 = reserved
+ *              4-7 = reserved
  *   [20:16] = reserved
  *   [15:0]  = used by other subsystems (e.g. TC).
  */
@@ -26,6 +27,7 @@
 enum {
 	MLX5E_ACCEL_FLOW_TAG_PROTO_NONE		= 0,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_IPSEC	= 1,
+	MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC	= 2,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_PSP		= 3,
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
index a15a0aff292f..aa7a28954861 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
@@ -752,12 +752,12 @@ static int mlx5e_macsec_add_rxsc(struct macsec_context *ctx)
 	 * a non-NULL md_dst with uninitialised contents.
 	 */
 	err = xa_alloc(&macsec->sc_xarray, &sc_xarray_element->fs_id, sc_xarray_element,
-		       XA_LIMIT(1, MLX5_MACEC_RX_FS_ID_MAX), GFP_KERNEL);
+		       XA_LIMIT(1, MLX5_MACSEC_RX_FS_ID_MAX), GFP_KERNEL);
 	if (err) {
 		if (err == -EBUSY)
 			netdev_err(ctx->netdev,
 				   "MACsec offload: unable to create entry for RX SC (%d Rx SCs already allocated)\n",
-				   MLX5_MACEC_RX_FS_ID_MAX);
+				   MLX5_MACSEC_RX_FS_ID_MAX);
 		goto destroy_md_dst;
 	}
 
@@ -1724,7 +1724,7 @@ void mlx5e_macsec_offload_handle_rx_skb(struct net_device *netdev,
 	if (!macsec)
 		return;
 
-	fs_id = MLX5_MACSEC_RX_METADAT_HANDLE(macsec_meta_data);
+	fs_id = MLX5_MACSEC_RX_METADATA_HANDLE(macsec_meta_data);
 
 	rcu_read_lock();
 	sc_xarray_element = xa_load(&macsec->sc_xarray, fs_id);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.h
index 27df72e23106..2066cb573f68 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.h
@@ -9,6 +9,7 @@
 #include <linux/mlx5/driver.h>
 #include <net/macsec.h>
 #include <net/dst_metadata.h>
+#include "en_accel/flow_tag.h"
 #include "lib/macsec_fs.h"
 
 struct mlx5e_priv;
@@ -31,7 +32,8 @@ static inline bool mlx5e_macsec_skb_is_offload(struct sk_buff *skb)
 
 static inline bool mlx5e_macsec_is_rx_flow(struct mlx5_cqe64 *cqe)
 {
-	return MLX5_MACSEC_METADATA_MARKER(be32_to_cpu(cqe->ft_metadata));
+	return mlx5e_accel_flow_tag_proto(cqe) ==
+		MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC;
 }
 
 void mlx5e_macsec_offload_handle_rx_skb(struct net_device *netdev, struct sk_buff *skb,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
index efc5167493c1..4ca3a6ded284 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
@@ -8,6 +8,7 @@
 #include <linux/mlx5/macsec.h>
 #include "fs_core.h"
 #include "lib/macsec_fs.h"
+#include "en_accel/flow_tag.h"
 #include "mlx5_core.h"
 
 /* MACsec TX flow steering */
@@ -45,9 +46,6 @@
 #define MLX5_SECTAG_HEADER_SIZE_WITHOUT_SCI 0x8
 #define MLX5_SECTAG_HEADER_SIZE_WITH_SCI (MLX5_SECTAG_HEADER_SIZE_WITHOUT_SCI + MACSEC_SCI_LEN)
 
-/* MACsec fs_id handling for steering */
-#define macsec_fs_set_rx_fs_id(fs_id) ((fs_id) | BIT(30))
-
 struct mlx5_sectag_header {
 	__be16 ethertype;
 	u8 tci_an;
@@ -1757,11 +1755,10 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
 	rx_tables = &rx_fs->tables;
 	ft_crypto = &rx_tables->ft_crypto;
 
-	/* Set bit[31 - 30] macsec marker - 0x01 */
 	/* Set bit[15-0] fs id */
 	MLX5_SET(set_action_in, action, action_type, MLX5_ACTION_TYPE_SET);
 	MLX5_SET(set_action_in, action, field, MLX5_ACTION_IN_FIELD_METADATA_REG_B);
-	MLX5_SET(set_action_in, action, data, macsec_fs_set_rx_fs_id(fs_id));
+	MLX5_SET(set_action_in, action, data, fs_id);
 	MLX5_SET(set_action_in, action, offset, 0);
 	MLX5_SET(set_action_in, action, length, 32);
 
@@ -1778,6 +1775,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
 	/* Rx crypto table with SCI rule */
 	macsec_fs_rx_setup_fte(spec, &flow_act, attrs, true);
 
+	spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
+	spec->flow_context.flow_tag =
+		FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+			   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
+
 	flow_act.modify_hdr = modify_hdr;
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
 			  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |
@@ -1803,6 +1805,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
 
 		macsec_fs_rx_setup_fte(spec, &flow_act, attrs, false);
 
+		spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
+		spec->flow_context.flow_tag =
+			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+				   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
+
 		flow_act.modify_hdr = modify_hdr;
 		flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
 				  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |
@@ -2160,8 +2167,8 @@ static int mlx5_macsec_fs_add_roce_rule_rx(struct mlx5_macsec_fs *macsec_fs, u32
 
 	spec->match_criteria_enable |= MLX5_MATCH_MISC_PARAMETERS_2;
 	MLX5_SET_TO_ONES(fte_match_param, spec->match_criteria, misc_parameters_2.metadata_reg_c_5);
-	MLX5_SET(fte_match_param, spec->match_value, misc_parameters_2.metadata_reg_c_5,
-		 macsec_fs_set_rx_fs_id(fs_id));
+	MLX5_SET(fte_match_param, spec->match_value,
+		 misc_parameters_2.metadata_reg_c_5, fs_id);
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_ALLOW;
 	new_rule = mlx5_add_flow_rules(rx_fs->roce.ft_macsec_op_check, spec, &flow_act,
 				       NULL, 0);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.h
index 15acaff43641..b8b8b412d5c3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.h
@@ -6,11 +6,10 @@
 
 #ifdef CONFIG_MLX5_MACSEC
 
-/* Bit31 - 30: MACsec marker, Bit15-0: MACsec id */
-#define MLX5_MACEC_RX_FS_ID_MAX USHRT_MAX /* Must be power of two */
-#define MLX5_MACSEC_RX_FS_ID_MASK MLX5_MACEC_RX_FS_ID_MAX
-#define MLX5_MACSEC_METADATA_MARKER(metadata)  ((((metadata) >> 30) & 0x3)  == 0x1)
-#define MLX5_MACSEC_RX_METADAT_HANDLE(metadata)  ((metadata) & MLX5_MACSEC_RX_FS_ID_MASK)
+/* MACsec fs_id in ft_metadata bits[15:0] */
+#define MLX5_MACSEC_RX_FS_ID_MAX USHRT_MAX /* Must be power of two */
+#define MLX5_MACSEC_RX_FS_ID_MASK MLX5_MACSEC_RX_FS_ID_MAX
+#define MLX5_MACSEC_RX_METADATA_HANDLE(metadata)  ((metadata) & MLX5_MACSEC_RX_FS_ID_MASK)
 
 /* MACsec TX flow steering */
 #define MLX5_ETH_WQE_FT_META_MACSEC_MASK \
-- 
2.44.0

[PATCH net-next V4 08/14] net/mlx5e: psp: Handle HW-decapsulated RX PSP packets

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

mlx5e_psp_offload_handle_rx_skb() handles RX PSP packets.

Add an additional flow tag marker and helpers for decapsulated PSP
packets and extend the handler to construct the PSP skb extension from
the CQE fields (SPI in ft_metadata and PSP version in flow_tag).

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/flow_tag.h    |  8 +++--
 .../mellanox/mlx5/core/en_accel/psp_rxtx.c    | 21 +++++++++--
 .../mellanox/mlx5/core/en_accel/psp_rxtx.h    | 36 ++++++++++++++++++-
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
index 1b6b0b3de738..cd10a74e2b27 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
@@ -17,18 +17,22 @@
  *              1 = IPsec
  *              2 = MACsec
  *              3 = PSP (HW decrypted, PSP header present)
- *              4-7 = reserved
- *   [20:16] = reserved
+ *              4 = PSP decap (HW decrypted & decapsulated)
+ *              5-7 = reserved
+ *   [20:17] = PSP version (4 bits, valid when proto == PSP decap)
+ *   16      = reserved
  *   [15:0]  = used by other subsystems (e.g. TC).
  */
 #define MLX5E_ACCEL_FLOW_TAG_MASK		GENMASK(23, 0)
 #define MLX5E_ACCEL_FLOW_TAG_PROTO_MASK		GENMASK(23, 21)
+#define MLX5E_ACCEL_FLOW_TAG_PSP_VER_MASK	GENMASK(20, 17)
 
 enum {
 	MLX5E_ACCEL_FLOW_TAG_PROTO_NONE		= 0,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_IPSEC	= 1,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC	= 2,
 	MLX5E_ACCEL_FLOW_TAG_PROTO_PSP		= 3,
+	MLX5E_ACCEL_FLOW_TAG_PROTO_PSP_DECAP	= 4,
 };
 
 static inline u32 mlx5e_accel_flow_tag(struct mlx5_cqe64 *cqe)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
index 348fd7a96261..2beffee14278 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
@@ -118,10 +118,25 @@ bool mlx5e_psp_offload_handle_rx_skb(struct net_device *netdev, struct sk_buff *
 {
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 	u16 dev_id = priv->psp->psd->id;
-	bool strip_icv = true;
-	u8 generation = 0;
+	struct psp_skb_ext *pse;
+
+	if (mlx5e_psp_is_decap(cqe)) {
+		/* UDP + PSP headers and PSP trailer removed by HW.
+		 * Construct the PSP extension from CQE metadata.
+		 */
+		pse = skb_ext_add(skb, SKB_EXT_PSP);
+		if (unlikely(!pse))
+			goto drop;
+
+		pse->spi = mlx5e_psp_get_spi(cqe);
+		pse->version = mlx5e_psp_get_version(cqe);
+		pse->dev_id = dev_id;
+		pse->generation = 0;
+		skb->decrypted = 1;
+		return false;
+	}
 
-	if (psp_dev_rcv(skb, dev_id, generation, strip_icv))
+	if (psp_dev_rcv(skb, dev_id, 0, true))
 		goto drop;
 
 	skb->decrypted = 1;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
index a26faf7cfc27..32a8330027c0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.h
@@ -81,10 +81,29 @@ static inline bool mlx5e_psp_is_rx_flow(struct mlx5_cqe64 *cqe)
 {
 	u32 proto = mlx5e_accel_flow_tag_proto(cqe);
 
-	return proto == MLX5E_ACCEL_FLOW_TAG_PROTO_PSP;
+	return proto == MLX5E_ACCEL_FLOW_TAG_PROTO_PSP ||
+		proto == MLX5E_ACCEL_FLOW_TAG_PROTO_PSP_DECAP;
 
 }
 
+static inline bool mlx5e_psp_is_decap(struct mlx5_cqe64 *cqe)
+{
+	u32 proto = mlx5e_accel_flow_tag_proto(cqe);
+
+	return proto == MLX5E_ACCEL_FLOW_TAG_PROTO_PSP_DECAP;
+}
+
+static inline u8 mlx5e_psp_get_version(struct mlx5_cqe64 *cqe)
+{
+	return FIELD_GET(MLX5E_ACCEL_FLOW_TAG_PSP_VER_MASK,
+			 mlx5e_accel_flow_tag(cqe));
+}
+
+static inline __be32 mlx5e_psp_get_spi(struct mlx5_cqe64 *cqe)
+{
+	return cqe->ft_metadata;
+}
+
 bool mlx5e_psp_offload_handle_rx_skb(struct net_device *netdev, struct sk_buff *skb,
 				     struct mlx5_cqe64 *cqe);
 #else
@@ -110,6 +129,21 @@ static inline bool mlx5e_psp_is_rx_flow(struct mlx5_cqe64 *cqe)
 	return false;
 }
 
+static inline bool mlx5e_psp_is_decap(struct mlx5_cqe64 *cqe)
+{
+	return false;
+}
+
+static inline u8 mlx5e_psp_get_version(struct mlx5_cqe64 *cqe)
+{
+	return 0;
+}
+
+static inline __be32 mlx5e_psp_get_spi(struct mlx5_cqe64 *cqe)
+{
+	return 0;
+}
+
 static inline bool mlx5e_psp_offload_handle_rx_skb(struct net_device *netdev,
 						   struct sk_buff *skb,
 						   struct mlx5_cqe64 *cqe)
-- 
2.44.0

[PATCH net-next V4 09/14] net/mlx5e: psp: Add an rx_decap steering table

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Introduce an additional steering table for PSP transport mode
decapsulation, containing:
- one rule per supported PSP version which does:
  - transport mode decap (removes UDP+PSP headers and PSP trailer)
    - recomputes iph->tot_len
    - recomputes IP checksum
    - reparses packet headers
  - copy SPI into reg_b (which ends up as cqe.ft_metadata)
  - set a decap marker and the PSP version in the flow_tag, so the RX
    handler can make sense of the packet
- default drop rule for unsupported PSP versions (per PSP spec).

Packets are forwarded to the previously added rx table, where:
- one rule forwards UDP traffic to the UDP default destination.
- default rule forwards traffic to the TTC table.

The reason is to avoid steering loops. If packets were to be injected
into the TTC directly after rx_decap, it may be possible to create a
steering loop with RX packets of the form IP|UDP|PSP|UDP|PSP...
The rx flow table guarantees that packets go through PSP steering at
most once.

The steering mode is saved in a new field 'fs.decap_enabled'.
Updating the mode is done through accel_psp_fs_rx_reconfigure(), which
creates the decap steering table if needed and possible. It then uses an
atomic rule update to redirect traffic to the new table.
This is now invoked with decap_wanted == false.
The intention is for failures creating the new table to not block
feature reconfig/PSP toggling. Decap table creation and traffic
redirection failures therefore result in a warning that PSP HW GRO will
not work.
Nothing happens on older HW/FW without the ability to decapsulate PSP
transport.

An upcoming patch will add dynamic reconfiguration of PSP steering based
on HW GRO.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en/fs.h   |   1 +
 .../mellanox/mlx5/core/en_accel/psp.c         | 249 +++++++++++++++++-
 2 files changed, 248 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h b/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
index 4973fb473ff0..a802f80d90be 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
@@ -98,6 +98,7 @@ enum {
 #if defined(CONFIG_MLX5_EN_PSP)
 	MLX5E_ACCEL_FS_PSP_FT_LEVEL = MLX5E_INNER_TTC_FT_LEVEL + 1,
 	MLX5E_ACCEL_FS_PSP_ERR_FT_LEVEL,
+	MLX5E_ACCEL_FS_PSP_DECAP_FT_LEVEL,
 	MLX5E_ACCEL_FS_PSP_RX_FT_LEVEL,
 #endif
 };
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
index 20d7f14fb4ac..84133cbf8689 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
@@ -23,6 +23,13 @@ enum accel_psp_syndrome {
 	PSP_BAD_TRAILER,
 };
 
+static const u8 psp_supported_versions[] = {
+	PSP_VERSION_HDR0_AES_GCM_128,
+	PSP_VERSION_HDR0_AES_GCM_256,
+};
+
+#define MLX5E_PSP_NUM_SUPPORTED_VERSIONS ARRAY_SIZE(psp_supported_versions)
+
 struct mlx5e_psp_tx_table {
 	struct mlx5_flow_namespace *ns;
 	struct mlx5_flow_table *ft;
@@ -43,6 +50,7 @@ struct mlx5e_psp_rx_decrypt_table {
 	struct mlx5_flow_table *ft;
 	struct mlx5_flow_group *miss_group;
 	struct mlx5_flow_handle *miss_rule;
+	struct mlx5_modify_hdr *modify_hdr;
 	struct mlx5_flow_handle *rule;
 };
 
@@ -53,6 +61,15 @@ struct mlx5e_psp_rx_table {
 	struct mlx5_flow_handle *udp_rules[ACCEL_FS_PSP_NUM_TYPES];
 };
 
+struct mlx5e_psp_rx_decap_table {
+	struct mlx5_flow_table *ft;
+	struct mlx5_flow_group *drop_group;
+	struct mlx5_modify_hdr *modify_hdr;
+	struct mlx5_pkt_reformat *reformat;
+	struct mlx5_flow_handle *rule[MLX5E_PSP_NUM_SUPPORTED_VERSIONS];
+	struct mlx5_flow_handle *unsupported_rule;
+};
+
 struct mlx5e_psp_fs {
 	struct mlx5_core_dev *mdev;
 	struct mlx5_fc *tx_counter;
@@ -64,9 +81,14 @@ struct mlx5e_psp_fs {
 	struct mlx5_fc *rx_auth_fail_counter;
 	struct mlx5_fc *rx_err_counter;
 	struct mlx5_fc *rx_bad_counter;
+	/* When set, steering is configured to decapsulate PSP (remove UDP+PSP
+	 * headers and PSP trailer) and hand off the SPI in cqe.ft_metadata.
+	 */
+	bool decap_enabled;
 
 	struct mlx5e_psp_rx_decrypt_table decrypt[ACCEL_FS_PSP_NUM_TYPES];
 	struct mlx5e_psp_rx_check_table check;
+	struct mlx5e_psp_rx_decap_table decap;
 	struct mlx5e_psp_rx_table rx;
 };
 
@@ -111,6 +133,15 @@ static void accel_psp_fs_del_flow_rule(struct mlx5_flow_handle **rule)
 	}
 }
 
+static void accel_psp_fs_dealloc_modify_hdr(struct mlx5_core_dev *dev,
+					    struct mlx5_modify_hdr **modhdr)
+{
+	if (*modhdr) {
+		mlx5_modify_header_dealloc(dev, *modhdr);
+		*modhdr = NULL;
+	}
+}
+
 static int accel_psp_fs_create_miss_group(struct mlx5_flow_table *ft,
 					  struct mlx5_flow_group **group)
 {
@@ -403,11 +434,164 @@ int accel_psp_fs_rx_check_ft_create(struct mlx5e_psp_fs *fs,
 	return err;
 }
 
+static
+void accel_psp_fs_rx_decap_ft_destroy(struct mlx5e_psp_fs *fs,
+				      struct mlx5e_psp_rx_decap_table *decap)
+{
+	int i;
+
+	accel_psp_fs_del_flow_rule(&decap->unsupported_rule);
+	for (i = 0; i < MLX5E_PSP_NUM_SUPPORTED_VERSIONS; i++)
+		accel_psp_fs_del_flow_rule(&decap->rule[i]);
+	if (decap->reformat) {
+		mlx5_packet_reformat_dealloc(fs->mdev, decap->reformat);
+		decap->reformat = NULL;
+	}
+	accel_psp_fs_dealloc_modify_hdr(fs->mdev, &decap->modify_hdr);
+	accel_psp_fs_destroy_flow_group(&decap->drop_group);
+	accel_psp_fs_destroy_ft(&decap->ft);
+	fs->decap_enabled = false;
+}
+
+static void setup_fte_psp_version(struct mlx5_flow_spec *spec, u8 version)
+{
+	void *misc_params_6;
+
+	memset(spec, 0, sizeof(*spec));
+	spec->match_criteria_enable |= MLX5_MATCH_MISC_PARAMETERS_6;
+	misc_params_6 = MLX5_ADDR_OF(fte_match_param, spec->match_criteria,
+				     misc_parameters_6);
+	MLX5_SET_TO_ONES(fte_match_set_misc6, misc_params_6, psp_version);
+	misc_params_6 = MLX5_ADDR_OF(fte_match_param, spec->match_value,
+				     misc_parameters_6);
+	MLX5_SET(fte_match_set_misc6, misc_params_6, psp_version, version);
+}
+
+static
+int accel_psp_fs_rx_decap_ft_create(struct mlx5e_psp_fs *fs,
+				    struct mlx5e_psp_rx_decap_table *decap)
+{
+	u8 action[MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto)] = {};
+	struct mlx5_pkt_reformat_params reformat_params = {};
+	struct mlx5_flow_table_attr ft_attr = {};
+	struct mlx5_flow_destination dest = {};
+	struct mlx5_core_dev *mdev = fs->mdev;
+	struct mlx5_pkt_reformat *reformat;
+	struct mlx5_modify_hdr *modify_hdr;
+	struct mlx5_flow_handle *rule;
+	struct mlx5_flow_spec *spec;
+	int i, err = 0;
+
+	spec = kvzalloc_obj(*spec);
+	if (!spec)
+		return -ENOMEM;
+
+	/* Create FT */
+	ft_attr.max_fte = 1 + MLX5E_PSP_NUM_SUPPORTED_VERSIONS;
+	ft_attr.level = MLX5E_ACCEL_FS_PSP_DECAP_FT_LEVEL;
+	ft_attr.prio = MLX5E_NIC_PRIO;
+	ft_attr.autogroup.num_reserved_entries = 1;
+	err = accel_psp_fs_create_ft(fs, &ft_attr, &decap->ft);
+	if (err) {
+		mlx5_core_err(mdev, "fail to create psp decap rx ft err=%d\n",
+			      err);
+		goto out_spec;
+	}
+
+	/* Create drop group */
+	err = accel_psp_fs_create_miss_group(decap->ft, &decap->drop_group);
+	if (err) {
+		mlx5_core_err(mdev,
+			      "fail to create psp decap rx drop_group err=%d\n",
+			      err);
+		goto out_err;
+	}
+
+	/* Add default drop rule */
+	err = accel_psp_add_drop_rule(decap->ft, NULL, fs->rx_bad_counter,
+				      &decap->unsupported_rule);
+	if (err) {
+		mlx5_core_err(mdev,
+			      "fail to create psp decap unsupported versions drop rule err=%d\n",
+			      err);
+		goto out_err;
+	}
+
+	/* modify_hdr: copy SPI from REG_C_0 to REG_B */
+	MLX5_SET(copy_action_in, action, action_type, MLX5_ACTION_TYPE_COPY);
+	MLX5_SET(copy_action_in, action, src_field,
+		 MLX5_ACTION_IN_FIELD_METADATA_REG_C_0);
+	MLX5_SET(copy_action_in, action, src_offset, 0);
+	MLX5_SET(copy_action_in, action, length, 0);  /* 0 = 32 bits */
+	MLX5_SET(copy_action_in, action, dst_field,
+		 MLX5_ACTION_IN_FIELD_METADATA_REG_B);
+	MLX5_SET(copy_action_in, action, dst_offset, 0);
+
+	modify_hdr = mlx5_modify_header_alloc(mdev, MLX5_FLOW_NAMESPACE_KERNEL,
+					      1, action);
+	if (IS_ERR(modify_hdr)) {
+		err = PTR_ERR(modify_hdr);
+		goto out_err;
+	}
+	decap->modify_hdr = modify_hdr;
+
+	/* pkt_reformat: decap PSP transport */
+	reformat_params.type = MLX5_REFORMAT_TYPE_REMOVE_PSP_TRANSPORT;
+	reformat = mlx5_packet_reformat_alloc(mdev, &reformat_params,
+					      MLX5_FLOW_NAMESPACE_KERNEL);
+	if (IS_ERR(reformat)) {
+		err = PTR_ERR(reformat);
+		goto out_err;
+	}
+	decap->reformat = reformat;
+
+	for (i = 0; i < MLX5E_PSP_NUM_SUPPORTED_VERSIONS; i++) {
+		u8 version = psp_supported_versions[i];
+		struct mlx5_flow_act flow_act = {};
+
+		/* match(version) => decap, copy SPI, fwd to rx FT */
+		setup_fte_psp_version(spec, version);
+
+		/*
+		 * Override the flow tag set in the decrypt table with
+		 * the decap PSP marker and version.
+		 */
+		spec->flow_context.flags = FLOW_CONTEXT_HAS_TAG;
+		spec->flow_context.flow_tag =
+			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
+				   MLX5E_ACCEL_FLOW_TAG_PROTO_PSP_DECAP) |
+			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PSP_VER_MASK, version);
+
+		flow_act.action = MLX5_FLOW_CONTEXT_ACTION_PACKET_REFORMAT |
+			MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
+			MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+		flow_act.pkt_reformat = reformat;
+		flow_act.modify_hdr = modify_hdr;
+		dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+		dest.ft = fs->rx.ft;
+
+		rule = mlx5_add_flow_rules(decap->ft, spec, &flow_act, &dest, 1);
+		if (IS_ERR(rule)) {
+			err = PTR_ERR(rule);
+			goto out_err;
+		}
+		decap->rule[i] = rule;
+	}
+	goto out_spec;
+
+out_err:
+	accel_psp_fs_rx_decap_ft_destroy(fs, decap);
+out_spec:
+	kvfree(spec);
+	return err;
+}
+
 static void
 accel_psp_fs_rx_decrypt_ft_destroy(struct mlx5e_psp_fs *fs,
 				   struct mlx5e_psp_rx_decrypt_table *decrypt)
 {
 	accel_psp_fs_del_flow_rule(&decrypt->rule);
+	accel_psp_fs_dealloc_modify_hdr(fs->mdev, &decrypt->modify_hdr);
 	accel_psp_fs_del_flow_rule(&decrypt->miss_rule);
 	accel_psp_fs_destroy_flow_group(&decrypt->miss_group);
 	accel_psp_fs_destroy_ft(&decrypt->ft);
@@ -427,10 +611,12 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 				  struct mlx5e_psp_rx_decrypt_table *decrypt,
 				  struct mlx5_flow_destination *default_dest)
 {
+	u8 action[MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto)] = {};
 	struct mlx5_flow_table_attr ft_attr = {};
 	struct mlx5_flow_destination dest = {};
 	struct mlx5_core_dev *mdev = fs->mdev;
 	MLX5_DECLARE_FLOW_ACT(flow_act);
+	struct mlx5_modify_hdr *modhdr;
 	struct mlx5_flow_handle *rule;
 	struct mlx5_flow_spec *spec;
 	int err = 0;
@@ -474,6 +660,24 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 	}
 	decrypt->miss_rule = rule;
 
+	/* Create modify_hdr to copy SPI to REG_C_0 */
+	MLX5_SET(copy_action_in, action, action_type, MLX5_ACTION_TYPE_COPY);
+	MLX5_SET(copy_action_in, action, src_field,
+		 MLX5_ACTION_IN_FIELD_PSP_HEADER_1);
+	MLX5_SET(copy_action_in, action, src_offset, 0);
+	MLX5_SET(copy_action_in, action, length, 0);  /* 0 = 32 bits */
+	MLX5_SET(copy_action_in, action, dst_field,
+		 MLX5_ACTION_IN_FIELD_METADATA_REG_C_0);
+	MLX5_SET(copy_action_in, action, dst_offset, 0);
+
+	modhdr = mlx5_modify_header_alloc(mdev, MLX5_FLOW_NAMESPACE_KERNEL, 1,
+					  action);
+	if (IS_ERR(modhdr)) {
+		err = PTR_ERR(modhdr);
+		goto out_err;
+	}
+	decrypt->modify_hdr = modhdr;
+
 	/* Add PSP RX decrypt rule */
 	setup_fte_udp_psp(spec, PSP_DEFAULT_UDP_PORT);
 
@@ -484,8 +688,10 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 			   MLX5E_ACCEL_FLOW_TAG_PROTO_PSP);
 
 	flow_act.crypto.type = MLX5_FLOW_CONTEXT_ENCRYPT_DECRYPT_TYPE_PSP;
-	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
-			  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT;
+	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |
+		MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
+		MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	flow_act.modify_hdr = modhdr;
 	dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
 	dest.ft = fs->check.ft;
 	rule = mlx5_add_flow_rules(decrypt->ft, spec, &flow_act, &dest, 1);
@@ -506,6 +712,42 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
 	return err;
 }
 
+static void accel_psp_fs_rx_reconfigure(struct mlx5e_psp_fs *fs,
+					bool decap_wanted)
+{
+	bool decap_supported =
+		MLX5_CAP_FLOWTABLE(fs->mdev,
+				   flow_table_properties_nic_receive.reformat_del_psp_transport);
+	bool decap_enable = decap_wanted && decap_supported;
+	struct mlx5_flow_destination dest = {};
+	int err;
+
+	if (decap_enable == fs->decap_enabled)
+		return;
+
+	/* Create the decap table if needed. */
+	if (decap_enable && !fs->decap.ft) {
+		err = accel_psp_fs_rx_decap_ft_create(fs, &fs->decap);
+		if (err)
+			goto out_err;
+	}
+
+	/* Redirect traffic to the correct table. */
+	dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+	dest.ft = decap_enable ? fs->decap.ft : fs->rx.ft;
+	err = mlx5_modify_rule_destination(fs->check.rule, &dest, NULL);
+	if (err)
+		goto out_err;
+
+	fs->decap_enabled = decap_enable;
+	return;
+
+out_err:
+	mlx5_core_warn(fs->mdev,
+		       "Failed to create/modify PSP decapsulation rules (err %d), HW GRO for PSP unavailable\n",
+		       err);
+}
+
 static void accel_psp_fs_rx_destroy(struct mlx5e_psp_fs *fs)
 {
 	struct mlx5_ttc_table *ttc = mlx5e_fs_get_ttc(fs->fs, false);
@@ -518,6 +760,7 @@ static void accel_psp_fs_rx_destroy(struct mlx5e_psp_fs *fs)
 		accel_psp_fs_rx_decrypt_ft_destroy(fs, &fs->decrypt[i]);
 	}
 	accel_psp_fs_rx_check_ft_destroy(&fs->check);
+	accel_psp_fs_rx_decap_ft_destroy(fs, &fs->decap);
 	accel_psp_fs_rx_ft_destroy(&fs->rx);
 	if (tc_blocked)
 		mlx5e_accel_unblock_tc_offload(fs->mdev);
@@ -565,6 +808,8 @@ static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
 		mlx5_ttc_fwd_dest(ttc, fs_psp2tt(i), &dest);
 	}
 
+	accel_psp_fs_rx_reconfigure(fs, false);
+
 	return 0;
 
 err_decrypt_ft:
-- 
2.44.0

[PATCH net-next V4 10/14] net/mlx5e: shampo: Flush session on PSP mismatch

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Flush SHAMPO session on PSP status change (no PSP -> PSP or vice-versa)
or on SPI/version mismatch. HW aggregates by 5-tuple but is unaware of
PSP fields, so sessions must be terminated when security parameters
change to avoid mixing packets from different PSP associations.

The session is also forcefully flushed if non-decapsulated packets are
received. This could happen if, for example, the decap table could not
be created so HW GRO is active but there's no decapsulation.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index e3f915beebe1..9eb11be7abe0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -2224,6 +2224,33 @@ static bool mlx5e_hw_gro_skb_has_enough_space(struct sk_buff *skb,
 		return page_size * nr_frags + data_bcnt <= GRO_LEGACY_MAX_SIZE;
 }
 
+static bool mlx5e_hw_gro_psp_match(struct sk_buff *skb, struct mlx5_cqe64 *cqe)
+{
+#ifdef CONFIG_MLX5_EN_PSP
+	struct psp_skb_ext *pse = skb_ext_find(skb, SKB_EXT_PSP);
+	bool is_psp = mlx5e_psp_is_rx_flow(cqe);
+
+	if (likely(!is_psp && !pse))
+		return true;
+
+	/* No match on PSP status change (no crypto -> crypto or vice-versa). */
+	if (unlikely(is_psp != !!pse))
+		return false;
+
+	/* SPI and version are only available in CQE metadata for decap flows.
+	 * Non-decap PSP cannot be matched here, force a flush.
+	 */
+	if (unlikely(!mlx5e_psp_is_decap(cqe)))
+		return false;
+
+	/* No match on security parameters change. */
+	return pse->spi == mlx5e_psp_get_spi(cqe) &&
+		pse->version == mlx5e_psp_get_version(cqe);
+#else
+	return true;
+#endif
+}
+
 static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
 {
 	u16 data_bcnt		= mpwrq_get_cqe_byte_cnt(cqe) - cqe->shampo.header_size;
@@ -2266,8 +2293,9 @@ static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cq
 	}
 
 	if (*skb &&
-	    !(match && mlx5e_hw_gro_skb_has_enough_space(*skb, data_bcnt,
-							 page_size))) {
+	    !(match &&
+	      mlx5e_hw_gro_skb_has_enough_space(*skb, data_bcnt, page_size) &&
+	      mlx5e_hw_gro_psp_match(*skb, cqe))) {
 		match = false;
 		mlx5e_shampo_flush_skb(rq, cqe, match);
 	}
-- 
2.44.0

[PATCH net-next V4 11/14] net/mlx5e: psp: Dynamically reconfigure based on SHAMPO mode

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Add an mlx5e_update_nic_rx() -> mlx5e_psp_update_rx() hook.
This allows PSP steering to reconfigure when netdev features change.

When this signal is received, toggle PSP between standard and
decap modes when SHAMPO mode changes, optionally creating the decap
steering table.

The toggling of traffic is atomic (a single steering rule update), so no
packets should get eaten/lost. It is probable though that the rxhash
will change, and decapsulated packets will be received by a different
core than before. Established PSP connections receiving traffic might
experience reordering/retransmits as a result.

Also enable decap when configuring the device if the mode indicates it.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/en_accel.h    |  5 ++++
 .../mellanox/mlx5/core/en_accel/psp.c         | 23 +++++++++++++++++--
 .../mellanox/mlx5/core/en_accel/psp.h         |  2 ++
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 10 +++++++-
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
index 8a2ea7616440..98709d48b801 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
@@ -251,6 +251,11 @@ static inline void mlx5e_accel_cleanup_rx(struct mlx5e_priv *priv)
 	mlx5_accel_psp_fs_cleanup_rx_tables(priv);
 }
 
+static inline void mlx5e_accel_update_rx(struct mlx5e_priv *priv)
+{
+	mlx5e_psp_update_rx(priv);
+}
+
 static inline int mlx5e_accel_init_tx(struct mlx5e_priv *priv)
 {
 	return mlx5e_ktls_init_tx(priv);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
index 84133cbf8689..c40d2ddf6af2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
@@ -92,6 +92,12 @@ struct mlx5e_psp_fs {
 	struct mlx5e_psp_rx_table rx;
 };
 
+static bool shampo_enabled(struct mlx5e_priv *priv)
+{
+	return priv->channels.params.packet_merge.type ==
+		MLX5E_PACKET_MERGE_SHAMPO;
+}
+
 /* PSP RX flow steering */
 static enum mlx5_traffic_types fs_psp2tt(enum accel_fs_psp_type i)
 {
@@ -767,6 +773,7 @@ static void accel_psp_fs_rx_destroy(struct mlx5e_psp_fs *fs)
 }
 
 static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
+				  bool decap_enable,
 				  struct netlink_ext_ack *extack)
 {
 	struct mlx5_ttc_table *ttc = mlx5e_fs_get_ttc(fs->fs, false);
@@ -808,7 +815,7 @@ static int accel_psp_fs_rx_create(struct mlx5e_psp_fs *fs,
 		mlx5_ttc_fwd_dest(ttc, fs_psp2tt(i), &dest);
 	}
 
-	accel_psp_fs_rx_reconfigure(fs, false);
+	accel_psp_fs_rx_reconfigure(fs, decap_enable);
 
 	return 0;
 
@@ -1078,7 +1085,8 @@ static int accel_psp_fs_create(struct mlx5e_priv *priv,
 {
 	int err;
 
-	err = accel_psp_fs_rx_create(priv->psp->fs, extack);
+	err = accel_psp_fs_rx_create(priv->psp->fs, shampo_enabled(priv),
+				     extack);
 	if (err)
 		return err;
 
@@ -1355,3 +1363,14 @@ void mlx5e_psp_cleanup(struct mlx5e_priv *priv)
 	priv->psp = NULL;
 	kfree(psp);
 }
+
+void mlx5e_psp_update_rx(struct mlx5e_priv *priv)
+{
+	struct mlx5e_psp *psp = priv->psp;
+
+	netdev_assert_locked(priv->netdev);
+	if (!psp || !psp->fs->check.ft)
+		return;
+
+	accel_psp_fs_rx_reconfigure(psp->fs, shampo_enabled(priv));
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.h
index 3f441e7dd55a..43571a039ea6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.h
@@ -49,6 +49,7 @@ int mlx5e_psp_register(struct mlx5e_priv *priv);
 void mlx5e_psp_unregister(struct mlx5e_priv *priv);
 int mlx5e_psp_init(struct mlx5e_priv *priv);
 void mlx5e_psp_cleanup(struct mlx5e_priv *priv);
+void mlx5e_psp_update_rx(struct mlx5e_priv *priv);
 #else
 static inline void mlx5_accel_psp_fs_cleanup_rx_tables(struct mlx5e_priv *priv) { }
 static inline void mlx5_accel_psp_fs_cleanup_tx_tables(struct mlx5e_priv *priv) { }
@@ -61,5 +62,6 @@ static inline int mlx5e_psp_register(struct mlx5e_priv *priv) { return 0; }
 static inline void mlx5e_psp_unregister(struct mlx5e_priv *priv) { }
 static inline int mlx5e_psp_init(struct mlx5e_priv *priv) { return 0; }
 static inline void mlx5e_psp_cleanup(struct mlx5e_priv *priv) { }
+static inline void mlx5e_psp_update_rx(struct mlx5e_priv *priv) { }
 #endif /* CONFIG_MLX5_EN_PSP */
 #endif /* __MLX5E_ACCEL_PSP_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index fc110a7d16e8..b5449d849838 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -6288,7 +6288,15 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 
 static int mlx5e_update_nic_rx(struct mlx5e_priv *priv)
 {
-	return mlx5e_refresh_tirs(priv->mdev, false, false);
+	int err;
+
+	err = mlx5e_refresh_tirs(priv->mdev, false, false);
+	if (err)
+		return err;
+
+	mlx5e_accel_update_rx(priv);
+
+	return 0;
 }
 
 static const struct mlx5e_profile mlx5e_nic_profile = {
-- 
2.44.0

[PATCH net-next V4 12/14] selftests: drv-net: psp: Extract shared helpers into psp_lib.py

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Put init_psp_dev() and the logic for requiring a psp version into a
shared lib, since they will soon be used by a second test file.

No functional change for psp.py.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 tools/testing/selftests/drivers/net/Makefile  |  1 +
 tools/testing/selftests/drivers/net/psp.py    | 77 ++++++-------------
 .../testing/selftests/drivers/net/psp_lib.py  | 57 ++++++++++++++
 3 files changed, 82 insertions(+), 53 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/psp_lib.py

diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index 1f21e17d8c22..8cea66fa323c 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -32,6 +32,7 @@ TEST_PROGS := \
 
 TEST_FILES := \
 	gro_lib.py \
+	psp_lib.py \
 # end of TEST_FILES
 
 # YNL files, must be before "include ..lib.mk"
diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 315648a770d0..47983fb8c7c2 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -11,6 +11,8 @@ import struct
 import termios
 import time
 
+from psp_lib import init_psp_dev, require_version
+
 from lib.py import defer
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import ksft_true, ksft_eq, ksft_ne, ksft_gt, ksft_raises
@@ -122,34 +124,13 @@ def _get_stat(cfg, key):
 # Test case boiler plate
 #
 
-def _init_psp_dev(cfg, use_psp_ifindex=False):
-    if not hasattr(cfg, 'psp_dev_id'):
-        # Figure out which local device we are testing against
-        # For NetDrvContEnv: use psp_ifindex instead of ifindex
-        target_ifindex = cfg.psp_ifindex if use_psp_ifindex else cfg.ifindex
-        for dev in cfg.pspnl.dev_get({}, dump=True):
-            if dev['ifindex'] == target_ifindex:
-                cfg.psp_info = dev
-                cfg.psp_dev_id = cfg.psp_info['id']
-                break
-        else:
-            raise KsftSkipEx("No PSP devices found")
-
-    # Enable PSP if necessary
-    cap = cfg.psp_info['psp-versions-cap']
-    ena = cfg.psp_info['psp-versions-ena']
-    if cap != ena:
-        cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': cap})
-        defer(cfg.pspnl.dev_set, {'id': cfg.psp_dev_id,
-                                  'psp-versions-ena': ena })
-
 #
 # Test cases
 #
 
 def dev_list_devices(cfg):
     """ Dump all devices """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     devices = cfg.pspnl.dev_get({}, dump=True)
 
@@ -161,7 +142,7 @@ def dev_list_devices(cfg):
 
 def dev_get_device(cfg):
     """ Get the device we intend to use """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     dev = cfg.pspnl.dev_get({'id': cfg.psp_dev_id})
     ksft_eq(dev['id'], cfg.psp_dev_id)
@@ -180,7 +161,7 @@ def dev_get_device_bad(cfg):
 
 def dev_rotate(cfg):
     """ Test key rotation """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     prev_rotations = _get_stat(cfg, 'key-rotations')
 
@@ -195,7 +176,7 @@ def dev_rotate(cfg):
 
 def dev_rotate_spi(cfg):
     """ Test key rotation and SPI check """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     top_a = top_b = 0
     with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
@@ -217,7 +198,7 @@ def dev_rotate_spi(cfg):
 
 def assoc_basic(cfg):
     """ Test creating associations """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
         assoc = cfg.pspnl.rx_assoc({"version": 0,
@@ -237,7 +218,7 @@ def assoc_basic(cfg):
 
 def assoc_bad_dev(cfg):
     """ Test creating associations with bad device ID """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
         with ksft_raises(NlError) as cm:
@@ -249,7 +230,7 @@ def assoc_bad_dev(cfg):
 
 def assoc_sk_only_conn(cfg):
     """ Test creating associations based on socket """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with _make_clr_conn(cfg) as s:
         assoc = cfg.pspnl.rx_assoc({"version": 0,
@@ -263,7 +244,7 @@ def assoc_sk_only_conn(cfg):
 
 def assoc_sk_only_mismatch(cfg):
     """ Test creating associations based on socket (dev mismatch) """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with _make_clr_conn(cfg) as s:
         with ksft_raises(NlError) as cm:
@@ -278,7 +259,7 @@ def assoc_sk_only_mismatch(cfg):
 
 def assoc_sk_only_mismatch_tx(cfg):
     """ Test creating associations based on socket (dev mismatch) """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with _make_clr_conn(cfg) as s:
         with ksft_raises(NlError) as cm:
@@ -296,7 +277,7 @@ def assoc_sk_only_mismatch_tx(cfg):
 
 def assoc_sk_only_unconn(cfg):
     """ Test creating associations based on socket (unconnected, should fail) """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
         with ksft_raises(NlError) as cm:
@@ -309,7 +290,7 @@ def assoc_sk_only_unconn(cfg):
 
 def assoc_version_mismatch(cfg):
     """ Test creating associations where Rx and Tx PSP versions do not match """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     versions = list(cfg.psp_info['psp-versions-cap'])
     if len(versions) < 2:
@@ -335,7 +316,7 @@ def assoc_version_mismatch(cfg):
 
 def assoc_twice(cfg):
     """ Test reusing Tx assoc for two sockets """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     def rx_assoc_check(s):
         assoc = cfg.pspnl.rx_assoc({"version": 0,
@@ -369,19 +350,9 @@ def assoc_twice(cfg):
 
 def _data_basic_send(cfg, version, ipver):
     """ Test basic data send """
-    _init_psp_dev(cfg)
-
-    # Version 0 is required by spec, don't let it skip
-    if version:
-        name = cfg.pspnl.consts["version"].entries_by_val[version].name
-        if name not in cfg.psp_info['psp-versions-cap']:
-            with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
-                with ksft_raises(NlError) as cm:
-                    cfg.pspnl.rx_assoc({"version": version,
-                                        "dev-id": cfg.psp_dev_id,
-                                        "sock-fd": s.fileno()})
-                ksft_eq(cm.exception.nl_msg.error, -errno.EOPNOTSUPP)
-            raise KsftSkipEx("PSP version not supported", name)
+    init_psp_dev(cfg)
+
+    require_version(cfg, version)
 
     s = _make_psp_conn(cfg, version, ipver)
 
@@ -418,7 +389,7 @@ def __bad_xfer_do(cfg, s, tx, version='hdr0-aes-gcm-128'):
 
 def data_send_bad_key(cfg):
     """ Test send data with bad key """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     s = _make_psp_conn(cfg)
 
@@ -433,7 +404,7 @@ def data_send_bad_key(cfg):
 
 def data_send_disconnect(cfg):
     """ Test socket close after sending data """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     with _make_psp_conn(cfg) as s:
         assoc = cfg.pspnl.rx_assoc({"version": 0,
@@ -451,7 +422,7 @@ def data_send_disconnect(cfg):
 
 
 def _data_mss_adjust(cfg, ipver):
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     # First figure out what the MSS would be without any adjustments
     s = _make_clr_conn(cfg, ipver)
@@ -491,7 +462,7 @@ def _data_mss_adjust(cfg, ipver):
 
 def data_stale_key(cfg):
     """ Test send on a double-rotated key """
-    _init_psp_dev(cfg)
+    init_psp_dev(cfg)
 
     prev_stale = _get_stat(cfg, 'stale-events')
     s = _make_psp_conn(cfg)
@@ -766,7 +737,7 @@ def _psp_dev_get_check_netkit_psp_assoc(cfg):
 
 def _dev_assoc_no_nsid(cfg):
     """ Test dev-assoc and dev-disassoc without nsid attribute """
-    _init_psp_dev(cfg, True)
+    init_psp_dev(cfg, True)
 
     # Associate without nsid - should look up ifindex in caller's netns
     cfg.pspnl.dev_assoc({'id': cfg.psp_dev_id,
@@ -800,7 +771,7 @@ def _psp_dev_assoc_cleanup_on_netkit_del(cfg):
     Creates a disposable netkit pair for this test to avoid destroying
     the shared environment.
     """
-    _init_psp_dev(cfg, True)
+    init_psp_dev(cfg, True)
     defer(delattr, cfg, 'psp_dev_id')
     defer(delattr, cfg, 'psp_info')
 
@@ -877,7 +848,7 @@ def _try_disassoc(cfg, psp_dev_id, ifindex, nsid=None):
 
 def _assoc_nk_guest(cfg):
     """Associate nk_guest with PSP device and register cleanup via defer()."""
-    _init_psp_dev(cfg, True)
+    init_psp_dev(cfg, True)
 
     cfg.pspnl.dev_assoc({'id': cfg.psp_dev_id,
                          'ifindex': cfg.nk_guest_ifindex,
diff --git a/tools/testing/selftests/drivers/net/psp_lib.py b/tools/testing/selftests/drivers/net/psp_lib.py
new file mode 100644
index 000000000000..d450db81cfc2
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/psp_lib.py
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0
+
+"""Shared helpers for the PSP selftests."""
+
+import errno
+import socket
+
+from lib.py import defer
+from lib.py import ksft_eq, ksft_raises
+from lib.py import KsftSkipEx
+from lib.py import NlError
+
+
+def require_version(cfg, version):
+    """Skip unless the device supports a PSP version.
+
+    Version 0 is required by the spec, so it never skips.  Before
+    skipping, check that the kernel rejects the unsupported version
+    properly - this is the only coverage that path gets.
+    """
+    if not version:
+        return
+
+    name = cfg.pspnl.consts["version"].entries_by_val[version].name
+    if name in cfg.psp_info['psp-versions-cap']:
+        return
+
+    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+        with ksft_raises(NlError) as cm:
+            cfg.pspnl.rx_assoc({"version": version,
+                                "dev-id": cfg.psp_dev_id,
+                                "sock-fd": s.fileno()})
+        ksft_eq(cm.exception.nl_msg.error, -errno.EOPNOTSUPP)
+    raise KsftSkipEx("PSP version not supported", name)
+
+
+def init_psp_dev(cfg, use_psp_ifindex=False):
+    """Find the PSP device under test and enable all supported versions."""
+    if not hasattr(cfg, 'psp_dev_id'):
+        # Figure out which local device we are testing against
+        # For NetDrvContEnv: use psp_ifindex instead of ifindex
+        target_ifindex = cfg.psp_ifindex if use_psp_ifindex else cfg.ifindex
+        for dev in cfg.pspnl.dev_get({}, dump=True):
+            if dev['ifindex'] == target_ifindex:
+                cfg.psp_info = dev
+                cfg.psp_dev_id = cfg.psp_info['id']
+                break
+        else:
+            raise KsftSkipEx("No PSP devices found")
+
+    # Enable PSP if necessary
+    cap = cfg.psp_info['psp-versions-cap']
+    ena = cfg.psp_info['psp-versions-ena']
+    if cap != ena:
+        cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': cap})
+        defer(cfg.pspnl.dev_set, {'id': cfg.psp_dev_id,
+                                  'psp-versions-ena': ena})
-- 
2.44.0

[PATCH net-next V4 13/14] selftests: net: gro: Add PSP encapsulation and encryption

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Add a PSP mode which wraps every transmitted frame in PSP transport
mode and encrypts them in SW with the help of OpenSSL.
The receiving NIC decrypts and decapsulates, so the frames seen on the
receiver on its AF_PACKET socket are identical to the non-PSP ones and
every geometry assertion in this file applies unchanged.

Sender packet construction is unchanged, PSP uses a different buffer to
avoid double encryption.

A few PSP-only cases are added: psp_spi_diff, psp_ver_diff, psp_mixed,
to cover coalescing scenarios which only exist for PSP.

OpenSSL is optional and scoped to this binary: gro is deployed by
several suites, so a missing libcrypto costs only the PSP mode.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 tools/testing/selftests/net/lib/Makefile |  16 ++
 tools/testing/selftests/net/lib/gro.c    | 337 ++++++++++++++++++++++-
 2 files changed, 351 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile
index ff83603397d0..ce67e8d3e4f6 100644
--- a/tools/testing/selftests/net/lib/Makefile
+++ b/tools/testing/selftests/net/lib/Makefile
@@ -22,4 +22,20 @@ TEST_INCLUDES := $(wildcard py/*.py sh/*.sh)
 
 include ../../lib.mk
 
+# gro optionally needs OpenSSL for PSP.
+PKG_CONFIG ?= pkg-config
+LIBCRYPTO_CFLAGS := $(shell $(PKG_CONFIG) --cflags libcrypto 2>/dev/null)
+LIBCRYPTO_LIBS := $(shell $(PKG_CONFIG) --libs libcrypto 2>/dev/null || \
+		    echo -lcrypto)
+HAVE_OPENSSL := $(shell echo 'int main(void) { return 0; }' | \
+		  $(CC) -x c - -include openssl/evp.h $(LIBCRYPTO_CFLAGS) \
+		  $(LIBCRYPTO_LIBS) -o /dev/null >/dev/null 2>&1 && echo 1)
+
+ifeq ($(HAVE_OPENSSL),1)
+$(OUTPUT)/gro: CFLAGS += -DHAVE_OPENSSL $(LIBCRYPTO_CFLAGS)
+$(OUTPUT)/gro: LDLIBS += $(LIBCRYPTO_LIBS)
+else
+$(warning libcrypto not found, gro will be built without PSP support)
+endif
+
 include ../bpf.mk
diff --git a/tools/testing/selftests/net/lib/gro.c b/tools/testing/selftests/net/lib/gro.c
index 7a333155de1a..fe4a4c0e17ae 100644
--- a/tools/testing/selftests/net/lib/gro.c
+++ b/tools/testing/selftests/net/lib/gro.c
@@ -64,6 +64,7 @@
 #define _GNU_SOURCE
 
 #include <arpa/inet.h>
+#include <endian.h>
 #include <errno.h>
 #include <error.h>
 #include <getopt.h>
@@ -79,14 +80,22 @@
 #include <netinet/ip.h>
 #include <netinet/ip6.h>
 #include <netinet/tcp.h>
+#include <netinet/udp.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <stdlib.h>
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
 
+#include <linux/psp.h>
+
+#ifdef HAVE_OPENSSL
+#include <openssl/evp.h>
+#endif
+
 #include "kselftest.h"
 #include "ksft.h"
 
@@ -110,6 +119,38 @@
 
 #define EXIT_OVER_COALESCE	42
 
+/* PSP transport mode encapsulation, as built by the sender:
+ *
+ *	[eth][IP][UDP dport=1000][PSP hdr][encrypted L4][ICV]
+ *
+ * Can't use the kernel-only include/net/psp/types.h header, so copy the
+ * encoding here.
+ */
+struct psphdr {
+	uint8_t		nexthdr;
+	uint8_t		hdrlen;
+	uint8_t		crypt_offset;
+	uint8_t		verfl;
+	uint32_t	spi;		/* big endian */
+	uint64_t	iv;		/* big endian */
+} __packed;
+
+#define PSP_UDP_PORT		1000
+#define PSP_UDP_LEN		sizeof(struct udphdr)
+#define PSP_HDR_LEN		sizeof(struct psphdr)
+#define PSP_ICV_LEN		16
+/* total frame growth: the inserted headers plus the trailing ICV */
+#define PSP_ENCAP_LEN		(PSP_UDP_LEN + PSP_HDR_LEN + PSP_ICV_LEN)
+#define PSP_HDRLEN_NOOPT	1	/* (1 + 1) * 8 == PSP_HDR_LEN */
+#define PSP_VERFL_ONE		0x01
+#define PSP_VERFL_VER_SHIFT	2
+#define PSP_MAX_ASSOC		2
+#define PSP_KEY_LEN_MAX		32
+
+enum {
+	OPT_PSP_ASSOC = 256,
+};
+
 #define ipv6_optlen(p)  (((p)->hdrlen+1) << 3) /* calculate IPv6 extension header len */
 #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
 
@@ -143,6 +184,9 @@ static bool pppoe;
 static uint64_t txtime_ns;
 static int num_flows = 4;
 static bool order_check;
+static bool psp_enabled;
+static int psp_idx;
+static int psp_num_assoc;
 
 #define CAPACITY_PAYLOAD_LEN 200
 
@@ -158,7 +202,8 @@ static int max_payload(void)
 
 static int calc_mss(void)
 {
-	return ASSUMED_MTU - (total_hdr_len - ETH_HLEN);
+	return ASSUMED_MTU - (total_hdr_len - ETH_HLEN) -
+		(psp_enabled ? PSP_ENCAP_LEN : 0);
 }
 
 static int num_large_pkt(void)
@@ -384,6 +429,215 @@ static void fill_transportlayer(void *buf, int seq_offset, int ack_offset,
 	tcph->check = tcp_checksum(tcph, payload_len);
 }
 
+#ifdef HAVE_OPENSSL
+
+struct psp_assoc {
+	uint32_t	spi;
+	uint8_t		key[PSP_KEY_LEN_MAX];
+	int		keylen;
+	int		version;
+	EVP_CIPHER_CTX	*ctx;
+};
+
+static struct psp_assoc psp_assocs[PSP_MAX_ASSOC];
+static uint64_t psp_next_iv = 1;
+
+static char psp_scratch[L2_HLEN_MAX + IP_MAXPACKET + PSP_ENCAP_LEN];
+
+static int psp_key_len(int version)
+{
+	switch (version) {
+	case PSP_VERSION_HDR0_AES_GCM_128:
+	case PSP_VERSION_HDR0_AES_GMAC_128:
+		return 16;
+	case PSP_VERSION_HDR0_AES_GCM_256:
+	case PSP_VERSION_HDR0_AES_GMAC_256:
+		return 32;
+	default:
+		error(1, 0, "psp: unknown version %d", version);
+		return 0;
+	}
+}
+
+static bool psp_version_is_gmac(int version)
+{
+	return version == PSP_VERSION_HDR0_AES_GMAC_128 ||
+	       version == PSP_VERSION_HDR0_AES_GMAC_256;
+}
+
+/* Parses one --psp-assoc "version,spi,key".
+ * The SPI and key (both hex) is a PSP RX assoc from the device under test.
+ */
+static void psp_parse_assoc(char *arg)
+{
+	char *ver_s, *spi_s, *key_s, *end;
+	const EVP_CIPHER *cipher;
+	struct psp_assoc *assoc;
+	unsigned long val;
+	size_t len, i;
+
+	if (psp_num_assoc == PSP_MAX_ASSOC)
+		error(1, 0, "psp: at most %d associations", PSP_MAX_ASSOC);
+	assoc = &psp_assocs[psp_num_assoc];
+
+	ver_s = strtok(arg, ",");
+	spi_s = strtok(NULL, ",");
+	key_s = strtok(NULL, ",");
+	if (!ver_s || !spi_s || !key_s || strtok(NULL, ","))
+		error(1, 0, "psp: --psp-assoc wants version,spi,key");
+
+	errno = 0;
+	val = strtoul(ver_s, &end, 10);
+	if (errno || end == ver_s || *end ||
+	    val > PSP_VERSION_HDR0_AES_GMAC_256)
+		error(1, 0, "psp: bad version '%s'", ver_s);
+	assoc->version = val;
+
+	errno = 0;
+	val = strtoul(spi_s, &end, 16);
+	if (errno || end == spi_s || *end || val > 0xffffffffUL)
+		error(1, 0, "psp: bad SPI '%s'", spi_s);
+	assoc->spi = val;
+
+	len = strlen(key_s);
+	if (len != (size_t)psp_key_len(assoc->version) * 2 ||
+	    strspn(key_s, "0123456789abcdefABCDEF") != len)
+		error(1, 0, "psp: version %d needs a %d byte key, got '%s'",
+		      assoc->version, psp_key_len(assoc->version), key_s);
+
+	assoc->keylen = len / 2;
+	for (i = 0; i < len; i += 2) {
+		unsigned int byte;
+
+		if (sscanf(key_s + i, "%2x", &byte) != 1)
+			error(1, 0, "psp: bad key '%s'", key_s);
+		assoc->key[i / 2] = byte;
+	}
+
+	cipher = assoc->keylen == 16 ? EVP_aes_128_gcm() : EVP_aes_256_gcm();
+	assoc->ctx = EVP_CIPHER_CTX_new();
+	if (!assoc->ctx)
+		error(1, 0, "psp: EVP_CIPHER_CTX_new");
+	if (EVP_EncryptInit_ex(assoc->ctx, cipher, NULL, assoc->key,
+			       NULL) != 1)
+		error(1, 0, "psp: EVP_EncryptInit_ex key");
+	psp_num_assoc++;
+	psp_enabled = true;
+}
+
+static void psp_crypt(struct psp_assoc *assoc, const uint8_t *nonce,
+		      const uint8_t *aad, int aad_len,
+		      uint8_t *data, int data_len, uint8_t *icv)
+{
+	uint8_t *crypt_data = psp_version_is_gmac(assoc->version) ? NULL : data;
+	EVP_CIPHER_CTX *ctx = assoc->ctx;
+	int outl;
+
+	if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, nonce) != 1)
+		error(1, 0, "psp: EVP_EncryptInit_ex nonce");
+	if (EVP_EncryptUpdate(ctx, NULL, &outl, aad, aad_len) != 1)
+		error(1, 0, "psp: EVP_EncryptUpdate aad");
+	if (EVP_EncryptUpdate(ctx, crypt_data, &outl, data, data_len) != 1)
+		error(1, 0, "psp: EVP_EncryptUpdate data");
+	if (EVP_EncryptFinal_ex(ctx, icv, &outl) != 1)
+		error(1, 0, "psp: EVP_EncryptFinal_ex");
+	if (EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, PSP_ICV_LEN,
+				icv) != 1)
+		error(1, 0, "psp: EVP_CTRL_GCM_GET_TAG");
+}
+
+/* Encapsulates & encrypts @pkt with PSP transport mode into psp_scratch.
+ * Returns the scratch buffer and updates *@lenp.
+ */
+static char *psp_encapsulate(const char *pkt, int *lenp)
+{
+	struct psp_assoc *assoc = &psp_assocs[psp_idx];
+	int ip_hlen, l4_off, l4_len, aad_len;
+	struct psphdr *psph;
+	struct udphdr *udph;
+	__be16 sport, dport;
+	uint8_t *data, *icv;
+	int len = *lenp;
+	uint8_t nexthdr;
+
+	if (len + PSP_ENCAP_LEN > (int)sizeof(psp_scratch))
+		error(1, 0, "psp: %d byte frame does not fit", len);
+
+	memcpy(psp_scratch, pkt, len);
+
+	if (proto == PF_INET) {
+		struct iphdr *iph = (struct iphdr *)(psp_scratch + ETH_HLEN);
+
+		ip_hlen = iph->ihl * 4;
+		nexthdr = iph->protocol;
+	} else {
+		struct ipv6hdr *ip6h =
+			(struct ipv6hdr *)(psp_scratch + ETH_HLEN);
+
+		ip_hlen = sizeof(*ip6h);
+		nexthdr = ip6h->nexthdr;
+	}
+
+	l4_off = ETH_HLEN + ip_hlen;
+	l4_len = len - l4_off;
+	if (l4_len < 4)
+		error(1, 0, "psp: no room for an L4 header");
+
+	sport = *(__be16 *)(psp_scratch + l4_off);
+	dport = *(__be16 *)(psp_scratch + l4_off + sizeof(sport));
+
+	memmove(psp_scratch + l4_off + PSP_UDP_LEN + PSP_HDR_LEN,
+		psp_scratch + l4_off, l4_len);
+
+	udph = (struct udphdr *)(psp_scratch + l4_off);
+	udph->source = sport ^ dport;
+	udph->dest = htons(PSP_UDP_PORT);
+	udph->len = htons(PSP_UDP_LEN + PSP_HDR_LEN + l4_len + PSP_ICV_LEN);
+	udph->check = 0;
+
+	psph = (struct psphdr *)(udph + 1);
+	psph->nexthdr = nexthdr;
+	psph->hdrlen = PSP_HDRLEN_NOOPT;
+	psph->crypt_offset = 0;  /* Matches the kernel. */
+	psph->verfl = (assoc->version << PSP_VERFL_VER_SHIFT) | PSP_VERFL_ONE;
+	psph->spi = htonl(assoc->spi);
+	psph->iv = htobe64(psp_next_iv++);
+
+	data = (uint8_t *)psph + PSP_HDR_LEN;
+	icv = (uint8_t *)psph + PSP_HDR_LEN + l4_len;
+	aad_len = PSP_HDR_LEN;
+
+	/* The GCM nonce is wire (spi, iv), which are contiguous. */
+	psp_crypt(assoc,
+		  (const uint8_t *)psph + offsetof(struct psphdr, spi),
+		  (const uint8_t *)psph, aad_len, data, l4_len, icv);
+
+	if (proto == PF_INET) {
+		struct iphdr *iph = (struct iphdr *)(psp_scratch + ETH_HLEN);
+
+		iph->tot_len = htons(ntohs(iph->tot_len) + PSP_ENCAP_LEN);
+		iph->protocol = IPPROTO_UDP;
+		iph->check = 0;
+		iph->check = checksum_fold(iph, ip_hlen, 0);
+	} else {
+		struct ipv6hdr *ip6h =
+			(struct ipv6hdr *)(psp_scratch + ETH_HLEN);
+
+		ip6h->payload_len =
+			htons(ntohs(ip6h->payload_len) + PSP_ENCAP_LEN);
+		ip6h->nexthdr = IPPROTO_UDP;
+	}
+
+	*lenp = len + PSP_ENCAP_LEN;
+	return psp_scratch;
+}
+#else
+static void psp_parse_assoc(char *arg)
+{
+	error(1, 0, "gro was built without OpenSSL, PSP is not available");
+}
+#endif  /* HAVE_OPENSSL */
+
 static void write_packet(int fd, char *buf, int len, struct sockaddr_ll *daddr)
 {
 	char control[CMSG_SPACE(sizeof(uint64_t))];
@@ -392,6 +646,11 @@ static void write_packet(int fd, char *buf, int len, struct sockaddr_ll *daddr)
 	struct cmsghdr *cm;
 	int ret = -1;
 
+#ifdef HAVE_OPENSSL
+	if (psp_enabled)
+		buf = psp_encapsulate(buf, &len);
+#endif
+
 	iov.iov_base = buf;
 	iov.iov_len = len;
 
@@ -572,6 +831,10 @@ static void send_large(int fd, struct sockaddr_ll *daddr, int remainder)
 	const int mss = calc_mss();
 	int i;
 
+	if (num_pkt > MAX_LARGE_PKT_CNT)
+		error(1, 0, "too many large packets: %d > %zu", num_pkt,
+		      (size_t)MAX_LARGE_PKT_CNT);
+
 	for (i = 0; i < num_pkt; i++)
 		create_packet(pkts[i], i * mss, 0, mss, 0);
 	create_packet(last, num_pkt * mss, 0, remainder, 0);
@@ -1111,6 +1374,39 @@ static void send_changed_pppoe_sid(int fd, struct sockaddr_ll *daddr)
 	write_packet(fd, buf, pkt_size, daddr);
 }
 
+/* PSP packets don't coalesce across SPIs or versions. */
+static void send_psp_assoc_switch(int fd, struct sockaddr_ll *daddr)
+{
+	static char buf[MAX_HDR_LEN + PAYLOAD_LEN];
+	int pkt_size = total_hdr_len + PAYLOAD_LEN;
+
+	if (psp_num_assoc < 2)
+		error(1, 0, "%s needs two PSP associations", testname);
+
+	create_packet(buf, 0, 0, PAYLOAD_LEN, 0);
+	write_packet(fd, buf, pkt_size, daddr);
+
+	create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
+	psp_idx = 1;
+	write_packet(fd, buf, pkt_size, daddr);
+	psp_idx = 0;
+}
+
+/* A PSP packet and a cleartext packet of the same flow don't coalesce. */
+static void send_psp_mixed(int fd, struct sockaddr_ll *daddr)
+{
+	static char buf[MAX_HDR_LEN + PAYLOAD_LEN];
+	int pkt_size = total_hdr_len + PAYLOAD_LEN;
+
+	create_packet(buf, 0, 0, PAYLOAD_LEN, 0);
+	write_packet(fd, buf, pkt_size, daddr);
+
+	create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
+	psp_enabled = false;
+	write_packet(fd, buf, pkt_size, daddr);
+	psp_enabled = true;
+}
+
 static void bind_packetsocket(int fd)
 {
 	struct sockaddr_ll daddr = {};
@@ -1559,6 +1855,16 @@ static void gro_sender(void)
 		usleep(fin_delay_us);
 		write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
 
+	/* PSP sub-tests */
+	} else if (strcmp(testname, "psp_spi_diff") == 0 ||
+		   strcmp(testname, "psp_ver_diff") == 0) {
+		send_psp_assoc_switch(txfd, &daddr);
+		usleep(fin_delay_us);
+		write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
+	} else if (strcmp(testname, "psp_mixed") == 0) {
+		send_psp_mixed(txfd, &daddr);
+		usleep(fin_delay_us);
+		write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
 	} else {
 		error(1, 0, "Unknown testcase: %s", testname);
 	}
@@ -1782,6 +2088,22 @@ static void gro_receiver(void)
 		printf("different PPPoE session ID doesn't coalesce: ");
 		check_recv_pkts(rxfd, correct_payload, 2);
 
+	/* PSP sub-tests */
+	} else if (strcmp(testname, "psp_spi_diff") == 0) {
+		correct_payload[0] = PAYLOAD_LEN;
+		correct_payload[1] = PAYLOAD_LEN;
+		printf("different PSP SPI doesn't coalesce: ");
+		check_recv_pkts(rxfd, correct_payload, 2);
+	} else if (strcmp(testname, "psp_ver_diff") == 0) {
+		correct_payload[0] = PAYLOAD_LEN;
+		correct_payload[1] = PAYLOAD_LEN;
+		printf("different PSP version doesn't coalesce: ");
+		check_recv_pkts(rxfd, correct_payload, 2);
+	} else if (strcmp(testname, "psp_mixed") == 0) {
+		correct_payload[0] = PAYLOAD_LEN;
+		correct_payload[1] = PAYLOAD_LEN;
+		printf("PSP and cleartext don't coalesce: ");
+		check_recv_pkts(rxfd, correct_payload, 2);
 	} else {
 		error(1, 0, "Test case error: unknown testname %s", testname);
 	}
@@ -1802,6 +2124,7 @@ static void parse_args(int argc, char **argv)
 		{ "ip6ip6", no_argument, NULL, 'E' },
 		{ "pppoev4", no_argument, NULL, 'p' },
 		{ "pppoev6", no_argument, NULL, 'P' },
+		{ "psp-assoc", required_argument, NULL, OPT_PSP_ASSOC },
 		{ "num-flows", required_argument, NULL, 'n' },
 		{ "rx", no_argument, NULL, 'r' },
 		{ "saddr", required_argument, NULL, 's' },
@@ -1813,8 +2136,12 @@ static void parse_args(int argc, char **argv)
 	};
 	int c;
 
-	while ((c = getopt_long(argc, argv, "46d:D:eEi:n:pPrs:S:t:ov", opts, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, "46d:D:eEi:n:pPrs:S:t:ov",
+			       opts, NULL)) != -1) {
 		switch (c) {
+		case OPT_PSP_ASSOC:
+			psp_parse_assoc(optarg);
+			break;
 		case '4':
 			proto = PF_INET;
 			ethhdr_proto = htons(ETH_P_IP);
@@ -1904,6 +2231,12 @@ int main(int argc, char **argv)
 		error(1, 0, "Protocol family is not ipv4 or ipv6");
 	}
 
+	if (psp_enabled && (ipip || ip6ip6 || pppoe))
+		error(1, 0, "--psp-assoc doesn't support ipip/ip6ip6/PPPoE");
+
+	if (!strncmp(testname, "psp_", 4) && !psp_enabled)
+		error(1, 0, "test %s requires --psp-assoc", testname);
+
 	read_MAC(src_mac, smac);
 	read_MAC(dst_mac, dmac);
 
-- 
2.44.0

[PATCH net-next V4 14/14] selftests: drv-net: Add PSP HW GRO conformance tests

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Add PSP conformance tests using the same gro helper binary as other gro
tests , but in --psp mode. SPIs are procured by psp_gro.py from a real
PSP device and handed off to the gro sender & receiver. The sender
crafts and encrypts packets in software, the receiver relies fully on
the HW to decrypt, decapsulate and do HW GRO.

Because the NIC decrypts and decapsulates before the frames reach the
receiver AF_PACKET tap, the gro receiver sees the plain frames and the
assertions remain exactly the same as other gro tests.

So these tests verify, at length, that the device does PSP HW-GRO the
same way as plain TCP.

Additional PSP-specific tests are defined to check that GRO doesn't
merge packets across PSP versions, SPIs, encryption-status, etc.

Some gro tests are not included because they don't work:
- ip_csum: the checksum is recomputed by psp_dev_rcv().
- tcp_csum: packets are marked with CHECKSUM_UNNECESSARY.
- ip_frag4/ip_frag6: PSP is incompatible with IP fragmentation.
- IPv6 extension header tests: PSP doesn't deal with IPv6 ext headers.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../testing/selftests/drivers/net/gro_lib.py  |  15 +-
 .../testing/selftests/drivers/net/hw/Makefile |  17 ++
 tools/testing/selftests/drivers/net/hw/config |   1 +
 .../selftests/drivers/net/hw/psp_gro.py       | 157 ++++++++++++++++++
 4 files changed, 186 insertions(+), 4 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/hw/psp_gro.py

diff --git a/tools/testing/selftests/drivers/net/gro_lib.py b/tools/testing/selftests/drivers/net/gro_lib.py
index 45ded8477a50..8edae7d736a9 100644
--- a/tools/testing/selftests/drivers/net/gro_lib.py
+++ b/tools/testing/selftests/drivers/net/gro_lib.py
@@ -189,7 +189,8 @@ def _setup_queue_count(cfg, num_queues):
 
 
 def _run_gro_bin(cfg, test_name, protocol=None, num_flows=None,
-                 order_check=False, verbose=False, fail=False):
+                 order_check=False, verbose=False, fail=False,
+                 common_args=None):
     """Run gro binary with given test and return the process result."""
     if not hasattr(cfg, "bin_remote"):
         cfg.bin_local = cfg.net_lib_dir / "gro"
@@ -217,6 +218,8 @@ def _run_gro_bin(cfg, test_name, protocol=None, num_flows=None,
         base_args.append("--order-check")
     if verbose:
         base_args.append("--verbose")
+    if common_args:
+        base_args += common_args
 
     args = " ".join(base_args)
 
@@ -341,8 +344,11 @@ def _gro_variants():
                 yield protocol, test_name
 
 
-def run_test(cfg, mode, protocol, test_name):
-    """Run a single GRO test with retries."""
+def run_test(cfg, mode, protocol, test_name, common_args=None):
+    """Run a single GRO test with retries.
+
+    common_args are extra arguments passed to both gro sender and receiver.
+    """
 
     ipver = "6" if protocol[-1] == "6" else "4"
     cfg.require_ipver(ipver)
@@ -356,7 +362,8 @@ def run_test(cfg, mode, protocol, test_name):
     for attempt in range(max_retries):
         fail_now = attempt >= max_retries - 1
         rx_proc = _run_gro_bin(cfg, test_name, protocol=protocol,
-                               verbose=True, fail=fail_now)
+                               verbose=True, fail=fail_now,
+                               common_args=common_args)
 
         if rx_proc.ret == 0:
             return
diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 6105be8e590f..daa7e52f236e 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -13,6 +13,20 @@ else
 $(warning excluding iouring tests, liburing not installed or too old)
 endif
 
+# psp_gro.py uses the gro binary in PSP mode, which requires OpenSSL.
+PKG_CONFIG ?= pkg-config
+HAVE_OPENSSL := $(shell echo 'int main(void) { return 0; }' | \
+		  $(CC) -x c - -include openssl/evp.h \
+		  $(shell $(PKG_CONFIG) --cflags --libs libcrypto 2>/dev/null \
+			  || echo -lcrypto) \
+		  -o /dev/null >/dev/null 2>&1 && echo 1)
+
+ifeq ($(HAVE_OPENSSL),1)
+COND_PROGS += psp_gro.py
+else
+$(warning excluding PSP GRO tests, libcrypto not installed)
+endif
+
 TEST_GEN_FILES := \
 	$(COND_GEN_FILES) \
 # end of TEST_GEN_FILES
@@ -40,6 +54,7 @@ TEST_PROGS = \
 	nk_qlease.py \
 	ntuple.py \
 	pp_alloc_fail.py \
+	$(COND_PROGS) \
 	rss_api.py \
 	rss_ctx.py \
 	rss_drv.py \
@@ -64,6 +79,8 @@ TEST_FILES := \
 
 TEST_INCLUDES := \
 	$(wildcard lib/py/*.py ../lib/py/*.py) \
+	../gro_lib.py \
+	../psp_lib.py \
 	../../../net/lib.sh \
 	../../../net/forwarding/ipip_lib.sh \
 	../../../net/forwarding/lib.sh \
diff --git a/tools/testing/selftests/drivers/net/hw/config b/tools/testing/selftests/drivers/net/hw/config
index d89a9ba17655..060ed1423bf3 100644
--- a/tools/testing/selftests/drivers/net/hw/config
+++ b/tools/testing/selftests/drivers/net/hw/config
@@ -8,6 +8,7 @@ CONFIG_INET6_ESP=y
 CONFIG_INET6_ESP_OFFLOAD=y
 CONFIG_INET_ESP=y
 CONFIG_INET_ESP_OFFLOAD=y
+CONFIG_INET_PSP=y
 CONFIG_IO_URING=y
 CONFIG_IPV6=y
 CONFIG_IPV6_GRE=y
diff --git a/tools/testing/selftests/drivers/net/hw/psp_gro.py b/tools/testing/selftests/drivers/net/hw/psp_gro.py
new file mode 100755
index 000000000000..3a069c944ae1
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/psp_gro.py
@@ -0,0 +1,157 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+PSP HW GRO conformance tests.
+
+This reuses the gro binary in --psp mode:
+The sender crafts encapsulated & SW-encrypted PSP packets with receiver's PSP
+rx-assoc, and the receiver's device decrypts and decapsulates the packets
+before an AF_PACKET tap gets to analyze them.
+
+All GRO conformance tests which could run with PSP are included.
+"""
+
+import socket
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_variants, KsftNamedVariant
+from lib.py import KsftSkipEx
+from lib.py import NetDrvEpEnv, PSPFamily
+from lib.py import defer
+
+# The helpers live next to their other users, gro_*.py and psp.py. Importing
+# lib.py above puts the selftests root on sys.path, which is what makes these
+# two resolve, so they have to stay after it - pylint wants the opposite.
+# pylint: disable=wrong-import-order,import-error
+from drivers.net.gro_lib import run_test
+from drivers.net.psp_lib import init_psp_dev, require_version
+# pylint: enable=wrong-import-order,import-error
+
+
+# Tests for both IP versions, a subset of gro tests. Missing:
+# - tcp_csum: PSP packets are validated by HW (with the ICV) and marked with
+# CHECKSUM_UNNECESSARY.
+_COMMON = ["data_same", "data_lrg_sml", "data_sml_lrg", "data_lrg_1byte",
+           "data_burst",
+           "ack",
+           "flags_psh", "flags_syn", "flags_rst", "flags_urg", "flags_cwr",
+           "tcp_seq", "tcp_ts", "tcp_opt",
+           "ip_ecn", "ip_tos",
+           "large_max", "large_rem",
+]
+
+# Tests specific to IPv4, a subset of gro tests. Missing:
+# - ip_csum: psp_dev_rcv() recomputes the IP checksum.
+# - ip_frag4: PSP is incompatible with IP fragmentation.
+_V4 = ["ip_ttl", "ip_opt",
+       "ip_id_df1_inc", "ip_id_df1_fixed",
+       "ip_id_df0_inc", "ip_id_df0_fixed",
+       "ip_id_df1_inc_fixed", "ip_id_df1_fixed_inc",
+]
+
+# No IPv6-only tests:
+# - ip_frag6: PSP doesn't support fragmentation
+# - ip_v6ext_same, ip_v6ext_diff: PSP doesn't support IPv6 ext headers.
+
+
+def _psp_variants():
+    for ver in range(4):
+        for proto in ["ipv4", "ipv6"]:
+            for test_name in _COMMON + (_V4 if proto == "ipv4" else []):
+                yield KsftNamedVariant(f"v{ver}_{proto}_{test_name}",
+                                       ver, proto, test_name)
+
+
+def _ip_variants():
+    """IPv4/IPv6."""
+    for proto in ("ipv4", "ipv6"):
+        yield KsftNamedVariant(proto, proto)
+
+
+def _psp_assoc(cfg, version=0):
+    # This socket receives no traffic, exists solely to own the rx assoc.
+    s = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
+    defer(s.close)
+    return cfg.pspnl.rx_assoc({"version": version, "dev-id": cfg.psp_dev_id,
+                               "sock-fd": s.fileno()})['rx-key']
+
+
+def _setup(cfg, version):
+    """Enables PSP on the device under test."""
+    init_psp_dev(cfg)
+    require_version(cfg, version)
+
+
+def _psp_args(cfg, versions):
+    """Produces PSP associations as gro binary --psp-assoc arguments."""
+    keys = [_psp_assoc(cfg, ver) for ver in versions]
+    return [f"--psp-assoc {ver},{key['spi']:x},{key['key'].hex()}"
+            for ver, key in zip(versions, keys)]
+
+
+def _run(cfg, test_name, protocol, versions):
+    """Sets up, associates and runs one gro test case under PSP + HW GRO."""
+    _setup(cfg, max(versions))
+
+    run_test(cfg, "hw", protocol, test_name,
+             common_args=_psp_args(cfg, versions))
+
+
+@ksft_variants(_psp_variants())
+def test_psp_gro(cfg, version, protocol, test_name):
+    """Runs one gro conformance case with PSP encapsulation."""
+    _run(cfg, test_name, protocol, [version])
+
+
+# PSP-specific GRO tests
+
+@ksft_variants(_ip_variants())
+def test_psp_spi_diff(cfg, protocol):
+    """Frames from two different SPIs must not coalesce."""
+    _run(cfg, "psp_spi_diff", protocol, [0, 0])
+
+
+@ksft_variants(_ip_variants())
+def test_psp_ver_diff(cfg, protocol):
+    """Frames from two different PSP versions must not coalesce."""
+    init_psp_dev(cfg)
+    if len(cfg.psp_info['psp-versions-cap']) < 2:
+        raise KsftSkipEx("Device supports a single PSP version")
+
+    _run(cfg, "psp_ver_diff", protocol, [0, 1])
+
+
+@ksft_variants(_ip_variants())
+def test_psp_mixed(cfg, protocol):
+    """A PSP frame must not coalesce with a clear text one."""
+    _run(cfg, "psp_mixed", protocol, [0])
+
+
+@ksft_variants(_ip_variants())
+def test_psp_after_reconfig(cfg, protocol):
+    """Verifies that decap still works after PSP off + on."""
+    _setup(cfg, 0)
+
+    cap = cfg.psp_info['psp-versions-cap']
+    cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': []})
+    cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': cap})
+
+    run_test(cfg, "hw", protocol, "data_same",
+             common_args=_psp_args(cfg, [0]))
+
+
+def main() -> None:
+    """ Ksft boiler plate main """
+
+    with NetDrvEpEnv(__file__, nsim_test=False) as cfg:
+        cfg.pspnl = PSPFamily()
+
+        ksft_run(cases=[test_psp_gro, test_psp_spi_diff, test_psp_ver_diff,
+                        test_psp_mixed, test_psp_after_reconfig],
+                 args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.44.0

Re: [PATCH net-next V4 12/14] selftests: drv-net: psp: Extract shared helpers into psp_lib.py

[Daniel Zahka]

On Tue Sep 8, 2026 at 2:52 AM EDT, Tariq Toukan wrote:
> From: Cosmin Ratiu <cratiu@nvidia.com>
>
> Put init_psp_dev() and the logic for requiring a psp version into a
> shared lib, since they will soon be used by a second test file.
>
> No functional change for psp.py.
>
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next V4 13/14] selftests: net: gro: Add PSP encapsulation and encryption

[Daniel Zahka]

On Tue Sep 8, 2026 at 2:52 AM EDT, Tariq Toukan wrote:
> From: Cosmin Ratiu <cratiu@nvidia.com>
>
> Add a PSP mode which wraps every transmitted frame in PSP transport
> mode and encrypts them in SW with the help of OpenSSL.
> The receiving NIC decrypts and decapsulates, so the frames seen on the
> receiver on its AF_PACKET socket are identical to the non-PSP ones and
> every geometry assertion in this file applies unchanged.
>
> Sender packet construction is unchanged, PSP uses a different buffer to
> avoid double encryption.
>
> A few PSP-only cases are added: psp_spi_diff, psp_ver_diff, psp_mixed,
> to cover coalescing scenarios which only exist for PSP.
>
> OpenSSL is optional and scoped to this binary: gro is deployed by
> several suites, so a missing libcrypto costs only the PSP mode.
>
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
...
> +/* PSP transport mode encapsulation, as built by the sender:
> + *
> + *	[eth][IP][UDP dport=1000][PSP hdr][encrypted L4][ICV]
> + *
> + * Can't use the kernel-only include/net/psp/types.h header, so copy the
> + * encoding here.
> + */
> +struct psphdr {
> +	uint8_t		nexthdr;
> +	uint8_t		hdrlen;
> +	uint8_t		crypt_offset;
> +	uint8_t		verfl;
> +	uint32_t	spi;		/* big endian */
> +	uint64_t	iv;		/* big endian */
> +} __packed;

My llm is complaining that it thinks __packed is not defined for
userspace code and being used as the name of an object being declared,
instead of an attribute. Otherwise:

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next V4 14/14] selftests: drv-net: Add PSP HW GRO conformance tests

[Daniel Zahka]

On Tue Sep 8, 2026 at 2:52 AM EDT, Tariq Toukan wrote:
> From: Cosmin Ratiu <cratiu@nvidia.com>
>
> Add PSP conformance tests using the same gro helper binary as other gro
> tests , but in --psp mode. SPIs are procured by psp_gro.py from a real
> PSP device and handed off to the gro sender & receiver. The sender
> crafts and encrypts packets in software, the receiver relies fully on
> the HW to decrypt, decapsulate and do HW GRO.
>
> Because the NIC decrypts and decapsulates before the frames reach the
> receiver AF_PACKET tap, the gro receiver sees the plain frames and the
> assertions remain exactly the same as other gro tests.
>
> So these tests verify, at length, that the device does PSP HW-GRO the
> same way as plain TCP.
>
> Additional PSP-specific tests are defined to check that GRO doesn't
> merge packets across PSP versions, SPIs, encryption-status, etc.
>
> Some gro tests are not included because they don't work:
> - ip_csum: the checksum is recomputed by psp_dev_rcv().

psp_dev_rcv() isn't on the hw gro path for mlx5, right?

> - tcp_csum: packets are marked with CHECKSUM_UNNECESSARY.

this also seems strange to me as the hw gro engine and its decision to
coalesce two packets sits upstream of this skb->ip_summed determination
in the driver, right?

> - ip_frag4/ip_frag6: PSP is incompatible with IP fragmentation.
> - IPv6 extension header tests: PSP doesn't deal with IPv6 ext headers.
>
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
...
> +
> +@ksft_variants(_ip_variants())
> +def test_psp_ver_diff(cfg, protocol):
> +    """Frames from two different PSP versions must not coalesce."""
> +    init_psp_dev(cfg)
> +    if len(cfg.psp_info['psp-versions-cap']) < 2:

nit: what the above checks is slightly different than what the _run()
below uses, e.g. dev supports versions 0 and 2.

> +        raise KsftSkipEx("Device supports a single PSP version")
> +
> +    _run(cfg, "psp_ver_diff", protocol, [0, 1])
> +

couple of minor comments:

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

Re: [PATCH net-next V4 09/14] net/mlx5e: psp: Add an rx_decap steering table

[Daniel Zahka]

On Tue Sep 8, 2026 at 2:52 AM EDT, Tariq Toukan wrote:
> From: Cosmin Ratiu <cratiu@nvidia.com>
>  
> +static void accel_psp_fs_rx_reconfigure(struct mlx5e_psp_fs *fs,
> +					bool decap_wanted)
> +{
> +	bool decap_supported =
> +		MLX5_CAP_FLOWTABLE(fs->mdev,
> +				   flow_table_properties_nic_receive.reformat_del_psp_transport);

not for this series, but this makes me think having some way of letting
the user know whether or not psp with hw gro is enabled would be nice,
so that they don't wonder why stuff isn't coalescing when they have old
fw like I do :) I could see a similar thing being useful for gso
features if the psp device can't segment something that it otherwise can
for non-psp.

> +	bool decap_enable = decap_wanted && decap_supported;
> +	struct mlx5_flow_destination dest = {};
> +	int err;
> +
> +	if (decap_enable == fs->decap_enabled)
> +		return;
> +
> +	/* Create the decap table if needed. */
> +	if (decap_enable && !fs->decap.ft) {
> +		err = accel_psp_fs_rx_decap_ft_create(fs, &fs->decap);
> +		if (err)
> +			goto out_err;
> +	}
> +
> +	/* Redirect traffic to the correct table. */
> +	dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
> +	dest.ft = decap_enable ? fs->decap.ft : fs->rx.ft;
> +	err = mlx5_modify_rule_destination(fs->check.rule, &dest, NULL);
> +	if (err)
> +		goto out_err;
> +
> +	fs->decap_enabled = decap_enable;
> +	return;
> +
> +out_err:
> +	mlx5_core_warn(fs->mdev,
> +		       "Failed to create/modify PSP decapsulation rules (err %d), HW GRO for PSP unavailable\n",
> +		       err);
> +}

Re: [PATCH net-next V4 01/14] net/mlx5e: Generalize TC <-> IPsec mutual exclusion

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 1

- [Medium] Silent removal of the eswitch-operation gate on the accel
  (IPsec packet-offload) rule-installation path.
- [Low] The patch fixes a real, pre-existing race on the TC/accel mutual-
  exclusion counters (both decrement paths were previously completely…

Pre-existing issues:
- [High] offload_block.num_accel is leaked on the driver's own flow-
  teardown path, permanently blocking all accel offload on the device.
- [Medium] Both unblock paths drop the counter before the corresponding
  hardware rules are actually gone, so the exclusion the mechanism…

--

net/mlx5e: Generalize TC <-> IPsec mutual exclusion

This generalizes the existing TC <-> IPsec mutual exclusion so other accel
protocols can use it: the helpers move to en_accel.h, the two mdev counters
move into a new offload_block struct, and the conditional eswitch write lock
is replaced by a dedicated mutex. Underflow warnings are added on both
decrement paths.

A few questions below.

The commit message says:

    - replace the conditional use of the esw write lock as a protection for
      incrementing the counter with a new mutex. The esw might not be
      available, and it wasn't used correctly on the decrement path anyway,
      allowing races to happen. Using a dedicated mutex for these counters
      makes it clear and avoids races.

[Severity: Low]

This paragraph describes an actual race fix, but the patch carries no
Fixes: tag; c8e350e62fc5 is only cited as background.

Before this patch both decrement sites were completely unserialized:

    static void mlx5e_ipsec_unblock_tc_offload(struct mlx5_core_dev *mdev)
    {
            mdev->num_block_tc--;
    }

and

    priv->mdev->num_block_ipsec--;

the latter reached from mlx5e_delete_flower(), which takes no esw lock at
all. Two concurrent unblocks on the same mlx5_core_dev (two xfrm SA
deletions, or an SA delete concurrent with tc filter del) can lose an
update and leave the counters wrong.

Would a Fixes: tag pointing at c8e350e62fc5 be appropriate here? As it
stands the race fix is entangled with the renames, the move to en_accel.h
and the new anonymous struct in include/linux/mlx5/driver.h, so it cannot
be picked up for stable as-is. Would splitting the locking fix from the
rename/move make that possible?

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> index 3f212e46fc2f9..8a2ea76164404 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> @@ -218,6 +218,28 @@ static inline void mlx5e_accel_tx_finish(struct mlx5e_txqsq *sq,
>  #endif
>  }
>  
> +static inline int mlx5e_accel_block_tc_offload(struct mlx5_core_dev *mdev)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&mdev->offload_block.lock);
> +	if (mdev->offload_block.num_accel)
> +		ret = -EBUSY;
> +	else
> +		mdev->offload_block.num_tc++;
> +	mutex_unlock(&mdev->offload_block.lock);
> +
> +	return ret;
> +}
> +
> +static inline void mlx5e_accel_unblock_tc_offload(struct mlx5_core_dev *mdev)
> +{
> +	mutex_lock(&mdev->offload_block.lock);
> +	if (!WARN_ON_ONCE(!mdev->offload_block.num_tc))
> +		mdev->offload_block.num_tc--;
> +	mutex_unlock(&mdev->offload_block.lock);
> +}
> +
>  static inline int mlx5e_accel_init_rx(struct mlx5e_priv *priv)
>  {
>  	return mlx5e_ktls_init_rx(priv);

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
> index 329608c59313b..74e0aa5b61331 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
> @@ -2574,53 +2575,12 @@ void mlx5e_accel_ipsec_fs_read_stats(struct mlx5e_priv *priv, void *ipsec_stats)
>  	}
>  }
>  
> -#ifdef CONFIG_MLX5_ESWITCH
> -static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev)
> -{
> -	struct mlx5_eswitch *esw = mdev->priv.eswitch;
> -	int err = 0;
> -
> -	if (esw) {
> -		err = mlx5_esw_lock(esw);
> -		if (err)
> -			return err;
> -	}
> -
> -	if (mdev->num_block_ipsec) {
> -		err = -EBUSY;
> -		goto unlock;
> -	}
> -
> -	mdev->num_block_tc++;
> -
> -unlock:
> -	if (esw)
> -		mlx5_esw_unlock(esw);
> -
> -	return err;
> -}
> -#else

[ ... ]

>  int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
>  {
>  	int err;
>  
>  	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) {
> -		err = mlx5e_ipsec_block_tc_offload(sa_entry->ipsec->mdev);
> +		err = mlx5e_accel_block_tc_offload(sa_entry->ipsec->mdev);
>  		if (err)
>  			return err;
>  	}

[ ... ]

> @@ -2652,7 +2612,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
>  		mlx5_packet_reformat_dealloc(mdev, ipsec_rule->pkt_reformat);
>  
>  	if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET)
> -		mlx5e_ipsec_unblock_tc_offload(mdev);
> +		mlx5e_accel_unblock_tc_offload(mdev);
>  
>  	if (sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_OUT) {
>  		tx_ft_put(sa_entry->ipsec, sa_entry->attrs.type);

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index b290beb4369ad..05c703795df4a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -4830,27 +4830,35 @@ static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx
>  	return true;
>  }
>  
> -static int mlx5e_tc_block_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
> +static int mlx5e_tc_block_accel_offload(struct net_device *filter,
> +					struct mlx5e_priv *priv)
>  {
>  	struct mlx5_core_dev *mdev = priv->mdev;
> +	int ret = 0;
>  
> -	if (!is_tc_ipsec_order_check_needed(filter, priv))
> +	if (!is_tc_accel_check_needed(filter, priv))
>  		return 0;
>  
> -	if (mdev->num_block_tc)
> -		return -EBUSY;
> -
> -	mdev->num_block_ipsec++;
> +	mutex_lock(&mdev->offload_block.lock);
> +	if (mdev->offload_block.num_tc)
> +		ret = -EBUSY;
> +	else
> +		mdev->offload_block.num_accel++;
> +	mutex_unlock(&mdev->offload_block.lock);

[Severity: High]

This isn't a bug introduced by this patch, since the same asymmetry existed
with num_block_ipsec, but can offload_block.num_accel be leaked when the
driver tears down its own flows?

The increment here is only undone by mlx5e_tc_unblock_accel_offload() from
mlx5e_delete_flower() or the mlx5e_configure_flower() error label. Flows
destroyed by the driver go through the bulk destructor instead:

    static void _mlx5e_tc_del_flow(void *ptr, void *arg)
    {
            struct mlx5e_tc_flow *flow = ptr;
            struct mlx5e_priv *priv = flow->priv;

            mlx5e_tc_del_flow(priv, flow);
            kfree(flow);
    }

which is passed to rhashtable_free_and_destroy() from mlx5e_tc_nic_cleanup()
and mlx5e_tc_ht_cleanup(), and never decrements the counter.

mlx5e_tc_nic_cleanup() is reachable without unregistering the netdev:

  _mlx5e_suspend() -> mlx5e_detach_netdev() -> mlx5e_cleanup_nic_rx() ->
  mlx5e_tc_nic_cleanup() -> rhashtable_free_and_destroy(&tc->ht,
                                                        _mlx5e_tc_del_flow, NULL)

i.e. devlink dev reload, PCI suspend and FW-reset/health recovery. On those
paths the cls_flower filters are still installed and still in tc->ht, so
they were never removed via mlx5e_delete_flower().

Since offload_block lives on mlx5_core_dev and nothing else in the tree
resets num_tc/num_accel, wouldn't num_accel stay above zero for the
lifetime of the driver instance, making every later
mlx5e_accel_block_tc_offload() return -EBUSY even though no TC rule exists?
That would refuse IPsec packet offload, and after the rest of this series
also MACsec and PSP enablement, until the module is reloaded.

The new WARN_ON_ONCE() checks only catch the opposite direction.

Would calling mlx5e_tc_unblock_accel_offload() (or an mdev-only variant of
it) from _mlx5e_tc_del_flow(), or resetting the counters on profile
detach, be the right place to close this?

>  
> -	return 0;
> +	return ret;
>  }
>  
> -static void mlx5e_tc_unblock_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
> +static void mlx5e_tc_unblock_accel_offload(struct net_device *filter,
> +					   struct mlx5e_priv *priv)
>  {
> -	if (!is_tc_ipsec_order_check_needed(filter, priv))
> +	if (!is_tc_accel_check_needed(filter, priv))
>  		return;
>  
> -	priv->mdev->num_block_ipsec--;
> +	mutex_lock(&priv->mdev->offload_block.lock);
> +	if (!WARN_ON_ONCE(!priv->mdev->offload_block.num_accel))
> +		priv->mdev->offload_block.num_accel--;
> +	mutex_unlock(&priv->mdev->offload_block.lock);
>  }

[ ... ]

> @@ -4957,7 +4965,7 @@ int mlx5e_delete_flower(struct net_device *dev, struct mlx5e_priv *priv,
>  	trace_mlx5e_delete_flower(f);
>  	mlx5e_flow_put(priv, flow);
>  
> -	mlx5e_tc_unblock_ipsec_offload(dev, priv);
> +	mlx5e_tc_unblock_accel_offload(dev, priv);

[Severity: Medium]

This is a pre-existing ordering, unchanged by the patch (the old
mlx5e_tc_unblock_ipsec_offload() call sat in the same place), but is the
counter dropped before the hardware rules are actually gone?

mlx5e_flow_put() only removes the rules when the last reference goes away:

    void mlx5e_flow_put(struct mlx5e_priv *priv, struct mlx5e_tc_flow *flow)
    {
            if (refcount_dec_and_test(&flow->refcnt)) {
                    mlx5e_tc_del_flow(priv, flow);
                    kfree_rcu(flow, rcu_head);
            }
    }

Other contexts legitimately hold references via mlx5e_flow_get() in
en/tc_tun_encap.c (encap/neigh update work) and in mlx5e_stats_flower().
With refcnt still above one, the TC rules remain in hardware while
mlx5e_tc_unblock_accel_offload() sets num_accel back to 0, so a concurrent

  ip xfrm state add ... offload packet dev <uplink>

can pass mlx5e_accel_block_tc_offload() and install its flow_tag rule next
to the still-live TC flow_tag rule, which is the overwrite the commit
message says must not happen.

The same shape exists in mlx5e_accel_ipsec_fs_del_rule(), where
mlx5e_accel_unblock_tc_offload() runs while the trailer/auth/sa_sel/replay
rules and the RX/TX steering tables are still installed.

>  	mlx5_esw_put(priv->mdev);
>  	return 0;
>  
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index b6e2c153b4f7d..8b36a96522ab7 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -3022,18 +3022,6 @@ int mlx5_esw_try_lock(struct mlx5_eswitch *esw)
>  	return esw->mode;
>  }
>  
> -int mlx5_esw_lock(struct mlx5_eswitch *esw)
> -{
> -	down_write(&esw->mode_lock);
> -
> -	if (esw->eswitch_operation_in_progress) {
> -		up_write(&esw->mode_lock);
> -		return -EBUSY;
> -	}
> -
> -	return 0;
> -}
> -

[Severity: Medium]

Does removing this also remove the eswitch-transition gate on the accel
side, and not just a counter lock?

mlx5_esw_lock() returned -EBUSY whenever esw->eswitch_operation_in_progress
was set, which aborted mlx5e_accel_ipsec_fs_add_rule() and
mlx5e_accel_ipsec_fs_add_pol() for the whole duration of a devlink eswitch
mode / inline-mode / encap-mode transition. That check was added on purpose
by commit baac8351f74c5 ("net/mlx5e: Reduce eswitch mode_lock protection
context"), which carried Fixes: c8e350e62fc5, and it mirrored the check
added to mlx5_esw_hold() for the TC path.

The replacement only takes the new private mutex:

    mutex_lock(&mdev->offload_block.lock);
    if (mdev->offload_block.num_accel)
            ret = -EBUSY;
    else
            mdev->offload_block.num_tc++;
    mutex_unlock(&mdev->offload_block.lock);

which the eswitch code knows nothing about. So

  ip xfrm state add ... offload packet dev <uplink>

can now proceed into rx_add_rule()/tx_add_rule() and create tables and rules
in the FDB namespace (ipsec_fs_get_ns() returns MLX5_FLOW_NAMESPACE_FDB for
XFRM_DEV_OFFLOAD_PACKET on an uplink rep) while
mlx5_devlink_eswitch_encap_mode_set() runs
esw_destroy_offloads_fdb_tables()/esw_create_offloads_fdb_tables() with
mode_lock dropped.

The commit message describes the esw lock only as "protection for
incrementing the counter" and doesn't mention the
eswitch_operation_in_progress -> -EBUSY behaviour going away. Was that
intentional? If so, could the commit message spell out why the gate is no
longer needed?

>  /**
>   * mlx5_esw_unlock() - Release write lock on esw mode lock
>   * @esw: eswitch device.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 04/14] net/mlx5e: macsec: Block TC offload when MACsec is enabled

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 1

- [High] Unbalanced TC-block reference across the MACsec teardown path:
  mlx5e_macsec_init_sa_fs() now takes one mdev->offload_block.num_tc…
- [Low] Commit-message scope vs. implementation: the message states the
  patch makes "sure that TC and MACsec cannot be both active at the same…

--

net/mlx5e: macsec: Block TC offload when MACsec is enabled

The patch takes a reference on the mlx5 TC/accel mutual exclusion counter
whenever a MACsec RX (decrypt) steering rule is installed, and drops it when
that rule is removed. It also holds a best-effort block across
macsec_upd_secy_hw_address() so the drain/re-add loops are not disturbed by a
concurrent TC filter add.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> index daff53ba7d09f..a15a0aff292ff 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c

[ ... ]

> @@ -324,6 +325,8 @@ static void mlx5e_macsec_cleanup_sa_fs(struct mlx5e_macsec *macsec,
>  	mlx5_macsec_fs_del_rule(macsec->mdev->macsec_fs, sa->macsec_rule, action, netdev,
>  				fs_id);
>  	sa->macsec_rule = NULL;
> +	if (!is_tx)
> +		mlx5e_accel_unblock_tc_offload(macsec->mdev);
>  }

[Severity: High]
This is the only place that gives back the count taken in
mlx5e_macsec_init_sa_fs(). Is mlx5e_macsec_cleanup() a second teardown path
that destroys the same RX rules without passing through here?

drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c:mlx5e_macsec_cleanup() {
	mlx5_notifier_unregister(mdev, &macsec->nb);
	mlx5_macsec_fs_cleanup(mdev->macsec_fs);
	destroy_workqueue(macsec->wq);
	mlx5e_macsec_aso_cleanup(&macsec->aso, mdev);
	mutex_destroy(&macsec->lock);
	kfree(macsec);
}

It never walks macsec_device_list_head / rx_sc->rx_sa[] and never calls
mlx5e_accel_unblock_tc_offload(), so wouldn't every RX SA rule still installed
at that moment leave mdev->offload_block.num_tc permanently incremented?

The path looks reachable without any mdo_del_rxsa/mdo_del_secy first:

mlx5_unload_one()/mlx5e_suspend() -> _mlx5e_suspend() ->
mlx5e_detach_netdev() -> mlx5e_nic_profile.disable == mlx5e_nic_disable() ->
mlx5e_macsec_cleanup()

In those flows (system suspend, devlink reload, health recovery) the lower
netdev is only netif_device_detach()ed, not unregistered, so the stacked
MACsec devices and their offloaded RX SAs are still live. mdev->offload_block
is initialized once in mlx5_mdev_init() and is not reset by unload/load.

After such an event, does every later TC flower add on that device fail
forever?

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:mlx5e_tc_block_accel_offload() {
	mutex_lock(&mdev->offload_block.lock);
	if (mdev->offload_block.num_tc)
		ret = -EBUSY;
	...
}

The WARN_ON_ONCE() in mlx5e_accel_unblock_tc_offload() would not fire here
because the leak is a missing decrement, so there is no diagnostic either.
Would it be more robust to drop the reference in mlx5e_macsec_cleanup(), or to
hold it per MACsec device in add_secy/del_secy instead of per RX rule?

>  
>  static void mlx5e_macsec_cleanup_sa(struct mlx5e_macsec *macsec,
> @@ -343,6 +346,7 @@ static int mlx5e_macsec_init_sa_fs(struct macsec_context *ctx,
>  	const struct macsec_tx_sc *tx_sc = &ctx->secy->tx_sc;
>  	struct mlx5_macsec_rule_attrs rule_attrs;
>  	union mlx5_macsec_rule *macsec_rule;
> +	int err = 0;
>  
>  	if (is_tx && tx_sc->encoding_sa != sa->assoc_num)
>  		return 0;
> @@ -353,13 +357,26 @@ static int mlx5e_macsec_init_sa_fs(struct macsec_context *ctx,
>  	rule_attrs.action = (is_tx) ? MLX5_ACCEL_MACSEC_ACTION_ENCRYPT :
>  				      MLX5_ACCEL_MACSEC_ACTION_DECRYPT;
>  
> +	if (!is_tx) {
> +		err = mlx5e_accel_block_tc_offload(priv->mdev);
> +		if (err)
> +			return err;
> +	}
> +

[Severity: Low]
This isn't a bug, but the commit message says the patch makes

    "sure that TC and MACsec cannot be both active at the same time"

while the block here is taken only for RX rules (!is_tx), and only for as long
as an RX rule exists. A device configured with just an active TX SA goes
through mlx5e_macsec_add_txsa() -> mlx5e_macsec_init_sa() ->
mlx5e_macsec_init_sa_fs() with is_tx=true and never touches
mdev->offload_block.num_tc, so TC flower filters are still accepted while
MACsec offload is active.

That scope looks functionally sufficient, since the follow-up patch
"net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag" only sets
FLOW_CONTEXT_HAS_TAG/flow_tag on the RX crypto rules in
macsec_fs_rx_add_rule(), the TX path keeps using MLX5_MACSEC_TX_METADATA, and
tc_act_can_offload_mark() limits TC marks to 16 bits so a TC filter cannot
forge the proto bits. Could the commit message state the RX-only scope
explicitly, the way the sibling IPsec patch does?

Related to the same narrow scope: because the block is per RX rule, a TC filter
can be installed during any window with no RX rule present (all RX SAs
inactive, or after a partially failed macsec_upd_secy_hw_address()). The next
RX SA activation then returns a bare -EBUSY from mlx5e_macsec_init_sa_fs()
with no netdev_err() or extack, unlike the other error paths in this file.
Would adding a message here help users understand the failure?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 09/14] net/mlx5e: psp: Add an rx_decap steering table

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] accel_psp_fs_rx_decap_ft_create()
  (drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c) creates the…
- [Medium] accel_psp_fs_rx_decrypt_ft_create() now unconditionally
  allocates a modify-header context that copies…

--

net/mlx5e: psp: Add an rx_decap steering table

This adds a new PSP RX decapsulation steering table with one rule per
supported PSP version (transport-mode decap, SPI copy into reg_b, flow tag
marker) plus a drop rule for unsupported versions. The mode is tracked in
fs.decap_enabled and switched through accel_psp_fs_rx_reconfigure(), which
is called with decap_wanted == false in this commit.

A couple of questions below.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> index 20d7f14fb4acd..84133cbf86895 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c

[ ... ]

> @@ -403,11 +434,164 @@ int accel_psp_fs_rx_check_ft_create(struct mlx5e_psp_fs *fs,
>  	return err;
>  }
>  
> +static
> +int accel_psp_fs_rx_decap_ft_create(struct mlx5e_psp_fs *fs,
> +				    struct mlx5e_psp_rx_decap_table *decap)
> +{

[ ... ]

> +	/* Create FT */
> +	ft_attr.max_fte = 1 + MLX5E_PSP_NUM_SUPPORTED_VERSIONS;
> +	ft_attr.level = MLX5E_ACCEL_FS_PSP_DECAP_FT_LEVEL;
> +	ft_attr.prio = MLX5E_NIC_PRIO;
> +	ft_attr.autogroup.num_reserved_entries = 1;
> +	err = accel_psp_fs_create_ft(fs, &ft_attr, &decap->ft);

[Severity: High]
Should ft_attr.flags here include MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT?

Every rule installed into this table below carries
MLX5_FLOW_CONTEXT_ACTION_PACKET_REFORMAT with a
MLX5_REFORMAT_TYPE_REMOVE_PSP_TRANSPORT context, but the table flag is the
only source of flow_table_context.reformat_en:

fs_cmd.c:mlx5_cmd_create_flow_table() {
	int en_encap = !!(ft->flags & MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT);
	...
	MLX5_SET(create_flow_table_in, in, flow_table_context.reformat_en,
		 en_encap);
}

The SWS and HWS backends derive reformat_en from the same flag, so with
flags == 0 the table is created with reformat disabled while its FTEs
request a reformat action.

The directly analogous header-removal case in the same driver does set the
flag, for the MACsec RX check table that hosts
MLX5_REFORMAT_TYPE_DEL_MACSEC:

lib/macsec_fs.c:macsec_fs_rx_create() {
	flow_table = macsec_fs_auto_group_table_create(ns,
						       MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT,
						       RX_CHECK_TABLE_LEVEL,
						       RX_CHECK_TABLE_NUM_FTE);
}

IPsec does the same on rx->ft.sa / tx->ft.sa in en_accel/ipsec_fs.c, and so
do esw/bridge.c, lib/fs_chains.c, esw/vporttbl.c and eswitch_offloads.c for
their reformat-bearing tables.

If firmware rejects the reformat FTE, mlx5_add_flow_rules() fails in the
version loop, accel_psp_fs_rx_decap_ft_create() unwinds, and
accel_psp_fs_rx_reconfigure() only prints "HW GRO for PSP unavailable", so
the feature stays inert even on HW/FW advertising
reformat_del_psp_transport. If a backend instead accepts the FTE without
performing the reformat, packets that still carry UDP+PSP headers get the
PSP-decap flow tag and are interpreted as decapsulated by the RX handler
added later in the series.

Note there is no software-side check in fs_core.c for this mismatch, so it
only shows up as a firmware syndrome at rule installation time. Also, as
IPsec shows (mlx5_eswitch_block_encap() before setting the flag), adding the
flag alone may not be the whole fix.

At the end of the series (selftests: drv-net: Add PSP HW GRO conformance
tests) psp.c still has no ft_attr.flags assignment and no
MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT anywhere.

> +	if (err) {
> +		mlx5_core_err(mdev, "fail to create psp decap rx ft err=%d\n",
> +			      err);
> +		goto out_spec;
> +	}

[ ... ]

> +	/* pkt_reformat: decap PSP transport */
> +	reformat_params.type = MLX5_REFORMAT_TYPE_REMOVE_PSP_TRANSPORT;
> +	reformat = mlx5_packet_reformat_alloc(mdev, &reformat_params,
> +					      MLX5_FLOW_NAMESPACE_KERNEL);
> +	if (IS_ERR(reformat)) {
> +		err = PTR_ERR(reformat);
> +		goto out_err;
> +	}
> +	decap->reformat = reformat;
> +
> +	for (i = 0; i < MLX5E_PSP_NUM_SUPPORTED_VERSIONS; i++) {

[ ... ]

> +		flow_act.action = MLX5_FLOW_CONTEXT_ACTION_PACKET_REFORMAT |
> +			MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
> +			MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> +		flow_act.pkt_reformat = reformat;
> +		flow_act.modify_hdr = modify_hdr;
> +		dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
> +		dest.ft = fs->rx.ft;
> +
> +		rule = mlx5_add_flow_rules(decap->ft, spec, &flow_act, &dest, 1);
> +		if (IS_ERR(rule)) {
> +			err = PTR_ERR(rule);
> +			goto out_err;
> +		}
> +		decap->rule[i] = rule;
> +	}
> +	goto out_spec;
> +
> +out_err:
> +	accel_psp_fs_rx_decap_ft_destroy(fs, decap);
> +out_spec:
> +	kvfree(spec);
> +	return err;
> +}
> +

[ ... ]

> @@ -474,6 +660,24 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
>  	}
>  	decrypt->miss_rule = rule;
>  
> +	/* Create modify_hdr to copy SPI to REG_C_0 */
> +	MLX5_SET(copy_action_in, action, action_type, MLX5_ACTION_TYPE_COPY);
> +	MLX5_SET(copy_action_in, action, src_field,
> +		 MLX5_ACTION_IN_FIELD_PSP_HEADER_1);
> +	MLX5_SET(copy_action_in, action, src_offset, 0);
> +	MLX5_SET(copy_action_in, action, length, 0);  /* 0 = 32 bits */
> +	MLX5_SET(copy_action_in, action, dst_field,
> +		 MLX5_ACTION_IN_FIELD_METADATA_REG_C_0);
> +	MLX5_SET(copy_action_in, action, dst_offset, 0);
> +
> +	modhdr = mlx5_modify_header_alloc(mdev, MLX5_FLOW_NAMESPACE_KERNEL, 1,
> +					  action);
> +	if (IS_ERR(modhdr)) {
> +		err = PTR_ERR(modhdr);
> +		goto out_err;
> +	}
> +	decrypt->modify_hdr = modhdr;
> +

[Severity: Medium]
Should this SPI copy be gated on the same capability as the decap table?

The only consumer of REG_C_0 here is the optional decap table, which
re-copies REG_C_0 into REG_B and is gated in accel_psp_fs_rx_reconfigure()
on:

	bool decap_supported =
		MLX5_CAP_FLOWTABLE(fs->mdev,
				   flow_table_properties_nic_receive.reformat_del_psp_transport);

with failures there being warn-only. On the decrypt path, however, the
MLX5_ACTION_IN_FIELD_PSP_HEADER_1 copy is unconditional, and a failure is
fatal for the whole feature:

accel_psp_fs_rx_create()
  accel_psp_fs_rx_decrypt_ft_create()   /* -> goto out_err */
    NL_SET_ERR_MSG(extack, "Failed creating RX decrypt steering table");

mlx5_cmd_modify_header_alloc() validates only num_actions against
max_modify_header_actions, so the source field is validated by firmware
alone and there is no capability bit for it. Both
MLX5_ACTION_IN_FIELD_PSP_HEADER_1 and the reformat_del_psp_transport bit
came in with commit bee40a7d0bd1 ("net/mlx5: Add PSP related fields to the
mlx5_ifc"), while PSP crypto offload shipped earlier with commit
04a3134f88a4. On FW that has PSP crypto offload but not the newer PSP header
field, does PSP offload stop being enable-able at all rather than just
losing decap/GRO?

That would not match the commit message:

    The intention is for failures creating the new table to not block
    feature reconfig/PSP toggling.
    ...
    Nothing happens on older HW/FW without the ability to decapsulate PSP
    transport.

The change to the pre-existing decrypt rule is not mentioned in the commit
message at all; could it be described there?

Two smaller points in the same block. This is the only failure branch of
accel_psp_fs_rx_decrypt_ft_create() without an mlx5_core_err(), while the
code it replaces did log ("fail to alloc psp set modify_header_id err=%d").
And since accel_psp_fs_rx_create() calls accel_psp_fs_rx_reconfigure(fs,
false) in this commit, the extra MOD_HDR action runs for every PSP RX packet
with nothing reading REG_C_0 until the follow-up patch enables decap. At the
end of the series the decrypt-side copy is still unconditional and still
unlogged.

>  	/* Add PSP RX decrypt rule */
>  	setup_fte_udp_psp(spec, PSP_DEFAULT_UDP_PORT);
>  
> @@ -484,8 +688,10 @@ accel_psp_fs_rx_decrypt_ft_create(struct mlx5e_psp_fs *fs,
>  			   MLX5E_ACCEL_FLOW_TAG_PROTO_PSP);
>  
>  	flow_act.crypto.type = MLX5_FLOW_CONTEXT_ENCRYPT_DECRYPT_TYPE_PSP;
> -	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
> -			  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT;
> +	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |
> +		MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
> +		MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> +	flow_act.modify_hdr = modhdr;
>  	dest.type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
>  	dest.ft = fs->check.ft;
>  	rule = mlx5_add_flow_rules(decrypt->ft, spec, &flow_act, &dest, 1);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 10/14] net/mlx5e: shampo: Flush session on PSP mismatch

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Understated impact / missing Fixes: tag.

--

net/mlx5e: shampo: Flush session on PSP mismatch

This adds mlx5e_hw_gro_psp_match() and wires it into the SHAMPO session
match test in mlx5e_handle_rx_cqe_mpwrq_shampo(), so a session is flushed
when the PSP status changes, when SPI/version differ, or when the CQE
indicates a non-decapsulated PSP packet.

(No Link: tags present in the commit header.)

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index e3f915beebe1..9eb11be7abe0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -2224,6 +2224,33 @@ static bool mlx5e_hw_gro_skb_has_enough_space(struct sk_buff *skb,
>  		return page_size * nr_frags + data_bcnt <= GRO_LEGACY_MAX_SIZE;
>  }
>  
> +static bool mlx5e_hw_gro_psp_match(struct sk_buff *skb, struct mlx5_cqe64 *cqe)
> +{
> +#ifdef CONFIG_MLX5_EN_PSP
> +	struct psp_skb_ext *pse = skb_ext_find(skb, SKB_EXT_PSP);
> +	bool is_psp = mlx5e_psp_is_rx_flow(cqe);
> +
> +	if (likely(!is_psp && !pse))
> +		return true;
> +
> +	/* No match on PSP status change (no crypto -> crypto or vice-versa). */
> +	if (unlikely(is_psp != !!pse))
> +		return false;
> +
> +	/* SPI and version are only available in CQE metadata for decap flows.
> +	 * Non-decap PSP cannot be matched here, force a flush.
> +	 */
> +	if (unlikely(!mlx5e_psp_is_decap(cqe)))
> +		return false;

[Severity: Medium]
Is the commit message understating what this particular hunk fixes?  The
message says:

    The session is also forcefully flushed if non-decapsulated packets are
    received. This could happen if, for example, the decap table could not
    be created so HW GRO is active but there's no decapsulation.

At the baseline commit (bc6fe9d301d5), non-decap looks like the only PSP
RX mode rather than an exceptional fallback:
mlx5e_psp_offload_handle_rx_skb() in en_accel/psp_rxtx.c only calls
psp_dev_rcv(), and mlx5e_psp_update_rx()/shampo_enabled() coupling between
PSP and NETIF_F_GRO_HW is only added by a later patch in this series.

If that is right, does the following sequence mis-deliver payload bytes
today, without this patch?

mlx5e_handle_rx_cqe_mpwrq_shampo()
  -> HW coalesces several PSP datagrams sharing the outer IP/UDP 5-tuple
     (mlx5e_shampo_update_hdr() -> mlx5e_shampo_update_ipv4_udp_hdr()
      sets SKB_GSO_UDP_L4, so UDP flows are aggregated too)
  -> mlx5e_shampo_complete_rx_cqe():

	if (NAPI_GRO_CB(skb)->count != 1)
		return false;

	if (mlx5e_build_rx_skb(cqe, cqe_bcnt, rq, skb))
		return true;

so mlx5e_build_rx_skb() -> mlx5e_psp_offload_handle_rx_skb() ->
psp_dev_rcv() runs only for the first record of a session.  psp_dev_rcv()
in net/psp/psp_main.c strips one 16-byte PSP header plus one 16-byte
trailer and adjusts iph->tot_len once, so do the PSP headers and ICVs of
records 2..N remain embedded in the payload handed to the socket?

If so, should the commit message say that PSP + HW GRO
(ethtool -K <dev> rx-gro-hw on, also implied by tcp-data-split) is broken
in the current tree, and should the patch carry a Fixes: tag so stable
kernels shipping mlx5e PSP pick it up?  A natural candidate looks like
29d7f433fcec ("net/mlx5e: Add Rx data path offload"), even if a backport
needs adjustment because mlx5e_psp_is_decap() is new in this series.

> +
> +	/* No match on security parameters change. */
> +	return pse->spi == mlx5e_psp_get_spi(cqe) &&
> +		pse->version == mlx5e_psp_get_version(cqe);
> +#else
> +	return true;
> +#endif
> +}
> +
>  static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
>  {
>  	u16 data_bcnt		= mpwrq_get_cqe_byte_cnt(cqe) - cqe->shampo.header_size;
> @@ -2266,8 +2293,9 @@ static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cq
>  	}
>  
>  	if (*skb &&
> -	    !(match && mlx5e_hw_gro_skb_has_enough_space(*skb, data_bcnt,
> -							 page_size))) {
> +	    !(match &&
> +	      mlx5e_hw_gro_skb_has_enough_space(*skb, data_bcnt, page_size) &&
> +	      mlx5e_hw_gro_psp_match(*skb, cqe))) {
>  		match = false;
>  		mlx5e_shampo_flush_skb(rq, cqe, match);
>  	}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 11/14] net/mlx5e: psp: Dynamically reconfigure based on SHAMPO mode

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In mlx5e_update_nic_rx()
  (drivers/net/ethernet/mellanox/mlx5/core/en_main.c:6289-6300) the…

--

net/mlx5e: psp: Dynamically reconfigure based on SHAMPO mode

This adds an mlx5e_update_nic_rx() -> mlx5e_accel_update_rx() ->
mlx5e_psp_update_rx() hook so PSP steering can switch between standard and
decap modes when the SHAMPO mode changes.  It also enables decap at fs
creation time when shampo_enabled() says so.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> index 84133cbf8689..c40d2ddf6af2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c

[ ... ]

> @@ -1355,3 +1363,14 @@ void mlx5e_psp_cleanup(struct mlx5e_priv *priv)
>  	priv->psp = NULL;
>  	kfree(psp);
>  }
> +
> +void mlx5e_psp_update_rx(struct mlx5e_priv *priv)
> +{
> +	struct mlx5e_psp *psp = priv->psp;
> +
> +	netdev_assert_locked(priv->netdev);
> +	if (!psp || !psp->fs->check.ft)
> +		return;
> +
> +	accel_psp_fs_rx_reconfigure(psp->fs, shampo_enabled(priv));
> +}

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index fc110a7d16e8..b5449d849838 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6288,7 +6288,15 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
>  
>  static int mlx5e_update_nic_rx(struct mlx5e_priv *priv)
>  {
> -	return mlx5e_refresh_tirs(priv->mdev, false, false);
> +	int err;
> +
> +	err = mlx5e_refresh_tirs(priv->mdev, false, false);
> +	if (err)
> +		return err;
> +
> +	mlx5e_accel_update_rx(priv);
> +
> +	return 0;
>  }

[Severity: Medium]
Should the new PSP hook be gated on the TIR refresh result?  The two
operations look unrelated (TIR self-loopback refresh versus the PSP check
rule destination), so a failing mlx5e_refresh_tirs() now also skips the PSP
steering reconfiguration.

mlx5e_refresh_tirs() does have reachable failure modes on devices where
MLX5_CAP_GEN(mdev, tis_tir_td_order) is 0, since it then calls
mlx5e_modify_tirs_lb() in en_common.c, which returns -ENOMEM when
mlx5e_tir_builder_alloc() fails and otherwise propagates the MODIFY_TIR
firmware error:

drivers/net/ethernet/mellanox/mlx5/core/en_common.c:mlx5e_modify_tirs_lb() {
	builder = mlx5e_tir_builder_alloc(true);
	if (!builder)
		return -ENOMEM;
	...
		err = mlx5e_tir_modify(tir, builder);
		if (err) {
	...
			break;
}

The live reconfiguration caller does not look at the return value either,
and it has already committed the new channel params by then:

drivers/net/ethernet/mellanox/mlx5/core/en_main.c:mlx5e_switch_priv_channels() {
	priv->channels = *new_chs;
	...
	priv->profile->update_rx(priv);
	...
	mlx5e_activate_priv_channels(priv);
}

In that case the datapath runs with the new packet_merge.type while
fs->decap_enabled still reflects the old mode, and nothing re-evaluates it,
because shampo_enabled() is only consulted from mlx5e_psp_update_rx() and
accel_psp_fs_create().  When SHAMPO has just been enabled the PSP check rule
keeps pointing at fs->rx.ft rather than fs->decap.ft, so HW GRO for PSP stays
unavailable, and unlike accel_psp_fs_rx_reconfigure()'s own error path there
is no mlx5_core_warn() to indicate it.

The RX path itself appears to tolerate the mismatch, since
mlx5e_psp_is_decap() keys off the per-CQE flow tag and en_rx.c flushes the
SHAMPO session for non-decap PSP packets, and the state self-heals on the
next channel switch or open.  Would calling mlx5e_accel_update_rx(priv)
unconditionally, or before mlx5e_refresh_tirs(), be preferable here?

This condition is still present at the end of the series
(a9e41034d482a6cbfe0487f9ed8050c5bf5c3ce1), where mlx5e_update_nic_rx() is
unchanged.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 13/14] selftests: net: gro: Add PSP encapsulation and encryption

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 7

- [Low] `struct psphdr { ... } __packed;` in
  tools/testing/selftests/net/lib/gro.c uses the `__packed` macro, but…
- [Low] The `psp_ver_diff` sub-test claims to verify that 'different PSP
  version doesn't coalesce', but it is implemented by the same…
- [Low] The file-level comment block of
  tools/testing/selftests/net/lib/gro.c enumerates every test case and…
- [Low] Concern (validated as NOT a defect): gro.c gains `--psp-assoc`
  and three psp_* sub-tests with no in-tree caller, making the feature…
- [Low] Concern (validated as NOT a defect): psp_encapsulate() hardcodes
  IPv6 header length and does not walk extension headers or reject…
- [Low] Concern (validated as NOT a defect): psp_encapsulate() recomputes
  the IPv4 header checksum from scratch, repairing the deliberately…
- [Low] Concern (validated as NOT a defect): each parsed PSP association
  allocates an EVP_CIPHER_CTX that is never freed, on error paths and at…

--

selftests: net: gro: Add PSP encapsulation and encryption

This adds an optional PSP transport mode to the gro selftest helper: frames
are encapsulated and encrypted in software with OpenSSL before being sent,
and the receiving NIC is expected to decrypt and decapsulate them. Three
PSP-only sub-tests (psp_spi_diff, psp_ver_diff, psp_mixed) and a new
--psp-assoc option are added, with libcrypto detection scoped to the gro
binary in the Makefile.

A few questions below.

> diff --git a/tools/testing/selftests/net/lib/gro.c b/tools/testing/selftests/net/lib/gro.c
> index 7a333155de1a..fe4a4c0e17ae 100644
> --- a/tools/testing/selftests/net/lib/gro.c
> +++ b/tools/testing/selftests/net/lib/gro.c

[ ... ]

> @@ -110,6 +119,38 @@
>  
>  #define EXIT_OVER_COALESCE	42
>  
> +/* PSP transport mode encapsulation, as built by the sender:
> + *
> + *	[eth][IP][UDP dport=1000][PSP hdr][encrypted L4][ICV]
> + *
> + * Can't use the kernel-only include/net/psp/types.h header, so copy the
> + * encoding here.
> + */
> +struct psphdr {
> +	uint8_t		nexthdr;
> +	uint8_t		hdrlen;
> +	uint8_t		crypt_offset;
> +	uint8_t		verfl;
> +	uint32_t	spi;		/* big endian */
> +	uint64_t	iv;		/* big endian */
> +} __packed;

[Severity: Low]

Where does __packed come from in this translation unit?

The include path for this binary is only:

  CFLAGS += -I../../../../../usr/include/ $(KHDR_INCLUDES)
  CFLAGS += -I../../

scripts/headers_install.sh textually rewrites the macro when exporting uapi
headers:

  s/(^|[^a-zA-Z0-9])__packed([^a-zA-Z0-9_]|$)/\1__attribute__((packed))\2/g

so the installed headers (including linux/psp.h) never define __packed, and
kselftest.h plus net/lib/ksft.h only pull in libc headers. In tools/ the
macro is defined by tools/include/linux/compiler.h and
tools/include/linux/compiler-gcc.h, neither of which is on this include
path.

Doesn't that make the declaration parse as a file-scope tentative definition
of an object named __packed of type struct psphdr, leaving the struct
unpacked?

The layouts happen to agree today (spi at offset 4, iv at offset 8,
sizeof == 16), so PSP_HDR_LEN and the offsetof() used for the nonce are
still right. But the unpacked struct carries 8-byte alignment while psph
lands at ETH_HLEN + 20 + 8 == 42 inside psp_scratch, i.e. 2 mod 8, so:

	psph->spi = htonl(assoc->spi);
	psph->iv = htobe64(psp_next_iv++);

are stores the compiler is entitled to treat as aligned. Would using
__attribute__((packed)) directly, or defining __packed locally as the sgx
and bpf selftests do, be preferable here?

> +
> +#define PSP_UDP_PORT		1000
> +#define PSP_UDP_LEN		sizeof(struct udphdr)
> +#define PSP_HDR_LEN		sizeof(struct psphdr)

[ ... ]

> @@ -1111,6 +1374,39 @@ static void send_changed_pppoe_sid(int fd, struct sockaddr_ll *daddr)
>  	write_packet(fd, buf, pkt_size, daddr);
>  }
>  
> +/* PSP packets don't coalesce across SPIs or versions. */
> +static void send_psp_assoc_switch(int fd, struct sockaddr_ll *daddr)
> +{
> +	static char buf[MAX_HDR_LEN + PAYLOAD_LEN];
> +	int pkt_size = total_hdr_len + PAYLOAD_LEN;
> +
> +	if (psp_num_assoc < 2)
> +		error(1, 0, "%s needs two PSP associations", testname);
> +
> +	create_packet(buf, 0, 0, PAYLOAD_LEN, 0);
> +	write_packet(fd, buf, pkt_size, daddr);
> +
> +	create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
> +	psp_idx = 1;
> +	write_packet(fd, buf, pkt_size, daddr);
> +	psp_idx = 0;
> +}

[Severity: Low]

Does psp_ver_diff actually exercise the version dimension?

Both psp_spi_diff and psp_ver_diff dispatch to send_psp_assoc_switch(),
which only flips psp_idx from association 0 to association 1. Since the two
RX associations always carry distinct device-allocated SPIs, the second
frame differs in SPI as well as version.

The kernel side compares the whole extension:

include/net/psp/functions.h:__psp_skb_coalesce_diff() {
	...
	diffs |= memcmp(a, b, sizeof(*a));
	...
}

over struct psp_skb_ext { __be32 spi; u16 dev_id; u8 generation;
u8 version; }, so the differing SPI alone already forces the flush.

Wouldn't psp_ver_diff then keep passing even if the version field were
dropped from the software or hardware comparison, making it a duplicate of
psp_spi_diff? Since SPIs are allocated by the device and two associations
cannot share one, would it make sense to at least assert
psp_assocs[0].version != psp_assocs[1].version in the sender, or note in a
comment that this case mainly exercises multi-version decap?

>  static void bind_packetsocket(int fd)
>  {
>  	struct sockaddr_ll daddr = {};

[ ... ]

> @@ -1559,6 +1855,16 @@ static void gro_sender(void)
>  		usleep(fin_delay_us);
>  		write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
>  
> +	/* PSP sub-tests */
> +	} else if (strcmp(testname, "psp_spi_diff") == 0 ||
> +		   strcmp(testname, "psp_ver_diff") == 0) {
> +		send_psp_assoc_switch(txfd, &daddr);
> +		usleep(fin_delay_us);
> +		write_packet(txfd, fin_pkt, total_hdr_len, &daddr);

[ ... ]

> @@ -1782,6 +2088,22 @@ static void gro_receiver(void)
>  		printf("different PPPoE session ID doesn't coalesce: ");
>  		check_recv_pkts(rxfd, correct_payload, 2);
>  
> +	/* PSP sub-tests */
> +	} else if (strcmp(testname, "psp_spi_diff") == 0) {
> +		correct_payload[0] = PAYLOAD_LEN;
> +		correct_payload[1] = PAYLOAD_LEN;
> +		printf("different PSP SPI doesn't coalesce: ");
> +		check_recv_pkts(rxfd, correct_payload, 2);
> +	} else if (strcmp(testname, "psp_ver_diff") == 0) {
> +		correct_payload[0] = PAYLOAD_LEN;
> +		correct_payload[1] = PAYLOAD_LEN;
> +		printf("different PSP version doesn't coalesce: ");
> +		check_recv_pkts(rxfd, correct_payload, 2);

[ ... ]

> @@ -1802,6 +2124,7 @@ static void parse_args(int argc, char **argv)
>  		{ "ip6ip6", no_argument, NULL, 'E' },
>  		{ "pppoev4", no_argument, NULL, 'p' },
>  		{ "pppoev6", no_argument, NULL, 'P' },
> +		{ "psp-assoc", required_argument, NULL, OPT_PSP_ASSOC },

[Severity: Low]

Should the file-level comment block at the top of gro.c be extended too?

It enumerates every sub-test family (data_*, flags_*, tcp_*, ip_*, large_*,
single, capacity), but psp_spi_diff, psp_ver_diff, psp_mixed and the new
--psp-assoc version,spi,key option are not listed there, and the last patch
of the series does not touch gro.c either.

The requirement that both sides be invoked with the same --psp-assoc
arguments is also undocumented, even though the receiver's expected sizes
depend on it:

static int calc_mss(void)
{
	return ASSUMED_MTU - (total_hdr_len - ETH_HLEN) -
		(psp_enabled ? PSP_ENCAP_LEN : 0);
}

Could that coupling be spelled out next to the other test descriptions?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 14/14] selftests: drv-net: Add PSP HW GRO conformance tests

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 · Low: 3

- [Medium] test_psp_ver_diff() in
  tools/testing/selftests/drivers/net/hw/psp_gro.py gates on the *count*…
- [Medium] test_psp_after_reconfig()
  (tools/testing/selftests/drivers/net/hw/psp_gro.py:132-141) disables…
- [Low] The new HAVE_OPENSSL probe in
  tools/testing/selftests/drivers/net/hw/Makefile (lines 16-28) decides…
- [Low] test_psp_ver_diff()
  (tools/testing/selftests/drivers/net/hw/psp_gro.py:116-122) claims to…
- [Low] The psp_gro.py module docstring (line 7: "This reuses the gro
  binary in --psp mode:") and the new Makefile comment ("psp_gro.py uses…

--

selftests: drv-net: Add PSP HW GRO conformance tests

This adds psp_gro.py, which drives the existing gro helper binary with PSP
associations obtained from a real PSP device, so the sender encrypts in
software and the receiver relies on the NIC for decrypt, decap and HW GRO.
It also plumbs a common_args parameter through gro_lib.run_test() and adds a
libcrypto probe to the hw Makefile.

No Link: tags in the commit header.

>     Add PSP conformance tests using the same gro helper binary as other gro
>     tests , but in --psp mode. SPIs are procured by psp_gro.py from a real

[Severity: Low]
The commit message, the new Makefile comment ("psp_gro.py uses the gro
binary in PSP mode") and the psp_gro.py docstring all describe a "--psp
mode", but the gro helper has no such option.

In tools/testing/selftests/net/lib/gro.c the only PSP entry in
long_options[] is:

	{ "psp-assoc", required_argument, NULL, OPT_PSP_ASSOC },

and psp_enabled is just a side effect of psp_parse_assoc().  _psp_args()
in psp_gro.py correctly emits --psp-assoc ver,spi,key.  Could the wording
be changed to --psp-assoc so nobody tries to pass --psp and gets an
unknown-option error from getopt_long()?

> diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> index 6105be8e590f..daa7e52f236e 100644
> --- a/tools/testing/selftests/drivers/net/hw/Makefile
> +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> @@ -13,6 +13,20 @@ else
>  $(warning excluding iouring tests, liburing not installed or too old)
>  endif
>  
> +# psp_gro.py uses the gro binary in PSP mode, which requires OpenSSL.
> +PKG_CONFIG ?= pkg-config
> +HAVE_OPENSSL := $(shell echo 'int main(void) { return 0; }' | \
> +		  $(CC) -x c - -include openssl/evp.h \
> +		  $(shell $(PKG_CONFIG) --cflags --libs libcrypto 2>/dev/null \
> +			  || echo -lcrypto) \
> +		  -o /dev/null >/dev/null 2>&1 && echo 1)
> +
> +ifeq ($(HAVE_OPENSSL),1)
> +COND_PROGS += psp_gro.py
> +else
> +$(warning excluding PSP GRO tests, libcrypto not installed)
> +endif

[Severity: Low]
Is the build host the right place to test this condition?  The PSP
software encryption runs in the gro binary on the remote endpoint, not on
the build host:

gro_lib.py:_run_gro_bin()
	cfg.bin_local = cfg.net_lib_dir / "gro"
	cfg.bin_remote = cfg.remote.deploy(cfg.bin_local)
	...
	tx_cmd = f"{cfg.bin_remote} {args} --iface {cfg.remote_ifname}"
	...
	cmd(tx_cmd, host=cfg.remote)

gro is dynamically linked against libcrypto:

tools/testing/selftests/net/lib/Makefile
	$(OUTPUT)/gro: LDLIBS += $(LIBCRYPTO_LIBS)

and Remote.deploy() only scp's the executable:

tools/testing/selftests/drivers/net/lib/py/remote_ssh.py
	cmd(f"scp {what} {self.name}:{file_name}")

So on a setup where the build host has libcrypto but the remote endpoint
lacks the runtime library, psp_gro.py is installed and every case fails
with a dynamic linker error on the sender instead of skipping.  Would a
runtime check on the remote be preferable here?

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/hw/psp_gro.py b/tools/testing/selftests/drivers/net/hw/psp_gro.py
> new file mode 100755
> index 000000000000..3a069c944ae1
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/psp_gro.py
> @@ -0,0 +1,157 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +"""
> +PSP HW GRO conformance tests.
> +
> +This reuses the gro binary in --psp mode:

[Severity: Low]
Same --psp naming question as in the commit message: the implemented
option in gro.c is --psp-assoc.

> +The sender crafts encapsulated & SW-encrypted PSP packets with receiver's PSP
> +rx-assoc, and the receiver's device decrypts and decapsulates the packets
> +before an AF_PACKET tap gets to analyze them.
> +
> +All GRO conformance tests which could run with PSP are included.
> +"""

[ ... ]

> +def _setup(cfg, version):
> +    """Enables PSP on the device under test."""
> +    init_psp_dev(cfg)
> +    require_version(cfg, version)
> +
> +
> +def _psp_args(cfg, versions):
> +    """Produces PSP associations as gro binary --psp-assoc arguments."""
> +    keys = [_psp_assoc(cfg, ver) for ver in versions]
> +    return [f"--psp-assoc {ver},{key['spi']:x},{key['key'].hex()}"
> +            for ver, key in zip(versions, keys)]
> +
> +
> +def _run(cfg, test_name, protocol, versions):
> +    """Sets up, associates and runs one gro test case under PSP + HW GRO."""
> +    _setup(cfg, max(versions))
> +
> +    run_test(cfg, "hw", protocol, test_name,
> +             common_args=_psp_args(cfg, versions))

[ ... ]

> +@ksft_variants(_ip_variants())
> +def test_psp_ver_diff(cfg, protocol):
> +    """Frames from two different PSP versions must not coalesce."""
> +    init_psp_dev(cfg)
> +    if len(cfg.psp_info['psp-versions-cap']) < 2:
> +        raise KsftSkipEx("Device supports a single PSP version")
> +
> +    _run(cfg, "psp_ver_diff", protocol, [0, 1])

[Severity: Medium]
Does the guard here match what the test then asks for?  It counts how
many versions the device advertises, but the following call hardcodes
versions 0 and 1.

psp-versions-cap is an independent flag bitmask in
Documentation/netlink/specs/psp.yaml:

      - name: psp-versions-cap
        doc: Bitmask of PSP versions supported by the device.
        type: u32
        enum: version
        enum-as-flags: true

so the supported set is not required to be a contiguous prefix starting
at 0.  On a device advertising, say, hdr0-aes-gcm-128 (0) and
hdr0-aes-gmac-128 (2), the length check passes and then:

psp_gro.py:_run()
	_setup(cfg, max([0, 1]))     /* == 1 */

psp_lib.py:require_version()
	name = cfg.pspnl.consts["version"].entries_by_val[version].name
	if name in cfg.psp_info['psp-versions-cap']:
		return
	...
	raise KsftSkipEx("PSP version not supported", name)

which skips with a reason unrelated to the guard, and the cross-version
non-coalescing assertion never runs on a device that could run it.

The existing peer test picks the versions from the actual caps instead:

tools/testing/selftests/drivers/net/psp.py:assoc_version_mismatch()
	versions = [cfg.pspnl.consts["version"].entries[v].value for v in versions]

Would deriving the two versions from cfg.psp_info['psp-versions-cap'] be
better here?  Today mlx5 happens to advertise exactly versions 0 and 1
(psp_supported_versions[] = { GCM_128, GCM_256 }), so this only bites the
next driver that advertises a GMAC version.

[Severity: Low]
This isn't a bug, but does psp_ver_diff assert anything that psp_spi_diff
does not?  The two associations passed here differ in version and in SPI,
since each rx_assoc allocates a fresh device SPI
(psp_nl_rx_assoc_doit() -> psd->ops->rx_spi_alloc()).

On the gro.c side both cases share the same sender helper, which only
flips psp_idx:

send_psp_assoc_switch()
	create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
	psp_idx = 1;
	write_packet(fd, buf, pkt_size, daddr);
	psp_idx = 0;

and the receive expectations are byte-for-byte the same as psp_spi_diff
(correct_payload[0] = correct_payload[1] = PAYLOAD_LEN, then
check_recv_pkts(rxfd, correct_payload, 2)); only the printf text differs.
A version-only difference does not look constructible from user space, so
this may be the closest achievable approximation - it is just worth noting
that the case would pass even if the GRO path ignored the version field.

[ ... ]

> +@ksft_variants(_ip_variants())
> +def test_psp_after_reconfig(cfg, protocol):
> +    """Verifies that decap still works after PSP off + on."""
> +    _setup(cfg, 0)
> +
> +    cap = cfg.psp_info['psp-versions-cap']
> +    cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': []})
> +    cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': cap})

[Severity: Medium]
Should the disable here be paired with a defer() for the restore?  As
written, the only thing that re-enables PSP is the very next dev_set()
call, with nothing covering an NlError from it or a kill between the two
calls.

init_psp_dev() only registers a rollback when the cached psp_info says the
enabled set differs from the caps:

tools/testing/selftests/drivers/net/psp_lib.py:init_psp_dev()
	if cap != ena:
		cfg.pspnl.dev_set({'id': cfg.psp_dev_id, 'psp-versions-ena': cap})
		defer(cfg.pspnl.dev_set, {'id': cfg.psp_dev_id,
					  'psp-versions-ena': ena})

so on a device discovered with PSP already fully enabled (cap == ena)
there is no defer at all.  If the re-enable does not happen, the NIC is
left with PSP offload off while cfg.psp_info still caches the old enabled
set, and later init_psp_dev() calls compare against that stale snapshot
and conclude nothing needs enabling - subsequent PSP tests then fail on
rx-assoc/decap in a way that looks like a driver problem.

Would registering the re-enable via defer() right after the disable, and
re-reading the device state rather than trusting the cached psp_info,
address that?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908065244.3799142-1-tariqt%40nvidia.com

Re: [PATCH net-next V4 00/14] net/mlx5e: Add support for HW-GRO to PSP

[Cosmin Ratiu]

On Tue, 2026-09-08 at 09:52 +0300, Tariq Toukan wrote:
> Hi,
> 
> Ingress PSP packets cannot be merged by the HW-GRO HW state machine
> because they are not decapsulated and the current HW-GRO state
> machine
> does not understand PSP.
> 
> This series by Cosmin decapsulates PSP packets in steering, which
> allows
> the now-decapsulated PSP packets (== TCP) to go through HW-GRO and be
> aggregated. The SPI and PSP version from the PSP header are handed
> off
> to the driver in the CQE metadata fields. They are used to terminate
> the
> HW GRO session on mismatch, and are required to construct the skb
> extension which is used higher up in the stack.
> 
> Some preparatory work needed to happen to allow that:
> - All accel protocol markers were moved away from ft_metadata into
>   flow_tag
> - Mutual exclusion between TC and accel protocols was added. Trying
> to
>   configure both IPsec and TC at the same time for example is now
>   detected at config time instead of doing weird things at runtime.
> 
> kperf tests on a pair of CX7 NICs with 200Gbps link speed:
> 
> Streams   Gbps no HW-GRO   Gbps HW-GRO   Speedup
> -------   --------------   -----------   -------
>       1               28            58     2.07x
>       2               67           102     1.52x
>       4              136           180     1.32x
>       8              175           183     1.05x
>         
> Regards,
> Tariq
> 
> Some internal Sashiko findings, plus Cosmin's comments:
> 
> > +        cfg.wait_hw_stats_settle()
> > +        after = cfg.netnl.qstats_get({"ifindex": cfg.ifindex},
> > dump=True)[0]
> Could this lead to flaky test failures on multiqueue NICs?
> By hardcoding the netlink dump array access to index [0], the test
> statically
> verifies statistics for the first queue only. Since the test sends
> traffic over
> an ephemeral random port, Receive Side Scaling (RSS) will hash this
> 4-tuple and
> could deliver the flow to any active RX queue. If the flow lands on a
> queue
> other than queue 0, will the test fail to observe the hardware GRO
> packet
> increments?
> 
> [CR] The premise is wrong. [0] is not queue 0, it's the result for
> the
> requested dev.
> 
> > @@ -1757,11 +1755,10 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs
> > *macsec_fs,
> >       rx_tables = &rx_fs->tables;
> >       ft_crypto = &rx_tables->ft_crypto;
> > 
> > -     /* Set bit[31 - 30] macsec marker - 0x01 */
> >       /* Set bit[15-0] fs id */
> >       MLX5_SET(set_action_in, action, action_type,
> > MLX5_ACTION_TYPE_SET);
> >       MLX5_SET(set_action_in, action, field,
> > MLX5_ACTION_IN_FIELD_METADATA_REG_B);
> > -     MLX5_SET(set_action_in, action, data,
> > macsec_fs_set_rx_fs_id(fs_id));
> > +     MLX5_SET(set_action_in, action, data, fs_id);
> Does removing the BIT(30) marker from the MACsec fs_id metadata cause
> a
> regression by exposing RoCE MACsec to an aliasing issue?
> By removing BIT(30) from the fs_id in this patch, the fs_id written
> to
> METADATA_REG_B becomes a small integer (e.g., 1, 2).
> Later in mlx5_macsec_fs_add_roce_rule_rx(), the hardware rule
> ft_macsec_op_check enforces MACsec on RoCE traffic by matching
> metadata_reg_c_5 exactly against this same fs_id value.
> Could any unencrypted packet that has its REG_B set to this small
> integer
> by another NIC feature (such as TC rules setting packet marks, or PSP
> IPsec
> decryption) falsely pass the MACsec check and bypass encryption
> enforcement?
> 
> [CR] Preexisting, TC rules could set the fsid to the the previous
> value even before.
> 'PSP IPsec' decryption doesn't make sense.
> 
> > @@ -1757,11 +1755,10 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs
> > *macsec_fs,
> >  	rx_tables = &rx_fs->tables;
> >  	ft_crypto = &rx_tables->ft_crypto;
> > 
> > -	/* Set bit[31 - 30] macsec marker - 0x01 */
> >  	/* Set bit[15-0] fs id */
> >  	MLX5_SET(set_action_in, action, action_type,
> > MLX5_ACTION_TYPE_SET);
> >  	MLX5_SET(set_action_in, action, field,
> > MLX5_ACTION_IN_FIELD_METADATA_REG_B);
> > -	MLX5_SET(set_action_in, action, data,
> > macsec_fs_set_rx_fs_id(fs_id));
> > +	MLX5_SET(set_action_in, action, data, fs_id);
> Does removing the BIT(30) marker allow metadata collisions in the
> hardware
> RoCE datapath?
> If an administrator configures a benign TC rule that sets a mark
> equal to
> the MACsec fs_id (which is just a 16-bit integer), could an attacker
> send
> unauthenticated RoCE traffic that matches the TC mark, falsely
> setting the
> metadata and bypassing the MACsec hardware authentication checks?
> 
> [CR] This series makes TC and MACsec mutually exclusive.
> 
> > @@ -2160,8 +2167,8 @@ static int
> > mlx5_macsec_fs_add_roce_rule_rx(struct mlx5_macsec_fs *macsec_fs,
> > u32
> > 
> >  	spec->match_criteria_enable |=
> > MLX5_MATCH_MISC_PARAMETERS_2;
> >  	MLX5_SET_TO_ONES(fte_match_param, spec->match_criteria,
> > misc_parameters_2.metadata_reg_c_5);
> > -	MLX5_SET(fte_match_param, spec->match_value,
> > misc_parameters_2.metadata_reg_c_5,
> > -		 macsec_fs_set_rx_fs_id(fs_id));
> > +	MLX5_SET(fte_match_param, spec->match_value,
> > +		 misc_parameters_2.metadata_reg_c_5, fs_id);
> By matching on the bare fs_id without the BIT(30) marker in
> metadata_reg_c_5, is this RoCE datapath rule now susceptible to the
> TC
> mark collision issue?
> 
> [CR] See above.
> 
> > @@ -1778,6 +1775,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs
> > *macsec_fs,
> >  	/* Rx crypto table with SCI rule */
> >  	macsec_fs_rx_setup_fte(spec, &flow_act, attrs, true);
> > 
> > +	spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
> > +	spec->flow_context.flow_tag =
> > +		FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
> > +			   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
> Can adding FLOW_CONTEXT_HAS_TAG to the shared MACsec crypto table
> corrupt
> RoCE Queue Pair Numbers (QPN)?
> This unconditionally adds a flow tag to the shared ft_crypto table,
> which
> processes both Ethernet and RoCE MACsec traffic. When
> FLOW_CONTEXT_HAS_TAG
> is set, the hardware writes the flow tag into the sop_drop_qpn field
> of
> the CQE.
> Will this cause the mlx5_ib driver to read the MACsec tag (value 2)
> instead
> of the actual destination QPN for RDMA traffic, breaking RoCE
> delivery?
> 
> [CR] The flow_tag is only given for Eth CQEs. For RoCE/RDMA, that CQE
> field is the QPN, and therefore mlx5_ib is unaffected.
> 
> > @@ -1803,6 +1805,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs
> > *macsec_fs,
> > 
> >  		macsec_fs_rx_setup_fte(spec, &flow_act, attrs,
> > false);
> > 
> > +		spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
> > +		spec->flow_context.flow_tag =
> > +			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK
> > ,
> > +				  
> > MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
> Is it safe to set the flow tag on the shared crypto table here as
> well?
> This risks corrupting the destination QPN for RoCE traffic as
> mentioned
> above.
> 
> [CR] Same as above, QPN isn't corrupted, those CQEs don't carry
> flow_tag.
> 
> > @@ -1717,7 +1717,7 @@ void
> > mlx5e_macsec_offload_handle_rx_skb(struct net_device *netdev,
> >       if (!macsec)
> >               return;
> > 
> > -     fs_id = MLX5_MACSEC_RX_METADAT_HANDLE(macsec_meta_data);
> > +     fs_id = MLX5_MACSEC_RX_METADATA_HANDLE(macsec_meta_data);
> > 
> >       rcu_read_lock();
> >       sc_xarray_element = xa_load(&macsec->sc_xarray, fs_id);
> This is a pre-existing issue, but can this xa_load() race with an
> immediate
> kfree() during deletion, causing a use-after-free?
> In the deletion path, mlx5e_macsec_del_rxsc() calls xa_erase() and
> then
> immediately frees the element without an RCU grace period:
> mlx5e_macsec_del_rxsc() {
>         ...
>         xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element-
> >fs_id);
>         dst_release(&rx_sc->md_dst->dst);
>         kfree(rx_sc->sc_xarray_element);
>         ...
> }
> Since mlx5e_macsec_offload_handle_rx_skb() retrieves
> sc_xarray_element
> locklessly under rcu_read_lock() via xa_load(), would a concurrent
> deletion
> result in dereferencing freed memory?
> 
> [CR] Preexisting.
> 
> > +static inline int mlx5e_accel_block_tc_offload(struct
> > mlx5_core_dev *mdev)
> > +{
> > +     int ret = 0;
> > +
> > +     mutex_lock(&mdev->offload_block.lock);
> > +     if (mdev->offload_block.num_accel)
> > +             ret = -EBUSY;
> > +     else
> > +             mdev->offload_block.num_tc++;
> This isn't a bug, but does the new naming here swap the semantics of
> the
> variables?
> In the original logic, num_block_tc counted the number of blockers on
> TC
> (which are IPsec/accel rules). By renaming it to num_tc,
> mlx5e_accel_block_tc_offload() now increments num_tc when an accel
> rule
> is added.
> Would it be clearer to keep "block" in the names, e.g., num_block_tc,
> so
> it doesn't look like num_tc is tracking the number of TC rules?
> 
> [CR] Intentional, "offload_block.num_{tc, accel}" sounds better.
> 
> > +static inline int mlx5e_accel_block_tc_offload(struct
> > mlx5_core_dev *mdev)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&mdev->offload_block.lock);
> > +	if (mdev->offload_block.num_accel)
> > +		ret = -EBUSY;
> > +	else
> > +		mdev->offload_block.num_tc++;
> Are the semantic meanings of the num_tc and num_accel variables
> inverted?
> When an Accel rule is added here in mlx5e_accel_block_tc_offload(),
> it
> increments num_tc.
> 
> [CR] Same thing, it's an intentional rename.
> 
> > @@ -384,6 +429,215 @@ static void fill_transportlayer(void *buf,
> > int seq_offset, int ack_offset,
> [ ... ]
> > +static char psp_scratch[L2_HLEN_MAX + IP_MAXPACKET +
> > PSP_ENCAP_LEN];
> [ ... ]
> > +/* Encapsulates & encrypts @pkt with PSP transport mode into
> > psp_scratch.
> > + * Returns the scratch buffer and updates *@lenp.
> > + */
> > +static char *psp_encapsulate(const char *pkt, int *lenp)
> > +{
> [ ... ]
> > +	memcpy(psp_scratch, pkt, len);
> > +
> > +	if (proto == PF_INET) {
> > +		struct iphdr *iph = (struct iphdr *)(psp_scratch +
> > ETH_HLEN);
> Does this code violate strict aliasing rules?
> Since psp_scratch is declared as a character array, casting it to an
> incompatible structure pointer like struct iphdr * (and later struct
> ipv6hdr *
> and struct udphdr *) violates C11 strict aliasing rules.
> Because the tools/ directory assumes standard -fstrict-aliasing
> optimizations
> are active, could this cause the compiler to incorrectly reorder or
> optimize
> away memory writes to these headers, potentially resulting in
> malformed
> packets and spurious test failures?
> 
> [CR] Maybe, but there are already 20+ similar things in the file.
> 
> V4:
> - Removed dead mlx5_esw_lock() (Daniel).
> - Rebased on top of f225a7317c18 ("selftests: drv-net: split gro.py
> into one test per coalescing mode") (Daniel).
> - Dropped the gro_lib.py extraction, made obsolete by above change.
> - Fixed some typos (Aleksandr).
> - Added CONFIG_INET_PSP=y to the test config (Daniel).
> 
> V3:
> https://lore.kernel.org/netdev/20260903085215.3691657-1-tariqt@nvidia.com/
> - Cleared fs->decap_enabled on config down (Daniel).
> - Made decap support optional (don't fail device reconfig on errors).
> - Used bitfield ops for accel protos & psp ver (Daniel).
> - Renamed psp_responder off -> len (Jakub).
> - Dedicated HW GRO PSP test (Jakub).
> - Extended HW GRO test coverage (Jakub).
> 
> V2:
> https://lore.kernel.org/netdev/20260804083535.2946459-1-tariqt@nvidia.com/
> - Use XFail in patch 13 (Jakub).
> 
> V1:
> https://lore.kernel.org/all/20260730091756.2543777-1-tariqt@nvidia.com/
> 
> Cosmin Ratiu (14):
>   net/mlx5e: Generalize TC <-> IPsec mutual exclusion
>   net/mlx5e: ipsec: Block TC offload when IPsec is enabled
>   net/mlx5e: psp: Block TC offload when PSP is enabled
>   net/mlx5e: macsec: Block TC offload when MACsec is enabled
>   net/mlx5e: psp: Move RX marker from ft_metadata to flow_tag
>   net/mlx5e: ipsec: Move RX marker from ft_metadata to flow_tag
>   net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag
>   net/mlx5e: psp: Handle HW-decapsulated RX PSP packets
>   net/mlx5e: psp: Add an rx_decap steering table
>   net/mlx5e: shampo: Flush session on PSP mismatch
>   net/mlx5e: psp: Dynamically reconfigure based on SHAMPO mode
>   selftests: drv-net: psp: Extract shared helpers into psp_lib.py
>   selftests: net: gro: Add PSP encapsulation and encryption
>   selftests: drv-net: Add PSP HW GRO conformance tests
> 
>  .../net/ethernet/mellanox/mlx5/core/en/fs.h   |   1 +
>  .../mellanox/mlx5/core/en_accel/en_accel.h    |  27 ++
>  .../mellanox/mlx5/core/en_accel/flow_tag.h    |  49 +++
>  .../mellanox/mlx5/core/en_accel/ipsec_fs.c    |  72 ++--
>  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |   8 +-
>  .../mellanox/mlx5/core/en_accel/macsec.c      |  34 +-
>  .../mellanox/mlx5/core/en_accel/macsec.h      |   4 +-
>  .../mellanox/mlx5/core/en_accel/psp.c         | 316 ++++++++++++++--
>  .../mellanox/mlx5/core/en_accel/psp.h         |   2 +
>  .../mellanox/mlx5/core/en_accel/psp_rxtx.c    |  21 +-
>  .../mellanox/mlx5/core/en_accel/psp_rxtx.h    |  44 ++-
>  .../net/ethernet/mellanox/mlx5/core/en_main.c |  10 +-
>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  32 +-
>  .../net/ethernet/mellanox/mlx5/core/en_tc.c   |  46 ++-
>  .../net/ethernet/mellanox/mlx5/core/en_tc.h   |   7 +-
>  .../net/ethernet/mellanox/mlx5/core/eswitch.c |  12 -
>  .../net/ethernet/mellanox/mlx5/core/eswitch.h |   1 -
>  .../mellanox/mlx5/core/lib/macsec_fs.c        |  21 +-
>  .../mellanox/mlx5/core/lib/macsec_fs.h        |   9 +-
>  .../net/ethernet/mellanox/mlx5/core/main.c    |   3 +
>  include/linux/mlx5/driver.h                   |   7 +-
>  tools/testing/selftests/drivers/net/Makefile  |   1 +
>  .../testing/selftests/drivers/net/gro_lib.py  |  15 +-
>  .../testing/selftests/drivers/net/hw/Makefile |  17 +
>  tools/testing/selftests/drivers/net/hw/config |   1 +
>  .../selftests/drivers/net/hw/psp_gro.py       | 157 ++++++++
>  tools/testing/selftests/drivers/net/psp.py    |  77 ++--
>  .../testing/selftests/drivers/net/psp_lib.py  |  57 +++
>  tools/testing/selftests/net/lib/Makefile      |  16 +
>  tools/testing/selftests/net/lib/gro.c         | 337
> +++++++++++++++++-
>  30 files changed, 1198 insertions(+), 206 deletions(-)
>  create mode 100644
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/flow_tag.h
>  create mode 100755 tools/testing/selftests/drivers/net/hw/psp_gro.py
>  create mode 100644 tools/testing/selftests/drivers/net/psp_lib.py
> 
> 
> base-commit: bc6fe9d301d59fe5cb236d646abe792bffae5fa6

Replying here instead of the individual messages, to make it easier to
read.

I read and understood the various bits and feedback from Daniel and
from Sashiko.
Some useful findings, everything possible will be addressed in the next
version, and there will be a net series with fixes for the TC exclusion
and macsec issues raised by Sashiko. This will unfortunately add a week
or so at least of dependency.

Cosmin.