Re: [PATCH 5/7] netdev: disable power save if required

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 6/15/23 14:24, James Prestwood wrote:
> Disable power save if the wiphy indicates its needed. Do this
> before issuing GET_LINK so the netdev doesn't signal its up until
> power save is disabled.
> ---
>   src/netdev.c | 80 +++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 66 insertions(+), 14 deletions(-)
> 

<snip>

> +static void netdev_disable_ps_cb(struct l_genl_msg *msg, void *user_data)
> +{
> +	int err = l_genl_msg_get_error(msg);
> +	uint32_t ifindex = L_PTR_TO_UINT(user_data);
> +
> +	/* Can't do anything about it but inform the user */
> +	if (err < 0) {
> +		l_error("Failed to disable power save for ifindex %u (%s: %d)",
> +				ifindex, strerror(-err), err);
> +		return;
> +	}
> +
> +	l_debug("Disabled power save for ifindex %u", ifindex);
> +}
> +
> +static void netdev_disable_ps_destroy(void *user_data)
> +{
> +	uint32_t ifindex = L_PTR_TO_UINT(user_data);
> +
> +	netdev_get_link(ifindex);

So why do we do this in the destroy callback?  What happens if this operation is 
canceled (maybe by hot-unplug?)

> +}
> +

<snip>

>   
> -	l_free(rtmmsg);
> +	if (wiphy_disable_power_save(wiphy)) {
> +		/* Wait to issue GET_LINK until PS is disabled */
> +		if (netdev_disable_power_save(ifindex))

Should we be saving a command id here so we can cancel this operation in case of 
hot-unplug?

Ideally we should switch to using l_genl_family_new per netdev so that all the 
outstanding commands are auto-canceled, but this might require some care.

> +			return netdev;
> +	}
>   
> -	netdev_setup_interface(netdev);
> +	netdev_get_link(ifindex);

We should be saving the command id here too, but as a separate fix.

>   
>   	return netdev;
>   }

Regards,
-Denis