| Message ID | 20230509143504.30382-4-fr0st61te@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Refactoring for GMA command | expand |
On 09/05/2023 16:35, Ivan Mikhaylov wrote: > Add the mac-address-increment option for specify MAC address taken by > any other sources. > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> > --- > .../devicetree/bindings/net/ethernet-controller.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > index 00be387984ac..6900098c5105 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > @@ -34,6 +34,14 @@ properties: > minItems: 6 > maxItems: 6 > > + mac-address-increment: > + $ref: /schemas/types.yaml#/definitions/int32 > + description: > + Specifies the MAC address increment to be added to the MAC address. > + Should be used in cases when there is a need to use MAC address > + different from one obtained by any other level, like u-boot or the > + NC-SI stack. We don't store MAC addresses in DT, but provide simple placeholder for firmware or bootloader. Why shall we store static "increment" part of MAC address? Can't the firmware give you proper MAC address? Best regards, Krzysztof
On Wed, 2023-05-10 at 16:48 +0200, Krzysztof Kozlowski wrote: > On 09/05/2023 16:35, Ivan Mikhaylov wrote: > > Add the mac-address-increment option for specify MAC address taken > > by > > any other sources. > > > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> > > --- > > .../devicetree/bindings/net/ethernet-controller.yaml | 8 > > ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet- > > controller.yaml b/Documentation/devicetree/bindings/net/ethernet- > > controller.yaml > > index 00be387984ac..6900098c5105 100644 > > --- a/Documentation/devicetree/bindings/net/ethernet- > > controller.yaml > > +++ b/Documentation/devicetree/bindings/net/ethernet- > > controller.yaml > > @@ -34,6 +34,14 @@ properties: > > minItems: 6 > > maxItems: 6 > > > > + mac-address-increment: > > + $ref: /schemas/types.yaml#/definitions/int32 > > + description: > > + Specifies the MAC address increment to be added to the MAC > > address. > > + Should be used in cases when there is a need to use MAC > > address > > + different from one obtained by any other level, like u-boot > > or the > > + NC-SI stack. > > We don't store MAC addresses in DT, but provide simple placeholder > for > firmware or bootloader. Why shall we store static "increment" part of > MAC address? Can't the firmware give you proper MAC address? > > Best regards, > Krzysztof > Krzysztof, maybe that's a point to make commit message with better explanation from my side. At current time there is at least two cases where I see it's possible to be used: 1. NC-SI 2. embedded At NC-SI level there is Get Mac Address command which provides to BMC mac address from the host which is same as host mac address, it happens at runtime and overrides old one. Also, this part was also to be discussed 2 years ago in this thread: https://lore.kernel.org/all/OF8E108F72.39D22E89-ON00258765.001E46EB-00258765.00251157@ibm.com/ Where Milton provided this information: DTMF spec DSP0222 NC-SI (network controller sideband interface) is a method to provide a BMC (Baseboard management controller) shared access to an external ethernet port for comunication to the management network in the outside world. The protocol describes ethernet packets that control selective bridging implemented in a host network controller to share its phy. Various NIC OEMs have added a query to find out the address the host is using, and some vendors have added code to query host nic and set the BMC mac to a fixed offset (current hard coded +1 from the host value). If this is compiled in the kernel, the NIC OEM is recognised and the BMC doesn't miss the NIC response the address is set once each time the NCSI stack reinitializes. This mechanism overrides any mac-address or local-mac-address or other assignment. DSP0222 https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110 In embedded case, sometimes you have different multiple ethernet interfaces which using one mac address which increments or decrements for particular interface, just for better explanation, there is patch with explanation which providing them such way of work: https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch In their rep a lot of dts using such option. Thanks.
On 11/05/2023 01:31, Ivan Mikhaylov wrote: > On Wed, 2023-05-10 at 16:48 +0200, Krzysztof Kozlowski wrote: >> On 09/05/2023 16:35, Ivan Mikhaylov wrote: >>> Add the mac-address-increment option for specify MAC address taken >>> by >>> any other sources. >>> >>> Signed-off-by: Paul Fertser <fercerpav@gmail.com> >>> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> >>> --- >>> .../devicetree/bindings/net/ethernet-controller.yaml | 8 >>> ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/ethernet- >>> controller.yaml b/Documentation/devicetree/bindings/net/ethernet- >>> controller.yaml >>> index 00be387984ac..6900098c5105 100644 >>> --- a/Documentation/devicetree/bindings/net/ethernet- >>> controller.yaml >>> +++ b/Documentation/devicetree/bindings/net/ethernet- >>> controller.yaml >>> @@ -34,6 +34,14 @@ properties: >>> minItems: 6 >>> maxItems: 6 >>> >>> + mac-address-increment: >>> + $ref: /schemas/types.yaml#/definitions/int32 >>> + description: >>> + Specifies the MAC address increment to be added to the MAC >>> address. >>> + Should be used in cases when there is a need to use MAC >>> address >>> + different from one obtained by any other level, like u-boot >>> or the >>> + NC-SI stack. >> >> We don't store MAC addresses in DT, but provide simple placeholder >> for >> firmware or bootloader. Why shall we store static "increment" part of >> MAC address? Can't the firmware give you proper MAC address? >> >> Best regards, >> Krzysztof >> > > Krzysztof, maybe that's a point to make commit message with better > explanation from my side. At current time there is at least two cases > where I see it's possible to be used: > > 1. NC-SI > 2. embedded > > At NC-SI level there is Get Mac Address command which provides to BMC > mac address from the host which is same as host mac address, it happens > at runtime and overrides old one. > > Also, this part was also to be discussed 2 years ago in this thread: > https://lore.kernel.org/all/OF8E108F72.39D22E89-ON00258765.001E46EB-00258765.00251157@ibm.com/ Which was not sent to Rob though... > > Where Milton provided this information: > > DTMF spec DSP0222 NC-SI (network controller sideband interface) > is a method to provide a BMC (Baseboard management controller) shared > access to an external ethernet port for comunication to the management > network in the outside world. The protocol describes ethernet packets > that control selective bridging implemented in a host network > controller > to share its phy. Various NIC OEMs have added a query to find out the > address the host is using, and some vendors have added code to query > host > nic and set the BMC mac to a fixed offset (current hard coded +1 from > the host value). If this is compiled in the kernel, the NIC OEM is > recognised and the BMC doesn't miss the NIC response the address is set > once each time the NCSI stack reinitializes. This mechanism overrides > any mac-address or local-mac-address or other assignment. > > DSP0222 > https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110 > > > In embedded case, sometimes you have different multiple ethernet > interfaces which using one mac address which increments or decrements > for particular interface, just for better explanation, there is patch > with explanation which providing them such way of work: > https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch > > In their rep a lot of dts using such option. None of these explain why this is property of the hardware. I understand that this is something you want Linux to do, but DT is not for that purpose. Do not encode system policies into DT and what above commit says is a policy. Best regards, Krzysztof
On 12/05/2023 13:28, Ivan Mikhaylov wrote: > On Fri, 2023-05-12 at 08:22 +0200, Krzysztof Kozlowski wrote: >> On 11/05/2023 01:31, Ivan Mikhaylov wrote: >>> On Wed, 2023-05-10 at 16:48 +0200, Krzysztof Kozlowski wrote: >>>> On 09/05/2023 16:35, Ivan Mikhaylov wrote: >>>>> Add the mac-address-increment option for specify MAC address >>>>> taken >>>>> by >>>>> any other sources. >>>>> >>>>> Signed-off-by: Paul Fertser <fercerpav@gmail.com> >>>>> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> >>>>> --- >>>>> .../devicetree/bindings/net/ethernet-controller.yaml | 8 >>>>> ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/net/ethernet- >>>>> controller.yaml >>>>> b/Documentation/devicetree/bindings/net/ethernet- >>>>> controller.yaml >>>>> index 00be387984ac..6900098c5105 100644 >>>>> --- a/Documentation/devicetree/bindings/net/ethernet- >>>>> controller.yaml >>>>> +++ b/Documentation/devicetree/bindings/net/ethernet- >>>>> controller.yaml >>>>> @@ -34,6 +34,14 @@ properties: >>>>> minItems: 6 >>>>> maxItems: 6 >>>>> >>>>> + mac-address-increment: >>>>> + $ref: /schemas/types.yaml#/definitions/int32 >>>>> + description: >>>>> + Specifies the MAC address increment to be added to the >>>>> MAC >>>>> address. >>>>> + Should be used in cases when there is a need to use MAC >>>>> address >>>>> + different from one obtained by any other level, like u- >>>>> boot >>>>> or the >>>>> + NC-SI stack. >>>> >>>> We don't store MAC addresses in DT, but provide simple >>>> placeholder >>>> for >>>> firmware or bootloader. Why shall we store static "increment" >>>> part of >>>> MAC address? Can't the firmware give you proper MAC address? >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> Krzysztof, maybe that's a point to make commit message with better >>> explanation from my side. At current time there is at least two >>> cases >>> where I see it's possible to be used: >>> >>> 1. NC-SI >>> 2. embedded >>> >>> At NC-SI level there is Get Mac Address command which provides to >>> BMC >>> mac address from the host which is same as host mac address, it >>> happens >>> at runtime and overrides old one. >>> >>> Also, this part was also to be discussed 2 years ago in this >>> thread: >>> https://lore.kernel.org/all/OF8E108F72.39D22E89-ON00258765.001E46EB-00258765.00251157@ibm.com/ >> >> Which was not sent to Rob though... >> >> >>> >>> Where Milton provided this information: >>> >>> DTMF spec DSP0222 NC-SI (network controller sideband interface) >>> is a method to provide a BMC (Baseboard management controller) >>> shared >>> access to an external ethernet port for comunication to the >>> management >>> network in the outside world. The protocol describes ethernet >>> packets >>> that control selective bridging implemented in a host network >>> controller >>> to share its phy. Various NIC OEMs have added a query to find out >>> the >>> address the host is using, and some vendors have added code to >>> query >>> host >>> nic and set the BMC mac to a fixed offset (current hard coded +1 >>> from >>> the host value). If this is compiled in the kernel, the NIC OEM is >>> recognised and the BMC doesn't miss the NIC response the address is >>> set >>> once each time the NCSI stack reinitializes. This mechanism >>> overrides >>> any mac-address or local-mac-address or other assignment. >>> >>> DSP0222 >>> https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110 >>> >>> >>> In embedded case, sometimes you have different multiple ethernet >>> interfaces which using one mac address which increments or >>> decrements >>> for particular interface, just for better explanation, there is >>> patch >>> with explanation which providing them such way of work: >>> https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch >>> >>> In their rep a lot of dts using such option. >> >> None of these explain why this is property of the hardware. I >> understand >> that this is something you want Linux to do, but DT is not for that >> purpose. Do not encode system policies into DT and what above commit >> says is a policy. >> > > Krzysztof, okay then to which DT subsystem it should belong? To > ftgmac100 after conversion? To my understanding, decision to add some numbers to MAC address does not look like DT property at all. Otherwise please help me to understand - why different boards with same device should have different offset/value? Anyway, commit msg also lacks any justification for this. Best regards, Krzysztof
On Fri, 2023-05-12 at 08:22 +0200, Krzysztof Kozlowski wrote: > On 11/05/2023 01:31, Ivan Mikhaylov wrote: > > On Wed, 2023-05-10 at 16:48 +0200, Krzysztof Kozlowski wrote: > > > On 09/05/2023 16:35, Ivan Mikhaylov wrote: > > > > Add the mac-address-increment option for specify MAC address > > > > taken > > > > by > > > > any other sources. > > > > > > > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > > > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> > > > > --- > > > > .../devicetree/bindings/net/ethernet-controller.yaml | 8 > > > > ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet- > > > > controller.yaml > > > > b/Documentation/devicetree/bindings/net/ethernet- > > > > controller.yaml > > > > index 00be387984ac..6900098c5105 100644 > > > > --- a/Documentation/devicetree/bindings/net/ethernet- > > > > controller.yaml > > > > +++ b/Documentation/devicetree/bindings/net/ethernet- > > > > controller.yaml > > > > @@ -34,6 +34,14 @@ properties: > > > > minItems: 6 > > > > maxItems: 6 > > > > > > > > + mac-address-increment: > > > > + $ref: /schemas/types.yaml#/definitions/int32 > > > > + description: > > > > + Specifies the MAC address increment to be added to the > > > > MAC > > > > address. > > > > + Should be used in cases when there is a need to use MAC > > > > address > > > > + different from one obtained by any other level, like u- > > > > boot > > > > or the > > > > + NC-SI stack. > > > > > > We don't store MAC addresses in DT, but provide simple > > > placeholder > > > for > > > firmware or bootloader. Why shall we store static "increment" > > > part of > > > MAC address? Can't the firmware give you proper MAC address? > > > > > > Best regards, > > > Krzysztof > > > > > > > Krzysztof, maybe that's a point to make commit message with better > > explanation from my side. At current time there is at least two > > cases > > where I see it's possible to be used: > > > > 1. NC-SI > > 2. embedded > > > > At NC-SI level there is Get Mac Address command which provides to > > BMC > > mac address from the host which is same as host mac address, it > > happens > > at runtime and overrides old one. > > > > Also, this part was also to be discussed 2 years ago in this > > thread: > > https://lore.kernel.org/all/OF8E108F72.39D22E89-ON00258765.001E46EB-00258765.00251157@ibm.com/ > > Which was not sent to Rob though... > > > > > > Where Milton provided this information: > > > > DTMF spec DSP0222 NC-SI (network controller sideband interface) > > is a method to provide a BMC (Baseboard management controller) > > shared > > access to an external ethernet port for comunication to the > > management > > network in the outside world. The protocol describes ethernet > > packets > > that control selective bridging implemented in a host network > > controller > > to share its phy. Various NIC OEMs have added a query to find out > > the > > address the host is using, and some vendors have added code to > > query > > host > > nic and set the BMC mac to a fixed offset (current hard coded +1 > > from > > the host value). If this is compiled in the kernel, the NIC OEM is > > recognised and the BMC doesn't miss the NIC response the address is > > set > > once each time the NCSI stack reinitializes. This mechanism > > overrides > > any mac-address or local-mac-address or other assignment. > > > > DSP0222 > > https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110 > > > > > > In embedded case, sometimes you have different multiple ethernet > > interfaces which using one mac address which increments or > > decrements > > for particular interface, just for better explanation, there is > > patch > > with explanation which providing them such way of work: > > https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch > > > > In their rep a lot of dts using such option. > > None of these explain why this is property of the hardware. I > understand > that this is something you want Linux to do, but DT is not for that > purpose. Do not encode system policies into DT and what above commit > says is a policy. > Krzysztof, okay then to which DT subsystem it should belong? To ftgmac100 after conversion? Thanks.
On Fri, 2023-05-12 at 11:24 +0200, Krzysztof Kozlowski wrote: > On 12/05/2023 13:28, Ivan Mikhaylov wrote: > > On Fri, 2023-05-12 at 08:22 +0200, Krzysztof Kozlowski wrote: > > > On 11/05/2023 01:31, Ivan Mikhaylov wrote: > > > > On Wed, 2023-05-10 at 16:48 +0200, Krzysztof Kozlowski wrote: > > > > > On 09/05/2023 16:35, Ivan Mikhaylov wrote: > > > > > > Add the mac-address-increment option for specify MAC > > > > > > address > > > > > > taken > > > > > > by > > > > > > any other sources. > > > > > > > > > > > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > > > > > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com> > > > > > > --- > > > > > > .../devicetree/bindings/net/ethernet-controller.yaml > > > > > > | 8 > > > > > > ++++++++ > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/net/ethernet- > > > > > > controller.yaml > > > > > > b/Documentation/devicetree/bindings/net/ethernet- > > > > > > controller.yaml > > > > > > index 00be387984ac..6900098c5105 100644 > > > > > > --- a/Documentation/devicetree/bindings/net/ethernet- > > > > > > controller.yaml > > > > > > +++ b/Documentation/devicetree/bindings/net/ethernet- > > > > > > controller.yaml > > > > > > @@ -34,6 +34,14 @@ properties: > > > > > > minItems: 6 > > > > > > maxItems: 6 > > > > > > > > > > > > + mac-address-increment: > > > > > > + $ref: /schemas/types.yaml#/definitions/int32 > > > > > > + description: > > > > > > + Specifies the MAC address increment to be added to > > > > > > the > > > > > > MAC > > > > > > address. > > > > > > + Should be used in cases when there is a need to use > > > > > > MAC > > > > > > address > > > > > > + different from one obtained by any other level, like > > > > > > u- > > > > > > boot > > > > > > or the > > > > > > + NC-SI stack. > > > > > > > > > > We don't store MAC addresses in DT, but provide simple > > > > > placeholder > > > > > for > > > > > firmware or bootloader. Why shall we store static "increment" > > > > > part of > > > > > MAC address? Can't the firmware give you proper MAC address? > > > > > > > > > > Best regards, > > > > > Krzysztof > > > > > > > > > > > > > Krzysztof, maybe that's a point to make commit message with > > > > better > > > > explanation from my side. At current time there is at least two > > > > cases > > > > where I see it's possible to be used: > > > > > > > > 1. NC-SI > > > > 2. embedded > > > > > > > > At NC-SI level there is Get Mac Address command which provides > > > > to > > > > BMC > > > > mac address from the host which is same as host mac address, it > > > > happens > > > > at runtime and overrides old one. > > > > > > > > Also, this part was also to be discussed 2 years ago in this > > > > thread: > > > > https://lore.kernel.org/all/OF8E108F72.39D22E89-ON00258765.001E46EB-00258765.00251157@ibm.com/ > > > > > > Which was not sent to Rob though... > > > > > > > > > > > > > > Where Milton provided this information: > > > > > > > > DTMF spec DSP0222 NC-SI (network controller sideband interface) > > > > is a method to provide a BMC (Baseboard management controller) > > > > shared > > > > access to an external ethernet port for comunication to the > > > > management > > > > network in the outside world. The protocol describes ethernet > > > > packets > > > > that control selective bridging implemented in a host network > > > > controller > > > > to share its phy. Various NIC OEMs have added a query to find > > > > out > > > > the > > > > address the host is using, and some vendors have added code to > > > > query > > > > host > > > > nic and set the BMC mac to a fixed offset (current hard coded > > > > +1 > > > > from > > > > the host value). If this is compiled in the kernel, the NIC > > > > OEM is > > > > recognised and the BMC doesn't miss the NIC response the > > > > address is > > > > set > > > > once each time the NCSI stack reinitializes. This mechanism > > > > overrides > > > > any mac-address or local-mac-address or other assignment. > > > > > > > > DSP0222 > > > > https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110 > > > > > > > > > > > > In embedded case, sometimes you have different multiple > > > > ethernet > > > > interfaces which using one mac address which increments or > > > > decrements > > > > for particular interface, just for better explanation, there is > > > > patch > > > > with explanation which providing them such way of work: > > > > https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch > > > > > > > > In their rep a lot of dts using such option. > > > > > > None of these explain why this is property of the hardware. I > > > understand > > > that this is something you want Linux to do, but DT is not for > > > that > > > purpose. Do not encode system policies into DT and what above > > > commit > > > says is a policy. > > > > > > > Krzysztof, okay then to which DT subsystem it should belong? To > > ftgmac100 after conversion? > > To my understanding, decision to add some numbers to MAC address does > not look like DT property at all. Otherwise please help me to > understand > - why different boards with same device should have different > offset/value? > > Anyway, commit msg also lacks any justification for this. > > Best regards, > Krzysztof > Krzysztof, essentially some PCIe network cards have like an additional *MII interface which connects directly to a BMC (separate SoC for managing a motherboard) and by sending special ethernet type frames over that connection (called NC-SI) the BMC can obtain MAC, get link parameters etc. So it's natural for a vendor to allocate two MACs per such a board with PCIe card intergrated, with one MAC "flashed into" the network card, under the assumption that the BMC should automatically use the next MAC. So it's the property of the hardware as the vendor designs it, not a matter of usage policy. Also at the nvmem binding tree is "nvmem-cell-cells" which is literally the same as what was proposed but on different level. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/nvmem?id=7e2805c203a6c8dc85c1cfda205161ed39ae82d5 Thanks.
On 16/05/2023 13:47, Ivan Mikhaylov wrote: hy this is property of the hardware. I >>>> understand >>>> that this is something you want Linux to do, but DT is not for >>>> that >>>> purpose. Do not encode system policies into DT and what above >>>> commit >>>> says is a policy. >>>> >>> >>> Krzysztof, okay then to which DT subsystem it should belong? To >>> ftgmac100 after conversion? >> >> To my understanding, decision to add some numbers to MAC address does >> not look like DT property at all. Otherwise please help me to >> understand >> - why different boards with same device should have different >> offset/value? >> >> Anyway, commit msg also lacks any justification for this. >> >> Best regards, >> Krzysztof >> > > Krzysztof, essentially some PCIe network cards have like an additional > *MII interface which connects directly to a BMC (separate SoC for > managing a motherboard) and by sending special ethernet type frames > over that connection (called NC-SI) the BMC can obtain MAC, get link > parameters etc. So it's natural for a vendor to allocate two MACs per > such a board with PCIe card intergrated, with one MAC "flashed into" > the network card, under the assumption that the BMC should Who makes the assumption that next MAC should differ by 1 or 2? > automatically use the next MAC. So it's the property of the hardware as > the vendor designs it, not a matter of usage policy. > > Also at the nvmem binding tree is "nvmem-cell-cells" which is literally > the same as what was proposed but on different level. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/nvmem?id=7e2805c203a6c8dc85c1cfda205161ed39ae82d5 How is this similar? This points the location of mac address on some NV storage. You add fixed value which should be added to the Ethernet. I might be missing the context but there is no DTS example nor user of this property, so how can I get such? Best regards, Krzysztof
On 17/05/2023 23:38, Ivan Mikhaylov wrote: > On Wed, 2023-05-17 at 10:36 +0200, Krzysztof Kozlowski wrote: >> On 16/05/2023 13:47, Ivan Mikhaylov wrote: >> hy this is property of the hardware. I >>>>>> understand >>>>>> that this is something you want Linux to do, but DT is not >>>>>> for >>>>>> that >>>>>> purpose. Do not encode system policies into DT and what above >>>>>> commit >>>>>> says is a policy. >>>>>> >>>>> >>>>> Krzysztof, okay then to which DT subsystem it should belong? To >>>>> ftgmac100 after conversion? >>>> >>>> To my understanding, decision to add some numbers to MAC address >>>> does >>>> not look like DT property at all. Otherwise please help me to >>>> understand >>>> - why different boards with same device should have different >>>> offset/value? I would like to remind this question. "why different boards with same device should have different offset/value?" It was literally ignored and you started explaining network cards and BMC. I don't understand why, but it does not help your case. Let me extend this question with one more: "Why for all your boards of one type, so using the same DTS, would you use one value of incrementing MAC address?" >>>> >>>> Anyway, commit msg also lacks any justification for this. >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> Krzysztof, essentially some PCIe network cards have like an >>> additional >>> *MII interface which connects directly to a BMC (separate SoC for >>> managing a motherboard) and by sending special ethernet type frames >>> over that connection (called NC-SI) the BMC can obtain MAC, get >>> link >>> parameters etc. So it's natural for a vendor to allocate two MACs >>> per >>> such a board with PCIe card intergrated, with one MAC "flashed >>> into" >>> the network card, under the assumption that the BMC should >> >> Who makes the assumption that next MAC should differ by 1 or 2? > > Krzysztof, in this above case BMC does, BMC should care about changing > it and doing it with current codebase without any options just by some > hardcoded numbers which is wrong. But you hard-code the number, just in BMC DTS. How does it differ from BMC hard-coding it differently? You encode policy - or software decisions - into Devicetree. > >> >>> automatically use the next MAC. So it's the property of the >>> hardware as >>> the vendor designs it, not a matter of usage policy. >>> >>> Also at the nvmem binding tree is "nvmem-cell-cells" which is >>> literally >>> the same as what was proposed but on different level. >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/nvmem?id=7e2805c203a6c8dc85c1cfda205161ed39ae82d5 >> >> How is this similar? This points the location of mac address on some >> NV >> storage. You add fixed value which should be added to the Ethernet. > > It's not the points the location, this particular option provides this > increment for mac addresses to make use of them with multiple > interfaces. Just part of above commit: > "It's used as a base for calculating addresses for multiple interfaces. > It's done by adding proper values. Actual offsets are picked by > manufacturers and vary across devices." > > It is same as we talked before about mac-address-increment in openwrt > project, if you want examples, you can look into their github. And same > as we trying to achieve here. > > https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch Awesome... so if project added wrong property to bindings, e.g. SW property, you find it as an argument for anyone else. No, that's not how it works. > > "Lots of embedded devices use the mac-address of other interface > extracted from nvmem cells and increments it by one or two. Add two > bindings to integrate this and directly use the right mac-address for > the interface. Some example are some routers that use the gmac > mac-address stored in the art partition and increments it by one for > the > wifi. mac-address-increment-byte bindings is used to tell what byte of > the mac-address has to be increased (if not defined the last byte is > increased) and mac-address-increment tells how much the byte decided > early has to be increased." > > Don't you see similarity with nvmem commit? Explanation is similar, but you are using wrong argument to justify the property. The MAC address is stored in some NVMEM cell. There is such NVMEM cell. That's the hardware property, thus it is justified in DT. Now how MAC address will be modified - by 1, 2, 3, 252 - is not related to that commit, because it is a software decision. Again, we are back to the previous question to which you answered "BMC will do it". I understand this is property for the BMC DTS, thus: Why for all your boards of one type, so using one DTS, would you use one value of incrementing MAC address? Why devices with same board cannot use different values? One board "1" and second "2" for MAC increments? I am sure that one customer could have it different. The choice how much you increment some MAC address is not a hardware property. It does not even look like a firmware property. If playing with this property was done by firmware, like we do for all MAC address fields, then I would expect here some references to it. Which you did not provide, I believe. > >> >> I might be missing the context but there is no DTS example nor user >> of >> this property, so how can I get such? >> > > I don't see it either in linux kernel DTS tree but it in DTS doc. > > Also, just a little bit history about older propositions > https://lore.kernel.org/all/?q=mac-address-increment > https://lore.kernel.org/all/20200919214941.8038-5-ansuelsmth@gmail.com/ I don't see any user there, except the same rejected proposal: https://lore.kernel.org/all/CAL_JsqKhyeh2=pJcpBKkh+s3FM__DY+VoYSYJLRUErrujTLn9A@mail.gmail.com/ If you want to convince us, please illustrate it in a real world upstreamed DTS (or explain why it cannot). Otherwise I don't see justification as it is not a hardware property. This is a NAK from me. Feel free to ping Rob in some later time, as he might have different opinion. Best regards, Krzysztof
On Wed, 2023-05-17 at 10:36 +0200, Krzysztof Kozlowski wrote: > On 16/05/2023 13:47, Ivan Mikhaylov wrote: > hy this is property of the hardware. I > > > > > understand > > > > > that this is something you want Linux to do, but DT is not > > > > > for > > > > > that > > > > > purpose. Do not encode system policies into DT and what above > > > > > commit > > > > > says is a policy. > > > > > > > > > > > > > Krzysztof, okay then to which DT subsystem it should belong? To > > > > ftgmac100 after conversion? > > > > > > To my understanding, decision to add some numbers to MAC address > > > does > > > not look like DT property at all. Otherwise please help me to > > > understand > > > - why different boards with same device should have different > > > offset/value? > > > > > > Anyway, commit msg also lacks any justification for this. > > > > > > Best regards, > > > Krzysztof > > > > > > > Krzysztof, essentially some PCIe network cards have like an > > additional > > *MII interface which connects directly to a BMC (separate SoC for > > managing a motherboard) and by sending special ethernet type frames > > over that connection (called NC-SI) the BMC can obtain MAC, get > > link > > parameters etc. So it's natural for a vendor to allocate two MACs > > per > > such a board with PCIe card intergrated, with one MAC "flashed > > into" > > the network card, under the assumption that the BMC should > > Who makes the assumption that next MAC should differ by 1 or 2? Krzysztof, in this above case BMC does, BMC should care about changing it and doing it with current codebase without any options just by some hardcoded numbers which is wrong. > > > automatically use the next MAC. So it's the property of the > > hardware as > > the vendor designs it, not a matter of usage policy. > > > > Also at the nvmem binding tree is "nvmem-cell-cells" which is > > literally > > the same as what was proposed but on different level. > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/nvmem?id=7e2805c203a6c8dc85c1cfda205161ed39ae82d5 > > How is this similar? This points the location of mac address on some > NV > storage. You add fixed value which should be added to the Ethernet. It's not the points the location, this particular option provides this increment for mac addresses to make use of them with multiple interfaces. Just part of above commit: "It's used as a base for calculating addresses for multiple interfaces. It's done by adding proper values. Actual offsets are picked by manufacturers and vary across devices." It is same as we talked before about mac-address-increment in openwrt project, if you want examples, you can look into their github. And same as we trying to achieve here. https://github.com/openwrt/openwrt/blob/master/target/linux/generic/pending-5.15/682-of_net-add-mac-address-increment-support.patch "Lots of embedded devices use the mac-address of other interface extracted from nvmem cells and increments it by one or two. Add two bindings to integrate this and directly use the right mac-address for the interface. Some example are some routers that use the gmac mac-address stored in the art partition and increments it by one for the wifi. mac-address-increment-byte bindings is used to tell what byte of the mac-address has to be increased (if not defined the last byte is increased) and mac-address-increment tells how much the byte decided early has to be increased." Don't you see similarity with nvmem commit? > > I might be missing the context but there is no DTS example nor user > of > this property, so how can I get such? > I don't see it either in linux kernel DTS tree but it in DTS doc. Also, just a little bit history about older propositions https://lore.kernel.org/all/?q=mac-address-increment https://lore.kernel.org/all/20200919214941.8038-5-ansuelsmth@gmail.com/ Thanks.
Hello Krzysztof, Let me try to clarify a bit on the particular usecase and answer your questions. Let's consider a server motherboard manufactured and sold by a single company. This motherboard includes I210 (Ethernet Controlleer) chip along with the other necessary parts right there, soldered to the PCB, non-replaceable. This I210 is connected to the host CPU with a PCIe lane and acts as a regular network adapter. In addition to that this chip is connected using NC-SI (management channel) to the BMC SoC (also permanently soldered to the board). There is a separate EEPROM connected directly to I210 which hosts its firmware and many operational parameters, including the MAC address. This EEPROM is not anyhow accessible by the BMC (the host can read/write it using special protocol over PCIe). Intel expects the board manufacturer to embed a MAC address from the manufacturer's range in the EEPROM configuration. But in many cases it's desirable to use a separate MAC address for the BMC (then I210 acts as if it has an integrated switch), so the board manufacturer can, by its internal policy, allocate two consecutive MAC addresses to each motherboard. The only way BMC can learn the MAC address used by I210 is by a special vendor-specific NC-SI command, and it can provide just a single address, the one used by the host. NC-SI is using Ethernet frames with a special type, so to execute this command the network driver needs to be (at least partially) functional. I do not really imagine nvmem getting support to read it this way. On Wed, May 17, 2023 at 09:26:35PM +0200, Krzysztof Kozlowski wrote: > I would like to remind this question. > "why different boards with same device should have different offset/value?" In the usecase we're aiming for the DT is describing a specific board from manufacturer that guarantees the offset to be correct, as none of the parts are replaceable and the MAC address is flashed into the I210 EEPROM during manufacturing. > Let me extend this question with one more: > "Why for all your boards of one type, so using the same DTS, would you > use one value of incrementing MAC address?" Here we assume that for all the boards supported by a particular DT the board manufacturer guarantees the MAC address offset by internal production policy, by allocating the addresses from the manufacturer's pool. > But you hard-code the number, just in BMC DTS. How does it differ from > BMC hard-coding it differently? > > You encode policy - or software decisions - into Devicetree. But MAC address of an Ethernet equipment is an inherent part of the hardware. It's just that we can't store it in an nvmem-addressable cell in this case, unfortunately. > Why devices with same board cannot use different values? One board "1" > and second "2" for MAC increments? I am sure that one customer could > have it different. You assume that the customers might be allocating their own MAC addresses for the network interface of a motherboard, that might be true if the customer gets such a board from an ODM. But such a customer not willing to follow the MAC address offsets policy is not much different from a customer who e.g. modifies flash partitions or storage format making the nvmem references invalid, and so requiring a separate DT. > If you want to convince us, please illustrate it in a real world > upstreamed DTS (or explain why it cannot). Otherwise I don't see > justification as it is not a hardware property. Can you please tell how you would imagine a responsible vendor tackle the usecase I outlined? Guess it's not by a startup script that would be getting a MAC address from an interface, applying the offset, and then change it on the same interface? Thank you for the review and discussion.
On 29/05/2023 22:59, Paul Fertser wrote: > Hello Krzysztof, > > Let me try to clarify a bit on the particular usecase and answer your > questions. > > Let's consider a server motherboard manufactured and sold by a single > company. This motherboard includes I210 (Ethernet Controlleer) chip > along with the other necessary parts right there, soldered to the PCB, > non-replaceable. This I210 is connected to the host CPU with a PCIe > lane and acts as a regular network adapter. In addition to that this > chip is connected using NC-SI (management channel) to the BMC SoC > (also permanently soldered to the board). > > There is a separate EEPROM connected directly to I210 which hosts its > firmware and many operational parameters, including the MAC > address. This EEPROM is not anyhow accessible by the BMC (the host can > read/write it using special protocol over PCIe). Intel expects the > board manufacturer to embed a MAC address from the manufacturer's > range in the EEPROM configuration. But in many cases it's desirable to > use a separate MAC address for the BMC (then I210 acts as if it has an > integrated switch), so the board manufacturer can, by its internal > policy, allocate two consecutive MAC addresses to each motherboard. > > The only way BMC can learn the MAC address used by I210 is by a > special vendor-specific NC-SI command, and it can provide just a > single address, the one used by the host. NC-SI is using Ethernet > frames with a special type, so to execute this command the network > driver needs to be (at least partially) functional. I do not really > imagine nvmem getting support to read it this way. > > On Wed, May 17, 2023 at 09:26:35PM +0200, Krzysztof Kozlowski wrote: >> I would like to remind this question. >> "why different boards with same device should have different offset/value?" > > In the usecase we're aiming for the DT is describing a specific board > from manufacturer that guarantees the offset to be correct, as none of > the parts are replaceable and the MAC address is flashed into the > I210 EEPROM during manufacturing. > >> Let me extend this question with one more: >> "Why for all your boards of one type, so using the same DTS, would you >> use one value of incrementing MAC address?" > > Here we assume that for all the boards supported by a particular DT > the board manufacturer guarantees the MAC address offset by internal > production policy, by allocating the addresses from the manufacturer's > pool. OK, embed such information in the commit or property description. > >> But you hard-code the number, just in BMC DTS. How does it differ from >> BMC hard-coding it differently? >> >> You encode policy - or software decisions - into Devicetree. > > But MAC address of an Ethernet equipment is an inherent part of the > hardware. It's just that we can't store it in an nvmem-addressable > cell in this case, unfortunately. > >> Why devices with same board cannot use different values? One board "1" >> and second "2" for MAC increments? I am sure that one customer could >> have it different. > > You assume that the customers might be allocating their own MAC > addresses for the network interface of a motherboard, that might be > true if the customer gets such a board from an ODM. But such a > customer not willing to follow the MAC address offsets policy is not > much different from a customer who e.g. modifies flash partitions or > storage format making the nvmem references invalid, and so requiring a > separate DT. > >> If you want to convince us, please illustrate it in a real world >> upstreamed DTS (or explain why it cannot). Otherwise I don't see >> justification as it is not a hardware property. > > Can you please tell how you would imagine a responsible vendor tackle > the usecase I outlined? I would imagine him to upstream the DTS. I asked yo illustrate it. There is still no DTS user for it so I have doubts it is used as intended. > Guess it's not by a startup script that would > be getting a MAC address from an interface, applying the offset, and > then change it on the same interface? > > Thank you for the review and discussion. > Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index 00be387984ac..6900098c5105 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -34,6 +34,14 @@ properties: minItems: 6 maxItems: 6 + mac-address-increment: + $ref: /schemas/types.yaml#/definitions/int32 + description: + Specifies the MAC address increment to be added to the MAC address. + Should be used in cases when there is a need to use MAC address + different from one obtained by any other level, like u-boot or the + NC-SI stack. + max-frame-size: $ref: /schemas/types.yaml#/definitions/uint32 description: