[PATCH] base-files: migrate old UCI network sections defining bridges

[Rafał Miłecki]

On 16.05.2021 12:10, Paul Oranje (BB) wrote:
> I like this setup. Just some questions and a tiny nit pick.
> Paul
> 
> Op 15 mei 2021, om 23:11 heeft Rafał Miłecki <zajec5 at gmail.com> het volgende geschreven:
>>
>> From: Rafał Miłecki <rafal at milecki.pl>
>>
>> Old "interface" sections for bridges were mixing layer 2 and layer 3.
>> Migrate them to the new styles that has:
>> 1. "device" UCI section for L2 bridge
>> 2. "interface" UCI section for L3 interface
> 
> IIUC for a non-bridge ifname refers to a netdev that will implicitly be created without an explicit UCI device section.
> IIUC a device (L2 bridge) can have a VLAN as port (e.g. when converting interface.ifname=eth1.10).
> IIUC for a bridge with an empty ifname a device section without ports will be created.

Sounds correct.

Please note that netifd actually parses "old" bridges and "new" bridges
with the same code. So this switching from "old" config style to the
"new" config style doesn't actually change much about bridges
processing.


> You probably also considered renaming the interface option ifname (devname). What made you decide to stick with ifname ?

I just didn't have time to handle that yet. I'll come later.


>> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
>> ---
>> .../files/etc/uci-defaults/11_network-bridge  | 29 +++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>> create mode 100644 package/base-files/files/etc/uci-defaults/11_network-bridge
>>
>> diff --git a/package/base-files/files/etc/uci-defaults/11_network-bridge b/package/base-files/files/etc/uci-defaults/11_network-bridge
>> new file mode 100644
>> index 0000000000..7a53e8e355
>> --- /dev/null
>> +++ b/package/base-files/files/etc/uci-defaults/11_network-bridge
>> @@ -0,0 +1,29 @@
>> +. /lib/functions.sh
>> +
>> +migrate_bridge() {
>> +	local config="$1"
> interface i.s.o. config ?

Took me a moment to decrypt your "i.s.o.".

A matter of taste, I don't have strong opiniong on this. In OpenWrt
config_foreach callbacks often use "config" variable name for the first
argument.


>> +	local type ifname
>> +
>> +	config_get type "$config" type
>> +	[ "$type" != "bridge" ] && return
>> +
>> +	config_get ifname "$config" ifname
>> +
>> +	uci -q batch <<-EOF
>> +		add network device
>> +		set network. at device[-1].name='br-$1'
> $config i.s.o. $1

Definitely.


>> +		set network. at device[-1].type='bridge'
>> +	EOF
>> +	for port in $ifname; do uci add_list network. at device[-1].ports="$port"; done
>> +
>> +	uci -q batch <<-EOF
>> +		delete network.$1.type
>> +		set network.$1.ifname='br-$1'
> $config i.s.o. $1

Good point.