[PATCH net 1/1] net: macsec: Fix XPN properties passing to macsec offload

[PATCH net 1/1] net: macsec: Fix XPN properties passing to macsec offload

[Emeel Hakim]

Currently macsec invokes HW offload path before reading extended
packet number (XPN) related user properties i.e. salt and short
secure channel identifier (ssci), hence preventing macsec XPN HW
offload.

Fix by moving macsec XPN properties reading prior to HW offload path.

Fixes: 48ef50fa866a ("macsec: Netlink support of XPN cipher suites")
Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
 drivers/net/macsec.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f1683ce6b561..e0da161d94c8 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1808,6 +1808,12 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info)
 
 	rx_sa->sc = rx_sc;
 
+	if (secy->xpn) {
+		rx_sa->ssci = nla_get_ssci(tb_sa[MACSEC_SA_ATTR_SSCI]);
+		nla_memcpy(rx_sa->key.salt.bytes, tb_sa[MACSEC_SA_ATTR_SALT],
+			   MACSEC_SALT_LEN);
+	}
+
 	/* If h/w offloading is available, propagate to the device */
 	if (macsec_is_offloaded(netdev_priv(dev))) {
 		const struct macsec_ops *ops;
@@ -1830,12 +1836,6 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info)
 			goto cleanup;
 	}
 
-	if (secy->xpn) {
-		rx_sa->ssci = nla_get_ssci(tb_sa[MACSEC_SA_ATTR_SSCI]);
-		nla_memcpy(rx_sa->key.salt.bytes, tb_sa[MACSEC_SA_ATTR_SALT],
-			   MACSEC_SALT_LEN);
-	}
-
 	nla_memcpy(rx_sa->key.id, tb_sa[MACSEC_SA_ATTR_KEYID], MACSEC_KEYID_LEN);
 	rcu_assign_pointer(rx_sc->sa[assoc_num], rx_sa);
 
@@ -2050,6 +2050,12 @@ static int macsec_add_txsa(struct sk_buff *skb, struct genl_info *info)
 	if (assoc_num == tx_sc->encoding_sa && tx_sa->active)
 		secy->operational = true;
 
+	if (secy->xpn) {
+		tx_sa->ssci = nla_get_ssci(tb_sa[MACSEC_SA_ATTR_SSCI]);
+		nla_memcpy(tx_sa->key.salt.bytes, tb_sa[MACSEC_SA_ATTR_SALT],
+			   MACSEC_SALT_LEN);
+	}
+
 	/* If h/w offloading is available, propagate to the device */
 	if (macsec_is_offloaded(netdev_priv(dev))) {
 		const struct macsec_ops *ops;
@@ -2072,12 +2078,6 @@ static int macsec_add_txsa(struct sk_buff *skb, struct genl_info *info)
 			goto cleanup;
 	}
 
-	if (secy->xpn) {
-		tx_sa->ssci = nla_get_ssci(tb_sa[MACSEC_SA_ATTR_SSCI]);
-		nla_memcpy(tx_sa->key.salt.bytes, tb_sa[MACSEC_SA_ATTR_SALT],
-			   MACSEC_SALT_LEN);
-	}
-
 	nla_memcpy(tx_sa->key.id, tb_sa[MACSEC_SA_ATTR_KEYID], MACSEC_KEYID_LEN);
 	rcu_assign_pointer(tx_sc->sa[assoc_num], tx_sa);
 
-- 
2.21.3

Re: [PATCH net 1/1] net: macsec: Fix XPN properties passing to macsec offload

[Jakub Kicinski]

On Tue, 9 Aug 2022 13:29:05 +0300 Emeel Hakim wrote:
> Currently macsec invokes HW offload path before reading extended
> packet number (XPN) related user properties i.e. salt and short
> secure channel identifier (ssci), hence preventing macsec XPN HW
> offload.
> 
> Fix by moving macsec XPN properties reading prior to HW offload path.
> 
> Fixes: 48ef50fa866a ("macsec: Netlink support of XPN cipher suites")
> Reviewed-by: Raed Salem <raeds@nvidia.com>
> Signed-off-by: Emeel Hakim <ehakim@nvidia.com>

Is there a driver in the tree which uses those values today?
I can't grep out any rx_sa->key accesses in the drivers at all :S

If there is none it's not really a fix.

RE: [PATCH net 1/1] net: macsec: Fix XPN properties passing to macsec offload

[Emeel Hakim]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, 11 August 2022 19:26
> To: Emeel Hakim <ehakim@nvidia.com>
> Cc: edumazet@google.com; mayflowerera@gmail.com; pabeni@redhat.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Raed Salem
> <raeds@nvidia.com>
> Subject: Re: [PATCH net 1/1] net: macsec: Fix XPN properties passing to macsec
> offload
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, 9 Aug 2022 13:29:05 +0300 Emeel Hakim wrote:
> > Currently macsec invokes HW offload path before reading extended
> > packet number (XPN) related user properties i.e. salt and short secure
> > channel identifier (ssci), hence preventing macsec XPN HW offload.
> >
> > Fix by moving macsec XPN properties reading prior to HW offload path.
> >
> > Fixes: 48ef50fa866a ("macsec: Netlink support of XPN cipher suites")
> > Reviewed-by: Raed Salem <raeds@nvidia.com>
> > Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
> 
> Is there a driver in the tree which uses those values today?
> I can't grep out any rx_sa->key accesses in the drivers at all :S
> 
> If there is none it's not really a fix.

Thanks for the review, agreed
will repost it with commit adjustment to net-next as part
of a macsec offload series.