diff mbox series

[net-next] net: enetc: Support ethernet aliases in dts.

Message ID 20250225214458.658993-1-shenwei.wang@nxp.com (mailing list archive)
State New
Headers show
Series [net-next] net: enetc: Support ethernet aliases in dts. | expand

Commit Message

Shenwei Wang Feb. 25, 2025, 9:44 p.m. UTC
Retrieve the "ethernet" alias ID from the DTS and assign it as the
interface name (e.g., "eth0", "eth1"). This ensures predictable naming
aligned with the DTS's configuration.

If no alias is defined, fall back to the kernel's default enumeration
to maintain backward compatibility.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Feb. 26, 2025, 2:57 p.m. UTC | #1
On Tue, Feb 25, 2025 at 03:44:58PM -0600, Shenwei Wang wrote:
> Retrieve the "ethernet" alias ID from the DTS and assign it as the
> interface name (e.g., "eth0", "eth1"). This ensures predictable naming
> aligned with the DTS's configuration.
> 
> If no alias is defined, fall back to the kernel's default enumeration
> to maintain backward compatibility.

GregKH and others will tell you this is a user space problem. Ethernet
names have never been stable, user space has always had to deal with
this problem. Please use a udev rule, systemd naming, etc.

	Andrew
Shenwei Wang Feb. 26, 2025, 3:07 p.m. UTC | #2
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, February 26, 2025 8:58 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Vladimir Oltean
> <vladimir.oltean@nxp.com>; Wei Fang <wei.fang@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> imx@lists.linux.dev; netdev@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH net-next] net: enetc: Support ethernet aliases in dts.
> On Tue, Feb 25, 2025 at 03:44:58PM -0600, Shenwei Wang wrote:
> > Retrieve the "ethernet" alias ID from the DTS and assign it as the
> > interface name (e.g., "eth0", "eth1"). This ensures predictable naming
> > aligned with the DTS's configuration.
> >
> > If no alias is defined, fall back to the kernel's default enumeration
> > to maintain backward compatibility.
> 
> GregKH and others will tell you this is a user space problem. Ethernet names have
> never been stable, user space has always had to deal with this problem. Please
> use a udev rule, systemd naming, etc.

Thanks Andrew.
Does it mean the ethernet aliases defined in the dts are no longer recommended or supported?

Thanks,
Shenwei

> 
>         Andrew
Vladimir Oltean Feb. 26, 2025, 3:10 p.m. UTC | #3
On Wed, Feb 26, 2025 at 05:07:15PM +0200, Shenwei Wang wrote:
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Wednesday, February 26, 2025 8:58 AM
> > To: Shenwei Wang <shenwei.wang@nxp.com>
> > Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Vladimir Oltean
> > <vladimir.oltean@nxp.com>; Wei Fang <wei.fang@nxp.com>; Clark Wang
> > <xiaoning.wang@nxp.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S.
> > Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>;
> > imx@lists.linux.dev; netdev@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > Subject: [EXT] Re: [PATCH net-next] net: enetc: Support ethernet aliases in dts.
> > On Tue, Feb 25, 2025 at 03:44:58PM -0600, Shenwei Wang wrote:
> > > Retrieve the "ethernet" alias ID from the DTS and assign it as the
> > > interface name (e.g., "eth0", "eth1"). This ensures predictable naming
> > > aligned with the DTS's configuration.
> > >
> > > If no alias is defined, fall back to the kernel's default enumeration
> > > to maintain backward compatibility.
> > 
> > GregKH and others will tell you this is a user space problem. Ethernet names have
> > never been stable, user space has always had to deal with this problem. Please
> > use a udev rule, systemd naming, etc.
> 
> Thanks Andrew.
> Does it mean the ethernet aliases defined in the dts are no longer recommended or supported?
> 
> Thanks,
> Shenwei

Aliases are recommended at least for U-Boot fixups of any kind (MAC
addresses, but not necessarily). You never want to perform a fixup based
on a hardcoded node path.
Vladimir Oltean Feb. 26, 2025, 3:47 p.m. UTC | #4
On Tue, Feb 25, 2025 at 03:44:58PM -0600, Shenwei Wang wrote:
> Retrieve the "ethernet" alias ID from the DTS and assign it as the
> interface name (e.g., "eth0", "eth1"). This ensures predictable naming
> aligned with the DTS's configuration.
> 
> If no alias is defined, fall back to the kernel's default enumeration
> to maintain backward compatibility.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index fc41078c4f5d..5ec8dc59e809 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -622,10 +622,20 @@ static int enetc4_pf_netdev_create(struct enetc_si *si)
>  	struct device *dev = &si->pdev->dev;
>  	struct enetc_ndev_priv *priv;
>  	struct net_device *ndev;
> +	char ifname[IFNAMSIZ];
>  	int err;
>  
> -	ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
> -				  si->num_tx_rings, si->num_rx_rings);
> +	err = of_alias_get_id(dev->of_node, "ethernet");
> +	if (err >= 0) {
> +		snprintf(ifname, IFNAMSIZ, "eth%d", err);
> +		ndev = alloc_netdev_mqs(sizeof(struct enetc_ndev_priv),
> +					ifname, NET_NAME_PREDICTABLE, ether_setup,
> +					si->num_tx_rings, si->num_rx_rings);
> +	} else {
> +		ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
> +					  si->num_tx_rings, si->num_rx_rings);
> +	}
> +
>  	if (!ndev)
>  		return  -ENOMEM;
>  

Shenwei, you don't want the kernel to attempt to be very smart about the
initial netdev naming. You will inevitably run into the situation where
"eth%d" is already the name chosen for the kernel for a different
net_device without a predictable name (e.g. e1000e PCIe card) which has
been allocated already. Then you'll want to fix that somehow, and the
stream of patches will never stop, because the kernel will never be able
to fulfill all requirements. Look at the udev naming rules for dpaa2 and
enetc on Layerscape and build something like that. DSA switch included -
the "label" device tree property is considered legacy.

Nacked-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Shenwei Wang Feb. 26, 2025, 4:46 p.m. UTC | #5
> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Wednesday, February 26, 2025 9:48 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Wei Fang <wei.fang@nxp.com>;
> Clark Wang <xiaoning.wang@nxp.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; imx@lists.linux.dev; netdev@vger.kernel.org; dl-
> linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH net-next] net: enetc: Support ethernet aliases in dts.
> 
> On Tue, Feb 25, 2025 at 03:44:58PM -0600, Shenwei Wang wrote:
> > Retrieve the "ethernet" alias ID from the DTS and assign it as the
> > interface name (e.g., "eth0", "eth1"). This ensures predictable naming
> > aligned with the DTS's configuration.
> >
> > If no alias is defined, fall back to the kernel's default enumeration
> > to maintain backward compatibility.
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > index fc41078c4f5d..5ec8dc59e809 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> > @@ -622,10 +622,20 @@ static int enetc4_pf_netdev_create(struct enetc_si
> *si)
> >  	struct device *dev = &si->pdev->dev;
> >  	struct enetc_ndev_priv *priv;
> >  	struct net_device *ndev;
> > +	char ifname[IFNAMSIZ];
> >  	int err;
> >
> > -	ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
> > -				  si->num_tx_rings, si->num_rx_rings);
> > +	err = of_alias_get_id(dev->of_node, "ethernet");
> > +	if (err >= 0) {
> > +		snprintf(ifname, IFNAMSIZ, "eth%d", err);
> > +		ndev = alloc_netdev_mqs(sizeof(struct enetc_ndev_priv),
> > +					ifname, NET_NAME_PREDICTABLE,
> ether_setup,
> > +					si->num_tx_rings, si->num_rx_rings);
> > +	} else {
> > +		ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
> > +					  si->num_tx_rings, si->num_rx_rings);
> > +	}
> > +
> >  	if (!ndev)
> >  		return  -ENOMEM;
> >
> 
> Shenwei, you don't want the kernel to attempt to be very smart about the initial
> netdev naming. You will inevitably run into the situation where "eth%d" is already
> the name chosen for the kernel for a different net_device without a predictable
> name (e.g. e1000e PCIe card) which has been allocated already. Then you'll want

The driver just provides an option to configure predictable interface naming. IMO, all 
those kind of naming conflicts should be managed by the DTS itself. 

> to fix that somehow, and the stream of patches will never stop, because the
> kernel will never be able to fulfill all requirements. Look at the udev naming rules
> for dpaa2 and enetc on Layerscape and build something like that. DSA switch

Even with the udev/systemd solution, you may still need to resolve the naming
conflict sometimes among multiple cards.

Thanks,
Shenwei

> included - the "label" device tree property is considered legacy.
> 
> Nacked-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Vladimir Oltean Feb. 26, 2025, 5:10 p.m. UTC | #6
On Wed, Feb 26, 2025 at 06:46:54PM +0200, Shenwei Wang wrote:
> > Shenwei, you don't want the kernel to attempt to be very smart about the initial
> > netdev naming. You will inevitably run into the situation where "eth%d" is already
> > the name chosen for the kernel for a different net_device without a predictable
> > name (e.g. e1000e PCIe card) which has been allocated already. Then you'll want
> 
> The driver just provides an option to configure predictable interface naming. IMO, all 
> those kind of naming conflicts should be managed by the DTS itself. 

Good luck adding of_alias_get_id() patches to (and others) PCIe NIC
drivers (with a real PCIe link layer, not Enhanced Allocation).

> > to fix that somehow, and the stream of patches will never stop, because the
> > kernel will never be able to fulfill all requirements. Look at the udev naming rules
> > for dpaa2 and enetc on Layerscape and build something like that. DSA switch
> 
> Even with the udev/systemd solution, you may still need to resolve the naming
> conflict sometimes among multiple cards.

Yet, the rootfs seems to be the only place to keep net device names that
is sufficiently flexible to cover the variability in use cases. Maybe
you're used to your developer position where you can immediately modify
the device tree for a board, but one is supposed to be able to configure
U-Boot to provide a fixed device tree (its own) to Linux. This is for
example used for Arm SystemReady IR compliance with distributions, to my
knowledge.
Shenwei Wang Feb. 26, 2025, 5:43 p.m. UTC | #7
> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Wednesday, February 26, 2025 11:11 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; Wei Fang <wei.fang@nxp.com>;
> Clark Wang <xiaoning.wang@nxp.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; imx@lists.linux.dev; netdev@vger.kernel.org; dl-
> linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH net-next] net: enetc: Support ethernet aliases in dts.
> 
> On Wed, Feb 26, 2025 at 06:46:54PM +0200, Shenwei Wang wrote:
> > > Shenwei, you don't want the kernel to attempt to be very smart about
> > > the initial netdev naming. You will inevitably run into the
> > > situation where "eth%d" is already the name chosen for the kernel
> > > for a different net_device without a predictable name (e.g. e1000e
> > > PCIe card) which has been allocated already. Then you'll want
> >
> > The driver just provides an option to configure predictable interface
> > naming. IMO, all those kind of naming conflicts should be managed by the DTS
> itself.
> 
> Good luck adding of_alias_get_id() patches to (and others) PCIe NIC drivers (with
> a real PCIe link layer, not Enhanced Allocation).
> 
> > > to fix that somehow, and the stream of patches will never stop,
> > > because the kernel will never be able to fulfill all requirements.
> > > Look at the udev naming rules for dpaa2 and enetc on Layerscape and
> > > build something like that. DSA switch
> >
> > Even with the udev/systemd solution, you may still need to resolve the
> > naming conflict sometimes among multiple cards.
> 
> Yet, the rootfs seems to be the only place to keep net device names that is
> sufficiently flexible to cover the variability in use cases. Maybe you're used to
> your developer position where you can immediately modify the device tree for a
> board, but one is supposed to be able to configure U-Boot to provide a fixed
> device tree (its own) to Linux. This is for example used for Arm SystemReady IR
> compliance with distributions, to my knowledge.

So my understanding here is that the primary purpose of Ethernet aliases in the 
kernel DTS is simply to provide U-Boot with easier access to specific Ethernet nodes?

Thanks,
Shenwei
Vladimir Oltean Feb. 26, 2025, 8:02 p.m. UTC | #8
On Wed, Feb 26, 2025 at 07:43:34PM +0200, Shenwei Wang wrote:
> So my understanding here is that the primary purpose of Ethernet aliases in the 
> kernel DTS is simply to provide U-Boot with easier access to specific Ethernet nodes?

This is what my limited experience is telling me, yes.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index fc41078c4f5d..5ec8dc59e809 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -622,10 +622,20 @@  static int enetc4_pf_netdev_create(struct enetc_si *si)
 	struct device *dev = &si->pdev->dev;
 	struct enetc_ndev_priv *priv;
 	struct net_device *ndev;
+	char ifname[IFNAMSIZ];
 	int err;
 
-	ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
-				  si->num_tx_rings, si->num_rx_rings);
+	err = of_alias_get_id(dev->of_node, "ethernet");
+	if (err >= 0) {
+		snprintf(ifname, IFNAMSIZ, "eth%d", err);
+		ndev = alloc_netdev_mqs(sizeof(struct enetc_ndev_priv),
+					ifname, NET_NAME_PREDICTABLE, ether_setup,
+					si->num_tx_rings, si->num_rx_rings);
+	} else {
+		ndev = alloc_etherdev_mqs(sizeof(struct enetc_ndev_priv),
+					  si->num_tx_rings, si->num_rx_rings);
+	}
+
 	if (!ndev)
 		return  -ENOMEM;