dwc(4) Ethernet MAC address setting/generating

[bz]

Summary

Currently dwc(4) supports an Ethernet MAC address from hardware (usually set by the boot loader), or by generating a random one with a "BSD" prefix for the OUI.

Change the logic to first try a HW set address, or if that fails check the FDT. If neither works return an error and after if_alloc() in the attach function use ether_gen_addr() to create a (random) Ethernet MAC under the FreeBSD OUI. If we do support getting a cpuid and setting the host uuid based on that the generated MAC should also be stable. This seems to be a better solution than using a pseudo-random on a locally administered OUI.

Setting the "local-mac-address" from loader can be done, e.g., on a NanoPC-T4 with:
fdt mkprop /ethernet@fe300000/local-mac-address [ 0x58 0x9c 0xfc 0xde 0xc0 0xde ]

Test Plan

Cannot test the HW (boot loader) set address currently; the other two options seem to work.

[manu]

The bindings (https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L16) seems to indicate
that mac-address should be used if it exists and is prefered to local-mac-address.
Maybe change dwc_get_hwaddr_fdt to handle that and make it a generic helper ?

[bz]

In D23145#507260, @manu wrote:

The bindings (https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L16) seems to indicate
that mac-address should be used if it exists and is prefered to local-mac-address.
Maybe change dwc_get_hwaddr_fdt to handle that and make it a generic helper ?

Should we support both as USB does here? https://svnweb.freebsd.org/base/head/sys/dev/usb/usb_fdt_support.c?annotate=347974#l100

If it were to become a "generic" function, where would we put it?

[manu]

In D23145#507266, @bz wrote:

In D23145#507260, @manu wrote:

The bindings (https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L16) seems to indicate
that mac-address should be used if it exists and is prefered to local-mac-address.
Maybe change dwc_get_hwaddr_fdt to handle that and make it a generic helper ?

Should we support both as USB does here? https://svnweb.freebsd.org/base/head/sys/dev/usb/usb_fdt_support.c?annotate=347974#l100

The usb code looks correct. yes.

If it were to become a "generic" function, where would we put it?

No idea :)

[kevans]

In D23145#507267, @manu wrote:

In D23145#507266, @bz wrote:

If it were to become a "generic" function, where would we put it?

No idea :)

I'd be tempted to stick such a thing in ^/sys/dev/fdt/fdt_common.c, since it's (edit: not) hooking much into ethernet stuff -- just determining a MAC address to use for a given node.

[manu]

In D23145#507269, @kevans wrote:

In D23145#507267, @manu wrote:

In D23145#507266, @bz wrote:

If it were to become a "generic" function, where would we put it?

No idea :)

I'd be tempted to stick such a thing in ^/sys/dev/fdt/fdt_common.c, since it's (edit: not) hooking much into ethernet stuff -- just determining a MAC address to use for a given node.

Yes that should be ok since we're only adding one net-related function, if we grew more net-related helpers we could always move those to dedicated file.

[bz]

Update the change factoring out the fdt logic to resemble the USB one.
Is this what you were thinking of?

(currently untested)

[bz]

Any comments on this? I'd love to get the change out of my tree soon..
I am also interested in "this is what we were thinking of" comments.

[kevans]

In D23145#520806, @bz wrote:

Any comments on this? I'd love to get the change out of my tree soon..
I am also interested in "this is what we were thinking of" comments.

I did wonder if the common fdt method should guarantee a MAC address and call ether_gen_addr() itself if it couldn't find an applicable fdt prop, maybe with an ENOENT to indicate that it was generated rather than found (0)- this would avoid drivers having to then call ether_gen_addr() themselves if it fails, which I kind of suspect might be the common case. I'm unsure, though.

[bz]

In D23145#520891, @kevans wrote:

In D23145#520806, @bz wrote:

Any comments on this? I'd love to get the change out of my tree soon..
I am also interested in "this is what we were thinking of" comments.

I did wonder if the common fdt method should guarantee a MAC address and call ether_gen_addr() itself if it couldn't find an applicable fdt prop, maybe with an ENOENT to indicate that it was generated rather than found (0)- this would avoid drivers having to then call ether_gen_addr() themselves if it fails, which I kind of suspect might be the common case. I'm unsure, though.

The problem is that in a few cases, such as dwc, we read the HW address before we reset and that is before we allocate an ifp; without an ifp you cannot call ether_gen_addr().
We could restructure a lot of driver code to accommodate this; but the NIC drivers are essentially all following the same logic order.

[bz]

Anyone else any comments?

[bz]

ping6? Or else it goes in... which I don't know if it's the best idea ...

[adrian]

sys/dev/fdt/fdt_common.c

644

My only request is to abstract out the all-0's or all-1's check into something that I can call from drivers. You actually do this in if_dwc.c :-)

I've had to handle the case of missing ethernet MACs before and it would be nice to be able to do a quick if (! not_assigned(mac)) { eth_gen_random() ; } call in the driver init path so I can program it into the hardware.

[bz]

I've observed cases where this logic doesn't help and I need the reverse order.
I'd love the input from embedded people before changing and/or committing it in the next days.

sys/dev/dwc/if_dwc.c

1037

So I have a "funny" case now where the HW presents me with a random garage MAC and this logic doesn't help me in this order because even if I (re-)set the FDT property the HW address is "valid" and hence I never make it to it.

What do we do in these cases? Should we do the opposite? Should we take it from the FDT if it is set there (given boot loaders might also set that if they find the HW address) and then one can re-set that to something properly and only if there's nothing set fall back and try to do the HW MAC?

Given this is likely used for all kinds of gadgets which do not have a unique MAC burned into an NVRAM when shipping it seems to me a better option nowadays to swap the order?