[v2,1/6] zoneconf.cgi: Change NIC display order, improve code
Commit Message
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(-)
Comments
Hi all,
you probably already saw this patchset last month :)
I have implemented your suggestions and decided to resubmit the entire patchset to keep it coherent.
The patches 1-4 are unchanged. Daniel and Michael have already tested them (thank you very much!).
Patch 5 makes zoneconf use the new zone detection functions in network-functions.pl. This also fixes bug #12568!
Patch 6 adds default values to the input fields. I decided to add a default VLAN ID as well, because these inputs suffered from the same usability issue as the STP priority fields.
I'm looking forward to your feedback!
Best regards,
Leo
P.S. @Daniel thanks for your help with re-submitting! I have read through all your emails again and decided that I can submit these patches now.
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi all,</p>
<p>you probably already saw this patchset last month :)<br>
</p>
<p>I have implemented your suggestions and decided to resubmit the
entire patchset to keep it coherent.<br>
The patches 1-4 are unchanged. Daniel and Michael have already
tested them (thank you very much!).</p>
<p>Patch 5 makes zoneconf use the new zone detection functions in
network-functions.pl. This also fixes bug #12568!<br>
Patch 6 adds default values to the input fields. I decided to add
a default VLAN ID as well, because these inputs suffered from the
same usability issue as the STP priority fields.</p>
<p><span style="color: rgb(26, 26, 27); font-family: "Noto
Sans", Arial, sans-serif; font-size: 14px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-thickness: initial; text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">I'm looking forward to your feedback!</span>
</p>
<p>Best regards,<br>
Leo</p>
<p>P.S. @Daniel thanks for your help with re-submitting! I have read
through all your emails again and decided that I can submit these
patches now.</p>
</body>
</html>
Hello,
> On 18 Feb 2021, at 14:36, Leo Hofmann <hofmann@leo-andres.de> wrote:
>
> Hi all,
>
> you probably already saw this patchset last month :)
>
> I have implemented your suggestions and decided to resubmit the entire patchset to keep it coherent.
> The patches 1-4 are unchanged. Daniel and Michael have already tested them (thank you very much!).
>
> Patch 5 makes zoneconf use the new zone detection functions in network-functions.pl. This also fixes bug #12568!
> Patch 6 adds default values to the input fields. I decided to add a default VLAN ID as well, because these inputs suffered from the same usability issue as the STP priority fields.
>
> I'm looking forward to your feedback!
Thank you very much for working on this.
Judging from the code, this looks very good. Nice and tidy and changes are split into small chunks that are very easy to review.
I will wait for some more people to give any feedback - and hopefully test it live - and merge it.
Well done!
-Michael
> Best regards,
> Leo
>
> P.S. @Daniel thanks for your help with re-submitting! I have read through all your emails again and decided that I can submit these patches now.
>
Hello Michael,
thank you for all your positive feedback and support! I like the working atmosphere here very much.
(Sorry for the "off-topic" mail, but I think this is worth mentioning and appreciating!)
Looking forward to the test results!
Best,
Leo
Am 19.02.2021 um 20:26 schrieb Michael Tremer:
> Hello,
>
>> On 18 Feb 2021, at 14:36, Leo Hofmann <hofmann@leo-andres.de> wrote:
>>
>> Hi all,
>>
>> you probably already saw this patchset last month :)
>>
>> I have implemented your suggestions and decided to resubmit the entire patchset to keep it coherent.
>> The patches 1-4 are unchanged. Daniel and Michael have already tested them (thank you very much!).
>>
>> Patch 5 makes zoneconf use the new zone detection functions in network-functions.pl. This also fixes bug #12568!
>> Patch 6 adds default values to the input fields. I decided to add a default VLAN ID as well, because these inputs suffered from the same usability issue as the STP priority fields.
>>
>> I'm looking forward to your feedback!
> Thank you very much for working on this.
>
> Judging from the code, this looks very good. Nice and tidy and changes are split into small chunks that are very easy to review.
>
> I will wait for some more people to give any feedback - and hopefully test it live - and merge it.
>
> Well done!
>
> -Michael
>
>> Best regards,
>> Leo
>>
>> P.S. @Daniel thanks for your help with re-submitting! I have read through all your emails again and decided that I can submit these patches now.
>>
*thumbs up*
Couldn’t agree more.
> On 21 Feb 2021, at 12:14, Leo Hofmann <hofmann@leo-andres.de> wrote:
>
> Hello Michael,
>
> thank you for all your positive feedback and support! I like the working atmosphere here very much.
> (Sorry for the "off-topic" mail, but I think this is worth mentioning and appreciating!)
>
> Looking forward to the test results!
>
> Best,
> Leo
>
> Am 19.02.2021 um 20:26 schrieb Michael Tremer:
>> Hello,
>>
>>> On 18 Feb 2021, at 14:36, Leo Hofmann <hofmann@leo-andres.de> wrote:
>>>
>>> Hi all,
>>>
>>> you probably already saw this patchset last month :)
>>>
>>> I have implemented your suggestions and decided to resubmit the entire patchset to keep it coherent.
>>> The patches 1-4 are unchanged. Daniel and Michael have already tested them (thank you very much!).
>>>
>>> Patch 5 makes zoneconf use the new zone detection functions in network-functions.pl. This also fixes bug #12568!
>>> Patch 6 adds default values to the input fields. I decided to add a default VLAN ID as well, because these inputs suffered from the same usability issue as the STP priority fields.
>>>
>>> I'm looking forward to your feedback!
>> Thank you very much for working on this.
>>
>> Judging from the code, this looks very good. Nice and tidy and changes are split into small chunks that are very easy to review.
>>
>> I will wait for some more people to give any feedback - and hopefully test it live - and merge it.
>>
>> Well done!
>>
>> -Michael
>>
>>> Best regards,
>>> Leo
>>>
>>> P.S. @Daniel thanks for your help with re-submitting! I have read through all your emails again and decided that I can submit these patches now.
>>>
@@ -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>