| Message ID | 1453907300-28283-2-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 27, 2016 at 04:08:19PM +0100, Thomas Petazzoni wrote: > On Armada 38x, the available network interfaces are: > > - port 0, at 0x70000 > - port 1, at 0x30000 > - port 2, at 0x34000 > > Due to the rule saying that DT nodes should be ordered by register > addresses, the network interfaces are probed in this order: > > - port 1, at 0x30000, which gets named eth0 > - port 2, at 0x34000, which gets named eth1 > - port 0, at 0x70000, which gets named eth2 > > (if all three ports are enabled at the board level) > > Unfortunately, the network subsystem doesn't provide any way to rename > network interfaces from the kernel (it can only be done from > userspace). So, the default naming of the network interfaces is very > confusing as it doesn't match the datasheet, nor the naming of the > interfaces in the bootloader, nor the naming of the interfaces on > labels printed on the board. > > For example, on the Armada 388 GP, the board has two ports, labelled > GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, which > isn't really obvious. > > In order to solve this, this patch proposes to exceptionaly violate > the rule of "order DT nodes by register address", and put the 0x70000 > node before the 0x30000 node, so that network interfaces get named in > a more natural way. The danger is that this will completely mess up people's existing scripts/distro configuration which are now used to the current labelling of the network ports. Not every distro provides a sane way to deal with this...
On Wed, Jan 27, 2016 at 07:31:44PM +0000, Russell King - ARM Linux wrote: (...) > > For example, on the Armada 388 GP, the board has two ports, labelled > > GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, which > > isn't really obvious. > > > > In order to solve this, this patch proposes to exceptionaly violate > > the rule of "order DT nodes by register address", and put the 0x70000 > > node before the 0x30000 node, so that network interfaces get named in > > a more natural way. > > The danger is that this will completely mess up people's existing > scripts/distro configuration which are now used to the current > labelling of the network ports. > > Not every distro provides a sane way to deal with this... Well, may be that's one more reason for fixing it before boards start to ship and run distro kernels. If some boards are already in the wild running on a BSP kernel, people will complain that mainline reorders their network ports and will stick to the BSP kernel instead. Just my two cents, Willy
On Wed, Jan 27, 2016 at 08:45:12PM +0100, Willy Tarreau wrote: > On Wed, Jan 27, 2016 at 07:31:44PM +0000, Russell King - ARM Linux wrote: > (...) > > > For example, on the Armada 388 GP, the board has two ports, labelled > > > GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, which > > > isn't really obvious. > > > > > > In order to solve this, this patch proposes to exceptionaly violate > > > the rule of "order DT nodes by register address", and put the 0x70000 > > > node before the 0x30000 node, so that network interfaces get named in > > > a more natural way. > > > > The danger is that this will completely mess up people's existing > > scripts/distro configuration which are now used to the current > > labelling of the network ports. > > > > Not every distro provides a sane way to deal with this... > > Well, may be that's one more reason for fixing it before boards start to > ship and run distro kernels. If some boards are already in the wild running > on a BSP kernel, people will complain that mainline reorders their network > ports and will stick to the BSP kernel instead. I've checked, and we believe not many people are using mainline kernels on clearfog. I'll have to adjust my debian jessie setup to fix the resulting carnage though. On the plus side, it means that mainline matches Marvell's kernel for interface naming. However, I will point out that changing the DT order may fix it for now, but if there's changes to the kernel initialisation which results in a different probe order (eg, because of a change to the way probing works, which is something that's been discussed a few times) it's likely that the ethX names will change again... In other words, what I'm saying is that there's no guarantee that changing the DT order will always result in the desired network device naming.
Hello Russell, On Fri, 29 Jan 2016 11:48:53 +0000, Russell King - ARM Linux wrote: > > Well, may be that's one more reason for fixing it before boards start to > > ship and run distro kernels. If some boards are already in the wild running > > on a BSP kernel, people will complain that mainline reorders their network > > ports and will stick to the BSP kernel instead. > > I've checked, and we believe not many people are using mainline kernels > on clearfog. I'll have to adjust my debian jessie setup to fix the > resulting carnage though. On the plus side, it means that mainline > matches Marvell's kernel for interface naming. > > However, I will point out that changing the DT order may fix it for now, > but if there's changes to the kernel initialisation which results in a > different probe order (eg, because of a change to the way probing works, > which is something that's been discussed a few times) it's likely that > the ethX names will change again... > > In other words, what I'm saying is that there's no guarantee that > changing the DT order will always result in the desired network device > naming. Yes, fully agreed, it is not a complete guarantee. But in "most cases", it will still lead to something that is a bit more logical for normal users. In its vendor BSP, Marvell has gone all the way to changing the link order of certain PCIe drivers, to make sure that the mvneta driver gets probed first. And indeed, without this hack, if you plug an e1000e PCIe NIC, it gets probed first and therefore assigned as eth0, while all the built-in network interfaces get shifted at eth1, eth2, etc. Really sad that the network subsystem doesn't follow the DT aliases to name network interfaces, but it has been rejected multiple times by Dave Miller as far as I know. In the absence of this, re-ordering the DT nodes is a cheap hack that makes things look a bit more logical in many situations. Best regards, Thomas
Hi Russell, On Fri, Jan 29, 2016 at 11:48:53AM +0000, Russell King - ARM Linux wrote: > On Wed, Jan 27, 2016 at 08:45:12PM +0100, Willy Tarreau wrote: > > On Wed, Jan 27, 2016 at 07:31:44PM +0000, Russell King - ARM Linux wrote: > > (...) > > > > For example, on the Armada 388 GP, the board has two ports, labelled > > > > GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, which > > > > isn't really obvious. > > > > > > > > In order to solve this, this patch proposes to exceptionaly violate > > > > the rule of "order DT nodes by register address", and put the 0x70000 > > > > node before the 0x30000 node, so that network interfaces get named in > > > > a more natural way. > > > > > > The danger is that this will completely mess up people's existing > > > scripts/distro configuration which are now used to the current > > > labelling of the network ports. > > > > > > Not every distro provides a sane way to deal with this... > > > > Well, may be that's one more reason for fixing it before boards start to > > ship and run distro kernels. If some boards are already in the wild running > > on a BSP kernel, people will complain that mainline reorders their network > > ports and will stick to the BSP kernel instead. > > I've checked, and we believe not many people are using mainline kernels > on clearfog. I'll have to adjust my debian jessie setup to fix the > resulting carnage though. On the plus side, it means that mainline > matches Marvell's kernel for interface naming. Well, now I'm one of these and I confirm that it's really painful to have a different ordering between the DTB provided in mainline and the DTB provided with the board. The worst thing is that using mainline, eth0 corresponds to the switch and appears up, so you don't immediately realize that it's not where your cable is connected :-/ Thus could we please get Thomas' patch to ensure that the board boots with similar interface naming with both the original and mainline kernel ? Thanks in advance, Willy
On Wed, 24 Feb 2016 19:41:14 +0100, Willy Tarreau <w@1wt.eu> wrote: > Hi Russell, > > On Fri, Jan 29, 2016 at 11:48:53AM +0000, Russell King - ARM Linux wrote: >> On Wed, Jan 27, 2016 at 08:45:12PM +0100, Willy Tarreau wrote: >> > On Wed, Jan 27, 2016 at 07:31:44PM +0000, Russell King - ARM Linux >> wrote: >> > (...) >> > > > For example, on the Armada 388 GP, the board has two ports, >> labelled >> > > > GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, >> which >> > > > isn't really obvious. >> > > > >> > > > In order to solve this, this patch proposes to exceptionaly >> violate >> > > > the rule of "order DT nodes by register address", and put the >> 0x70000 >> > > > node before the 0x30000 node, so that network interfaces get >> named in >> > > > a more natural way. >> > > >> > > The danger is that this will completely mess up people's existing >> > > scripts/distro configuration which are now used to the current >> > > labelling of the network ports. >> > > >> > > Not every distro provides a sane way to deal with this... >> > >> > Well, may be that's one more reason for fixing it before boards start >> to >> > ship and run distro kernels. If some boards are already in the wild >> running >> > on a BSP kernel, people will complain that mainline reorders their >> network >> > ports and will stick to the BSP kernel instead. >> >> I've checked, and we believe not many people are using mainline kernels >> on clearfog. I'll have to adjust my debian jessie setup to fix the >> resulting carnage though. On the plus side, it means that mainline >> matches Marvell's kernel for interface naming. > > Well, now I'm one of these and I confirm that it's really painful to > have a different ordering between the DTB provided in mainline and the > DTB provided with the board. The worst thing is that using mainline, > eth0 corresponds to the switch and appears up, so you don't immediately > realize that it's not where your cable is connected :-/ > > Thus could we please get Thomas' patch to ensure that the board boots > with similar interface naming with both the original and mainline kernel > ? Shouldn't this change be device specific then instead of messing up the dtsi? Cheers, Imre
On Wed, Feb 24, 2016 at 08:02:09PM +0100, Imre Kaloz wrote: > >>I've checked, and we believe not many people are using mainline kernels > >>on clearfog. I'll have to adjust my debian jessie setup to fix the > >>resulting carnage though. On the plus side, it means that mainline > >>matches Marvell's kernel for interface naming. > > > >Well, now I'm one of these and I confirm that it's really painful to > >have a different ordering between the DTB provided in mainline and the > >DTB provided with the board. The worst thing is that using mainline, > >eth0 corresponds to the switch and appears up, so you don't immediately > >realize that it's not where your cable is connected :-/ > > > >Thus could we please get Thomas' patch to ensure that the board boots > >with similar interface naming with both the original and mainline kernel > >? > > Shouldn't this change be device specific then instead of messing up the > dtsi? According to what Thomas explained, it's related to the ordering of the NICs in the SoC's datasheet, so there are little chances that anyone would see them enumerated differently on their board. Willy
On Wed, Feb 24, 2016 at 07:41:14PM +0100, Willy Tarreau wrote: > Hi Russell, > Well, now I'm one of these and I confirm that it's really painful to > have a different ordering between the DTB provided in mainline and the > DTB provided with the board. The worst thing is that using mainline, > eth0 corresponds to the switch and appears up, so you don't immediately > realize that it's not where your cable is connected :-/ > > Thus could we please get Thomas' patch to ensure that the board boots > with similar interface naming with both the original and mainline kernel ? I didn't say no to it, I merely asked a few pertinent questions and made some pertinent points. Let me restate: * Today, people who switch between mainline and vendor kernels experience some pain due to the NIC order changing. * Mainline has had support for Armada 38x for 2 years now, which is long enough for it to have gained users. AFAICS, there haven't been any complaints about the different NIC ordering. Changing the NIC ordering is going to cause breakage to these users when they migrate across the change. By making the change, we're effectively telling these mainline-only users "we don't care about your setups, we're going to break them" because that's exactly what we're going to do. Of course, if no one complains about the change, you've got away with it. I think the folk who want to make this change should be prepared for userspace breakage reports: Linus feels very strongly about zero userspace regressions, as we all should know. What I'm saying, therefore, is make the change, but if you have one report that someone's userspace setup has broken as a result of the change, the change must be reverted.
On Wed, Feb 24, 2016 at 10:33:50PM +0000, Russell King - ARM Linux wrote: > On Wed, Feb 24, 2016 at 07:41:14PM +0100, Willy Tarreau wrote: > > Hi Russell, > > Well, now I'm one of these and I confirm that it's really painful to > > have a different ordering between the DTB provided in mainline and the > > DTB provided with the board. The worst thing is that using mainline, > > eth0 corresponds to the switch and appears up, so you don't immediately > > realize that it's not where your cable is connected :-/ > > > > Thus could we please get Thomas' patch to ensure that the board boots > > with similar interface naming with both the original and mainline kernel ? > > I didn't say no to it, I merely asked a few pertinent questions and > made some pertinent points. Yes I'm well aware, which is why I was bringing some feedback. > Let me restate: > > * Today, people who switch between mainline and vendor kernels > experience some pain due to the NIC order changing. > > * Mainline has had support for Armada 38x for 2 years now, which is > long enough for it to have gained users. I didn't realize it had been *that* long, I thought early support was very limited, but I can indeed see that the first dtsi was already fairly complete (though lots of patches came up recently, including one to disable IP checksumming on jumbo frames that's only 3 months old). > AFAICS, there haven't been > any complaints about the different NIC ordering. Changing the NIC > ordering is going to cause breakage to these users when they migrate > across the change. Yes, and the problem is that there are two expected breakages : going from LSP to "old mainline" and going from "old mainline" to "new mainline". Since boards ship with LSP by default, I think that not doing the change will lead to causing breakage at least once for each user going to mainline, while doing it will only cause breakage to users who have already experienced breakage. > By making the change, we're effectively telling these mainline-only > users "we don't care about your setups, we're going to break them" > because that's exactly what we're going to do. Not exactly, we're telling "if you moved away from the original kernel, you have already experienced breakage so apparently you know how to fix it, and if it's the first time you move away from it you won't notice anything" :-) > Of course, if no one complains about the change, you've got away > with it. > > I think the folk who want to make this change should be prepared for > userspace breakage reports: Linus feels very strongly about zero > userspace regressions, as we all should know. Oh yes and I've always agreed with him regarding this! > What I'm saying, therefore, is make the change, but if you have one > report that someone's userspace setup has broken as a result of the > change, the change must be reverted. Makes sense. If any breakage is reported, it would be interested to know if it's on a multi-port board or single-port one. Thanks, Willy
Willy, On Wed, 24 Feb 2016 19:41:14 +0100, Willy Tarreau wrote: > Thus could we please get Thomas' patch to ensure that the board boots > with similar interface naming with both the original and mainline kernel ? My patch has already been applied by the mvebu maintainers in the mvebu/dt branch: http://git.infradead.org/linux-mvebu.git/commit/cb4f71c4298853db0c6751b1209e4535956f136c. Best regards, Thomas
Dear Imre Kaloz, On Wed, 24 Feb 2016 20:02:09 +0100, Imre Kaloz wrote: > Shouldn't this change be device specific then instead of messing up the > dtsi? It's the order in which the nodes are defined at the top-most level that counts. So if you change the order of the nodes in the .dts files, it won't change anything. Best regards, Thomas
Hello, On Wed, 24 Feb 2016 22:33:50 +0000, Russell King - ARM Linux wrote: > I didn't say no to it, I merely asked a few pertinent questions and > made some pertinent points. > > Let me restate: > > * Today, people who switch between mainline and vendor kernels > experience some pain due to the NIC order changing. > > * Mainline has had support for Armada 38x for 2 years now, which is > long enough for it to have gained users. AFAICS, there haven't been > any complaints about the different NIC ordering. Changing the NIC > ordering is going to cause breakage to these users when they migrate > across the change. > > By making the change, we're effectively telling these mainline-only > users "we don't care about your setups, we're going to break them" > because that's exactly what we're going to do. > > Of course, if no one complains about the change, you've got away > with it. I agree with you that the change isn't perfect, it's really a matter of trade-off. The perfect change would be to be able to help the network subsystem in its naming of network interfaces, but this has always been rejected. It would have indeed been better to add this DT node ordering work-around earlier, but we didn't do it, and we're in the present. So again, yes the proposed change is not "great", but I believe it's relatively reasonable. If we indeed get feedback that it's breaking things for too many people, I guess we'll just revert. I don't mind if things remain as they are today, i.e with a weird ordering of the network interfaces. It's just annoying for new people using the platform, but it's not horrible either. Best regards, Thomas
Hi Thomas, On Thu, Feb 25, 2016 at 11:31:37AM +0100, Thomas Petazzoni wrote: > My patch has already been applied by the mvebu maintainers in > the mvebu/dt branch: > http://git.infradead.org/linux-mvebu.git/commit/cb4f71c4298853db0c6751b1209e4535956f136c. Ah thanks, I didn't notice! Best regards, willy
On Thu, 25 Feb 2016 11:36:10 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Wed, 24 Feb 2016 22:33:50 +0000, Russell King - ARM Linux wrote: > >> I didn't say no to it, I merely asked a few pertinent questions and >> made some pertinent points. >> >> Let me restate: >> >> * Today, people who switch between mainline and vendor kernels >> experience some pain due to the NIC order changing. >> >> * Mainline has had support for Armada 38x for 2 years now, which is >> long enough for it to have gained users. AFAICS, there haven't been >> any complaints about the different NIC ordering. Changing the NIC >> ordering is going to cause breakage to these users when they migrate >> across the change. >> >> By making the change, we're effectively telling these mainline-only >> users "we don't care about your setups, we're going to break them" >> because that's exactly what we're going to do. >> >> Of course, if no one complains about the change, you've got away >> with it. > > I agree with you that the change isn't perfect, it's really a matter of > trade-off. The perfect change would be to be able to help the network > subsystem in its naming of network interfaces, but this has always been > rejected. It would have indeed been better to add this DT node ordering > work-around earlier, but we didn't do it, and we're in the present. > > So again, yes the proposed change is not "great", but I believe it's > relatively reasonable. If we indeed get feedback that it's breaking > things for too many people, I guess we'll just revert. I don't mind if > things remain as they are today, i.e with a weird ordering of the > network interfaces. It's just annoying for new people using the > platform, but it's not horrible either. I can check during the weekend if the mainlined Linksys boards get broken because of this. Imre
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi index e8b7f67..b50784d 100644 --- a/arch/arm/boot/dts/armada-38x.dtsi +++ b/arch/arm/boot/dts/armada-38x.dtsi @@ -429,6 +429,27 @@ reg = <0x22000 0x1000>; }; + /* + * As a special exception to the "order by + * register address" rule, the eth0 node is + * placed here to ensure that it gets + * registered as the first interface, since + * the network subsystem doesn't allow naming + * interfaces using DT aliases. Without this, + * the ordering of interfaces is different + * from the one used in U-Boot and the + * labeling of interfaces on the boards, which + * is very confusing for users. + */ + eth0: ethernet@70000 { + compatible = "marvell,armada-370-neta"; + reg = <0x70000 0x4000>; + interrupts-extended = <&mpic 8>; + clocks = <&gateclk 4>; + tx-csum-limit = <9800>; + status = "disabled"; + }; + eth1: ethernet@30000 { compatible = "marvell,armada-370-neta"; reg = <0x30000 0x4000>; @@ -493,15 +514,6 @@ }; }; - eth0: ethernet@70000 { - compatible = "marvell,armada-370-neta"; - reg = <0x70000 0x4000>; - interrupts-extended = <&mpic 8>; - clocks = <&gateclk 4>; - tx-csum-limit = <9800>; - status = "disabled"; - }; - mdio: mdio@72004 { #address-cells = <1>; #size-cells = <0>;
On Armada 38x, the available network interfaces are: - port 0, at 0x70000 - port 1, at 0x30000 - port 2, at 0x34000 Due to the rule saying that DT nodes should be ordered by register addresses, the network interfaces are probed in this order: - port 1, at 0x30000, which gets named eth0 - port 2, at 0x34000, which gets named eth1 - port 0, at 0x70000, which gets named eth2 (if all three ports are enabled at the board level) Unfortunately, the network subsystem doesn't provide any way to rename network interfaces from the kernel (it can only be done from userspace). So, the default naming of the network interfaces is very confusing as it doesn't match the datasheet, nor the naming of the interfaces in the bootloader, nor the naming of the interfaces on labels printed on the board. For example, on the Armada 388 GP, the board has two ports, labelled GE0 and GE1. One has to know that GE0 is eth1 and GE1 is eth0, which isn't really obvious. In order to solve this, this patch proposes to exceptionaly violate the rule of "order DT nodes by register address", and put the 0x70000 node before the 0x30000 node, so that network interfaces get named in a more natural way. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- arch/arm/boot/dts/armada-38x.dtsi | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)