]> git.baikalelectronics.ru Git - kernel.git/commitdiff
Revert "net: macsec: report real_dev features when HW offloading is enabled"
authorSabrina Dubroca <sd@queasysnail.net>
Wed, 2 Nov 2022 21:33:12 +0000 (22:33 +0100)
committerDavid S. Miller <davem@davemloft.net>
Fri, 4 Nov 2022 10:43:55 +0000 (10:43 +0000)
This reverts commit 8480405e591a82a0594e2e274365f9ead724e334.

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>
Reviewed-by: Antoine Tenart <atenart@kernel.org>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/macsec.c

index c891b60937a7f0118147fc6b1e27f78f0df6ab0d..b3f76e8071f24d17dd2da5be385a6c2e200151a5 100644 (file)
@@ -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;