Re: [PATCH net-next v6 1/2] macsec: add support for IFLA_MACSEC_OFFLOAD in macsec_changelink

[Sabrina Dubroca <sd@queasysnail.net>]

2023-01-06, 15:35:50 +0200, ehakim@nvidia.com wrote:
[...]
> +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();

Why are you putting rtnl_lock() back down here? You just moved it
above get_dev_from_nl with commit f3b4a00f0f62 ("net: macsec: fix net
device access prior to holding a lock"), now you're pretty much
reverting that fix.

> +
> +	ret = macsec_update_offload(dev, offload);
>  
> -rollback:
> -	macsec->offload = prev_offload;
> -out:
>  	rtnl_unlock();
> +
>  	return ret;
>  }
>  

-- 
Sabrina