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

Jose Ignacio Tornos Martinez <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.netdev
Message-ID <[email protected]>
All the comments below fall into pre-existing issues, concerns already
addressed in previous versions, out-of-scope items, or extreme edge
cases. No code changes are considered necessary for a new version.
Details for each comment:

> [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?

The scenario requires passing an invalid MAC address (multicast, all-zero)
while the VF is resetting. -EBUSY is still a correct response, the VF
cannot perform any operation during reset regardless of the address.

If a caller retries on -EBUSY, the reset completes quickly (milliseconds
to seconds) and the next attempt returns -EADDRNOTAVAIL as expected.
This is a transient errno difference, not a functional issue.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.