[PATCH net v2 0/5] macsec: offload-related fixes

[PATCH net v2 0/5] macsec: offload-related fixes

[Sabrina Dubroca]

I'm working on a dummy offload for macsec on netdevsim. It just has a
small SecY and RXSC table so I can trigger failures easily on the
ndo_* side. It has exposed a couple of issues.

The first patch is a revert of commit c850240b6c41 ("net: macsec:
report real_dev features when HW offloading is enabled"). That commit
tried to improve the performance of macsec offload by taking advantage
of some of the NIC's features, but in doing so, broke macsec offload
when the lower device supports both macsec and ipsec offload, as the
ipsec offload feature flags were copied from the real device. Since
the macsec device doesn't provide xdo_* ops, the XFRM core rejects the
registration of the new macsec device in xfrm_api_check.

I'm working on re-adding those feature flags when offload is
available, but I haven't fully solved that yet. I think it would be
safer to do that second part in net-next considering how complex
feature interactions tend to be.

v2:
 - better describe the issue introduced by commit c850240b6c41 (Leon
   Romanovsky)
 - drop unnecessary !! (Leon Romanovsky)

Sabrina Dubroca (5):
  Revert "net: macsec: report real_dev features when HW offloading is
    enabled"
  macsec: delete new rxsc when offload fails
  macsec: fix secy->n_rx_sc accounting
  macsec: fix detection of RXSCs when toggling offloading
  macsec: clear encryption keys from the stack after setting up offload

 drivers/net/macsec.c | 51 ++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

-- 
2.38.0

[PATCH net v2 1/5] Revert "net: macsec: report real_dev features when HW offloading is enabled"

[Sabrina Dubroca]

This reverts commit c850240b6c4132574a00f2da439277ab94265b66.

That commit tried to improve the performance of macsec offload by
taking advantage of some of the NIC's features, but in doing so, broke
macsec offload when the lower device supports both macsec and ipsec
offload, as the ipsec offload feature flags (mainly NETIF_F_HW_ESP)
were copied from the real device. Since the macsec device doesn't
provide xdo_* ops, the XFRM core rejects the registration of the new
macsec device in xfrm_api_check.

Example perf trace when running
  ip link add link eni1np1 type macsec port 4 offload mac

    ip   737 [003]   795.477676: probe:xfrm_dev_event__REGISTER      name="macsec0" features=0x1c000080014869
              xfrm_dev_event+0x3a
              notifier_call_chain+0x47
              register_netdevice+0x846
              macsec_newlink+0x25a

    ip   737 [003]   795.477687:   probe:xfrm_dev_event__return      ret=0x8002 (NOTIFY_BAD)
             notifier_call_chain+0x47
             register_netdevice+0x846
             macsec_newlink+0x25a

dev->features includes NETIF_F_HW_ESP (0x04000000000000), so
xfrm_api_check returns NOTIFY_BAD because we don't have
dev->xfrmdev_ops on the macsec device.

We could probably propagate GSO and a few other features from the
lower device, similar to macvlan. This will be done in a future patch.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index c891b60937a7..b3f76e8071f2 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2654,11 +2654,6 @@ static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
 	if (ret)
 		goto rollback;
 
-	/* Force features update, since they are different for SW MACSec and
-	 * HW offloading cases.
-	 */
-	netdev_update_features(dev);
-
 	rtnl_unlock();
 	return 0;
 
@@ -3432,16 +3427,9 @@ static netdev_tx_t macsec_start_xmit(struct sk_buff *skb,
 	return ret;
 }
 
-#define SW_MACSEC_FEATURES \
+#define MACSEC_FEATURES \
 	(NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST)
 
-/* If h/w offloading is enabled, use real device features save for
- *   VLAN_FEATURES - they require additional ops
- *   HW_MACSEC - no reason to report it
- */
-#define REAL_DEV_FEATURES(dev) \
-	((dev)->features & ~(NETIF_F_VLAN_FEATURES | NETIF_F_HW_MACSEC))
-
 static int macsec_dev_init(struct net_device *dev)
 {
 	struct macsec_dev *macsec = macsec_priv(dev);
@@ -3458,12 +3446,8 @@ static int macsec_dev_init(struct net_device *dev)
 		return err;
 	}
 
-	if (macsec_is_offloaded(macsec)) {
-		dev->features = REAL_DEV_FEATURES(real_dev);
-	} else {
-		dev->features = real_dev->features & SW_MACSEC_FEATURES;
-		dev->features |= NETIF_F_LLTX | NETIF_F_GSO_SOFTWARE;
-	}
+	dev->features = real_dev->features & MACSEC_FEATURES;
+	dev->features |= NETIF_F_LLTX | NETIF_F_GSO_SOFTWARE;
 
 	dev->needed_headroom = real_dev->needed_headroom +
 			       MACSEC_NEEDED_HEADROOM;
@@ -3495,10 +3479,7 @@ static netdev_features_t macsec_fix_features(struct net_device *dev,
 	struct macsec_dev *macsec = macsec_priv(dev);
 	struct net_device *real_dev = macsec->real_dev;
 
-	if (macsec_is_offloaded(macsec))
-		return REAL_DEV_FEATURES(real_dev);
-
-	features &= (real_dev->features & SW_MACSEC_FEATURES) |
+	features &= (real_dev->features & MACSEC_FEATURES) |
 		    NETIF_F_GSO_SOFTWARE | NETIF_F_SOFT_FEATURES;
 	features |= NETIF_F_LLTX;
 
-- 
2.38.0

[PATCH net v2 2/5] macsec: delete new rxsc when offload fails

[Sabrina Dubroca]

Currently we get an inconsistent state:
 - netlink returns the error to userspace
 - the RXSC is installed but not offloaded

Then the device could get confused when we try to add an RXSA, because
the RXSC isn't supposed to exist.

Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index b3f76e8071f2..0d6fe34b91ae 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1876,7 +1876,6 @@ static int macsec_add_rxsc(struct sk_buff *skb, struct genl_info *info)
 	struct macsec_rx_sc *rx_sc;
 	struct nlattr *tb_rxsc[MACSEC_RXSC_ATTR_MAX + 1];
 	struct macsec_secy *secy;
-	bool was_active;
 	int ret;
 
 	if (!attrs[MACSEC_ATTR_IFINDEX])
@@ -1904,7 +1903,6 @@ static int macsec_add_rxsc(struct sk_buff *skb, struct genl_info *info)
 		return PTR_ERR(rx_sc);
 	}
 
-	was_active = rx_sc->active;
 	if (tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE])
 		rx_sc->active = !!nla_get_u8(tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE]);
 
@@ -1931,7 +1929,8 @@ static int macsec_add_rxsc(struct sk_buff *skb, struct genl_info *info)
 	return 0;
 
 cleanup:
-	rx_sc->active = was_active;
+	del_rx_sc(secy, sci);
+	free_rx_sc(rx_sc);
 	rtnl_unlock();
 	return ret;
 }
-- 
2.38.0

[PATCH net v2 3/5] macsec: fix secy->n_rx_sc accounting

[Sabrina Dubroca]

secy->n_rx_sc is supposed to be the number of _active_ rxsc's within a
secy. This is then used by macsec_send_sci to help decide if we should
add the SCI to the header or not.

This logic is currently broken when we create a new RXSC and turn it
off at creation, as create_rx_sc always sets ->active to true (and
immediately uses that to increment n_rx_sc), and only later
macsec_add_rxsc sets rx_sc->active.

Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: drop unnecessary !! (Leon Romanovsky)

 drivers/net/macsec.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 0d6fe34b91ae..48e2dd2d5778 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1413,7 +1413,8 @@ static struct macsec_rx_sc *del_rx_sc(struct macsec_secy *secy, sci_t sci)
 	return NULL;
 }
 
-static struct macsec_rx_sc *create_rx_sc(struct net_device *dev, sci_t sci)
+static struct macsec_rx_sc *create_rx_sc(struct net_device *dev, sci_t sci,
+					 bool active)
 {
 	struct macsec_rx_sc *rx_sc;
 	struct macsec_dev *macsec;
@@ -1437,7 +1438,7 @@ static struct macsec_rx_sc *create_rx_sc(struct net_device *dev, sci_t sci)
 	}
 
 	rx_sc->sci = sci;
-	rx_sc->active = true;
+	rx_sc->active = active;
 	refcount_set(&rx_sc->refcnt, 1);
 
 	secy = &macsec_priv(dev)->secy;
@@ -1876,6 +1877,7 @@ static int macsec_add_rxsc(struct sk_buff *skb, struct genl_info *info)
 	struct macsec_rx_sc *rx_sc;
 	struct nlattr *tb_rxsc[MACSEC_RXSC_ATTR_MAX + 1];
 	struct macsec_secy *secy;
+	bool active = true;
 	int ret;
 
 	if (!attrs[MACSEC_ATTR_IFINDEX])
@@ -1897,15 +1899,16 @@ static int macsec_add_rxsc(struct sk_buff *skb, struct genl_info *info)
 	secy = &macsec_priv(dev)->secy;
 	sci = nla_get_sci(tb_rxsc[MACSEC_RXSC_ATTR_SCI]);
 
-	rx_sc = create_rx_sc(dev, sci);
+
+	if (tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE])
+		active = nla_get_u8(tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE]);
+
+	rx_sc = create_rx_sc(dev, sci, active);
 	if (IS_ERR(rx_sc)) {
 		rtnl_unlock();
 		return PTR_ERR(rx_sc);
 	}
 
-	if (tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE])
-		rx_sc->active = !!nla_get_u8(tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE]);
-
 	if (macsec_is_offloaded(netdev_priv(dev))) {
 		const struct macsec_ops *ops;
 		struct macsec_context ctx;
-- 
2.38.0

[PATCH net v2 4/5] macsec: fix detection of RXSCs when toggling offloading

[Sabrina Dubroca]

macsec_is_configured incorrectly uses secy->n_rx_sc to check if some
RXSCs exist. secy->n_rx_sc only counts the number of active RXSCs, but
there can also be inactive SCs as well, which may be stored in the
driver (in case we're disabling offloading), or would have to be
pushed to the device (in case we're trying to enable offloading).

As long as RXSCs active on creation and never turned off, the issue is
not visible.

Fixes: dcb780fb2795 ("net: macsec: add nla support for changing the offloading selection")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 48e2dd2d5778..5a2c1bd65d89 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2572,7 +2572,7 @@ static bool macsec_is_configured(struct macsec_dev *macsec)
 	struct macsec_tx_sc *tx_sc = &secy->tx_sc;
 	int i;
 
-	if (secy->n_rx_sc > 0)
+	if (secy->rx_sc)
 		return true;
 
 	for (i = 0; i < MACSEC_NUM_AN; i++)
-- 
2.38.0

[PATCH net v2 5/5] macsec: clear encryption keys from the stack after setting up offload

[Sabrina Dubroca]

macsec_add_rxsa and macsec_add_txsa copy the key to an on-stack
offloading context to pass it to the drivers, but leaves it there when
it's done. Clear it with memzero_explicit as soon as it's not needed
anymore.

Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 5a2c1bd65d89..7710d549012f 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -1839,6 +1839,7 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info)
 		       secy->key_len);
 
 		err = macsec_offload(ops->mdo_add_rxsa, &ctx);
+		memzero_explicit(ctx.sa.key, secy->key_len);
 		if (err)
 			goto cleanup;
 	}
@@ -2082,6 +2083,7 @@ static int macsec_add_txsa(struct sk_buff *skb, struct genl_info *info)
 		       secy->key_len);
 
 		err = macsec_offload(ops->mdo_add_txsa, &ctx);
+		memzero_explicit(ctx.sa.key, secy->key_len);
 		if (err)
 			goto cleanup;
 	}
-- 
2.38.0

Re: [PATCH net v2 0/5] macsec: offload-related fixes

[Antoine Tenart]

Quoting Sabrina Dubroca (2022-10-26 23:56:22)
> I'm working on a dummy offload for macsec on netdevsim. It just has a
> small SecY and RXSC table so I can trigger failures easily on the
> ndo_* side. It has exposed a couple of issues.
> 
> The first patch is a revert of commit c850240b6c41 ("net: macsec:
> report real_dev features when HW offloading is enabled"). That commit
> tried to improve the performance of macsec offload by taking advantage
> of some of the NIC's features, but in doing so, broke macsec offload
> when the lower device supports both macsec and ipsec offload, as the
> ipsec offload feature flags were copied from the real device. Since
> the macsec device doesn't provide xdo_* ops, the XFRM core rejects the
> registration of the new macsec device in xfrm_api_check.
> 
> I'm working on re-adding those feature flags when offload is
> available, but I haven't fully solved that yet. I think it would be
> safer to do that second part in net-next considering how complex
> feature interactions tend to be.
> 
> v2:
>  - better describe the issue introduced by commit c850240b6c41 (Leon
>    Romanovsky)
>  - drop unnecessary !! (Leon Romanovsky)
> 
> Sabrina Dubroca (5):
>   Revert "net: macsec: report real_dev features when HW offloading is
>     enabled"
>   macsec: delete new rxsc when offload fails
>   macsec: fix secy->n_rx_sc accounting
>   macsec: fix detection of RXSCs when toggling offloading
>   macsec: clear encryption keys from the stack after setting up offload

Series,
Reviewed-by: Antoine Tenart <atenart@kernel.org>

Thanks!

Re: [PATCH net v2 3/5] macsec: fix secy->n_rx_sc accounting

[Jakub Kicinski]

On Wed, 26 Oct 2022 23:56:25 +0200 Sabrina Dubroca wrote:
>  
> -	rx_sc = create_rx_sc(dev, sci);
> +
> +	if (tb_rxsc[MACSEC_RXSC_ATTR_ACTIVE])

nit: double new line :(

Re: [PATCH net v2 1/5] Revert "net: macsec: report real_dev features when HW offloading is enabled"

[Leon Romanovsky]

On Wed, Oct 26, 2022 at 11:56:23PM +0200, Sabrina Dubroca wrote:
> This reverts commit c850240b6c4132574a00f2da439277ab94265b66.
> 
> That commit tried to improve the performance of macsec offload by
> taking advantage of some of the NIC's features, but in doing so, broke
> macsec offload when the lower device supports both macsec and ipsec
> offload, as the ipsec offload feature flags (mainly NETIF_F_HW_ESP)
> were copied from the real device. Since the macsec device doesn't
> provide xdo_* ops, the XFRM core rejects the registration of the new
> macsec device in xfrm_api_check.
> 
> Example perf trace when running
>   ip link add link eni1np1 type macsec port 4 offload mac
> 
>     ip   737 [003]   795.477676: probe:xfrm_dev_event__REGISTER      name="macsec0" features=0x1c000080014869
>               xfrm_dev_event+0x3a
>               notifier_call_chain+0x47
>               register_netdevice+0x846
>               macsec_newlink+0x25a
> 
>     ip   737 [003]   795.477687:   probe:xfrm_dev_event__return      ret=0x8002 (NOTIFY_BAD)
>              notifier_call_chain+0x47
>              register_netdevice+0x846
>              macsec_newlink+0x25a
> 
> dev->features includes NETIF_F_HW_ESP (0x04000000000000), so
> xfrm_api_check returns NOTIFY_BAD because we don't have
> dev->xfrmdev_ops on the macsec device.
> 
> We could probably propagate GSO and a few other features from the
> lower device, similar to macvlan. This will be done in a future patch.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  drivers/net/macsec.c | 27 ++++-----------------------
>  1 file changed, 4 insertions(+), 23 deletions(-)
> 

It is still mystery for me why mlx5 works.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net v2 2/5] macsec: delete new rxsc when offload fails

[Leon Romanovsky]

On Wed, Oct 26, 2022 at 11:56:24PM +0200, Sabrina Dubroca wrote:
> Currently we get an inconsistent state:
>  - netlink returns the error to userspace
>  - the RXSC is installed but not offloaded
> 
> Then the device could get confused when we try to add an RXSA, because
> the RXSC isn't supposed to exist.
> 
> Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  drivers/net/macsec.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net v2 3/5] macsec: fix secy->n_rx_sc accounting

[Leon Romanovsky]

On Wed, Oct 26, 2022 at 11:56:25PM +0200, Sabrina Dubroca wrote:
> secy->n_rx_sc is supposed to be the number of _active_ rxsc's within a
> secy. This is then used by macsec_send_sci to help decide if we should
> add the SCI to the header or not.
> 
> This logic is currently broken when we create a new RXSC and turn it
> off at creation, as create_rx_sc always sets ->active to true (and
> immediately uses that to increment n_rx_sc), and only later
> macsec_add_rxsc sets rx_sc->active.
> 
> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
> v2: drop unnecessary !! (Leon Romanovsky)
> 

With comment from Jakub.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net v2 4/5] macsec: fix detection of RXSCs when toggling offloading

[Leon Romanovsky]

On Wed, Oct 26, 2022 at 11:56:26PM +0200, Sabrina Dubroca wrote:
> macsec_is_configured incorrectly uses secy->n_rx_sc to check if some
> RXSCs exist. secy->n_rx_sc only counts the number of active RXSCs, but
> there can also be inactive SCs as well, which may be stored in the
> driver (in case we're disabling offloading), or would have to be
> pushed to the device (in case we're trying to enable offloading).
> 
> As long as RXSCs active on creation and never turned off, the issue is
> not visible.
> 
> Fixes: dcb780fb2795 ("net: macsec: add nla support for changing the offloading selection")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  drivers/net/macsec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH net v2 5/5] macsec: clear encryption keys from the stack after setting up offload

[Leon Romanovsky]

On Wed, Oct 26, 2022 at 11:56:27PM +0200, Sabrina Dubroca wrote:
> macsec_add_rxsa and macsec_add_txsa copy the key to an on-stack
> offloading context to pass it to the drivers, but leaves it there when
> it's done. Clear it with memzero_explicit as soon as it's not needed
> anymore.
> 
> Fixes: 3cf3227a21d1 ("net: macsec: hardware offloading infrastructure")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  drivers/net/macsec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

RE: [PATCH net v2 1/5] Revert "net: macsec: report real_dev features when HW offloading is enabled"

[Raed Salem]

>On Wed, Oct 26, 2022 at 11:56:23PM +0200, Sabrina Dubroca wrote:
>> This reverts commit c850240b6c4132574a00f2da439277ab94265b66.
>>
>> That commit tried to improve the performance of macsec offload by
>> taking advantage of some of the NIC's features, but in doing so, broke
>> macsec offload when the lower device supports both macsec and ipsec
>> offload, as the ipsec offload feature flags (mainly NETIF_F_HW_ESP)
>> were copied from the real device. Since the macsec device doesn't
>> provide xdo_* ops, the XFRM core rejects the registration of the new
>> macsec device in xfrm_api_check.
>>
>> Example perf trace when running
>>   ip link add link eni1np1 type macsec port 4 offload mac
>>
>>     ip   737 [003]   795.477676: probe:xfrm_dev_event__REGISTER
>name="macsec0" features=0x1c000080014869
>>               xfrm_dev_event+0x3a
>>               notifier_call_chain+0x47
>>               register_netdevice+0x846
>>               macsec_newlink+0x25a
>>
>>     ip   737 [003]   795.477687:   probe:xfrm_dev_event__return      ret=0x8002
>(NOTIFY_BAD)
>>              notifier_call_chain+0x47
>>              register_netdevice+0x846
>>              macsec_newlink+0x25a
>>
>> dev->features includes NETIF_F_HW_ESP (0x04000000000000), so
>> xfrm_api_check returns NOTIFY_BAD because we don't have
>> dev->xfrmdev_ops on the macsec device.
>>
>> We could probably propagate GSO and a few other features from the
>> lower device, similar to macvlan. This will be done in a future patch.
>>
>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>> ---
>>  drivers/net/macsec.c | 27 ++++-----------------------
>>  1 file changed, 4 insertions(+), 23 deletions(-)
>>
>
>It is still mystery for me why mlx5 works.
I think it works when the offload enablement on the macsec device is done after the macsec netdev creation stage i.e.:
Ip link add link eni1np1 type macsec port 4
Then 
ip macsec offload $MACSEC_IF mac
I think this path skips xfrm core rejection
>
>Thanks,
>Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

RE: [PATCH net v2 1/5] Revert "net: macsec: report real_dev features when HW offloading is enabled"

[Raed Salem]

>>On Wed, Oct 26, 2022 at 11:56:23PM +0200, Sabrina Dubroca wrote:
>>> This reverts commit c850240b6c4132574a00f2da439277ab94265b66.
>>>
>>> That commit tried to improve the performance of macsec offload by
>>> taking advantage of some of the NIC's features, but in doing so,
>>> broke macsec offload when the lower device supports both macsec and
>>> ipsec offload, as the ipsec offload feature flags (mainly
>>> NETIF_F_HW_ESP) were copied from the real device. Since the macsec
>>> device doesn't provide xdo_* ops, the XFRM core rejects the
>>> registration of the new macsec device in xfrm_api_check.
>>>
>>> Example perf trace when running
>>>   ip link add link eni1np1 type macsec port 4 offload mac
>>>
>>>     ip   737 [003]   795.477676: probe:xfrm_dev_event__REGISTER
>>name="macsec0" features=0x1c000080014869
>>>               xfrm_dev_event+0x3a
>>>               notifier_call_chain+0x47
>>>               register_netdevice+0x846
>>>               macsec_newlink+0x25a
>>>
>>>     ip   737 [003]   795.477687:   probe:xfrm_dev_event__return
>ret=0x8002
>>(NOTIFY_BAD)
>>>              notifier_call_chain+0x47
>>>              register_netdevice+0x846
>>>              macsec_newlink+0x25a
>>>
>>> dev->features includes NETIF_F_HW_ESP (0x04000000000000), so
>>> xfrm_api_check returns NOTIFY_BAD because we don't have
>>> dev->xfrmdev_ops on the macsec device.
>>>
>>> We could probably propagate GSO and a few other features from the
>>> lower device, similar to macvlan. This will be done in a future patch.
>>>
>>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>>> ---
>>>  drivers/net/macsec.c | 27 ++++-----------------------
>>>  1 file changed, 4 insertions(+), 23 deletions(-)
>>>
>>
>>It is still mystery for me why mlx5 works.
>I think it works when the offload enablement on the macsec device is done
>after the macsec netdev creation stage i.e.:
>Ip link add link eni1np1 type macsec port 4 Then ip macsec offload
>$MACSEC_IF mac I think this path skips xfrm core rejection
Further check it shows that xfrm_api_check fails in both configuration flavors,
in first method because of register_netdevice call the second method because of
netdev_update_features call, however, using the second method (that "works") masks
this error so user space is not aware of it, hence why the second method works.
>>
>>Thanks,
>>Reviewed-by: Leon Romanovsky <leonro@nvidia.com>