[PATCH v2] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[PATCH v2] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Ian Wienand]

As noted in the original commit 685343fc3ba6 ("net: add
name_assign_type netdev attribute")

  ... when the kernel has given the interface a name using global
  device enumeration based on order of discovery (ethX, wlanY, etc)
  ... are labelled NET_NAME_ENUM.

That describes this case, so set the default for the devices here to
NET_NAME_ENUM to better help userspace tools to know if they might
like to rename them.

This is inspired by inconsistent interface renaming between both
distributions and within different releases of distributions;
particularly with Xen's xen-netfront driver which gets it's device
from here and is not renamed under CentOS 8, but is under CentOS 9.
Of course it is ultimately up to userspace (systemd/udev) what happens
to interfaces marked with with this flag, but providing the naming
info brings it inline with other common interfaces such as virtio, and
should ensure better general consistency of renaming behaviour into
the future.

Signed-off-by: Ian Wienand <iwienand@redhat.com>
---
 net/ethernet/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index ebcc812735a4..62b89d6f54fd 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL(ether_setup);
 struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
 				      unsigned int rxqs)
 {
-	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
+	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_ENUM,
 				ether_setup, txqs, rxqs);
 }
 EXPORT_SYMBOL(alloc_etherdev_mqs);
-- 
2.35.1

Re: [PATCH v2] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Jakub Kicinski]

On Tue,  5 Apr 2022 10:15:00 +1000 Ian Wienand wrote:
> As noted in the original commit 685343fc3ba6 ("net: add
> name_assign_type netdev attribute")
> 
>   ... when the kernel has given the interface a name using global
>   device enumeration based on order of discovery (ethX, wlanY, etc)
>   ... are labelled NET_NAME_ENUM.
> 
> That describes this case, so set the default for the devices here to
> NET_NAME_ENUM to better help userspace tools to know if they might
> like to rename them.

Adding Tom and David to CC, please make sure you CC people whose
commits you're mentioning. They may know something.

> This is inspired by inconsistent interface renaming between both
> distributions and within different releases of distributions;

This worries me. Why is UNKNOWN and ENUM treated differently?
Can you point me to the code which pays attention to the assign type?

> particularly with Xen's xen-netfront driver which gets it's device
> from here and is not renamed under CentOS 8, but is under CentOS 9.
> Of course it is ultimately up to userspace (systemd/udev) what happens
> to interfaces marked with with this flag, but providing the naming
> info brings it inline with other common interfaces such as virtio, and
> should ensure better general consistency of renaming behaviour into
> the future.

Can you spell out how netfront gets a different type to virtio?
I see alloc_etherdev_mq() in both cases.

> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index ebcc812735a4..62b89d6f54fd 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -391,7 +391,7 @@ EXPORT_SYMBOL(ether_setup);
>  struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
>  				      unsigned int rxqs)
>  {
> -	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
> +	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_ENUM,
>  				ether_setup, txqs, rxqs);
>  }
>  EXPORT_SYMBOL(alloc_etherdev_mqs);

Re: [PATCH v2] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Ian Wienand]

Thanks for review

On Tue, Apr 05, 2022 at 12:41:03PM -0700, Jakub Kicinski wrote:
> Can you spell out how netfront gets a different type to virtio?
> I see alloc_etherdev_mq() in both cases.

Yeah I've been doing further testing to narrow this down, and I think
I've been confused by the renaming happening during the initrd steps.

It seems that renamed devices (no matter what the driver) will have
their name_assign_type set to NET_NAME_USER; which [1] gives away as
coming from the rtnl_newlink path.  virtio devices were renamed in
init phase in my testing environment, which is why
/sys/class/net/<iface>/net_name_type works for them by the time
interactive login starts -- not because they explicitly flag
themselves.  Sorry for not recognising that earlier.

> This worries me. Why is UNKNOWN and ENUM treated differently?
> Can you point me to the code which pays attention to the assign type?

Yeah, I'll have to retract that claim; it remains unclear to me why
CentOS 8-stream does not rename netfront devices (systemd 239) and
CentOS 9-stream does (systemd 249).

systemd only seems to use NET_NAME_ENUM in an informational way to
print a warning when you're using a .link file to set network info for
a device that might change names [2].

Perhaps this still has some utility in making that warning more
useful?

[1] https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/iplink.c#n65
[2] https://github.com/systemd/systemd/blob/main/src/udev/net/link-config.c#L446

Re: [PATCH v2] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Jakub Kicinski]

On Wed, 6 Apr 2022 11:56:51 +1000 Ian Wienand wrote:
> Thanks for review
> 
> On Tue, Apr 05, 2022 at 12:41:03PM -0700, Jakub Kicinski wrote:
> > Can you spell out how netfront gets a different type to virtio?
> > I see alloc_etherdev_mq() in both cases.  
> 
> Yeah I've been doing further testing to narrow this down, and I think
> I've been confused by the renaming happening during the initrd steps.
> 
> It seems that renamed devices (no matter what the driver) will have
> their name_assign_type set to NET_NAME_USER; which [1] gives away as
> coming from the rtnl_newlink path.  virtio devices were renamed in
> init phase in my testing environment, which is why
> /sys/class/net/<iface>/net_name_type works for them by the time
> interactive login starts -- not because they explicitly flag
> themselves.  Sorry for not recognising that earlier.
> 
> > This worries me. Why is UNKNOWN and ENUM treated differently?
> > Can you point me to the code which pays attention to the assign type?  
> 
> Yeah, I'll have to retract that claim; it remains unclear to me why
> CentOS 8-stream does not rename netfront devices (systemd 239) and
> CentOS 9-stream does (systemd 249).
> 
> systemd only seems to use NET_NAME_ENUM in an informational way to
> print a warning when you're using a .link file to set network info for
> a device that might change names [2].
> 
> Perhaps this still has some utility in making that warning more
> useful?

Okay, all good then. I was worried some user space is refusing to
rename UNKNOWN. I have no objection to changing UNKNOWN -> ENUM.
We just need the commit message to be updated.

[PATCH v3] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Ian Wienand]

As noted in the original commit 685343fc3ba6 ("net: add
name_assign_type netdev attribute")

  ... when the kernel has given the interface a name using global
  device enumeration based on order of discovery (ethX, wlanY, etc)
  ... are labelled NET_NAME_ENUM.

That describes this case, so set the default for the devices here to
NET_NAME_ENUM.  Current popular network setup tools like systemd use
this only to warn if you're setting static settings on interfaces that
might change, so it is expected this only leads to better user
information, but not changing of interfaces, etc.

Signed-off-by: Ian Wienand <iwienand@redhat.com>
---
 net/ethernet/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index ebcc812735a4..62b89d6f54fd 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -391,7 +391,7 @@ EXPORT_SYMBOL(ether_setup);
 struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
 				      unsigned int rxqs)
 {
-	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
+	return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_ENUM,
 				ether_setup, txqs, rxqs);
 }
 EXPORT_SYMBOL(alloc_etherdev_mqs);
-- 
2.35.1

Re: [PATCH v3] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Jakub Kicinski]

On Wed,  6 Apr 2022 19:36:36 +1000 Ian Wienand wrote:
> As noted in the original commit 685343fc3ba6 ("net: add
> name_assign_type netdev attribute")
> 
>   ... when the kernel has given the interface a name using global
>   device enumeration based on order of discovery (ethX, wlanY, etc)
>   ... are labelled NET_NAME_ENUM.
> 
> That describes this case, so set the default for the devices here to
> NET_NAME_ENUM.  Current popular network setup tools like systemd use
> this only to warn if you're setting static settings on interfaces that
> might change, so it is expected this only leads to better user
> information, but not changing of interfaces, etc.
> 
> Signed-off-by: Ian Wienand <iwienand@redhat.com>

Acked-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH v3] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  6 Apr 2022 19:36:36 +1000 you wrote:
> As noted in the original commit 685343fc3ba6 ("net: add
> name_assign_type netdev attribute")
> 
>   ... when the kernel has given the interface a name using global
>   device enumeration based on order of discovery (ethX, wlanY, etc)
>   ... are labelled NET_NAME_ENUM.
> 
> [...]

Here is the summary with links:
  - [v3] net/ethernet : set default assignment identifier to NET_NAME_ENUM
    https://git.kernel.org/netdev/net-next/c/e9f656b7a214

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html