2023-02-03 14:16:59

by Janne Grunau

[permalink] [raw]
Subject: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

The Devicetree Specification, Release v0.3 specifies in section 4.3.1
a "Network Class Binding". This covers MAC address and maximal frame
size properties. "local-mac-address" and "mac-address" with a fixed
address-size of 48 bits is already in the ethernet-controller.yaml
schema so move those over.
I think the only commonly used values for address-size are 48 and 64
bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
mac-address size based on the address-size. This seems to be an side
effect of the array definition and I was not able to restrict "minItems"
or "maxItems" based on the address-size value in an "if"-"then"-"else"
block.
An easy way out would be to restrict address-size to 48-bits for now.

I've ignored "max-frame-size" since the description in
ethernet-controller.yaml claims there is a contradiction in the
Devicetree specification. I suppose it is describing the property
"max-frame-size" with "Specifies maximum packet length ...".

My understanding from the dt-schema README is that network-class.yaml
should live in the dt-schema repository since it describes properties
from the Devicetree specification. How is the synchronization handled in
this case? The motivation for this series is to fix dtbs_check failures
for Apple silicon devices both in the tree and upcoming ones.

Signed-off-by: Janne Grunau <[email protected]>
---
Janne Grunau (3):
dt-bindings: net: Add network-class schema for mac-address properties
dt-bindings: wireless: bcm4329-fmac: Use network-class.yaml schema
dt-bindings: wireless: silabs,wfx: Use network-class.yaml

.../bindings/net/ethernet-controller.yaml | 18 +---------
.../devicetree/bindings/net/network-class.yaml | 40 ++++++++++++++++++++++
.../bindings/net/wireless/brcm,bcm4329-fmac.yaml | 5 ++-
.../bindings/net/wireless/silabs,wfx.yaml | 5 +--
4 files changed, 46 insertions(+), 22 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230203-dt-bindings-network-class-8367edd679d2

Best regards,
--
Janne Grunau <[email protected]>



2023-02-03 14:25:49

by Janne Grunau

[permalink] [raw]
Subject: [PATCH RFC 2/3] dt-bindings: wireless: bcm4329-fmac: Use network-class.yaml schema

The network-class schema specifies local-mac-address as used in the
bcm4329-fmac device nodes of Apple silicon devices
(arch/arm64/boot/dts/apple).
Fixes `make dtbs_check` for those devices.

Signed-off-by: Janne Grunau <[email protected]>
---
.../devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
index fec1cc9b9a08..55b0a21acb96 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
@@ -116,11 +116,14 @@ properties:
NVRAM. This would normally be filled in by the bootloader from platform
configuration data.

+allOf:
+ - $ref: /schemas/net/network-class.yaml#
+
required:
- compatible
- reg

-additionalProperties: false
+unevaluatedProperties: false

examples:
- |

--
2.39.1


2023-02-03 14:41:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

On Fri, Feb 3, 2023 at 7:56 AM Janne Grunau <[email protected]> wrote:
>
> The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> a "Network Class Binding". This covers MAC address and maximal frame
> size properties. "local-mac-address" and "mac-address" with a fixed
> address-size of 48 bits is already in the ethernet-controller.yaml
> schema so move those over.
> I think the only commonly used values for address-size are 48 and 64
> bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
> mac-address size based on the address-size. This seems to be an side
> effect of the array definition and I was not able to restrict "minItems"
> or "maxItems" based on the address-size value in an "if"-"then"-"else"
> block.
> An easy way out would be to restrict address-size to 48-bits for now.

I've never seen 64-bits used...

> I've ignored "max-frame-size" since the description in
> ethernet-controller.yaml claims there is a contradiction in the
> Devicetree specification. I suppose it is describing the property
> "max-frame-size" with "Specifies maximum packet length ...".

Please include it and we'll fix the spec. It is clearly wrong. 2 nios
boards use 1518 and the consumer for them says it is MTU. Everything
else clearly uses mtu with 1500 or 9000.

> My understanding from the dt-schema README is that network-class.yaml
> should live in the dt-schema repository since it describes properties
> from the Devicetree specification. How is the synchronization handled in
> this case? The motivation for this series is to fix dtbs_check failures
> for Apple silicon devices both in the tree and upcoming ones.

Let's add it to the kernel, then later we can copy it to dtschema,
bump the minimum version the kernel requires, and delete the kernel
copy.

Rob

2023-02-06 16:32:01

by Janne Grunau

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

On 2023-02-03 08:41:28 -0600, Rob Herring wrote:
> On Fri, Feb 3, 2023 at 7:56 AM Janne Grunau <[email protected]> wrote:
> >
> > The Devicetree Specification, Release v0.3 specifies in section 4.3.1
> > a "Network Class Binding". This covers MAC address and maximal frame
> > size properties. "local-mac-address" and "mac-address" with a fixed
> > address-size of 48 bits is already in the ethernet-controller.yaml
> > schema so move those over.
> > I think the only commonly used values for address-size are 48 and 64
> > bits (EUI-48 and EUI-64). Unfortunately I was not able to restrict the
> > mac-address size based on the address-size. This seems to be an side
> > effect of the array definition and I was not able to restrict "minItems"
> > or "maxItems" based on the address-size value in an "if"-"then"-"else"
> > block.
> > An easy way out would be to restrict address-size to 48-bits for now.
>
> I've never seen 64-bits used...

ZigBee and 6LoWPAN use 64-bits for example. Let's hardcode 48 bits for
now as that's what all in-tree devicetrees implicitly use. If needed it
can be changed later.

> > I've ignored "max-frame-size" since the description in
> > ethernet-controller.yaml claims there is a contradiction in the
> > Devicetree specification. I suppose it is describing the property
> > "max-frame-size" with "Specifies maximum packet length ...".
>
> Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> boards use 1518 and the consumer for them says it is MTU. Everything
> else clearly uses mtu with 1500 or 9000.

Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include
it with the description of ethernet-controller.yaml which specifies it
as MTU.

Janne

2023-02-07 01:34:56

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

> > > I've ignored "max-frame-size" since the description in
> > > ethernet-controller.yaml claims there is a contradiction in the
> > > Devicetree specification. I suppose it is describing the property
> > > "max-frame-size" with "Specifies maximum packet length ...".
> >
> > Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> > boards use 1518 and the consumer for them says it is MTU. Everything
> > else clearly uses mtu with 1500 or 9000.
>
> Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include
> it with the description of ethernet-controller.yaml which specifies it
> as MTU.

You need to be careful here. Frame and MTU are different things.

The IEEE 802.3 standard says nothing about MTU. I believe MTU is an IP
concept. It is the size of the SDU an Ethernet PDU can carry. This is
typically 1500.

Historically, the max Ethernet frame size was 1518. But with 802.1Q
which added the VLAN header, all modern hardware actual uses 1522 to
accommodate the extra 4 bytes VLAN header. So i would not actually put
max-frame-size = <1518> anywhere, because it will get copy/pasted and
break VLAN setups.

It looks like the ibm,emac.txt makes this error, max-frame-size =
<5dc>; 0x5dc is 1500. And there are a few powerpc .dtc using
1500/0x5dc, which are probably broken.

Andrew

2023-02-07 07:10:22

by Janne Grunau

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] dt-bindings: net: Add network-class.yaml schema

On 2023-02-07 02:34:41 +0100, Andrew Lunn wrote:
> > > > I've ignored "max-frame-size" since the description in
> > > > ethernet-controller.yaml claims there is a contradiction in the
> > > > Devicetree specification. I suppose it is describing the property
> > > > "max-frame-size" with "Specifies maximum packet length ...".
> > >
> > > Please include it and we'll fix the spec. It is clearly wrong. 2 nios
> > > boards use 1518 and the consumer for them says it is MTU. Everything
> > > else clearly uses mtu with 1500 or 9000.
> >
> > Ok, the example in the pdf is 'max-frame-size = <1518>;'. I'll include
> > it with the description of ethernet-controller.yaml which specifies it
> > as MTU.
>
> You need to be careful here. Frame and MTU are different things.

yes, we are aware. The description in of the property in
Documentation/devicetree/bindings/net/ethernet-controller.yaml is:

| Maximum transfer unit (IEEE defined MTU), rather than the
| maximum frame size (there\'s contradiction in the Devicetree
| Specification).

The description for the property in the Devicetree is:

| Specifies maximum packet length in bytes that the physical interface
| can send and receive.

While the "packet length" in the description is a little confusing this
seems to refer to the ethernet frame size.

> The IEEE 802.3 standard says nothing about MTU. I believe MTU is an IP
> concept. It is the size of the SDU an Ethernet PDU can carry. This is
> typically 1500.
>
> Historically, the max Ethernet frame size was 1518. But with 802.1Q
> which added the VLAN header, all modern hardware actual uses 1522 to
> accommodate the extra 4 bytes VLAN header. So i would not actually put
> max-frame-size = <1518> anywhere, because it will get copy/pasted and
> break VLAN setups.
>
> It looks like the ibm,emac.txt makes this error, max-frame-size =
> <5dc>; 0x5dc is 1500. And there are a few powerpc .dtc using
> 1500/0x5dc, which are probably broken.

I would not say it is an error. The specification/name and use of
"max-frame-size" has clearly diverged. All 4 in-tree users of this
property interpret it as MTU. With the exception of the 2 nios2 boards
Rob found all device trees use either 1500, 3800 or 9000 as
'max-frame-size'.

I think Rob's plan to deal with this conflict between specification and
actual use is to accept the use and update the description in the
specification. This results in a "max-frame-size" property which
describes the maximal payload / MTU. The upside of this is that we can
leave all devicetrees and drivers unchanged and avoid breaking
out-of-tree users.

I'll fix the 2 nios2 boards since those currently end up with a MTU of
1518 in altera_tse_main.c.

Janne