[PATCH net v7 0/4] Add update_pn flag

[PATCH net v7 0/4] Add update_pn flag

[Radu Pirea (NXP OSS)]

Patches extracted from
https://lore.kernel.org/all/20230928084430.1882670-1-radu-nicolae.pirea@oss.nxp.com/
Update_pn flag will let the offloaded MACsec implementations to know when
the PN is updated.

Radu P.

Radu Pirea (NXP OSS) (4):
  net: macsec: indicate next pn update when offloading
  octeontx2-pf: mcs: update PN only when update_pn is true
  net: phy: mscc: macsec: reject PN update requests
  net/mlx5e: macsec: use update_pn flag instead of PN comparation

 .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c   | 13 +++++++++----
 .../ethernet/mellanox/mlx5/core/en_accel/macsec.c   |  4 ++--
 drivers/net/macsec.c                                |  2 ++
 drivers/net/phy/mscc/mscc_macsec.c                  |  6 ++++++
 include/net/macsec.h                                |  1 +
 5 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH net v7 1/4] net: macsec: indicate next pn update when offloading

[Radu Pirea (NXP OSS)]

Indicate next PN update using update_pn flag in macsec_context.
Offloaded MACsec implementations does not know whether or not the
MACSEC_SA_ATTR_PN attribute was passed for an SA update and assume
that next PN should always updated, but this is not always true.

The PN can be reset to its initial value using the following command:
$ ip macsec set macsec0 tx sa 0 off #octeontx2-pf case

Or, the update PN command will succeed even if the driver does not support
PN updates.
$ ip macsec set macsec0 tx sa 0 pn 1 on #mscc phy driver case

Comparing the initial PN with the new PN value is not a solution. When
the user updates the PN using its initial value the command will
succeed, even if the driver does not support it. Like this:
$ ip macsec add macsec0 tx sa 0 pn 1 on key 00 \
ead3664f508eb06c40ac7104cdae4ce5
$ ip macsec set macsec0 tx sa 0 pn 1 on #mlx5 case

Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
---
Changes in v7:
- removed update_pn description. I will add description as part of the
following patch in v7
https://patchwork.kernel.org/project/netdevbpf/patch/20230928084430.1882670-3-radu-nicolae.pirea@oss.nxp.com/

Changes in v6:
- changed update_pn description

Changes in v5:
- none

Changes in v4:
- patch added in v4

 drivers/net/macsec.c | 2 ++
 include/net/macsec.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index b7e151439c48..c5cd4551c67c 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2383,6 +2383,7 @@ static int macsec_upd_txsa(struct sk_buff *skb, struct genl_info *info)
 
 		ctx.sa.assoc_num = assoc_num;
 		ctx.sa.tx_sa = tx_sa;
+		ctx.sa.update_pn = !!prev_pn.full64;
 		ctx.secy = secy;
 
 		ret = macsec_offload(ops->mdo_upd_txsa, &ctx);
@@ -2476,6 +2477,7 @@ static int macsec_upd_rxsa(struct sk_buff *skb, struct genl_info *info)
 
 		ctx.sa.assoc_num = assoc_num;
 		ctx.sa.rx_sa = rx_sa;
+		ctx.sa.update_pn = !!prev_pn.full64;
 		ctx.secy = secy;
 
 		ret = macsec_offload(ops->mdo_upd_rxsa, &ctx);
diff --git a/include/net/macsec.h b/include/net/macsec.h
index 75a6f4863c83..ebf9bc54036a 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -258,6 +258,7 @@ struct macsec_context {
 	struct macsec_secy *secy;
 	struct macsec_rx_sc *rx_sc;
 	struct {
+		bool update_pn;
 		unsigned char assoc_num;
 		u8 key[MACSEC_MAX_KEY_LEN];
 		union {
-- 
2.34.1

[PATCH net v7 2/4] octeontx2-pf: mcs: update PN only when update_pn is true

[Radu Pirea (NXP OSS)]

When updating SA, update the PN only when the update_pn flag is true.
Otherwise, the PN will be reset to its previous value using the
following command and this should not happen:
$ ip macsec set macsec0 tx sa 0 on

Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware offloading")
Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
---
Changes in v7:
- fixed update_pn check in cn10k_mdo_upd_txsa

Changes in v6:
- patch added in v6

 .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c   | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
index 59b138214af2..6cc7a78968fc 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
@@ -1357,10 +1357,12 @@ static int cn10k_mdo_upd_txsa(struct macsec_context *ctx)
 
 	if (netif_running(secy->netdev)) {
 		/* Keys cannot be changed after creation */
-		err = cn10k_write_tx_sa_pn(pfvf, txsc, sa_num,
-					   sw_tx_sa->next_pn);
-		if (err)
-			return err;
+		if (ctx->sa.update_pn) {
+			err = cn10k_write_tx_sa_pn(pfvf, txsc, sa_num,
+						   sw_tx_sa->next_pn);
+			if (err)
+				return err;
+		}
 
 		err = cn10k_mcs_link_tx_sa2sc(pfvf, secy, txsc,
 					      sa_num, sw_tx_sa->active);
@@ -1529,6 +1531,9 @@ static int cn10k_mdo_upd_rxsa(struct macsec_context *ctx)
 		if (err)
 			return err;
 
+		if (!ctx->sa.update_pn)
+			return 0;
+
 		err = cn10k_mcs_write_rx_sa_pn(pfvf, rxsc, sa_num,
 					       rx_sa->next_pn);
 		if (err)
-- 
2.34.1

[PATCH net v7 3/4] net: phy: mscc: macsec: reject PN update requests

[Radu Pirea (NXP OSS)]

Updating the PN is not supported.
Return -EINVAL if update_pn is true.

The following command succeeded, but it should fail because the driver
does not update the PN:
ip macsec set macsec0 tx sa 0 pn 232 on

Fixes: 28c5107aa904 ("net: phy: mscc: macsec support")
Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
---
Changes in v7:
- none

Changes in v6:
- patch added in v6

 drivers/net/phy/mscc/mscc_macsec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c
index 018253a573b8..4f39ba63a9a9 100644
--- a/drivers/net/phy/mscc/mscc_macsec.c
+++ b/drivers/net/phy/mscc/mscc_macsec.c
@@ -849,6 +849,9 @@ static int vsc8584_macsec_upd_rxsa(struct macsec_context *ctx)
 	struct macsec_flow *flow;
 	int ret;
 
+	if (ctx->sa.update_pn)
+		return -EINVAL;
+
 	flow = vsc8584_macsec_find_flow(ctx, MACSEC_INGR);
 	if (IS_ERR(flow))
 		return PTR_ERR(flow);
@@ -900,6 +903,9 @@ static int vsc8584_macsec_upd_txsa(struct macsec_context *ctx)
 	struct macsec_flow *flow;
 	int ret;
 
+	if (ctx->sa.update_pn)
+		return -EINVAL;
+
 	flow = vsc8584_macsec_find_flow(ctx, MACSEC_EGR);
 	if (IS_ERR(flow))
 		return PTR_ERR(flow);
-- 
2.34.1

[PATCH net v7 4/4] net/mlx5e: macsec: use update_pn flag instead of PN comparation

[Radu Pirea (NXP OSS)]

When updating the SA, use the new update_pn flags instead of comparing the
new PN with the initial one.

Comparing the initial PN value with the new value will allow the user
to update the SA using the initial PN value as a parameter like this:
$ ip macsec add macsec0 tx sa 0 pn 1 on key 00 \
ead3664f508eb06c40ac7104cdae4ce5
$ ip macsec set macsec0 tx sa 0 pn 1 off

Fixes: 8ff0ac5be144 ("net/mlx5: Add MACsec offload Tx command support")
Fixes: aae3454e4d4c ("net/mlx5e: Add MACsec offload Rx command support")
Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@oss.nxp.com>
---
Changes in v7:
- none

Changes in v6:
- patch added in v6

 drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 4 ++--
 1 file changed, 2 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 c9c1db971652..d4ebd8743114 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
@@ -580,7 +580,7 @@ static int mlx5e_macsec_upd_txsa(struct macsec_context *ctx)
 		goto out;
 	}
 
-	if (tx_sa->next_pn != ctx_tx_sa->next_pn_halves.lower) {
+	if (ctx->sa.update_pn) {
 		netdev_err(netdev, "MACsec offload: update TX sa %d PN isn't supported\n",
 			   assoc_num);
 		err = -EINVAL;
@@ -973,7 +973,7 @@ static int mlx5e_macsec_upd_rxsa(struct macsec_context *ctx)
 		goto out;
 	}
 
-	if (rx_sa->next_pn != ctx_rx_sa->next_pn_halves.lower) {
+	if (ctx->sa.update_pn) {
 		netdev_err(ctx->netdev,
 			   "MACsec offload update RX sa %d PN isn't supported\n",
 			   assoc_num);
-- 
2.34.1

Re: [PATCH net v7 0/4] Add update_pn flag

[Sabrina Dubroca]

2023-10-05, 21:06:32 +0300, Radu Pirea (NXP OSS) wrote:
> Patches extracted from
> https://lore.kernel.org/all/20230928084430.1882670-1-radu-nicolae.pirea@oss.nxp.com/
> Update_pn flag will let the offloaded MACsec implementations to know when
> the PN is updated.
> 
> Radu P.
> 
> Radu Pirea (NXP OSS) (4):
>   net: macsec: indicate next pn update when offloading
>   octeontx2-pf: mcs: update PN only when update_pn is true
>   net: phy: mscc: macsec: reject PN update requests
>   net/mlx5e: macsec: use update_pn flag instead of PN comparation

Thanks Radu! For the series:
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>


While reviewing this, I noticed that octeon can leave the HW in an
inconsistent state during upd_txsa and upd_rxsa: these ops do 2
separate changes that can both fail, and if the 2nd change fails, we
don't roll back the first change. This is an older issue (not
introduced by this patch) and can be looked at later (I don't know
what happens to the HW and why setting the PN would fail, maybe it's
not recoverable at that point).

-- 
Sabrina

Re: [PATCH net v7 0/4] Add update_pn flag

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu,  5 Oct 2023 21:06:32 +0300 you wrote:
> Patches extracted from
> https://lore.kernel.org/all/20230928084430.1882670-1-radu-nicolae.pirea@oss.nxp.com/
> Update_pn flag will let the offloaded MACsec implementations to know when
> the PN is updated.
> 
> Radu P.
> 
> [...]

Here is the summary with links:
  - [net,v7,1/4] net: macsec: indicate next pn update when offloading
    https://git.kernel.org/netdev/net/c/0412cc846a1e
  - [net,v7,2/4] octeontx2-pf: mcs: update PN only when update_pn is true
    https://git.kernel.org/netdev/net/c/4dcf38ae3ca1
  - [net,v7,3/4] net: phy: mscc: macsec: reject PN update requests
    https://git.kernel.org/netdev/net/c/e0a8c918daa5
  - [net,v7,4/4] net/mlx5e: macsec: use update_pn flag instead of PN comparation
    https://git.kernel.org/netdev/net/c/fde2f2d7f23d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html