All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
@ 2022-12-08 11:55 ehakim
  2022-12-08 11:55 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: ehakim @ 2022-12-08 11:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: raeds, davem, edumazet, kuba, pabeni, netdev, sd, atenart, jiri,
	Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

Add support for changing Macsec offload selection through the
netlink layer by implementing the relevant changes in
macsec_changelink.

Since the handling in macsec_changelink is similar to macsec_upd_offload,
update macsec_upd_offload to use a common helper function to avoid
duplication.

Example for setting offload for a macsec device:
    ip link set macsec0 type macsec offload mac

Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
V3 -> V4: - Dont pass whole attributes data to macsec_update_offload, just pass relevant attribute.
		  - Fix code style.
		  - Remove macsec_changelink_upd_offload
V2 -> V3: - Split the original patch into 3 patches, the macsec_rtnl_policy related change (separate patch)
                       to be sent to "net" branch as a fix.
                 - Change the original patch title to make it clear that it's only adding IFLA_MACSEC_OFFLOAD
                   to changelink
V1 -> V2: Add common helper to avoid duplicating code

 drivers/net/macsec.c | 100 +++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index d73b9d535b7a..abfe4a612a2d 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -2583,38 +2583,16 @@ static bool macsec_is_configured(struct macsec_dev *macsec)
 	return false;
 }
 
-static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
+static int macsec_update_offload(struct net_device *dev, enum macsec_offload offload)
 {
-	struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
-	enum macsec_offload offload, prev_offload;
-	int (*func)(struct macsec_context *ctx);
-	struct nlattr **attrs = info->attrs;
-	struct net_device *dev;
+	enum macsec_offload prev_offload;
 	const struct macsec_ops *ops;
 	struct macsec_context ctx;
 	struct macsec_dev *macsec;
-	int ret;
-
-	if (!attrs[MACSEC_ATTR_IFINDEX])
-		return -EINVAL;
-
-	if (!attrs[MACSEC_ATTR_OFFLOAD])
-		return -EINVAL;
-
-	if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX,
-					attrs[MACSEC_ATTR_OFFLOAD],
-					macsec_genl_offload_policy, NULL))
-		return -EINVAL;
+	int ret = 0;
 
-	dev = get_dev_from_nl(genl_info_net(info), attrs);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
 	macsec = macsec_priv(dev);
 
-	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
-		return -EINVAL;
-
-	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
 	if (macsec->offload == offload)
 		return 0;
 
@@ -2627,43 +2605,65 @@ static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
 	if (netif_running(dev))
 		return -EBUSY;
 
-	rtnl_lock();
-
-	prev_offload = macsec->offload;
-	macsec->offload = offload;
-
 	/* Check if the device already has rules configured: we do not support
 	 * rules migration.
 	 */
-	if (macsec_is_configured(macsec)) {
-		ret = -EBUSY;
-		goto rollback;
-	}
+	if (macsec_is_configured(macsec))
+		return -EBUSY;
+
+	prev_offload = macsec->offload;
 
 	ops = __macsec_get_ops(offload == MACSEC_OFFLOAD_OFF ? prev_offload : offload,
 			       macsec, &ctx);
-	if (!ops) {
+	if (!ops)
 		ret = -EOPNOTSUPP;
-		goto rollback;
-	}
 
-	if (prev_offload == MACSEC_OFFLOAD_OFF)
-		func = ops->mdo_add_secy;
-	else
-		func = ops->mdo_del_secy;
+	macsec->offload = offload;
 
 	ctx.secy = &macsec->secy;
-	ret = macsec_offload(func, &ctx);
+	ret = offload == MACSEC_OFFLOAD_OFF ? macsec_offload(ops->mdo_del_secy, &ctx)
+					    : macsec_offload(ops->mdo_add_secy, &ctx);
+
 	if (ret)
-		goto rollback;
+		macsec->offload = prev_offload;
 
-	rtnl_unlock();
-	return 0;
+	return ret;
+}
+
+static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
+	struct nlattr **attrs = info->attrs;
+	enum macsec_offload offload;
+	struct net_device *dev;
+	int ret;
+
+	if (!attrs[MACSEC_ATTR_IFINDEX])
+		return -EINVAL;
+
+	if (!attrs[MACSEC_ATTR_OFFLOAD])
+		return -EINVAL;
+
+	if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX,
+					attrs[MACSEC_ATTR_OFFLOAD],
+					macsec_genl_offload_policy, NULL))
+		return -EINVAL;
+
+	dev = get_dev_from_nl(genl_info_net(info), attrs);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
 
-rollback:
-	macsec->offload = prev_offload;
+	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
+		return -EINVAL;
+
+	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
+
+	rtnl_lock();
+
+	ret = macsec_update_offload(dev, offload);
 
 	rtnl_unlock();
+
 	return ret;
 }
 
@@ -3831,6 +3831,12 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
 	if (ret)
 		goto cleanup;
 
+	if (data[IFLA_MACSEC_OFFLOAD]) {
+		ret = macsec_update_offload(dev, nla_get_u8(data[IFLA_MACSEC_OFFLOAD]));
+		if (ret)
+			goto cleanup;
+	}
+
 	/* If h/w offloading is available, propagate to the device */
 	if (macsec_is_offloaded(macsec)) {
 		const struct macsec_ops *ops;
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump
  2022-12-08 11:55 [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink ehakim
@ 2022-12-08 11:55 ` ehakim
  2022-12-08 12:32   ` Jiri Pirko
  2022-12-08 12:32 ` [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink Jiri Pirko
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: ehakim @ 2022-12-08 11:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: raeds, davem, edumazet, kuba, pabeni, netdev, sd, atenart, jiri,
	Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

Support dumping offload netlink attribute in macsec's device
attributes dump.
Change macsec_get_size to consider the offload attribute in
the calculations of the required room for dumping the device
netlink attributes.

Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
V1 -> V2: Update commit message

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

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index abfe4a612a2d..85208a64f259 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -4237,16 +4237,22 @@ static size_t macsec_get_size(const struct net_device *dev)
 		nla_total_size(1) + /* IFLA_MACSEC_SCB */
 		nla_total_size(1) + /* IFLA_MACSEC_REPLAY_PROTECT */
 		nla_total_size(1) + /* IFLA_MACSEC_VALIDATION */
+		nla_total_size(1) + /* IFLA_MACSEC_OFFLOAD */
 		0;
 }
 
 static int macsec_fill_info(struct sk_buff *skb,
 			    const struct net_device *dev)
 {
-	struct macsec_secy *secy = &macsec_priv(dev)->secy;
-	struct macsec_tx_sc *tx_sc = &secy->tx_sc;
+	struct macsec_tx_sc *tx_sc;
+	struct macsec_dev *macsec;
+	struct macsec_secy *secy;
 	u64 csid;
 
+	macsec = macsec_priv(dev);
+	secy = &macsec->secy;
+	tx_sc = &secy->tx_sc;
+
 	switch (secy->key_len) {
 	case MACSEC_GCM_AES_128_SAK_LEN:
 		csid = secy->xpn ? MACSEC_CIPHER_ID_GCM_AES_XPN_128 : MACSEC_DEFAULT_CIPHER_ID;
@@ -4271,6 +4277,7 @@ static int macsec_fill_info(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_MACSEC_SCB, tx_sc->scb) ||
 	    nla_put_u8(skb, IFLA_MACSEC_REPLAY_PROTECT, secy->replay_protect) ||
 	    nla_put_u8(skb, IFLA_MACSEC_VALIDATION, secy->validate_frames) ||
+	    nla_put_u8(skb, IFLA_MACSEC_OFFLOAD, macsec->offload) ||
 	    0)
 		goto nla_put_failure;
 
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-08 11:55 [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink ehakim
  2022-12-08 11:55 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
@ 2022-12-08 12:32 ` Jiri Pirko
  2022-12-08 22:44 ` Sabrina Dubroca
  2022-12-09  2:32 ` Jakub Kicinski
  3 siblings, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2022-12-08 12:32 UTC (permalink / raw)
  To: ehakim
  Cc: linux-kernel, raeds, davem, edumazet, kuba, pabeni, netdev, sd,
	atenart

Thu, Dec 08, 2022 at 12:55:16PM CET, ehakim@nvidia.com wrote:
>From: Emeel Hakim <ehakim@nvidia.com>
>
>Add support for changing Macsec offload selection through the
>netlink layer by implementing the relevant changes in
>macsec_changelink.
>
>Since the handling in macsec_changelink is similar to macsec_upd_offload,
>update macsec_upd_offload to use a common helper function to avoid
>duplication.
>
>Example for setting offload for a macsec device:
>    ip link set macsec0 type macsec offload mac
>
>Reviewed-by: Raed Salem <raeds@nvidia.com>
>Signed-off-by: Emeel Hakim <ehakim@nvidia.com>

Looks fine to me.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump
  2022-12-08 11:55 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
@ 2022-12-08 12:32   ` Jiri Pirko
  0 siblings, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2022-12-08 12:32 UTC (permalink / raw)
  To: ehakim
  Cc: linux-kernel, raeds, davem, edumazet, kuba, pabeni, netdev, sd,
	atenart

Thu, Dec 08, 2022 at 12:55:17PM CET, ehakim@nvidia.com wrote:
>From: Emeel Hakim <ehakim@nvidia.com>
>
>Support dumping offload netlink attribute in macsec's device
>attributes dump.
>Change macsec_get_size to consider the offload attribute in
>the calculations of the required room for dumping the device
>netlink attributes.
>
>Reviewed-by: Raed Salem <raeds@nvidia.com>
>Signed-off-by: Emeel Hakim <ehakim@nvidia.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-08 11:55 [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink ehakim
  2022-12-08 11:55 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
  2022-12-08 12:32 ` [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink Jiri Pirko
@ 2022-12-08 22:44 ` Sabrina Dubroca
  2022-12-09  2:32 ` Jakub Kicinski
  3 siblings, 0 replies; 11+ messages in thread
From: Sabrina Dubroca @ 2022-12-08 22:44 UTC (permalink / raw)
  To: ehakim
  Cc: linux-kernel, raeds, davem, edumazet, kuba, pabeni, netdev,
	atenart, jiri

2022-12-08, 13:55:16 +0200, ehakim@nvidia.com wrote:
> From: Emeel Hakim <ehakim@nvidia.com>
> 
> Add support for changing Macsec offload selection through the
> netlink layer by implementing the relevant changes in
> macsec_changelink.
> 
> Since the handling in macsec_changelink is similar to macsec_upd_offload,
> update macsec_upd_offload to use a common helper function to avoid
> duplication.
> 
> Example for setting offload for a macsec device:
>     ip link set macsec0 type macsec offload mac
> 
> Reviewed-by: Raed Salem <raeds@nvidia.com>
> Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
> ---
> V3 -> V4: - Dont pass whole attributes data to macsec_update_offload, just pass relevant attribute.
> 		  - Fix code style.
> 		  - Remove macsec_changelink_upd_offload
> V2 -> V3: - Split the original patch into 3 patches, the macsec_rtnl_policy related change (separate patch)
>                        to be sent to "net" branch as a fix.
>                  - Change the original patch title to make it clear that it's only adding IFLA_MACSEC_OFFLOAD
>                    to changelink
> V1 -> V2: Add common helper to avoid duplicating code
> 
>  drivers/net/macsec.c | 100 +++++++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index d73b9d535b7a..abfe4a612a2d 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -2583,38 +2583,16 @@ static bool macsec_is_configured(struct macsec_dev *macsec)
>  	return false;
>  }
>  
> -static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
> +static int macsec_update_offload(struct net_device *dev, enum macsec_offload offload)
>  {
> -	struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
> -	enum macsec_offload offload, prev_offload;
> -	int (*func)(struct macsec_context *ctx);
> -	struct nlattr **attrs = info->attrs;
> -	struct net_device *dev;
> +	enum macsec_offload prev_offload;
>  	const struct macsec_ops *ops;
>  	struct macsec_context ctx;
>  	struct macsec_dev *macsec;
> -	int ret;
> -
> -	if (!attrs[MACSEC_ATTR_IFINDEX])
> -		return -EINVAL;
> -
> -	if (!attrs[MACSEC_ATTR_OFFLOAD])
> -		return -EINVAL;
> -
> -	if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX,
> -					attrs[MACSEC_ATTR_OFFLOAD],
> -					macsec_genl_offload_policy, NULL))
> -		return -EINVAL;
> +	int ret = 0;
>  
> -	dev = get_dev_from_nl(genl_info_net(info), attrs);
> -	if (IS_ERR(dev))
> -		return PTR_ERR(dev);
>  	macsec = macsec_priv(dev);
>  
> -	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
> -		return -EINVAL;
> -
> -	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
>  	if (macsec->offload == offload)
>  		return 0;
>  
> @@ -2627,43 +2605,65 @@ static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
>  	if (netif_running(dev))
>  		return -EBUSY;
>  
> -	rtnl_lock();
> -
> -	prev_offload = macsec->offload;
> -	macsec->offload = offload;
> -
>  	/* Check if the device already has rules configured: we do not support
>  	 * rules migration.
>  	 */
> -	if (macsec_is_configured(macsec)) {
> -		ret = -EBUSY;
> -		goto rollback;
> -	}
> +	if (macsec_is_configured(macsec))
> +		return -EBUSY;
> +
> +	prev_offload = macsec->offload;
>  
>  	ops = __macsec_get_ops(offload == MACSEC_OFFLOAD_OFF ? prev_offload : offload,
>  			       macsec, &ctx);
> -	if (!ops) {
> +	if (!ops)
>  		ret = -EOPNOTSUPP;

		return -EOPNOTSUPP;

It's probably not a problem because macsec_check_offload should catch
that, but it looks wrong.

> -		goto rollback;
> -	}
>  
> -	if (prev_offload == MACSEC_OFFLOAD_OFF)
> -		func = ops->mdo_add_secy;
> -	else
> -		func = ops->mdo_del_secy;
> +	macsec->offload = offload;
>  
>  	ctx.secy = &macsec->secy;
> -	ret = macsec_offload(func, &ctx);
> +	ret = offload == MACSEC_OFFLOAD_OFF ? macsec_offload(ops->mdo_del_secy, &ctx)
> +					    : macsec_offload(ops->mdo_add_secy, &ctx);
> +

nit: unnecessary blank line (sorry, I should have spotted that earlier)

>  	if (ret)
> -		goto rollback;
> +		macsec->offload = prev_offload;
>  
> -	rtnl_unlock();
> -	return 0;
> +	return ret;
> +}
> +
> +static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1];
> +	struct nlattr **attrs = info->attrs;
> +	enum macsec_offload offload;
> +	struct net_device *dev;
> +	int ret;
> +
> +	if (!attrs[MACSEC_ATTR_IFINDEX])
> +		return -EINVAL;
> +
> +	if (!attrs[MACSEC_ATTR_OFFLOAD])
> +		return -EINVAL;
> +
> +	if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX,
> +					attrs[MACSEC_ATTR_OFFLOAD],
> +					macsec_genl_offload_policy, NULL))
> +		return -EINVAL;
> +
> +	dev = get_dev_from_nl(genl_info_net(info), attrs);
> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
>  
> -rollback:
> -	macsec->offload = prev_offload;
> +	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
> +		return -EINVAL;
> +
> +	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> +
> +	rtnl_lock();
> +
> +	ret = macsec_update_offload(dev, offload);
>  
>  	rtnl_unlock();
> +
>  	return ret;
>  }
>  
> @@ -3831,6 +3831,12 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (ret)
>  		goto cleanup;
>  
> +	if (data[IFLA_MACSEC_OFFLOAD]) {
> +		ret = macsec_update_offload(dev, nla_get_u8(data[IFLA_MACSEC_OFFLOAD]));
> +		if (ret)
> +			goto cleanup;
> +	}
> +
>  	/* If h/w offloading is available, propagate to the device */
>  	if (macsec_is_offloaded(macsec)) {
>  		const struct macsec_ops *ops;

-- 
Sabrina


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-08 11:55 [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink ehakim
                   ` (2 preceding siblings ...)
  2022-12-08 22:44 ` Sabrina Dubroca
@ 2022-12-09  2:32 ` Jakub Kicinski
  2022-12-09  2:33   ` Jakub Kicinski
  3 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2022-12-09  2:32 UTC (permalink / raw)
  To: ehakim
  Cc: linux-kernel, raeds, davem, edumazet, pabeni, netdev, sd, atenart,
	jiri

On Thu, 8 Dec 2022 13:55:16 +0200 ehakim@nvidia.com wrote:
> +	dev = get_dev_from_nl(genl_info_net(info), attrs);

What prevents this dev from disappearing before...

> +	if (IS_ERR(dev))
> +		return PTR_ERR(dev);
> +	if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE])
> +		return -EINVAL;
> +
> +	offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> +
> +	rtnl_lock();

... we finally take the lock?

I think you're just moving this code, but still.

> +	ret = macsec_update_offload(dev, offload);
>  
>  	rtnl_unlock();

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-09  2:32 ` Jakub Kicinski
@ 2022-12-09  2:33   ` Jakub Kicinski
  2022-12-09 10:18     ` Emeel Hakim
  0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2022-12-09  2:33 UTC (permalink / raw)
  To: ehakim
  Cc: linux-kernel, raeds, davem, edumazet, pabeni, netdev, sd, atenart,
	jiri

On Thu, 8 Dec 2022 18:32:44 -0800 Jakub Kicinski wrote:
> I think you're just moving this code, but still.

And by "by still" I mean - it's still a bug, so it needs to be fixed
first.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-09  2:33   ` Jakub Kicinski
@ 2022-12-09 10:18     ` Emeel Hakim
  2022-12-09 11:18       ` Emeel Hakim
  0 siblings, 1 reply; 11+ messages in thread
From: Emeel Hakim @ 2022-12-09 10:18 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: linux-kernel@vger.kernel.org, Raed Salem, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, netdev@vger.kernel.org,
	sd@queasysnail.net, atenart@kernel.org, jiri@resnulli.us



> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Friday, 9 December 2022 4:33
> To: Emeel Hakim <ehakim@nvidia.com>
> Cc: linux-kernel@vger.kernel.org; Raed Salem <raeds@nvidia.com>;
> davem@davemloft.net; edumazet@google.com; pabeni@redhat.com;
> netdev@vger.kernel.org; sd@queasysnail.net; atenart@kernel.org; jiri@resnulli.us
> Subject: Re: [PATCH net-next v4 1/2] macsec: add support for
> IFLA_MACSEC_OFFLOAD in macsec_changelink
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 8 Dec 2022 18:32:44 -0800 Jakub Kicinski wrote:
> > I think you're just moving this code, but still.
> 
> And by "by still" I mean - it's still a bug, so it needs to be fixed first.

The code added by those patches does not use the rtnl_lock, the lock is just getting moved as part of sharing similar code,
but the new code is still not using it, I don’t think those  patches need to wait until a fix of an existing locking issue as long
as the new code is not inserting any bugs.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink
  2022-12-09 10:18     ` Emeel Hakim
@ 2022-12-09 11:18       ` Emeel Hakim
  0 siblings, 0 replies; 11+ messages in thread
From: Emeel Hakim @ 2022-12-09 11:18 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: linux-kernel@vger.kernel.org, Raed Salem, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, netdev@vger.kernel.org,
	sd@queasysnail.net, atenart@kernel.org, jiri@resnulli.us



> -----Original Message-----
> From: Emeel Hakim
> Sent: Friday, 9 December 2022 12:19
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: linux-kernel@vger.kernel.org; Raed Salem <raeds@nvidia.com>;
> davem@davemloft.net; edumazet@google.com; pabeni@redhat.com;
> netdev@vger.kernel.org; sd@queasysnail.net; atenart@kernel.org; jiri@resnulli.us
> Subject: RE: [PATCH net-next v4 1/2] macsec: add support for
> IFLA_MACSEC_OFFLOAD in macsec_changelink
> 
> 
> 
> > -----Original Message-----
> > From: Jakub Kicinski <kuba@kernel.org>
> > Sent: Friday, 9 December 2022 4:33
> > To: Emeel Hakim <ehakim@nvidia.com>
> > Cc: linux-kernel@vger.kernel.org; Raed Salem <raeds@nvidia.com>;
> > davem@davemloft.net; edumazet@google.com; pabeni@redhat.com;
> > netdev@vger.kernel.org; sd@queasysnail.net; atenart@kernel.org;
> > jiri@resnulli.us
> > Subject: Re: [PATCH net-next v4 1/2] macsec: add support for
> > IFLA_MACSEC_OFFLOAD in macsec_changelink
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Thu, 8 Dec 2022 18:32:44 -0800 Jakub Kicinski wrote:
> > > I think you're just moving this code, but still.
> >
> > And by "by still" I mean - it's still a bug, so it needs to be fixed first.
> 
> The code added by those patches does not use the rtnl_lock, the lock is just getting
> moved as part of sharing similar code, but the new code is still not using it, I don’t
> think those  patches need to wait until a fix of an existing locking issue as long as
> the new code is not inserting any bugs.

I will send a fix patch shortly as requested.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump
  2023-01-05  7:57 [PATCH net-next v6 0/2] Add support to offload macsec using netlink update ehakim
@ 2023-01-05  7:57 ` ehakim
  0 siblings, 0 replies; 11+ messages in thread
From: ehakim @ 2023-01-05  7:57 UTC (permalink / raw)
  To: netdev; +Cc: raeds, davem, edumazet, kuba, pabeni, sd, atenart, Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

Support dumping offload netlink attribute in macsec's device
attributes dump.
Change macsec_get_size to consider the offload attribute in
the calculations of the required room for dumping the device
netlink attributes.

Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
V1 -> V2: Update commit message
 drivers/net/macsec.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 1974c59977aa..0cff5083e661 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -4238,16 +4238,22 @@ static size_t macsec_get_size(const struct net_device *dev)
 		nla_total_size(1) + /* IFLA_MACSEC_SCB */
 		nla_total_size(1) + /* IFLA_MACSEC_REPLAY_PROTECT */
 		nla_total_size(1) + /* IFLA_MACSEC_VALIDATION */
+		nla_total_size(1) + /* IFLA_MACSEC_OFFLOAD */
 		0;
 }
 
 static int macsec_fill_info(struct sk_buff *skb,
 			    const struct net_device *dev)
 {
-	struct macsec_secy *secy = &macsec_priv(dev)->secy;
-	struct macsec_tx_sc *tx_sc = &secy->tx_sc;
+	struct macsec_tx_sc *tx_sc;
+	struct macsec_dev *macsec;
+	struct macsec_secy *secy;
 	u64 csid;
 
+	macsec = macsec_priv(dev);
+	secy = &macsec->secy;
+	tx_sc = &secy->tx_sc;
+
 	switch (secy->key_len) {
 	case MACSEC_GCM_AES_128_SAK_LEN:
 		csid = secy->xpn ? MACSEC_CIPHER_ID_GCM_AES_XPN_128 : MACSEC_DEFAULT_CIPHER_ID;
@@ -4272,6 +4278,7 @@ static int macsec_fill_info(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_MACSEC_SCB, tx_sc->scb) ||
 	    nla_put_u8(skb, IFLA_MACSEC_REPLAY_PROTECT, secy->replay_protect) ||
 	    nla_put_u8(skb, IFLA_MACSEC_VALIDATION, secy->validate_frames) ||
+	    nla_put_u8(skb, IFLA_MACSEC_OFFLOAD, macsec->offload) ||
 	    0)
 		goto nla_put_failure;
 
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump
  2023-01-05  8:04 [PATCH net-next 0/2] Add support to offload macsec using netlink update ehakim
@ 2023-01-05  8:04 ` ehakim
  0 siblings, 0 replies; 11+ messages in thread
From: ehakim @ 2023-01-05  8:04 UTC (permalink / raw)
  To: netdev; +Cc: raeds, davem, edumazet, kuba, pabeni, sd, atenart, Emeel Hakim

From: Emeel Hakim <ehakim@nvidia.com>

Support dumping offload netlink attribute in macsec's device
attributes dump.
Change macsec_get_size to consider the offload attribute in
the calculations of the required room for dumping the device
netlink attributes.

Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Emeel Hakim <ehakim@nvidia.com>
---
V1 -> V2: Update commit message
 drivers/net/macsec.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 1974c59977aa..0cff5083e661 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -4238,16 +4238,22 @@ static size_t macsec_get_size(const struct net_device *dev)
 		nla_total_size(1) + /* IFLA_MACSEC_SCB */
 		nla_total_size(1) + /* IFLA_MACSEC_REPLAY_PROTECT */
 		nla_total_size(1) + /* IFLA_MACSEC_VALIDATION */
+		nla_total_size(1) + /* IFLA_MACSEC_OFFLOAD */
 		0;
 }
 
 static int macsec_fill_info(struct sk_buff *skb,
 			    const struct net_device *dev)
 {
-	struct macsec_secy *secy = &macsec_priv(dev)->secy;
-	struct macsec_tx_sc *tx_sc = &secy->tx_sc;
+	struct macsec_tx_sc *tx_sc;
+	struct macsec_dev *macsec;
+	struct macsec_secy *secy;
 	u64 csid;
 
+	macsec = macsec_priv(dev);
+	secy = &macsec->secy;
+	tx_sc = &secy->tx_sc;
+
 	switch (secy->key_len) {
 	case MACSEC_GCM_AES_128_SAK_LEN:
 		csid = secy->xpn ? MACSEC_CIPHER_ID_GCM_AES_XPN_128 : MACSEC_DEFAULT_CIPHER_ID;
@@ -4272,6 +4278,7 @@ static int macsec_fill_info(struct sk_buff *skb,
 	    nla_put_u8(skb, IFLA_MACSEC_SCB, tx_sc->scb) ||
 	    nla_put_u8(skb, IFLA_MACSEC_REPLAY_PROTECT, secy->replay_protect) ||
 	    nla_put_u8(skb, IFLA_MACSEC_VALIDATION, secy->validate_frames) ||
+	    nla_put_u8(skb, IFLA_MACSEC_OFFLOAD, macsec->offload) ||
 	    0)
 		goto nla_put_failure;
 
-- 
2.21.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-01-05  8:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-08 11:55 [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink ehakim
2022-12-08 11:55 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
2022-12-08 12:32   ` Jiri Pirko
2022-12-08 12:32 ` [PATCH net-next v4 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink Jiri Pirko
2022-12-08 22:44 ` Sabrina Dubroca
2022-12-09  2:32 ` Jakub Kicinski
2022-12-09  2:33   ` Jakub Kicinski
2022-12-09 10:18     ` Emeel Hakim
2022-12-09 11:18       ` Emeel Hakim
  -- strict thread matches above, loose matches on Subject: below --
2023-01-05  7:57 [PATCH net-next v6 0/2] Add support to offload macsec using netlink update ehakim
2023-01-05  7:57 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim
2023-01-05  8:04 [PATCH net-next 0/2] Add support to offload macsec using netlink update ehakim
2023-01-05  8:04 ` [PATCH net-next v2 2/2] macsec: dump IFLA_MACSEC_OFFLOAD attribute as part of macsec dump ehakim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.