[OpenWrt-Devel] [PATCH v2 1/2] base-files: always store label MAC address in uci system config

[Adrian Schmutzler]

Hi,

> -----Original Message-----
> From: Kristian Evensen [mailto:kristian.evensen at gmail.com]
> Sent: Montag, 4. November 2019 14:57
> To: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> Cc: OpenWrt Development List <openwrt-devel at lists.openwrt.org>
> Subject: Re: [OpenWrt-Devel] [PATCH v2 1/2] base-files: always store label MAC
> address in uci system config
> 
> Hi Adrian,
> 
> On Mon, Nov 4, 2019 at 11:44 AM Adrian Schmutzler
> <freifunk at adrianschmutzler.de> wrote:
> >
> > If set, label MAC address is available from one of two sources,
> > device tree or board.json. So far, the function get_mac_label
> > was meant for retrieving the address, while an option in uci
> > system config was specified only for case 2 (board.json).
> >
> > Since this has been perceived as counter-intuitive, this patch
> > changes front-end access to the label MAC address:
> > During first-boot, the label MAC address will be written to uci
> > system config file for both cases, no matter whether is was
> > specified in DT or in board.json (via 02_network). A user of
> > the label MAC address will then read the value from
> > system. at system[0].label_macaddr, which is easier and more intuitive
> > than using a function and still have an uci value set.
> >
> > Since this is only changing the access to the label MAC address, it
> > won't interfere with the addresses stored in the code base so far.
> >
> > Signed-off-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> 
> I am not an authority on anything, but I don't think a "runtime" value
> like the label mac should be stored in a persistent configuration
> file. For example, if someone makes a mistake with the label MAC, you
> would need a uci-default script for fixing up the config. You also
> have the issue of devices that have already been installed, without a
> uci-defaults script they will not have a label mac in UCI.
> 
> Instead, I would keep board.json as the source for label MAC and have
> get_mac_label parse the JSON-file. I guess you might also have to
> patch the generation of board.json to include the device tree. At
> least I think that board.json is as easy to work with as uci from
> scripts, Luci, etc.

I also thought about that.

However, I'm not aware of a case where board.json is used for anything else than setting up config files, like in a script with user-interaction etc.
So, I hesitate to start with that (as it might also be counter-intuitive for some people).

Concerning address fixes: There might also be users that don't want the address to change if it has been wrong for some time and they have implemented that particular address into their system. This might be more relevant for downstream projects than for use cases directly in OpenWrt, but this is actually a somewhat separate aspect. Despite, having the uci option will allow users to change the address (if it's wrong or for other reasons) or provide it when upstream support is not there (yet). Just having a function reading from DT/board.json would be much less user-friendly there.
However, I do admit that my arguments here are practical, while you are right that strictly the label MAC address is a device parameter that should not be altered by the user.

> You also
> have the issue of devices that have already been installed, without a
> uci-defaults script they will not have a label mac in UCI.

That's a valid argument I haven't thought about before, and it eliminates/weakens most of my arguments from the previous paragraph.

So, if I want to bring the label MAC address to users updating to 20.xx/master, I'd either have to use board.json in get_mac_label, or add label_macaddr for those not having it even though /etc/config/system already exists. One could still think about providing a label_macaddr option in uci only for _overwriting_ the provided value.

I see, I will have to think about that (again) for some time ...

Thanks for the input

Adrian


> 
> BR,
> Kristian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20191104/900fe8e1/attachment.sig>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel