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

[PATCH] 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.

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] net/ethernet : set default assignment identifier to NET_NAME_ENUM

[Andrew Lunn]

On Fri, Apr 01, 2022 at 05:34:30PM +1100, 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.

Hi Ian

Is this potentially an ABI change, and we will get surprises when
interfaces which were not previously renamed now are? It would be nice
to see some justification this is actually safe to do.

   Andrew

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

[Ian Wienand]

Thanks for review

On Fri, Apr 01, 2022 at 03:13:27PM +0200, Andrew Lunn wrote:
> On Fri, Apr 01, 2022 at 05:34:30PM +1100, 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.

> Is this potentially an ABI change

I don't think this counts as an ABI change; it's a fixed-size flag and
designed to be updated with more specific information on a
case-by-case basis?

> and we will get surprises when
> interfaces which were not previously renamed now are? It would be nice
> to see some justification this is actually safe to do.

I came to this through inconsistency of behaviour in a heterogeneous
OpenStack cloud environment.  Most of the clouds are kvm-based and
have NICS using virtio which fills in
/sys/class/net/<interface>/name_assign_type and this bubbles up
through udev/systemd/general magic to get the devices renamed.
However we have one outlier using Xen and the xen-netfront driver,
which I traced to here, where name_assign_type isn't set.  So I think
it's a consistency win to have it reporting itself as NET_NAME_ENUM
here.

Thanks,

-i

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

[Andrew Lunn]

> > and we will get surprises when
> > interfaces which were not previously renamed now are? It would be nice
> > to see some justification this is actually safe to do.
> 
> I came to this through inconsistency of behaviour in a heterogeneous
> OpenStack cloud environment.  Most of the clouds are kvm-based and
> have NICS using virtio which fills in
> /sys/class/net/<interface>/name_assign_type and this bubbles up
> through udev/systemd/general magic to get the devices renamed.
> However we have one outlier using Xen and the xen-netfront driver,
> which I traced to here, where name_assign_type isn't set.

Hi Ian

So is there a risk here that Xen user suddenly find their network
interfaces renamed, where as before they were not, and their
networking thus breaks?

Consistency is good, but it is hard to do in retrospect when there are
deployed systems potentially dependent on the inconsistency.

	 Andrew

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

[Ian Wienand]

On Sat, Apr 02, 2022 at 08:40:08PM +0200, Andrew Lunn wrote:
> So is there a risk here that Xen user suddenly find their network
> interfaces renamed, where as before they were not, and their
> networking thus breaks?

Well, this is actually what "got" us.  The interfaces *are* renamed on
CentOS 9-stream, but not on earlier releases, because systemd makes
different choices [1].  Tomorrow someone might "fix" something in that
systemd/udev chain that flips interfaces without specific use flags
back to not being renamed again.  I'm certain it would vary based on
what distro and release you chose to boot.

> Consistency is good, but it is hard to do in retrospect when there are
> deployed systems potentially dependent on the inconsistency.

As noted, it is already the case that if your names are falling into
this path, they are unstable? (there are many pages for every distro
that go on and on about this, systemd/udev interactions, rules, link
files, and so on; e.g. [2]).

I get what you are saying, that in a fixed virtual environment booting
some relatively fixed distro, perhaps the names are "stable enough"
and nobody has bothered updating this yet, so everyone is probably
happy enough with what they have.

But ultimately it seems like nobody is regression testing this, and
all it takes is a seemingly unrelated change to struct layout or list
walking and things might change anyway.  Then the answer would then be
-- well sorry about that but we never guaranteed that anyway.

Reflecting reality and labeling the interface as named in a
unstable way just seems like the way forward here, to me.

-i

[1] For too much detail; it wasn't actually the interface renaming
    that was the problem, as such; but udev issuing a "move" event
    rather than an "add" event that our custom udev rules didn't match
    for (our bug, uncovered by this).  Of course it's more
    interesting, because the Xen cloud is the only one that doesn't
    use DHCP.  So although we were missing the event on the other
    clouds where things were renamed, we didn't notice because that
    rule was ultimatley a no-op in that situation anyway as it
    auto-configures.  It was only on Xen that networking was not
    configured.

[2] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/configuring_and_managing_networking/consistent-network-interface-device-naming_configuring-and-managing-networking

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

[Andrew Lunn]

On Mon, Apr 04, 2022 at 10:55:27AM +1000, Ian Wienand wrote:
> On Sat, Apr 02, 2022 at 08:40:08PM +0200, Andrew Lunn wrote:
> > So is there a risk here that Xen user suddenly find their network
> > interfaces renamed, where as before they were not, and their
> > networking thus breaks?
> 
> Well, this is actually what "got" us.  The interfaces *are* renamed on
> CentOS 9-stream, but not on earlier releases, because systemd makes
> different choices [1].  Tomorrow someone might "fix" something in that
> systemd/udev chain that flips interfaces without specific use flags
> back to not being renamed again.  I'm certain it would vary based on
> what distro and release you chose to boot.
> 
> > Consistency is good, but it is hard to do in retrospect when there are
> > deployed systems potentially dependent on the inconsistency.
> 
> As noted, it is already the case that if your names are falling into
> this path, they are unstable? (there are many pages for every distro
> that go on and on about this, systemd/udev interactions, rules, link
> files, and so on; e.g. [2]).
> 
> I get what you are saying, that in a fixed virtual environment booting
> some relatively fixed distro, perhaps the names are "stable enough"
> and nobody has bothered updating this yet, so everyone is probably
> happy enough with what they have.
> 
> But ultimately it seems like nobody is regression testing this, and
> all it takes is a seemingly unrelated change to struct layout or list
> walking and things might change anyway.  Then the answer would then be
> -- well sorry about that but we never guaranteed that anyway.
> 
> Reflecting reality and labeling the interface as named in a
> unstable way just seems like the way forward here, to me.

Hi Ian

Please send a v2 and include this consideration in the commit message.
It is then clear you have thought about the issue, why you think it is
O.K, etc. We can then let others decide if it should be merged.

If it does cause a regression and somebody thinks it should be
reverted, your reasoning why it is O.K. is also easy to see etc.

     Andrew