Refactor duplicate perl code and add comments
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 53 +++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 0914ceb78..bf46ab0c7 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -26,6 +26,7 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl";
+###--- HTML HEAD ---### my $extraHead = <<END <style> table#zoneconf { @@ -105,7 +106,9 @@ my $extraHead = <<END <script src="/include/zoneconf.js"></script> END ; +###--- END HTML HEAD ---###
+### Read configuration ### my %ethsettings = (); my %vlansettings = (); my %cgiparams = (); @@ -119,7 +122,7 @@ my $restart_notice = ""; &Header::showhttpheaders();
# Define all zones we will check for NIC assignment -my @zones = ("green", "red", "orange", "blue"); +my @zones = ("red", "green", "orange", "blue");
# Get all physical NICs present opendir(my $dh, "/sys/class/net/"); @@ -153,6 +156,21 @@ foreach (@nics) { } }
+### Functions ### + +# Check if a zone is in IP mode or in PPP, PPPoE, VDSL, ... mode +sub is_zonetype_ip { + my $zone_type = shift; + return ($zone_type eq "STATIC" || $zone_type eq "DHCP"); +} + +# Check if a zone is activated (device assigned) +sub is_zone_activated { + my $zone = uc shift; + return ($ethsettings{"${zone}_DEV"} ne ""); +} + +### START PAGE ### &Header::openpage($Lang::tr{"zoneconf title"}, 1, $extraHead); &Header::openbigbox('100%', 'center');
@@ -195,6 +213,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } }
+ # skip NIC/VLAN assignment and additional zone options for RED in PPP mode next; }
@@ -278,6 +297,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } }
+ # validation failed, show error message and exit if ($VALIDATE_error) { &Header::openbox('100%', 'left', $Lang::tr{"error"});
@@ -290,16 +310,17 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { exit 0; }
+ # new settings are valid, write configuration files &General::writehash("${General::swroot}/ethernet/settings",%ethsettings); &General::writehash("${General::swroot}/ethernet/vlans",%vlansettings);
$restart_notice = $Lang::tr{'zoneconf notice reboot'}; }
-&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"}); - ### START OF TABLE ###
+&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"}); + print <<END <form method='post' enctype='multipart/form-data'> <table id="zoneconf"> @@ -311,19 +332,16 @@ END # Fill the table header with all activated zones foreach (@zones) { my $uc = uc $_; - my $dev_name = $ethsettings{"${uc}_DEV"};
- if ($dev_name eq "") { # If the zone is not activated, don't show it - next; - } + # If the zone is not activated, don't show it + next unless is_zone_activated($_);
- # If the zone is in PPP mode, don't show a mode dropdown + # If the red zone is in PPP mode, don't show a mode dropdown if ($uc eq "RED") { my $red_type = $ethsettings{"RED_TYPE"}; - my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));
- if ($red_restricted) { - print "\t\t<td class='heading $_'>$uc ($red_type)</td>\n"; + unless (is_zonetype_ip($red_type)) { + print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n";
next; # We're done here } @@ -354,6 +372,7 @@ END
print "\t</tr>\n";
+# NIC assignment matrix foreach (@nics) { my $mac = $_->[0]; my $nic = $_->[1]; @@ -365,19 +384,14 @@ foreach (@nics) { # Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) { my $uc = uc $_; - my $dev_name = $ethsettings{"${uc}_DEV"}; my $highlight = "";
- if ($dev_name eq "") { # Again, skip the zone if it is not activated - next; - } + # If the zone is not activated, don't show it + next unless is_zone_activated($_);
if ($uc eq "RED") { - my $red_type = $ethsettings{"RED_TYPE"}; - my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP")); - # VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ... - if ($red_restricted) { + unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) { my $checked = "";
if ($mac eq $ethsettings{"${uc}_MACADDR"}) { @@ -449,6 +463,7 @@ END print "\t</tr>\n"; }
+# footer and submit button print <<END </table>
Simplify borders, load more colors from header and add dividers
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 42 ++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index bf46ab0c7..13543d244 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -32,39 +32,59 @@ my $extraHead = <<END table#zoneconf { width: 100%; border-collapse: collapse; + border-style: hidden; table-layout: fixed; }
+ /* row height */ #zoneconf tr { height: 4em; } + /* section separators */ + #zoneconf tr.divider-top { + border-top: 2px solid $Header::bordercolour; + } + #zoneconf tr.divider-bottom { + border-bottom: 2px solid $Header::bordercolour; + }
+ /* table cells */ #zoneconf td { padding: 5px 10px; - border: 0.5px solid black; + border-left: 0.5px solid $Header::bordercolour; text-align: center; }
- /* dark grey header cells */ + /* grey header cells */ #zoneconf td.heading { - background-color: grey; + background-color: lightgrey; color: white; } - #zoneconf td.heading::first-line { + #zoneconf td.heading.bold::first-line { font-weight: bold; line-height: 1.6; }
- /* narrow left column */ + /* narrow left column with background color */ #zoneconf tr > td:first-child { width: 11em; } + #zoneconf tr.nic-row > td:first-child { + background-color: darkgray; + } + #zoneconf tr.nic-row { + border-bottom: 0.5px solid $Header::bordercolour; + }
/* alternating row background color */ + #zoneconf tr { + background-color: $Header::table2colour; + } #zoneconf tr:nth-child(2n+3) { - background-color: #F0F0F0; + background-color: $Header::table1colour; }
+ /* special cell colors */ #zoneconf td.green { background-color: $Header::colourgreen; } @@ -83,8 +103,6 @@ my $extraHead = <<END
#zoneconf td.topleft { background-color: $Header::pagecolour; - border-top-style: none; - border-left-style: none; }
input.vlanid { @@ -324,7 +342,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { print <<END <form method='post' enctype='multipart/form-data'> <table id="zoneconf"> - <tr> + <tr class="divider-bottom"> <td class="topleft"></td> END ; @@ -359,7 +377,7 @@ foreach (@zones) { }
print <<END - <td class='heading $_'>$uc<br> + <td class='heading bold $_'>$uc<br> <select name="MODE $uc"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> @@ -378,8 +396,8 @@ foreach (@nics) { my $nic = $_->[1]; my $wlan = $_->[2];
- print "\t<tr>\n"; - print "\t\t<td class='heading'>$nic<br>$mac</td>\n"; + print "\t<tr class='nic-row'>\n"; + print "\t\t<td class='heading bold'>$nic<br>$mac</td>\n";
# Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) {
Changes & new features: - Add CSS for STP options, add texts to language files - Read STP settings from ethernet configuration and display inputs - Validate and save STP settings
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 100 ++++++++++++++++++++++++++++++++++++++ langs/de/cgi-bin/de.pl | 4 ++ langs/en/cgi-bin/en.pl | 4 ++ 3 files changed, 108 insertions(+)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 13543d244..1d30450ed 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -40,6 +40,13 @@ my $extraHead = <<END #zoneconf tr { height: 4em; } + #zoneconf tr.half-height { + height: 2em; + } + #zoneconf tr.half-height > td { + padding: 2px 10px; + } + /* section separators */ #zoneconf tr.divider-top { border-top: 2px solid $Header::bordercolour; @@ -75,6 +82,9 @@ my $extraHead = <<END #zoneconf tr.nic-row { border-bottom: 0.5px solid $Header::bordercolour; } + #zoneconf tr.option-row > td:first-child { + background-color: gray; + }
/* alternating row background color */ #zoneconf tr { @@ -108,6 +118,9 @@ my $extraHead = <<END input.vlanid { width: 4em; } + input.stp-priority { + width: 5em; + }
#submit-container { width: 100%; @@ -313,6 +326,25 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } elsif ($zone_mode eq "MACVTAP") { $ethsettings{"${uc}_MODE"} = "macvtap"; } + + # STP options + # (this has already been skipped when RED is in PPP mode, so we don't need to check for PPP here) + $ethsettings{"${uc}_STP"} = ""; + my $stp_enabled = $cgiparams{"STP-$uc"} eq "on"; + my $stp_priority = $cgiparams{"STP-PRIORITY-$uc"}; + + if($stp_enabled) { + unless($ethsettings{"${uc}_MODE"} eq "bridge") { # STP is only available in bridge mode + $VALIDATE_error = $Lang::tr{"zoneconf val stp zone mode error"}; + last; + } + unless (looks_like_number($stp_priority) && ($stp_priority >= 1) && ($stp_priority <= 65535)) { # STP bridge priority range: 1..65535 + $VALIDATE_error = $Lang::tr{"zoneconf val stp priority range error"}; + last; + } + $ethsettings{"${uc}_STP"} = "on"; # network-hotplug-bridges expects "on" + $ethsettings{"${uc}_STP_PRIORITY"} = $stp_priority; + } }
# validation failed, show error message and exit @@ -481,6 +513,74 @@ END print "\t</tr>\n"; }
+# STP options +my @stp_html = (); # form fields buffer (two rows) + +foreach (@zones) { # load settings and prepare form elements for each zone + my $uc = uc $_; + + # skip if zone is not activated + next unless is_zone_activated($_); + + # STP is not available if the RED interface is set to PPP, PPPoE, VDSL, ... + if ($uc eq "RED") { + unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) { + push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cell + next; + } + } + + # load configuration + my $stp_available = $ethsettings{"${uc}_MODE"} eq "bridge"; # STP is only available in bridge mode + my $stp_enabled = $ethsettings{"${uc}_STP"} eq "on"; + my $stp_priority = $ethsettings{"${uc}_STP_PRIORITY"}; + + # form element modifiers + my $checked = ""; + my $disabled = ""; + $checked = "checked" if ($stp_available && $stp_enabled); + $disabled = "disabled" unless $stp_available; + + # enable checkbox HTML + my $row_1 = <<END + <td> + <input type="checkbox" name="STP-$uc" $disabled $checked> + </td> +END +; + $disabled = "disabled" unless $stp_enabled; # STP priority can't be entered if STP is disabled + + # priority input box HTML + my $row_2 = <<END + <td> + <input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled> + </td> +END +; + # add fields to buffer + push(@stp_html, [$row_1, $row_2]); +} + +# print two rows of prepared form elements +print <<END + <tr class="half-height divider-top option-row"> + <td class="heading bold">$Lang::tr{"zoneconf stp enable"}</td> +END +; +foreach (@stp_html) { + print $_->[0]; # row 1 +} +print <<END + </tr> + <tr class="half-height option-row"> + <td class="heading">$Lang::tr{"zoneconf stp priority"}</td> +END +; +foreach (@stp_html) { + print $_->[1]; # row 2 +} +print "\t</tr>\n"; + # footer and submit button print <<END </table> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl index 38c9783f8..f3e4aca7e 100644 --- a/langs/de/cgi-bin/de.pl +++ b/langs/de/cgi-bin/de.pl @@ -2977,9 +2977,13 @@ 'zoneconf nicmode default' => 'Normal', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Bitte einen Neustart durchführen, um die Änderungen zu übernehmen.', +'zoneconf stp enable' => 'STP aktivieren', +'zoneconf stp priority' => 'Brücke Priorität', 'zoneconf title' => 'Zonen einrichten', 'zoneconf val native assignment error' => 'Eine Netzwerkkarte kann nicht von mehreren Zonen nativ verwendet werden.', 'zoneconf val ppp assignment error' => 'Die Netzwerkkarte, die von RED im PPP-Modus verwendet wird, kann keiner anderen Zone zugeordnet werden.', +'zoneconf val stp priority range error' => 'STP Brücke Priorität muss im Bereich 1-65535 liegen', +'zoneconf val stp zone mode error' => 'STP kann nur aktiviert werden, wenn sich die Zone im Brückenmodus befindet', 'zoneconf val vlan amount assignment error' => 'Pro Zone kann nur ein VLAN verwendet werden.', 'zoneconf val vlan tag assignment error' => 'Pro Netzwerkkarte kann derselbe VLAN-Tag nur einmal verwendet werden.', 'zoneconf val zoneslave amount error' => 'Wenn eine Zone nicht im Brückenmodus ist, kann ihr nur eine Netzwerkkarte zugewiesen werden.', diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl index 500913240..649139c73 100644 --- a/langs/en/cgi-bin/en.pl +++ b/langs/en/cgi-bin/en.pl @@ -3025,9 +3025,13 @@ 'zoneconf nicmode default' => 'Default', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Please reboot to apply your changes.', +'zoneconf stp enable' => 'STP enable', +'zoneconf stp priority' => 'Bridge Priority', 'zoneconf title' => 'Zone Configuration', 'zoneconf val native assignment error' => 'A NIC cannot be accessed natively by more than one zone.', 'zoneconf val ppp assignment error' => 'The NIC used for RED in PPP mode cannot be accessed by any other zone.', +'zoneconf val stp priority range error' => 'STP bridge priority must be in the range of 1-65535', +'zoneconf val stp zone mode error' => 'STP can only be enabled if the zone is in bridge mode', 'zoneconf val vlan amount assignment error' => 'A zone cannot have more than one VLAN assigned.', 'zoneconf val vlan tag assignment error' => 'You cannot use the same VLAN tag more than once per NIC.', 'zoneconf val zoneslave amount error' => 'A zone that is not in bridge mode can't have more than one NIC assigned',
Hi,
no problems after applying all four patches so far. Everything works as expected. Further tests are planned for the next days.
But there is one thing that we should append.
If there is no priority configured the priority should be set to 32768.
- Daniel
Am 22.12.20 um 21:06 schrieb Leo-Andres Hofmann:
Changes & new features:
- Add CSS for STP options, add texts to language files
- Read STP settings from ethernet configuration and display inputs
- Validate and save STP settings
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 100 ++++++++++++++++++++++++++++++++++++++ langs/de/cgi-bin/de.pl | 4 ++ langs/en/cgi-bin/en.pl | 4 ++ 3 files changed, 108 insertions(+)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 13543d244..1d30450ed 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -40,6 +40,13 @@ my $extraHead = <<END #zoneconf tr { height: 4em; }
- #zoneconf tr.half-height {
height: 2em;- }
- #zoneconf tr.half-height > td {
padding: 2px 10px;- }
- /* section separators */ #zoneconf tr.divider-top { border-top: 2px solid $Header::bordercolour;
@@ -75,6 +82,9 @@ my $extraHead = <<END #zoneconf tr.nic-row { border-bottom: 0.5px solid $Header::bordercolour; }
#zoneconf tr.option-row > td:first-child {
background-color: gray;}
/* alternating row background color */ #zoneconf tr {
@@ -108,6 +118,9 @@ my $extraHead = <<END input.vlanid { width: 4em; }
input.stp-priority {
width: 5em;}
#submit-container { width: 100%;
@@ -313,6 +326,25 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } elsif ($zone_mode eq "MACVTAP") { $ethsettings{"${uc}_MODE"} = "macvtap"; }
# STP options# (this has already been skipped when RED is in PPP mode, so we don't need to check for PPP here)$ethsettings{"${uc}_STP"} = "";my $stp_enabled = $cgiparams{"STP-$uc"} eq "on";my $stp_priority = $cgiparams{"STP-PRIORITY-$uc"};if($stp_enabled) {unless($ethsettings{"${uc}_MODE"} eq "bridge") { # STP is only available in bridge mode$VALIDATE_error = $Lang::tr{"zoneconf val stp zone mode error"};last;}unless (looks_like_number($stp_priority) && ($stp_priority >= 1) && ($stp_priority <= 65535)) { # STP bridge priority range: 1..65535$VALIDATE_error = $Lang::tr{"zoneconf val stp priority range error"};last;}$ethsettings{"${uc}_STP"} = "on"; # network-hotplug-bridges expects "on"$ethsettings{"${uc}_STP_PRIORITY"} = $stp_priority;}}
# validation failed, show error message and exit
@@ -481,6 +513,74 @@ END print "\t</tr>\n"; }
+# STP options +my @stp_html = (); # form fields buffer (two rows)
+foreach (@zones) { # load settings and prepare form elements for each zone
- my $uc = uc $_;
- # skip if zone is not activated
- next unless is_zone_activated($_);
- # STP is not available if the RED interface is set to PPP, PPPoE, VDSL, ...
- if ($uc eq "RED") {
unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cellnext;}- }
- # load configuration
- my $stp_available = $ethsettings{"${uc}_MODE"} eq "bridge"; # STP is only available in bridge mode
- my $stp_enabled = $ethsettings{"${uc}_STP"} eq "on";
- my $stp_priority = $ethsettings{"${uc}_STP_PRIORITY"};
- # form element modifiers
- my $checked = "";
- my $disabled = "";
- $checked = "checked" if ($stp_available && $stp_enabled);
- $disabled = "disabled" unless $stp_available;
- # enable checkbox HTML
- my $row_1 = <<END
<td><input type="checkbox" name="STP-$uc" $disabled $checked></td>+END +;
- $disabled = "disabled" unless $stp_enabled; # STP priority can't be entered if STP is disabled
- # priority input box HTML
- my $row_2 = <<END
<td><input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled></td>+END +;
- # add fields to buffer
- push(@stp_html, [$row_1, $row_2]);
+}
+# print two rows of prepared form elements +print <<END
<tr class="half-height divider-top option-row">
<td class="heading bold">$Lang::tr{"zoneconf stp enable"}</td>+END +; +foreach (@stp_html) {
- print $_->[0]; # row 1
+} +print <<END
</tr>
<tr class="half-height option-row">
<td class="heading">$Lang::tr{"zoneconf stp priority"}</td>+END +; +foreach (@stp_html) {
- print $_->[1]; # row 2
+} +print "\t</tr>\n";
- # footer and submit button print <<END
</table>
diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl index 38c9783f8..f3e4aca7e 100644 --- a/langs/de/cgi-bin/de.pl +++ b/langs/de/cgi-bin/de.pl @@ -2977,9 +2977,13 @@ 'zoneconf nicmode default' => 'Normal', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Bitte einen Neustart durchführen, um die Änderungen zu übernehmen.', +'zoneconf stp enable' => 'STP aktivieren', +'zoneconf stp priority' => 'Brücke Priorität', 'zoneconf title' => 'Zonen einrichten', 'zoneconf val native assignment error' => 'Eine Netzwerkkarte kann nicht von mehreren Zonen nativ verwendet werden.', 'zoneconf val ppp assignment error' => 'Die Netzwerkkarte, die von RED im PPP-Modus verwendet wird, kann keiner anderen Zone zugeordnet werden.', +'zoneconf val stp priority range error' => 'STP Brücke Priorität muss im Bereich 1-65535 liegen', +'zoneconf val stp zone mode error' => 'STP kann nur aktiviert werden, wenn sich die Zone im Brückenmodus befindet', 'zoneconf val vlan amount assignment error' => 'Pro Zone kann nur ein VLAN verwendet werden.', 'zoneconf val vlan tag assignment error' => 'Pro Netzwerkkarte kann derselbe VLAN-Tag nur einmal verwendet werden.', 'zoneconf val zoneslave amount error' => 'Wenn eine Zone nicht im Brückenmodus ist, kann ihr nur eine Netzwerkkarte zugewiesen werden.', diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl index 500913240..649139c73 100644 --- a/langs/en/cgi-bin/en.pl +++ b/langs/en/cgi-bin/en.pl @@ -3025,9 +3025,13 @@ 'zoneconf nicmode default' => 'Default', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Please reboot to apply your changes.', +'zoneconf stp enable' => 'STP enable', +'zoneconf stp priority' => 'Bridge Priority', 'zoneconf title' => 'Zone Configuration', 'zoneconf val native assignment error' => 'A NIC cannot be accessed natively by more than one zone.', 'zoneconf val ppp assignment error' => 'The NIC used for RED in PPP mode cannot be accessed by any other zone.', +'zoneconf val stp priority range error' => 'STP bridge priority must be in the range of 1-65535', +'zoneconf val stp zone mode error' => 'STP can only be enabled if the zone is in bridge mode', 'zoneconf val vlan amount assignment error' => 'A zone cannot have more than one VLAN assigned.', 'zoneconf val vlan tag assignment error' => 'You cannot use the same VLAN tag more than once per NIC.', 'zoneconf val zoneslave amount error' => 'A zone that is not in bridge mode can't have more than one NIC assigned',
Hi,
On 6 Jan 2021, at 13:20, Daniel Weismüller daniel.weismueller@ipfire.org wrote:
Hi,
no problems after applying all four patches so far. Everything works as expected. Further tests are planned for the next days.
But there is one thing that we should append.
If there is no priority configured the priority should be set to 32768.
Yes, currently the configuration cannot be saved without putting something into that field (as it should be). But it is not obvious for the user that this should be the case. I would also recommend setting the “required” attribute to the input fields.
Apart from that, I can also say that this looks good to be merged.
-Michael
Daniel
Am 22.12.20 um 21:06 schrieb Leo-Andres Hofmann:
Changes & new features:
- Add CSS for STP options, add texts to language files
- Read STP settings from ethernet configuration and display inputs
- Validate and save STP settings
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 100 ++++++++++++++++++++++++++++++++++++++ langs/de/cgi-bin/de.pl | 4 ++ langs/en/cgi-bin/en.pl | 4 ++ 3 files changed, 108 insertions(+)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 13543d244..1d30450ed 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -40,6 +40,13 @@ my $extraHead = <<END #zoneconf tr { height: 4em; }
- #zoneconf tr.half-height {
height: 2em;- }
- #zoneconf tr.half-height > td {
padding: 2px 10px;- }
- /* section separators */ #zoneconf tr.divider-top { border-top: 2px solid $Header::bordercolour;
@@ -75,6 +82,9 @@ my $extraHead = <<END #zoneconf tr.nic-row { border-bottom: 0.5px solid $Header::bordercolour; }
#zoneconf tr.option-row > td:first-child {
background-color: gray;}
/* alternating row background color */ #zoneconf tr {
@@ -108,6 +118,9 @@ my $extraHead = <<END input.vlanid { width: 4em; }
input.stp-priority {
width: 5em;}
#submit-container { width: 100%;
@@ -313,6 +326,25 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } elsif ($zone_mode eq "MACVTAP") { $ethsettings{"${uc}_MODE"} = "macvtap"; }
# STP options# (this has already been skipped when RED is in PPP mode, so we don't need to check for PPP here)$ethsettings{"${uc}_STP"} = "";my $stp_enabled = $cgiparams{"STP-$uc"} eq "on";my $stp_priority = $cgiparams{"STP-PRIORITY-$uc"};if($stp_enabled) {unless($ethsettings{"${uc}_MODE"} eq "bridge") { # STP is only available in bridge mode$VALIDATE_error = $Lang::tr{"zoneconf val stp zone mode error"};last;}unless (looks_like_number($stp_priority) && ($stp_priority >= 1) && ($stp_priority <= 65535)) { # STP bridge priority range: 1..65535$VALIDATE_error = $Lang::tr{"zoneconf val stp priority range error"};last;}$ethsettings{"${uc}_STP"} = "on"; # network-hotplug-bridges expects "on"$ethsettings{"${uc}_STP_PRIORITY"} = $stp_priority;}}
# validation failed, show error message and exit
@@ -481,6 +513,74 @@ END print "\t</tr>\n"; }
+# STP options +my @stp_html = (); # form fields buffer (two rows)
+foreach (@zones) { # load settings and prepare form elements for each zone
- my $uc = uc $_;
- # skip if zone is not activated
- next unless is_zone_activated($_);
- # STP is not available if the RED interface is set to PPP, PPPoE, VDSL, ...
- if ($uc eq "RED") {
unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cellnext;}- }
- # load configuration
- my $stp_available = $ethsettings{"${uc}_MODE"} eq "bridge"; # STP is only available in bridge mode
- my $stp_enabled = $ethsettings{"${uc}_STP"} eq "on";
- my $stp_priority = $ethsettings{"${uc}_STP_PRIORITY"};
- # form element modifiers
- my $checked = "";
- my $disabled = "";
- $checked = "checked" if ($stp_available && $stp_enabled);
- $disabled = "disabled" unless $stp_available;
- # enable checkbox HTML
- my $row_1 = <<END
<td><input type="checkbox" name="STP-$uc" $disabled $checked></td>+END +;
- $disabled = "disabled" unless $stp_enabled; # STP priority can't be entered if STP is disabled
- # priority input box HTML
- my $row_2 = <<END
<td><input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled></td>+END +;
- # add fields to buffer
- push(@stp_html, [$row_1, $row_2]);
+}
+# print two rows of prepared form elements +print <<END
<tr class="half-height divider-top option-row">
<td class="heading bold">$Lang::tr{"zoneconf stp enable"}</td>+END +; +foreach (@stp_html) {
- print $_->[0]; # row 1
+} +print <<END
</tr>
<tr class="half-height option-row">
<td class="heading">$Lang::tr{"zoneconf stp priority"}</td>+END +; +foreach (@stp_html) {
- print $_->[1]; # row 2
+} +print "\t</tr>\n";
# footer and submit button print <<END
</table> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl index 38c9783f8..f3e4aca7e 100644 --- a/langs/de/cgi-bin/de.pl +++ b/langs/de/cgi-bin/de.pl @@ -2977,9 +2977,13 @@ 'zoneconf nicmode default' => 'Normal', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Bitte einen Neustart durchführen, um die Änderungen zu übernehmen.', +'zoneconf stp enable' => 'STP aktivieren', +'zoneconf stp priority' => 'Brücke Priorität', 'zoneconf title' => 'Zonen einrichten', 'zoneconf val native assignment error' => 'Eine Netzwerkkarte kann nicht von mehreren Zonen nativ verwendet werden.', 'zoneconf val ppp assignment error' => 'Die Netzwerkkarte, die von RED im PPP-Modus verwendet wird, kann keiner anderen Zone zugeordnet werden.', +'zoneconf val stp priority range error' => 'STP Brücke Priorität muss im Bereich 1-65535 liegen', +'zoneconf val stp zone mode error' => 'STP kann nur aktiviert werden, wenn sich die Zone im Brückenmodus befindet', 'zoneconf val vlan amount assignment error' => 'Pro Zone kann nur ein VLAN verwendet werden.', 'zoneconf val vlan tag assignment error' => 'Pro Netzwerkkarte kann derselbe VLAN-Tag nur einmal verwendet werden.', 'zoneconf val zoneslave amount error' => 'Wenn eine Zone nicht im Brückenmodus ist, kann ihr nur eine Netzwerkkarte zugewiesen werden.', diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl index 500913240..649139c73 100644 --- a/langs/en/cgi-bin/en.pl +++ b/langs/en/cgi-bin/en.pl @@ -3025,9 +3025,13 @@ 'zoneconf nicmode default' => 'Default', 'zoneconf nicmode macvtap' => 'MacVTap', 'zoneconf notice reboot' => 'Please reboot to apply your changes.', +'zoneconf stp enable' => 'STP enable', +'zoneconf stp priority' => 'Bridge Priority', 'zoneconf title' => 'Zone Configuration', 'zoneconf val native assignment error' => 'A NIC cannot be accessed natively by more than one zone.', 'zoneconf val ppp assignment error' => 'The NIC used for RED in PPP mode cannot be accessed by any other zone.', +'zoneconf val stp priority range error' => 'STP bridge priority must be in the range of 1-65535', +'zoneconf val stp zone mode error' => 'STP can only be enabled if the zone is in bridge mode', 'zoneconf val vlan amount assignment error' => 'A zone cannot have more than one VLAN assigned.', 'zoneconf val vlan tag assignment error' => 'You cannot use the same VLAN tag more than once per NIC.', 'zoneconf val zoneslave amount error' => 'A zone that is not in bridge mode can\'t have more than one NIC assigned',
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de --- html/cgi-bin/zoneconf.cgi | 6 +++--- html/html/include/zoneconf.js | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 1d30450ed..eb6cd0e66 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -410,7 +410,7 @@ foreach (@zones) {
print <<END <td class='heading bold $_'>$uc<br> - <select name="MODE $uc"> + <select name="MODE $uc" data-zone="$uc" onchange="changeZoneMode(this)"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> <option value="MACVTAP" $mode_selected{"MACVTAP"}>$Lang::tr{"zoneconf nicmode macvtap"}</option> @@ -544,7 +544,7 @@ foreach (@zones) { # load settings and prepare form elements for each zone # enable checkbox HTML my $row_1 = <<END <td> - <input type="checkbox" name="STP-$uc" $disabled $checked> + <input type="checkbox" id="STP-$uc" name="STP-$uc" data-zone="$uc" onchange="changeEnableSTP(this)" $disabled $checked> </td> END ; @@ -553,7 +553,7 @@ END # priority input box HTML my $row_2 = <<END <td> - <input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled> + <input type="number" class="stp-priority" id="STP-PRIORITY-$uc" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled> </td> END ; diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js index d76f0ab68..d27a79bc8 100644 --- a/html/html/include/zoneconf.js +++ b/html/html/include/zoneconf.js @@ -54,3 +54,30 @@ function highlightAccess(selectObj) { //if interface is assigned, highlight table cell in zone color colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE')); } + +//update zone mode +function changeZoneMode(selectObj) { + if(!(selectObj && ('zone' in selectObj.dataset))) { + return; //required parameters are missing + } + + // STP enable checkbox + let stpEnable = document.getElementById('STP-' + selectObj.dataset.zone); + if(stpEnable) { + stpEnable.disabled = (selectObj.value !== 'BRIDGE'); //STP is available if zone is in bridge mode + stpEnable.checked = stpEnable.checked && (! stpEnable.disabled); //un-check if disabled + stpEnable.dispatchEvent(new Event('change')); + } +} + +//STP enable checkbox change toggles priority input +function changeEnableSTP(inputObj) { + if(!(inputObj && ('zone' in inputObj.dataset))) { + return; //required parameters are missing + } + + let priority = document.getElementById('STP-PRIORITY-' + inputObj.dataset.zone); + if(priority) { + priority.disabled = inputObj.disabled || (! inputObj.checked); + } +}
Hi Daniel,
just in time for the holidays, I finished my work on the zoneconf.cgi STP GUI. I'm not sure how to test STP in my development setup, but I think I got it right. I would appreciate if you could test these patches, thank you :)
Best regards & happy holidays Leo
Am 22.12.2020 um 21:06 schrieb Leo-Andres Hofmann:
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 6 +++--- html/html/include/zoneconf.js | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 1d30450ed..eb6cd0e66 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -410,7 +410,7 @@ foreach (@zones) {
print <<END <td class='heading bold $_'>$uc<br>
<select name="MODE $uc">
<select name="MODE $uc" data-zone="$uc" onchange="changeZoneMode(this)"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> <option value="MACVTAP" $mode_selected{"MACVTAP"}>$Lang::tr{"zoneconf nicmode macvtap"}</option>@@ -544,7 +544,7 @@ foreach (@zones) { # load settings and prepare form elements for each zone # enable checkbox HTML my $row_1 = <<END <td>
<input type="checkbox" name="STP-$uc" $disabled $checked>
<input type="checkbox" id="STP-$uc" name="STP-$uc" data-zone="$uc" onchange="changeEnableSTP(this)" $disabled $checked></td> END ;
@@ -553,7 +553,7 @@ END # priority input box HTML my $row_2 = <<END <td>
<input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled>
<input type="number" class="stp-priority" id="STP-PRIORITY-$uc" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled></td> END ;
diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js index d76f0ab68..d27a79bc8 100644 --- a/html/html/include/zoneconf.js +++ b/html/html/include/zoneconf.js @@ -54,3 +54,30 @@ function highlightAccess(selectObj) { //if interface is assigned, highlight table cell in zone color colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE')); }
+//update zone mode +function changeZoneMode(selectObj) {
- if(!(selectObj && ('zone' in selectObj.dataset))) {
return; //required parameters are missing- }
- // STP enable checkbox
- let stpEnable = document.getElementById('STP-' + selectObj.dataset.zone);
- if(stpEnable) {
stpEnable.disabled = (selectObj.value !== 'BRIDGE'); //STP is available if zone is in bridge modestpEnable.checked = stpEnable.checked && (! stpEnable.disabled); //un-check if disabledstpEnable.dispatchEvent(new Event('change'));- }
+}
+//STP enable checkbox change toggles priority input +function changeEnableSTP(inputObj) {
- if(!(inputObj && ('zone' in inputObj.dataset))) {
return; //required parameters are missing- }
- let priority = document.getElementById('STP-PRIORITY-' + inputObj.dataset.zone);
- if(priority) {
priority.disabled = inputObj.disabled || (! inputObj.checked);- }
+}
Hi,
This patchset looks good to me, but I would like to hear back from Daniel for some test results :)
Breaking people’s networks would be really bad. So let’s give this a good test.
-Michael
On 22 Dec 2020, at 21:15, Leo Hofmann hofmann@leo-andres.de wrote:
Hi Daniel,
just in time for the holidays, I finished my work on the zoneconf.cgi STP GUI. I'm not sure how to test STP in my development setup, but I think I got it right. I would appreciate if you could test these patches, thank you :)
Best regards & happy holidays Leo
Am 22.12.2020 um 21:06 schrieb Leo-Andres Hofmann:
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 6 +++--- html/html/include/zoneconf.js | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 1d30450ed..eb6cd0e66 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -410,7 +410,7 @@ foreach (@zones) { print <<END <td class='heading bold $_'>$uc<br>
<select name="MODE $uc">
<select name="MODE $uc" data-zone="$uc" onchange="changeZoneMode(this)"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> <option value="MACVTAP" $mode_selected{"MACVTAP"}>$Lang::tr{"zoneconf nicmode macvtap"}</option>@@ -544,7 +544,7 @@ foreach (@zones) { # load settings and prepare form elements for each zone # enable checkbox HTML my $row_1 = <<END <td>
<input type="checkbox" name="STP-$uc" $disabled $checked>
<input type="checkbox" id="STP-$uc" name="STP-$uc" data-zone="$uc" onchange="changeEnableSTP(this)" $disabled $checked></td>
END ; @@ -553,7 +553,7 @@ END # priority input box HTML my $row_2 = <<END <td>
<input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled>
<input type="number" class="stp-priority" id="STP-PRIORITY-$uc" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled></td>
END ; diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js index d76f0ab68..d27a79bc8 100644 --- a/html/html/include/zoneconf.js +++ b/html/html/include/zoneconf.js @@ -54,3 +54,30 @@ function highlightAccess(selectObj) { //if interface is assigned, highlight table cell in zone color colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE')); }
+//update zone mode +function changeZoneMode(selectObj) {
- if(!(selectObj && ('zone' in selectObj.dataset))) {
return; //required parameters are missing- }
- // STP enable checkbox
- let stpEnable = document.getElementById('STP-' + selectObj.dataset.zone);
- if(stpEnable) {
stpEnable.disabled = (selectObj.value !== 'BRIDGE'); //STP is available if zone is in bridge modestpEnable.checked = stpEnable.checked && (! stpEnable.disabled); //un-check if disabledstpEnable.dispatchEvent(new Event('change'));- }
+}
+//STP enable checkbox change toggles priority input +function changeEnableSTP(inputObj) {
- if(!(inputObj && ('zone' in inputObj.dataset))) {
return; //required parameters are missing- }
- let priority = document.getElementById('STP-PRIORITY-' + inputObj.dataset.zone);
- if(priority) {
priority.disabled = inputObj.disabled || (! inputObj.checked);- }
+}
Hi Michael,
thank you very much for reviewing my patches! I agree, we should test this thoroughly. I would rather submit new patches than break someones network :)
Regarding your other mail, I looked into moving the "is_zone_active"... functions to the general-functions.pl file. I still need to make sure that these functions are universally applicable, then I will submit them.
Regards, Leo
Am 27.12.2020 um 12:05 schrieb Michael Tremer:
Hi,
This patchset looks good to me, but I would like to hear back from Daniel for some test results :)
Breaking people’s networks would be really bad. So let’s give this a good test.
-Michael
On 22 Dec 2020, at 21:15, Leo Hofmann hofmann@leo-andres.de wrote:
Hi Daniel,
just in time for the holidays, I finished my work on the zoneconf.cgi STP GUI. I'm not sure how to test STP in my development setup, but I think I got it right. I would appreciate if you could test these patches, thank you :)
Best regards & happy holidays Leo
Am 22.12.2020 um 21:06 schrieb Leo-Andres Hofmann:
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 6 +++--- html/html/include/zoneconf.js | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 1d30450ed..eb6cd0e66 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -410,7 +410,7 @@ foreach (@zones) { print <<END <td class='heading bold $_'>$uc<br>
<select name="MODE $uc">
<select name="MODE $uc" data-zone="$uc" onchange="changeZoneMode(this)"> <option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option> <option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option> <option value="MACVTAP" $mode_selected{"MACVTAP"}>$Lang::tr{"zoneconf nicmode macvtap"}</option>@@ -544,7 +544,7 @@ foreach (@zones) { # load settings and prepare form elements for each zone # enable checkbox HTML my $row_1 = <<END <td>
<input type="checkbox" name="STP-$uc" $disabled $checked>
<input type="checkbox" id="STP-$uc" name="STP-$uc" data-zone="$uc" onchange="changeEnableSTP(this)" $disabled $checked></td> END ;
@@ -553,7 +553,7 @@ END # priority input box HTML my $row_2 = <<END <td>
<input type="number" class="stp-priority" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled>
<input type="number" class="stp-priority" id="STP-PRIORITY-$uc" name="STP-PRIORITY-$uc" min="1" max="65535" value="$stp_priority" $disabled></td> END ;
diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js index d76f0ab68..d27a79bc8 100644 --- a/html/html/include/zoneconf.js +++ b/html/html/include/zoneconf.js @@ -54,3 +54,30 @@ function highlightAccess(selectObj) { //if interface is assigned, highlight table cell in zone color colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE')); }
+//update zone mode +function changeZoneMode(selectObj) {
- if(!(selectObj && ('zone' in selectObj.dataset))) {
return; //required parameters are missing- }
- // STP enable checkbox
- let stpEnable = document.getElementById('STP-' + selectObj.dataset.zone);
- if(stpEnable) {
stpEnable.disabled = (selectObj.value !== 'BRIDGE'); //STP is available if zone is in bridge modestpEnable.checked = stpEnable.checked && (! stpEnable.disabled); //un-check if disabledstpEnable.dispatchEvent(new Event('change'));- }
+}
+//STP enable checkbox change toggles priority input +function changeEnableSTP(inputObj) {
- if(!(inputObj && ('zone' in inputObj.dataset))) {
return; //required parameters are missing- }
- let priority = document.getElementById('STP-PRIORITY-' + inputObj.dataset.zone);
- if(priority) {
priority.disabled = inputObj.disabled || (! inputObj.checked);- }
+}
Hi,
On 22 Dec 2020, at 21:06, Leo-Andres Hofmann hofmann@leo-andres.de wrote:
Refactor duplicate perl code and add comments
Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de
html/cgi-bin/zoneconf.cgi | 53 +++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index 0914ceb78..bf46ab0c7 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -26,6 +26,7 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl";
+###--- HTML HEAD ---### my $extraHead = <<END
<style> table#zoneconf { @@ -105,7 +106,9 @@ my $extraHead = <<END <script src="/include/zoneconf.js"></script> END ; +###--- END HTML HEAD ---### +### Read configuration ### my %ethsettings = (); my %vlansettings = (); my %cgiparams = (); @@ -119,7 +122,7 @@ my $restart_notice = ""; &Header::showhttpheaders(); # Define all zones we will check for NIC assignment -my @zones = ("green", "red", "orange", "blue"); +my @zones = ("red", "green", "orange", "blue"); # Get all physical NICs present opendir(my $dh, "/sys/class/net/"); @@ -153,6 +156,21 @@ foreach (@nics) { } } +### Functions ### + +# Check if a zone is in IP mode or in PPP, PPPoE, VDSL, ... mode +sub is_zonetype_ip { + my $zone_type = shift; + return ($zone_type eq "STATIC" || $zone_type eq "DHCP"); +} + +# Check if a zone is activated (device assigned) +sub is_zone_activated { + my $zone = uc shift; + return ($ethsettings{"${zone}_DEV"} ne ""); +}
Should we not have functions like this in network-functions.pl, so that we do not have to rewrite them every time?
We also have functions for this in general-functions.pl which use the CONFIG_TYPE setting from /var/ipfire/ethernet/settings. That should actually be the correct way to do this.
Maybe this is one for the list of things to tidy up :)
+### START PAGE ### &Header::openpage($Lang::tr{"zoneconf title"}, 1, $extraHead); &Header::openbigbox('100%', 'center');
@@ -195,6 +213,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } }
# skip NIC/VLAN assignment and additional zone options for RED in PPP mode next;
I like well-commented code.
@@ -278,6 +297,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { } }
- # validation failed, show error message and exit if ($VALIDATE_error) { &Header::openbox('100%', 'left', $Lang::tr{"error"});
@@ -290,16 +310,17 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { exit 0; }
# new settings are valid, write configuration files &General::writehash("${General::swroot}/ethernet/settings",%ethsettings); &General::writehash("${General::swroot}/ethernet/vlans",%vlansettings);
$restart_notice = $Lang::tr{'zoneconf notice reboot'};
}
-&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"});
### START OF TABLE ###
+&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"});
print <<END
<form method='post' enctype='multipart/form-data'> <table id="zoneconf"> @@ -311,19 +332,16 @@ END # Fill the table header with all activated zones foreach (@zones) { my $uc = uc $_; - my $dev_name = $ethsettings{"${uc}_DEV"};
- if ($dev_name eq "") { # If the zone is not activated, don't show it
next;- }
- # If the zone is not activated, don't show it
- next unless is_zone_activated($_);
- # If the zone is in PPP mode, don't show a mode dropdown
- # If the red zone is in PPP mode, don't show a mode dropdown if ($uc eq "RED") { my $red_type = $ethsettings{"RED_TYPE"};
my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));if ($red_restricted) {print "\t\t<td class='heading $_'>$uc ($red_type)</td>\n";
unless (is_zonetype_ip($red_type)) {print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n"; next; # We're done here}
@@ -354,6 +372,7 @@ END
print "\t</tr>\n";
+# NIC assignment matrix foreach (@nics) { my $mac = $_->[0]; my $nic = $_->[1]; @@ -365,19 +384,14 @@ foreach (@nics) { # Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) { my $uc = uc $_;
my $dev_name = $ethsettings{"${uc}_DEV"};my $highlight = "";
if ($dev_name eq "") { # Again, skip the zone if it is not activatednext;}
# If the zone is not activated, don't show itnext unless is_zone_activated($_);
And this is a good change, because it clearly says what the code is supposed to do.
if ($uc eq "RED") {
my $red_type = $ethsettings{"RED_TYPE"};my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));# VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ...if ($red_restricted) {
unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) { my $checked = ""; if ($mac eq $ethsettings{"${uc}_MACADDR"}) {@@ -449,6 +463,7 @@ END print "\t</tr>\n"; }
+# footer and submit button print <<END
</table>
-- 2.27.0.windows.1
-
Daniel Weismüller -
Leo Hofmann -
Leo-Andres Hofmann
-
Michael Tremer