2022-08-09 10:39:22

by Emeel Hakim

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

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 <[email protected]>
Signed-off-by: Emeel Hakim <[email protected]>
---
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

2022-08-11 17:18:00

by Jakub Kicinski

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

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 <[email protected]>
> Signed-off-by: Emeel Hakim <[email protected]>

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.

2022-08-14 07:43:56

by Emeel Hakim

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

> -----Original Message-----
> From: Jakub Kicinski <[email protected]>
> Sent: Thursday, 11 August 2022 19:26
> To: Emeel Hakim <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Raed Salem
> <[email protected]>
> 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 <[email protected]>
> > Signed-off-by: Emeel Hakim <[email protected]>
>
> 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.