Message ID | 20221227082517.8675-1-ehakim@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next,1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink | expand |
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@nvidia.com> > Sent: Tuesday, 27 December 2022 10:25 > To: netdev@vger.kernel.org > Cc: Raed Salem <raeds@nvidia.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; > sd@queasysnail.net; atenart@kernel.org; Emeel Hakim <ehakim@nvidia.com> > Subject: [PATCH net-next 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in > macsec_changelink > > 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> > --- > 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
On Tue, 3 Jan 2023 08:55:56 +0000 Emeel Hakim wrote: > 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? net-next was closed, please resend now.
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;