[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA1PR12MB6353281A503DF0337FE98655ABF49@IA1PR12MB6353.namprd12.prod.outlook.com>
Date: Tue, 3 Jan 2023 08:55:56 +0000
From: Emeel Hakim <ehakim@...dia.com>
To: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Raed Salem <raeds@...dia.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"sd@...asysnail.net" <sd@...asysnail.net>,
"atenart@...nel.org" <atenart@...nel.org>
Subject: RE: [PATCH net-next 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD
in macsec_changelink
I see in Patchwork that this patch status is "Deferred" , I was asked to send
a fix for accessing a net device prior to holding a lock and that is done (fix is accepted).
Is there a reason of the deferred status , is it pending anything from my side?
> -----Original Message-----
> From: Emeel Hakim <ehakim@...dia.com>
> Sent: Tuesday, 27 December 2022 10:25
> To: netdev@...r.kernel.org
> Cc: Raed Salem <raeds@...dia.com>; davem@...emloft.net;
> edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com;
> sd@...asysnail.net; atenart@...nel.org; Emeel Hakim <ehakim@...dia.com>
> Subject: [PATCH net-next 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in
> macsec_changelink
>
> From: Emeel Hakim <ehakim@...dia.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@...dia.com>
> Signed-off-by: Emeel Hakim <ehakim@...dia.com>
> ---
> drivers/net/macsec.c | 116 +++++++++++++++++++++----------------------
> 1 file changed, 57 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index
> bf8ac7a3ded7..1974c59977aa 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -2583,95 +2583,87 @@ 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 = 0;
>
> - 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;
> -
> - rtnl_lock();
> -
> - dev = get_dev_from_nl(genl_info_net(info), attrs);
> - if (IS_ERR(dev)) {
> - ret = PTR_ERR(dev);
> - goto out;
> - }
> macsec = macsec_priv(dev);
>
> - if (!tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]) {
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]);
> if (macsec->offload == offload)
> - goto out;
> + return 0;
>
> /* Check if the offloading mode is supported by the underlying layers */
> if (offload != MACSEC_OFFLOAD_OFF &&
> !macsec_check_offload(offload, macsec)) {
> - ret = -EOPNOTSUPP;
> - goto out;
> + return -EOPNOTSUPP;
> }
>
> /* Check if the net device is busy. */
> - if (netif_running(dev)) {
> - ret = -EBUSY;
> - goto out;
> - }
> -
> - prev_offload = macsec->offload;
> - macsec->offload = offload;
> + if (netif_running(dev))
> + return -EBUSY;
>
> /* 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) {
> - ret = -EOPNOTSUPP;
> - goto rollback;
> - }
> + if (!ops)
> + return -EOPNOTSUPP;
>
> - 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);
> +
> + 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);
>
> -rollback:
> - macsec->offload = prev_offload;
> -out:
> rtnl_unlock();
> +
> return ret;
> }
>
> @@ -3840,6 +3832,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
Powered by blists - more mailing lists