From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 3/4] zoneconf.cgi: Add STP options to GUI
Date: Wed, 06 Jan 2021 15:07:59 +0000
Message-ID: <52ED21F1-07D8-4B0A-A2EA-4177BD594B12@ipfire.org>
In-Reply-To: <91d79df7-1157-b826-fa44-c2a7b383bb9b@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============4316272388990723088=="
List-Id: <development.lists.ipfire.org>

--===============4316272388990723088==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

> On 6 Jan 2021, at 13:20, Daniel Weism=C3=BCller <daniel.weismueller(a)ipfir=
e.org> wrote:
>=20
> Hi,
>=20
> no problems after applying all four patches so far.=20
> Everything works as expected.=20
> Further tests are planned for the next days.
>=20
> But there is one thing that we should append.
>=20
> If there is no priority configured the priority should be set to 32768.

Yes, currently the configuration cannot be saved without putting something in=
to 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 =E2=80=9Crequired=E2=
=80=9D attribute to the input fields.

Apart from that, I can also say that this looks good to be merged.

-Michael

>=20
> -
> Daniel
>=20
>=20
>=20
> 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
>>=20
>> Signed-off-by: Leo-Andres Hofmann=20
>> <hofmann(a)leo-andres.de>
>>=20
>> ---
>>  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(+)
>>=20
>> 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 =3D <<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 =3D <<END
>>  	#zoneconf tr.nic-row {
>>  		border-bottom: 0.5px solid $Header::bordercolour;
>>  	}
>> +	#zoneconf tr.option-row > td:first-child {
>> +			background-color: gray;
>> +	}
>> =20
>>  	/* alternating row background color */
>>  	#zoneconf tr {
>> @@ -108,6 +118,9 @@ my $extraHead =3D <<END
>>  	input.vlanid {
>>  		width: 4em;
>>  	}
>> +	input.stp-priority {
>> +		width: 5em;
>> +	}
>> =20
>>  	#submit-container {
>>  		width: 100%;
>> @@ -313,6 +326,25 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
>>  		} elsif ($zone_mode eq "MACVTAP") {
>>  			$ethsettings{"${uc}_MODE"} =3D "macvtap";
>>  		}
>> +
>> +		# STP options
>> +		# (this has already been skipped when RED is in PPP mode, so we don't n=
eed to check for PPP here)
>> +		$ethsettings{"${uc}_STP"} =3D "";
>> +		my $stp_enabled =3D $cgiparams{"STP-$uc"} eq "on";
>> +		my $stp_priority =3D $cgiparams{"STP-PRIORITY-$uc"};
>> +
>> +		if($stp_enabled) {
>> +			unless($ethsettings{"${uc}_MODE"} eq "bridge") { # STP is only availab=
le in bridge mode
>> +				$VALIDATE_error =3D $Lang::tr{"zoneconf val stp zone mode error"};
>> +				last;
>> +			}
>> +			unless (looks_like_number($stp_priority) && ($stp_priority >=3D 1) && =
($stp_priority <=3D 65535)) { # STP bridge priority range: 1..65535
>> +				$VALIDATE_error =3D $Lang::tr{"zoneconf val stp priority range error"=
};
>> +				last;
>> +			}
>> +			$ethsettings{"${uc}_STP"} =3D "on"; # network-hotplug-bridges expects =
"on"
>> +			$ethsettings{"${uc}_STP_PRIORITY"} =3D $stp_priority;
>> +		}
>>  	}
>> =20
>>  	# validation failed, show error message and exit
>> @@ -481,6 +513,74 @@ END
>>  	print "\t</tr>\n";
>>  }
>> =20
>> +# STP options
>> +my @stp_html =3D (); # form fields buffer (two rows)
>> +
>> +foreach (@zones) { # load settings and prepare form elements for each zone
>> +	my $uc =3D 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 =3D $ethsettings{"${uc}_MODE"} eq "bridge"; # STP is o=
nly available in bridge mode
>> +	my $stp_enabled =3D $ethsettings{"${uc}_STP"} eq "on";
>> +	my $stp_priority =3D $ethsettings{"${uc}_STP_PRIORITY"};
>> +
>> +	# form element modifiers
>> +	my $checked =3D "";
>> +	my $disabled =3D "";
>> +	$checked =3D "checked" if ($stp_available && $stp_enabled);
>> +	$disabled =3D "disabled" unless $stp_available;
>> +
>> +	# enable checkbox HTML
>> +	my $row_1 =3D <<END
>> +		<td>
>> +			<input type=3D"checkbox" name=3D"STP-$uc" $disabled $checked>
>> +		</td>
>> +END
>> +;
>> +	$disabled =3D "disabled" unless $stp_enabled; # STP priority can't be en=
tered if STP is disabled
>> +
>> +	# priority input box HTML
>> +	my $row_2 =3D <<END
>> +		<td>
>> +			<input type=3D"number" class=3D"stp-priority" name=3D"STP-PRIORITY-$uc=
" min=3D"1" max=3D"65535" value=3D"$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=3D"half-height divider-top option-row">
>> +		<td class=3D"heading bold">$Lang::tr{"zoneconf stp enable"}</td>
>> +END
>> +;
>> +foreach (@stp_html) {
>> +	print $_->[0]; # row 1
>> +}
>> +print <<END
>> +	</tr>
>> +	<tr class=3D"half-height option-row">
>> +		<td class=3D"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' =3D> 'Normal',
>>  'zoneconf nicmode macvtap' =3D> 'MacVTap',
>>  'zoneconf notice reboot' =3D> 'Bitte einen Neustart durchf=C3=BChren, um =
die =C3=84nderungen zu =C3=BCbernehmen.',
>> +'zoneconf stp enable' =3D> 'STP aktivieren',
>> +'zoneconf stp priority' =3D> 'Br=C3=BCcke Priorit=C3=A4t',
>>  'zoneconf title' =3D> 'Zonen einrichten',
>>  'zoneconf val native assignment error' =3D> 'Eine Netzwerkkarte kann nich=
t von mehreren Zonen nativ verwendet werden.',
>>  'zoneconf val ppp assignment error' =3D> 'Die Netzwerkkarte, die von RED =
im PPP-Modus verwendet wird, kann keiner anderen Zone zugeordnet werden.',
>> +'zoneconf val stp priority range error' =3D> 'STP Br=C3=BCcke Priorit=C3=
=A4t muss im Bereich 1-65535 liegen',
>> +'zoneconf val stp zone mode error' =3D> 'STP kann nur aktiviert werden, w=
enn sich die Zone im Br=C3=BCckenmodus befindet',
>>  'zoneconf val vlan amount assignment error' =3D> 'Pro Zone kann nur ein V=
LAN verwendet werden.',
>>  'zoneconf val vlan tag assignment error' =3D> 'Pro Netzwerkkarte kann der=
selbe VLAN-Tag nur einmal verwendet werden.',
>>  'zoneconf val zoneslave amount error' =3D> 'Wenn eine Zone nicht im Br=C3=
=BCckenmodus 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' =3D> 'Default',
>>  'zoneconf nicmode macvtap' =3D> 'MacVTap',
>>  'zoneconf notice reboot' =3D> 'Please reboot to apply your changes.',
>> +'zoneconf stp enable' =3D> 'STP enable',
>> +'zoneconf stp priority' =3D> 'Bridge Priority',
>>  'zoneconf title' =3D> 'Zone Configuration',
>>  'zoneconf val native assignment error' =3D> 'A NIC cannot be accessed nat=
ively by more than one zone.',
>>  'zoneconf val ppp assignment error' =3D> 'The NIC used for RED in PPP mod=
e cannot be accessed by any other zone.',
>> +'zoneconf val stp priority range error' =3D> 'STP bridge priority must be=
 in the range of 1-65535',
>> +'zoneconf val stp zone mode error' =3D> 'STP can only be enabled if the z=
one is in bridge mode',
>>  'zoneconf val vlan amount assignment error' =3D> 'A zone cannot have more=
 than one VLAN assigned.',
>>  'zoneconf val vlan tag assignment error' =3D> 'You cannot use the same VL=
AN tag more than once per NIC.',
>>  'zoneconf val zoneslave amount error' =3D> 'A zone that is not in bridge =
mode can\'t have more than one NIC assigned',
>>=20


--===============4316272388990723088==--