[v2,5/6] zoneconf.cgi: Import network-functions.pl
Commit Message
Remove custom functions and use network-functions.pl instead to detect
the available zones correctly. This also removes the requirement that
a device must be assigned for a zone to become visible/configurable.
Fixes: #12568
Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
---
html/cgi-bin/zoneconf.cgi | 37 ++++++++-----------------------------
1 file changed, 8 insertions(+), 29 deletions(-)
Comments
> On 18 Feb 2021, at 14:30, Leo-Andres Hofmann <hofmann@leo-andres.de> wrote:
>
> Remove custom functions and use network-functions.pl instead to detect
> the available zones correctly. This also removes the requirement that
> a device must be assigned for a zone to become visible/configurable.
>
> Fixes: #12568
>
> Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
> ---
> html/cgi-bin/zoneconf.cgi | 37 ++++++++-----------------------------
> 1 file changed, 8 insertions(+), 29 deletions(-)
>
> diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
> index eb6cd0e66..9d01d06ce 100644
> --- a/html/cgi-bin/zoneconf.cgi
> +++ b/html/cgi-bin/zoneconf.cgi
> @@ -25,6 +25,7 @@ use Scalar::Util qw(looks_like_number);
> require '/var/ipfire/general-functions.pl';
> require "${General::swroot}/lang.pl";
> require "${General::swroot}/header.pl";
> +require "${General::swroot}/network-functions.pl";
>
> ###--- HTML HEAD ---###
> my $extraHead = <<END
> @@ -152,8 +153,8 @@ my $restart_notice = "";
> &Header::getcgihash(\%cgiparams);
> &Header::showhttpheaders();
>
> -# Define all zones we will check for NIC assignment
> -my @zones = ("red", "green", "orange", "blue");
> +# Get all network zones that are currently enabled
> +my @zones = Network::get_available_network_zones();
>
> # Get all physical NICs present
> opendir(my $dh, "/sys/class/net/");
> @@ -187,20 +188,6 @@ 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');
> @@ -211,7 +198,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
> my %VALIDATE_nic_check = ();
> my $VALIDATE_error = "";
>
> - foreach (@zones) {
> + # Loop trough all known zones to ensure a complete configuration file is created
> + foreach (@Network::known_network_zones) {
Quite a good idea to have a global constant for this :)
> my $uc = uc $_;
> my $slave_string = "";
> my $zone_mode = $cgiparams{"MODE $uc"};
> @@ -383,14 +371,11 @@ END
> foreach (@zones) {
> my $uc = uc $_;
>
> - # If the zone is not activated, don't show it
> - next unless is_zone_activated($_);
> -
> # If the red zone is in PPP mode, don't show a mode dropdown
> if ($uc eq "RED") {
> my $red_type = $ethsettings{"RED_TYPE"};
>
> - unless (is_zonetype_ip($red_type)) {
> + unless (Network::is_red_mode_ip()) {
Wouldn’t perl complain if this isn’t called with an “&”?
> print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n";
>
> next; # We're done here
> @@ -436,12 +421,9 @@ foreach (@nics) {
> my $uc = uc $_;
> my $highlight = "";
>
> - # If the zone is not activated, don't show it
> - next unless is_zone_activated($_);
> -
> if ($uc eq "RED") {
> # VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ...
> - unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {
> + unless (Network::is_red_mode_ip()) {
> my $checked = "";
>
> if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
> @@ -519,12 +501,9 @@ 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"})) {
> + unless (Network::is_red_mode_ip()) {
Same as above.
> push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cell
> next;
> }
> --
> 2.27.0.windows.1
>
-Michael
> Gesendet: Freitag, 19. Februar 2021 um 20:24 Uhr
> Von: "Michael Tremer" <michael.tremer@ipfire.org>
> An: "Leo-Andres Hofmann" <hofmann@leo-andres.de>
> Cc: development@lists.ipfire.org
> Betreff: Re: [PATCH v2 5/6] zoneconf.cgi: Import network-functions.pl
>
>
>
> > On 18 Feb 2021, at 14:30, Leo-Andres Hofmann <hofmann@leo-andres.de> wrote:
> >
> > Remove custom functions and use network-functions.pl instead to detect
> > the available zones correctly. This also removes the requirement that
> > a device must be assigned for a zone to become visible/configurable.
> >
> > Fixes: #12568
> >
> > Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
> > ---
> > html/cgi-bin/zoneconf.cgi | 37 ++++++++-----------------------------
> > 1 file changed, 8 insertions(+), 29 deletions(-)
> >
> > diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
> > index eb6cd0e66..9d01d06ce 100644
> > --- a/html/cgi-bin/zoneconf.cgi
> > +++ b/html/cgi-bin/zoneconf.cgi
> > @@ -25,6 +25,7 @@ use Scalar::Util qw(looks_like_number);
> > require '/var/ipfire/general-functions.pl';
> > require "${General::swroot}/lang.pl";
> > require "${General::swroot}/header.pl";
> > +require "${General::swroot}/network-functions.pl";
> >
> > ###--- HTML HEAD ---###
> > my $extraHead = <<END
> > @@ -152,8 +153,8 @@ my $restart_notice = "";
> > &Header::getcgihash(\%cgiparams);
> > &Header::showhttpheaders();
> >
> > -# Define all zones we will check for NIC assignment
> > -my @zones = ("red", "green", "orange", "blue");
> > +# Get all network zones that are currently enabled
> > +my @zones = Network::get_available_network_zones();
> >
> > # Get all physical NICs present
> > opendir(my $dh, "/sys/class/net/");
> > @@ -187,20 +188,6 @@ 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');
> > @@ -211,7 +198,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
> > my %VALIDATE_nic_check = ();
> > my $VALIDATE_error = "";
> >
> > - foreach (@zones) {
> > + # Loop trough all known zones to ensure a complete configuration file is created
> > + foreach (@Network::known_network_zones) {
>
> Quite a good idea to have a global constant for this :)
>
> > my $uc = uc $_;
> > my $slave_string = "";
> > my $zone_mode = $cgiparams{"MODE $uc"};
> > @@ -383,14 +371,11 @@ END
> > foreach (@zones) {
> > my $uc = uc $_;
> >
> > - # If the zone is not activated, don't show it
> > - next unless is_zone_activated($_);
> > -
> > # If the red zone is in PPP mode, don't show a mode dropdown
> > if ($uc eq "RED") {
> > my $red_type = $ethsettings{"RED_TYPE"};
> >
> > - unless (is_zonetype_ip($red_type)) {
> > + unless (Network::is_red_mode_ip()) {
>
> Wouldn’t perl complain if this isn’t called with an “&”?
>
No. This is the normal call now. The syntax with '&' brings some (minimal) restrictions.
> > print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n";
> >
> > next; # We're done here
> > @@ -436,12 +421,9 @@ foreach (@nics) {
> > my $uc = uc $_;
> > my $highlight = "";
> >
> > - # If the zone is not activated, don't show it
> > - next unless is_zone_activated($_);
> > -
> > if ($uc eq "RED") {
> > # VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ...
> > - unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {
> > + unless (Network::is_red_mode_ip()) {
> > my $checked = "";
> >
> > if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
> > @@ -519,12 +501,9 @@ 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"})) {
> > + unless (Network::is_red_mode_ip()) {
>
> Same as above.
>
> > push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cell
> > next;
> > }
> > --
> > 2.27.0.windows.1
> >
>
> -Michael
Am 20.02.2021 um 12:07 schrieb Bernhard Bitsch:
>
>> Gesendet: Freitag, 19. Februar 2021 um 20:24 Uhr
>> Von: "Michael Tremer" <michael.tremer@ipfire.org>
>> An: "Leo-Andres Hofmann" <hofmann@leo-andres.de>
>> Cc: development@lists.ipfire.org
>> Betreff: Re: [PATCH v2 5/6] zoneconf.cgi: Import network-functions.pl
>>
>>
>>
>>> On 18 Feb 2021, at 14:30, Leo-Andres Hofmann <hofmann@leo-andres.de> wrote:
>>>
>>> Remove custom functions and use network-functions.pl instead to detect
>>> the available zones correctly. This also removes the requirement that
>>> a device must be assigned for a zone to become visible/configurable.
>>>
>>> Fixes: #12568
>>>
>>> Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
>>> ---
>>> html/cgi-bin/zoneconf.cgi | 37 ++++++++-----------------------------
>>> 1 file changed, 8 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
>>> index eb6cd0e66..9d01d06ce 100644
>>> --- a/html/cgi-bin/zoneconf.cgi
>>> +++ b/html/cgi-bin/zoneconf.cgi
>>> @@ -25,6 +25,7 @@ use Scalar::Util qw(looks_like_number);
>>> require '/var/ipfire/general-functions.pl';
>>> require "${General::swroot}/lang.pl";
>>> require "${General::swroot}/header.pl";
>>> +require "${General::swroot}/network-functions.pl";
>>>
>>> ###--- HTML HEAD ---###
>>> my $extraHead = <<END
>>> @@ -152,8 +153,8 @@ my $restart_notice = "";
>>> &Header::getcgihash(\%cgiparams);
>>> &Header::showhttpheaders();
>>>
>>> -# Define all zones we will check for NIC assignment
>>> -my @zones = ("red", "green", "orange", "blue");
>>> +# Get all network zones that are currently enabled
>>> +my @zones = Network::get_available_network_zones();
>>>
>>> # Get all physical NICs present
>>> opendir(my $dh, "/sys/class/net/");
>>> @@ -187,20 +188,6 @@ 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');
>>> @@ -211,7 +198,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
>>> my %VALIDATE_nic_check = ();
>>> my $VALIDATE_error = "";
>>>
>>> - foreach (@zones) {
>>> + # Loop trough all known zones to ensure a complete configuration file is created
>>> + foreach (@Network::known_network_zones) {
>> Quite a good idea to have a global constant for this :)
>>
>>> my $uc = uc $_;
>>> my $slave_string = "";
>>> my $zone_mode = $cgiparams{"MODE $uc"};
>>> @@ -383,14 +371,11 @@ END
>>> foreach (@zones) {
>>> my $uc = uc $_;
>>>
>>> - # If the zone is not activated, don't show it
>>> - next unless is_zone_activated($_);
>>> -
>>> # If the red zone is in PPP mode, don't show a mode dropdown
>>> if ($uc eq "RED") {
>>> my $red_type = $ethsettings{"RED_TYPE"};
>>>
>>> - unless (is_zonetype_ip($red_type)) {
>>> + unless (Network::is_red_mode_ip()) {
>> Wouldn’t perl complain if this isn’t called with an “&”?
>>
> No. This is the normal call now. The syntax with '&' brings some (minimal) restrictions.
I agree with Bernhard, that's how i understood it too.
As far as I know, you only have to use "&" if you want to call a function before you define it. Otherwise, the current advice is to actually avoid it.
Since I included the "Network" package right at the beginning, perl should already know about this function at this point.
>
>>> print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n";
>>>
>>> next; # We're done here
>>> @@ -436,12 +421,9 @@ foreach (@nics) {
>>> my $uc = uc $_;
>>> my $highlight = "";
>>>
>>> - # If the zone is not activated, don't show it
>>> - next unless is_zone_activated($_);
>>> -
>>> if ($uc eq "RED") {
>>> # VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ...
>>> - unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {
>>> + unless (Network::is_red_mode_ip()) {
>>> my $checked = "";
>>>
>>> if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
>>> @@ -519,12 +501,9 @@ 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"})) {
>>> + unless (Network::is_red_mode_ip()) {
>> Same as above.
>>
>>> push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cell
>>> next;
>>> }
>>> --
>>> 2.27.0.windows.1
>>>
>> -Michael
@@ -25,6 +25,7 @@ use Scalar::Util qw(looks_like_number);
require '/var/ipfire/general-functions.pl';
require "${General::swroot}/lang.pl";
require "${General::swroot}/header.pl";
+require "${General::swroot}/network-functions.pl";
###--- HTML HEAD ---###
my $extraHead = <<END
@@ -152,8 +153,8 @@ my $restart_notice = "";
&Header::getcgihash(\%cgiparams);
&Header::showhttpheaders();
-# Define all zones we will check for NIC assignment
-my @zones = ("red", "green", "orange", "blue");
+# Get all network zones that are currently enabled
+my @zones = Network::get_available_network_zones();
# Get all physical NICs present
opendir(my $dh, "/sys/class/net/");
@@ -187,20 +188,6 @@ 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');
@@ -211,7 +198,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
my %VALIDATE_nic_check = ();
my $VALIDATE_error = "";
- foreach (@zones) {
+ # Loop trough all known zones to ensure a complete configuration file is created
+ foreach (@Network::known_network_zones) {
my $uc = uc $_;
my $slave_string = "";
my $zone_mode = $cgiparams{"MODE $uc"};
@@ -383,14 +371,11 @@ END
foreach (@zones) {
my $uc = uc $_;
- # If the zone is not activated, don't show it
- next unless is_zone_activated($_);
-
# If the red zone is in PPP mode, don't show a mode dropdown
if ($uc eq "RED") {
my $red_type = $ethsettings{"RED_TYPE"};
- unless (is_zonetype_ip($red_type)) {
+ unless (Network::is_red_mode_ip()) {
print "\t\t<td class='heading bold $_'>$uc ($red_type)</td>\n";
next; # We're done here
@@ -436,12 +421,9 @@ foreach (@nics) {
my $uc = uc $_;
my $highlight = "";
- # If the zone is not activated, don't show it
- next unless is_zone_activated($_);
-
if ($uc eq "RED") {
# VLANs/Bridging is not possible if the RED interface is set to PPP, PPPoE, VDSL, ...
- unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {
+ unless (Network::is_red_mode_ip()) {
my $checked = "";
if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
@@ -519,12 +501,9 @@ 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"})) {
+ unless (Network::is_red_mode_ip()) {
push(@stp_html, ["\t\t<td></td>\n", "\t\t<td></td>\n"]); # print empty cell
next;
}