On 2/13/25 21:39, Tantilov, Emil S wrote:
On 2/12/2025 10:21 AM, Simon Horman wrote:
On Mon, Feb 10, 2025 at 06:38:51PM -0800, Emil Tantilov wrote:
Current init logic ignores the error code from register_netdev(),
which will cause WARN_ON() on attempt to unregister it, if there was
one,
and there is no info for the user that the creation of the netdev
failed.
Hi Emil,
I'm wondering if we could reduce indentation and lines longer
than 80 characters in the above like this (completely untested!):
I was mostly trying to focus on the fix itself, since this patch is -net
bound. The >80 line came about from the introduction of the local netdev
and it seemed cleaner to keep it in one line. I can just split the check
as in the original code.
for (index = 0; index < adapter->max_vports; index++) {
struct idpf_vport_config *vport_config = adapter-
>vport_config[index];
struct net_device *netdev = adapter->netdevs[index];
if (!netdev ||
test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags))
continue;
Again, because its mainly to add the error checking I am not sure if its
OK to re-shuffle the logic.
err = register_netdev(netdev);
if (err) {
dev_err(&pdev->dev, "failed to register netdev for vport
%d: %pe\n",
index, ERR_PTR(err));
continue;
}
set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
}
Don't mind re-spinning (and testing) v2 with the proposed change, if
it's not infringing on the guidelines for submission to -net.
Thanks,
Emil
Emil, you are right that we generally don't want to do refactors on -net
submissions. In this particular case your code was just replacing the
body of the loop. Simon's snippet did the same, just with less
indentation.
The "early return" (or "continue") style is better, also in this case.
