luci-mod-network: don't implicitly move bridge opts into device sections
authorJo-Philipp Wich <jo@mein.io>
Mon, 29 Mar 2021 14:00:10 +0000 (16:00 +0200)
committerJo-Philipp Wich <jo@mein.io>
Mon, 29 Mar 2021 14:00:10 +0000 (16:00 +0200)
When setting bridge and device specific options such 'stp' or
'igmp_snooping', LuCI so far transparently created or reused a
`config device` section and set the corresponding option there.

In the case of bridges, this triggers multiple problems:

 - When implicitely creating a `config device` section referring to the
   bridge device, the legacy bridge configuration of the corresponding
   interface is disabled, causing a broken configuration on subsequent
   save operations

 - Netifd does not appear to properly merge bridge settings from config
   device and config interface sections, leading to an incoherent
   configuration state

In order to avoid that issue, do not automatically migrate bridge specific
options.

Fixes: #4948
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
modules/luci-mod-network/htdocs/luci-static/resources/tools/network.js

index 66d4eae9e2faa106c1c532e54aacda34612d0299..829ab1496385d265f7d9979d758d5367e085008f 100644 (file)
@@ -604,64 +604,64 @@ return baseclass.extend({
                };
                o.depends('type', 'bridge');
 
                };
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, gensection, form.Flag, 'bridge_empty', _('Bring up empty bridge'), _('Bring up the bridge interface even if no ports are attached'));
+               o = this.replaceOption(s, gensection, form.Flag, 'bridge_empty', _('Bring up empty bridge'), _('Bring up the bridge interface even if no ports are attached'));
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Value, 'priority', _('Priority'));
+               o = this.replaceOption(s, advsection, form.Value, 'priority', _('Priority'));
                o.placeholder = '32767';
                o.datatype = 'range(0, 65535)';
                o.depends('type', 'bridge');
 
                o.placeholder = '32767';
                o.datatype = 'range(0, 65535)';
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Value, 'ageing_time', _('Ageing time'), _('Timeout in seconds for learned MAC addresses in the forwarding database'));
+               o = this.replaceOption(s, advsection, form.Value, 'ageing_time', _('Ageing time'), _('Timeout in seconds for learned MAC addresses in the forwarding database'));
                o.placeholder = '30';
                o.datatype = 'uinteger';
                o.depends('type', 'bridge');
 
                o.placeholder = '30';
                o.datatype = 'uinteger';
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Flag, 'stp', _('Enable <abbr title="Spanning Tree Protocol">STP</abbr>'), _('Enables the Spanning Tree Protocol on this bridge'));
+               o = this.replaceOption(s, advsection, form.Flag, 'stp', _('Enable <abbr title="Spanning Tree Protocol">STP</abbr>'), _('Enables the Spanning Tree Protocol on this bridge'));
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Value, 'hello_time', _('Hello interval'), _('Interval in seconds for STP hello packets'));
+               o = this.replaceOption(s, advsection, form.Value, 'hello_time', _('Hello interval'), _('Interval in seconds for STP hello packets'));
                o.placeholder = '2';
                o.datatype = 'range(1, 10)';
                o.depends({ type: 'bridge', stp: '1' });
 
                o.placeholder = '2';
                o.datatype = 'range(1, 10)';
                o.depends({ type: 'bridge', stp: '1' });
 
-               o = this.addOption(s, advsection, form.Value, 'forward_delay', _('Forward delay'), _('Time in seconds to spend in listening and learning states'));
+               o = this.replaceOption(s, advsection, form.Value, 'forward_delay', _('Forward delay'), _('Time in seconds to spend in listening and learning states'));
                o.placeholder = '15';
                o.datatype = 'range(2, 30)';
                o.depends({ type: 'bridge', stp: '1' });
 
                o.placeholder = '15';
                o.datatype = 'range(2, 30)';
                o.depends({ type: 'bridge', stp: '1' });
 
-               o = this.addOption(s, advsection, form.Value, 'max_age', _('Maximum age'), _('Timeout in seconds until topology updates on link loss'));
+               o = this.replaceOption(s, advsection, form.Value, 'max_age', _('Maximum age'), _('Timeout in seconds until topology updates on link loss'));
                o.placeholder = '20';
                o.datatype = 'range(6, 40)';
                o.depends({ type: 'bridge', stp: '1' });
 
 
                o.placeholder = '20';
                o.datatype = 'range(6, 40)';
                o.depends({ type: 'bridge', stp: '1' });
 
 
-               o = this.addOption(s, advsection, form.Flag, 'igmp_snooping', _('Enable <abbr title="Internet Group Management Protocol">IGMP</abbr> snooping'), _('Enables IGMP snooping on this bridge'));
+               o = this.replaceOption(s, advsection, form.Flag, 'igmp_snooping', _('Enable <abbr title="Internet Group Management Protocol">IGMP</abbr> snooping'), _('Enables IGMP snooping on this bridge'));
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
                o.default = o.disabled;
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Value, 'hash_max', _('Maximum snooping table size'));
+               o = this.replaceOption(s, advsection, form.Value, 'hash_max', _('Maximum snooping table size'));
                o.placeholder = '512';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', igmp_snooping: '1' });
 
                o.placeholder = '512';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', igmp_snooping: '1' });
 
-               o = this.addOption(s, advsection, form.Flag, 'multicast_querier', _('Enable multicast querier'));
+               o = this.replaceOption(s, advsection, form.Flag, 'multicast_querier', _('Enable multicast querier'));
                o.defaults = { '1': [{'igmp_snooping': '1'}], '0': [{'igmp_snooping': '0'}] };
                o.depends('type', 'bridge');
 
                o.defaults = { '1': [{'igmp_snooping': '1'}], '0': [{'igmp_snooping': '0'}] };
                o.depends('type', 'bridge');
 
-               o = this.addOption(s, advsection, form.Value, 'robustness', _('Robustness'), _('The robustness value allows tuning for the expected packet loss on the network. If a network is expected to be lossy, the robustness value may be increased. IGMP is robust to (Robustness-1) packet losses'));
+               o = this.replaceOption(s, advsection, form.Value, 'robustness', _('Robustness'), _('The robustness value allows tuning for the expected packet loss on the network. If a network is expected to be lossy, the robustness value may be increased. IGMP is robust to (Robustness-1) packet losses'));
                o.placeholder = '2';
                o.datatype = 'min(1)';
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
                o.placeholder = '2';
                o.datatype = 'min(1)';
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
-               o = this.addOption(s, advsection, form.Value, 'query_interval', _('Query interval'), _('Interval in centiseconds between multicast general queries. By varying the value, an administrator may tune the number of IGMP messages on the subnet; larger values cause IGMP Queries to be sent less often'));
+               o = this.replaceOption(s, advsection, form.Value, 'query_interval', _('Query interval'), _('Interval in centiseconds between multicast general queries. By varying the value, an administrator may tune the number of IGMP messages on the subnet; larger values cause IGMP Queries to be sent less often'));
                o.placeholder = '12500';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
                o.placeholder = '12500';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
-               o = this.addOption(s, advsection, form.Value, 'query_response_interval', _('Query response interval'), _('The max response time in centiseconds inserted into the periodic general queries. By varying the value, an administrator may tune the burstiness of IGMP messages on the subnet; larger values make the traffic less bursty, as host responses are spread out over a larger interval'));
+               o = this.replaceOption(s, advsection, form.Value, 'query_response_interval', _('Query response interval'), _('The max response time in centiseconds inserted into the periodic general queries. By varying the value, an administrator may tune the burstiness of IGMP messages on the subnet; larger values make the traffic less bursty, as host responses are spread out over a larger interval'));
                o.placeholder = '1000';
                o.datatype = 'uinteger';
                o.validate = function(section_id, value) {
                o.placeholder = '1000';
                o.datatype = 'uinteger';
                o.validate = function(section_id, value) {
@@ -675,7 +675,7 @@ return baseclass.extend({
                };
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
                };
                o.depends({ type: 'bridge', multicast_querier: '1' });
 
-               o = this.addOption(s, advsection, form.Value, 'last_member_interval', _('Last member interval'), _('The max response time in centiseconds inserted into group-specific queries sent in response to leave group messages. It is also the amount of time between group-specific query messages. This value may be tuned to modify the "leave latency" of the network. A reduced value results in reduced time to detect the loss of the last member of a group'));
+               o = this.replaceOption(s, advsection, form.Value, 'last_member_interval', _('Last member interval'), _('The max response time in centiseconds inserted into group-specific queries sent in response to leave group messages. It is also the amount of time between group-specific query messages. This value may be tuned to modify the "leave latency" of the network. A reduced value results in reduced time to detect the loss of the last member of a group'));
                o.placeholder = '100';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', multicast_querier: '1' });
                o.placeholder = '100';
                o.datatype = 'uinteger';
                o.depends({ type: 'bridge', multicast_querier: '1' });