diff mbox series

Enable MACsec HW offload in driver_macsec_linux if supported

Message ID 752e48ea-9f11-59c7-779f-faf6ca3cbe71@ammitzboell-consult.dk
State Changes Requested
Headers show
Series Enable MACsec HW offload in driver_macsec_linux if supported | expand

Commit Message

Benny Lønstrup Ammitzbøll Oct. 28, 2022, 2:59 p.m. UTC

Comments

Jouni Malinen Nov. 26, 2022, 10:14 a.m. UTC | #1
On Fri, Oct 28, 2022 at 04:59:56PM +0200, Benny Lønstrup Ammitzbøll wrote:

> +static int macsec_drv_try_enable_hw_offload(void *priv, enum macsec_offload offl)

This breaks the build with many existing systems since enum
macsec_offload is not defined. In other words, this would need some kind
of conditional build mechanism to try to include this only if the needed
defines are available.

> +	msg = msg_prepare(MACSEC_CMD_UPD_OFFLOAD, ctx, drv->ifi);

Same applies for these MACSEC*OFFLOAD* values

> @@ -1180,7 +1220,28 @@ static int macsec_drv_create_transmit_sc(
> +	/* Try to enable MACsec hardware offloading */
> +	err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_MAC);
> +	if (err) {
> +		err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_PHY);
> +		if (err) {
> +			wpa_printf(MSG_INFO, DRV_PREFIX
> +				   "%s: MACSEC_OFFLOAD_OFF",
> +				   drv->common.ifname);
> +		} else {
> +			wpa_printf(MSG_INFO, DRV_PREFIX
> +				   "%s: MACSEC_OFFLOAD_PHY",
> +				   drv->common.ifname);
> +		}
> +	} else {
> +		wpa_printf(MSG_INFO, DRV_PREFIX
> +			   "%s: MACSEC_OFFLOAD_MAC",
> +			   drv->common.ifname);
> +	}
>  }

Missing return 0 at the end of the function.
Benny Lønstrup Ammitzbøll Nov. 28, 2022, 1:04 p.m. UTC | #2
On 26.11.2022 11.14, Jouni Malinen wrote:
> On Fri, Oct 28, 2022 at 04:59:56PM +0200, Benny Lønstrup Ammitzbøll wrote:
>
>> +static int macsec_drv_try_enable_hw_offload(void *priv, enum macsec_offload offl)
> This breaks the build with many existing systems since enum
> macsec_offload is not defined. In other words, this would need some kind
> of conditional build mechanism to try to include this only if the needed
> defines are available.
>
>> +	msg = msg_prepare(MACSEC_CMD_UPD_OFFLOAD, ctx, drv->ifi);
> Same applies for these MACSEC*OFFLOAD* values
Since this is the driver code for linux MACsec I assume that those many 
existing systems are older versions of linux from before the 
macsec_offload was added. How do I check for this?
>
>> @@ -1180,7 +1220,28 @@ static int macsec_drv_create_transmit_sc(
>> +	/* Try to enable MACsec hardware offloading */
>> +	err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_MAC);
>> +	if (err) {
>> +		err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_PHY);
>> +		if (err) {
>> +			wpa_printf(MSG_INFO, DRV_PREFIX
>> +				   "%s: MACSEC_OFFLOAD_OFF",
>> +				   drv->common.ifname);
>> +		} else {
>> +			wpa_printf(MSG_INFO, DRV_PREFIX
>> +				   "%s: MACSEC_OFFLOAD_PHY",
>> +				   drv->common.ifname);
>> +		}
>> +	} else {
>> +		wpa_printf(MSG_INFO, DRV_PREFIX
>> +			   "%s: MACSEC_OFFLOAD_MAC",
>> +			   drv->common.ifname);
>> +	}
>>   }
> Missing return 0 at the end of the function.
Thank you, I will add it.
>
Jouni Malinen Nov. 28, 2022, 3:54 p.m. UTC | #3
On Mon, Nov 28, 2022 at 02:04:19PM +0100, Benny Lønstrup Ammitzbøll wrote:
> On 26.11.2022 11.14, Jouni Malinen wrote:
> > This breaks the build with many existing systems since enum
> > macsec_offload is not defined. In other words, this would need some kind
> > of conditional build mechanism to try to include this only if the needed
> > defines are available.

> Since this is the driver code for linux MACsec I assume that those many
> existing systems are older versions of linux from before the macsec_offload
> was added. How do I check for this?

The most convenient option would be to be able to do #ifdef on something
new defined in the header file that defines these items. Alas, that
might not be available. If there is no convenient automated mechanism
for this, this might require a new wpa_supplicant/hostapd build option
to add safely.
diff mbox series

Patch

From 65efbc9348fb767dd77d2f04418bececadeca71f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Benny=20L=C3=B8nstrup=20Ammitzb=C3=B8ll?= <bla@zeuxion.com>
Date: Fri, 28 Oct 2022 16:47:27 +0200
Subject: [PATCH] Enable MACsec HW offload in driver_macsec_linux if supported.

Signed-off-by: Benny Ammitzboell <benny@ammitzboell-consult.dk>
---
 src/drivers/driver_macsec_linux.c | 63 ++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/src/drivers/driver_macsec_linux.c b/src/drivers/driver_macsec_linux.c
index b609bbf38..21697d6c5 100644
--- a/src/drivers/driver_macsec_linux.c
+++ b/src/drivers/driver_macsec_linux.c
@@ -677,6 +677,46 @@  out_free_msg:
 }
 
 
+/**
+ * macsec_drv_try_enable_hw_offload - Try to enable hardware offload if possible
+ * @priv: Private driver interface data
+ * Returns: 0 on success, -1 on failure (or if not supported)
+ */
+static int macsec_drv_try_enable_hw_offload(void *priv, enum macsec_offload offl)
+{
+	struct macsec_drv_data *drv = priv;
+	struct macsec_genl_ctx *ctx = &drv->ctx;
+	struct nl_msg *msg;
+	struct nlattr *nest;
+	int ret = -1;
+
+	wpa_printf(MSG_DEBUG, DRV_PREFIX "%s trying offl=%u", __func__, offl);
+
+	msg = msg_prepare(MACSEC_CMD_UPD_OFFLOAD, ctx, drv->ifi);
+	if (!msg)
+		return ret;
+
+	nest = nla_nest_start(msg, MACSEC_ATTR_OFFLOAD);
+	if (!nest)
+		goto nla_put_failure;
+
+	NLA_PUT_U8(msg, MACSEC_OFFLOAD_ATTR_TYPE, offl);
+
+	nla_nest_end(msg, nest);
+
+	ret = nl_send_recv(ctx->sk, msg);
+	if (ret < 0) {
+		wpa_printf(MSG_ERROR,
+			   DRV_PREFIX "failed to communicate: %d (%s)",
+			   ret, nl_geterror(-ret));
+	}
+
+ nla_put_failure:
+	nlmsg_free(msg);
+	return ret;
+}
+
+
 /**
  * macsec_drv_get_receive_lowest_pn - Get receive lowest PN
  * @priv: Private driver interface data
@@ -1180,7 +1220,28 @@  static int macsec_drv_create_transmit_sc(
 
 	/* In case some settings have already been done but we couldn't apply
 	 * them. */
-	return try_commit(drv);
+	err = try_commit(drv);
+	if (err)
+		return err;
+
+	/* Try to enable MACsec hardware offloading */
+	err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_MAC);
+	if (err) {
+		err = macsec_drv_try_enable_hw_offload(drv, MACSEC_OFFLOAD_PHY);
+		if (err) {
+			wpa_printf(MSG_INFO, DRV_PREFIX
+				   "%s: MACSEC_OFFLOAD_OFF",
+				   drv->common.ifname);
+		} else {
+			wpa_printf(MSG_INFO, DRV_PREFIX
+				   "%s: MACSEC_OFFLOAD_PHY",
+				   drv->common.ifname);
+		}
+	} else {
+		wpa_printf(MSG_INFO, DRV_PREFIX
+			   "%s: MACSEC_OFFLOAD_MAC",
+			   drv->common.ifname);
+	}
 }
 
 
-- 
2.37.4