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

[Sabrina Dubroca <sd@queasysnail.net>]

Sorry to delay this again, there's something to fix in this version.
I hope you're not too frustrated by the review process.

2023-01-11, 10:11:11 +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 macsec_dev *macsec;
> +	struct net_device *dev;
> +	int ret;

        int ret = 0;

Otherwise we can return with ret uninitialized when macsec->offload ==
offload.

(unfortunately the compiler warning is disabled in the kernel Makefile)

[...]
> @@ -3840,8 +3837,17 @@ static int macsec_changelink(struct net_device *dev, struct nlattr *tb[],
>  	if (ret)
>  		goto cleanup;
>  
> +	if (data[IFLA_MACSEC_OFFLOAD]) {
> +		offload = nla_get_u8(data[IFLA_MACSEC_OFFLOAD]);
> +		if (macsec->offload != offload) {
> +			macsec_offload_state_change = true;
> +			ret = macsec_update_offload(dev, offload);
> +			if (ret)
> +				goto cleanup;
> +		}
> +	}

nit: there was a blank line here in the previous version, please bring it back.

>  	/* If h/w offloading is available, propagate to the device */
> -	if (macsec_is_offloaded(macsec)) {
> +	if (!macsec_offload_state_change && macsec_is_offloaded(macsec)) {
>  		const struct macsec_ops *ops;
>  		struct macsec_context ctx;

Thanks.

-- 
Sabrina