diff mbox series

[RFC,10/18] net: macsec: enable HW offloading by default (when available)

Message ID 20200214150258.390-11-irusskikh@marvell.com
State RFC
Delegated to: David Miller
Headers show
Series net: atlantic: MACSec support for AQC devices | expand

Commit Message

Igor Russkikh Feb. 14, 2020, 3:02 p.m. UTC 
From: Mark Starovoytov <mstarovoitov@marvell.com>

This patch makes HW offload to be enabled by default (when available).
This patch along with the next one (reporting real_dev features) are
both required to fix the issue described below.

Issue description:
real_dev features are disabled upon macsec creation.

Root cause:
Features limitation (specific to SW MACSec limitation) is being applied
to HW offloaded case as well.
This causes 'set_features' request on the real_dev with reduced feature
set due to chain propagation.
IF SW MACSec limitations are not applied to HW offloading case (see the
next path), then we still face an issue, because SW MACSec is enabled by
default.

Proposed solution:
Enable HW offloading by default (when available).

Signed-off-by: Mark Starovoytov <mstarovoitov@marvell.com>
Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
---
 drivers/net/macsec.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Antoine Tenart Feb. 21, 2020, 6:04 p.m. UTC | #1 
Hello,

On Fri, Feb 14, 2020 at 06:02:50PM +0300, Igor Russkikh wrote:
> From: Mark Starovoytov <mstarovoitov@marvell.com>
> 
> This patch makes HW offload to be enabled by default (when available).
> This patch along with the next one (reporting real_dev features) are
> both required to fix the issue described below.
> 
> Issue description:
> real_dev features are disabled upon macsec creation.
> 
> Root cause:
> Features limitation (specific to SW MACSec limitation) is being applied
> to HW offloaded case as well.
> This causes 'set_features' request on the real_dev with reduced feature
> set due to chain propagation.
> IF SW MACSec limitations are not applied to HW offloading case (see the
> next path), then we still face an issue, because SW MACSec is enabled by
> default.
> 
> Proposed solution:
> Enable HW offloading by default (when available).

I would say enabling offloading by default is a no-go, and was discussed
when MACsec offloading was initially proposed. But as you said in the
cover letter, a good way to do this would be to allow setting the
offloading mode when the MACsec interface is created (-> an option in
iproute2).

Thanks,
Antoine
diff mbox series

Patch

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 0ee647238996..38403037cea0 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3816,8 +3816,13 @@  static int macsec_newlink(struct net *net, struct net_device *dev,
 
 	macsec->real_dev = real_dev;
 
-	/* MACsec offloading is off by default */
-	macsec->offload = MACSEC_OFFLOAD_OFF;
+	/* If h/w offloading is available, enable it by default */
+	if (real_dev->features & NETIF_F_HW_MACSEC && real_dev->macsec_ops)
+		macsec->offload = MACSEC_OFFLOAD_MAC;
+	else if (real_dev->phydev && real_dev->phydev->macsec_ops)
+		macsec->offload = MACSEC_OFFLOAD_PHY;
+	else
+		macsec->offload = MACSEC_OFFLOAD_OFF;
 
 	if (data && data[IFLA_MACSEC_ICV_LEN])
 		icv_len = nla_get_u8(data[IFLA_MACSEC_ICV_LEN]);
@@ -3860,6 +3865,20 @@  static int macsec_newlink(struct net *net, struct net_device *dev,
 			goto del_dev;
 	}
 
+	/* If h/w offloading is available, propagate to the device */
+	if (macsec_is_offloaded(macsec)) {
+		const struct macsec_ops *ops;
+		struct macsec_context ctx;
+
+		ops = macsec_get_ops(macsec, &ctx);
+		if (ops) {
+			ctx.secy = &macsec->secy;
+			err = macsec_offload(ops->mdo_add_secy, &ctx);
+			if (err)
+				goto del_dev;
+		}
+	}
+
 	err = register_macsec_dev(real_dev, dev);
 	if (err < 0)
 		goto del_dev;