search

openwrt / netifd

[MIRROR] OpenWrt Network interface configuration daemon
https://git.openwrt.org/?p=project/netifd.git;
17 stars 19 forks source link

config: network-device is incompatible with jshn #13

Closed MadnessASAP closed 10 months ago

MadnessASAP commented 11 months ago

An oversight in openwrts uci-defaults.sh ucidef_set_networkdevice* functions resulted in a hyphen (-) being used insted of a underscore (_) this causes problems for jshn which will translate the former to the latter. This causes config_init_board to return null on boards that set network?device using uci-defaults (or jshn).

This patch corrects the issue by defaulting to network_device with a fallback to network-device if nothing is returned.

Signed-off-by: Michael 'ASAP' Weinrich

MadnessASAP commented 11 months ago

Related https://github.com/openwrt/openwrt/pull/13622

howels commented 10 months ago

Please can someone review this? Unfortunately 23.05 is removing the MAC address for RTL838x switches.

svanheule commented 10 months ago

As I mentioned in the OpenWrt PR, I don't think we should support both versions. The underscore version (i.e. the only one fully supported by jshn) is enough IMHO, and AFAICT board.json shouldn't be expected to be stable anyway.

Perhaps a Fixes could also be added: Fixes: 42c48866f1c1 ("config: parse default mac address from board.json")

The way I see it, jshn just shouldn't perform silent key normalization and should be more consistent in normalizing, or should just not normalize at all and (consistently) throw an error instead.

Ansuel commented 10 months ago

The way I see it, jshn just shouldn't perform silent key normalization and should be more consistent in normalizing, or should just not normalize at all and (consistently) throw an error instead.

ehhh i feel this is one of the case where dev needs to be aware that something must not be used... (due to limitation of the template format used for example) maybe normalization should be denied for sections and just used for some value???

svanheule commented 10 months ago

I'll see if I can make some time this week whip up a patch for libubox to discuss.

MadnessASAP commented 10 months ago

If board.json is truly considered to be unstable then my opinion is that support for the hyphen should be dropped entirely. I just wanted to guard against the possibility that someone, somewhere is counting on the old behavior.

Let me know what you'd like to see and I'll make the change.

Ansuel commented 10 months ago

Handled by 5590a80e2566d378be955f61c287a63fb3bdf329