Re: [PATCH net 01/10] iavf: return EBUSY if reset in progress or not ready during MAC change

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
iavf: return EBUSY if reset in progress or not ready during MAC change

This adds an early bail-out in iavf_set_mac() so that a MAC address change
requested while the VF is resetting, or before the adapter reaches
__IAVF_DOWN, returns -EBUSY instead of waiting with the netdev instance
lock held.

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 29b8403a066bc..630388e9d28cb 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -1042,6 +1042,9 @@ static int iavf_set_mac(struct net_device *netdev, void *p)
>  	struct sockaddr *addr = p;
>  	int ret;
>  
> +	if (iavf_is_reset_in_progress(adapter) || adapter->state < __IAVF_DOWN)
> +		return -EBUSY;
> +
>  	if (!is_valid_ether_addr(addr->sa_data))
>  		return -EADDRNOTAVAIL;

[Severity: Low]
Is the placement of the new gate ahead of is_valid_ether_addr()
intentional?

With this ordering, a request carrying an address that can never be
accepted (all-zero, multicast, other non-unicast MAC) now reports -EBUSY
rather than -EADDRNOTAVAIL whenever iavf_is_reset_in_progress(adapter) is
true or adapter->state < __IAVF_DOWN.

The core does not validate the address itself, so the driver check is the
only one on the rtnetlink/ioctl/sysfs paths:

net/core/dev.c:netif_set_mac_address() {
	if (!ops->ndo_set_mac_address)
		return -EOPNOTSUPP;
	if (ss->ss_family != dev->type)
		return -EINVAL;
	if (!netif_device_present(dev))
		return -ENODEV;
	...
		err = ops->ndo_set_mac_address(dev, ss);
}

Since -EBUSY advertises a transient condition, callers that retry on EBUSY
would loop on input that can never succeed. Would moving the state gate
below the is_valid_ether_addr() check work equally well for the stated
goal?

The ordering is still present at the end of the series, and the commit
message does not mention the change in returned errno for invalid
addresses. Could that be noted, or the checks reordered?