[PATCH 1/2] network-functions.pl: Add MAC address compare function

[PATCH 1/2] network-functions.pl: Add MAC address compare function

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 config/cfgroot/network-functions.pl | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/config/cfgroot/network-functions.pl b/config/cfgroot/network-functions.pl
index e94404f05..9908ee8ab 100644
--- a/config/cfgroot/network-functions.pl
+++ b/config/cfgroot/network-functions.pl
@@ -431,6 +431,18 @@ sub valid_mac($) {
 	return $mac =~ /^([0-9A-Fa-f]{2}[:]){5}([0-9A-Fa-f]{2})$/;
 }
 
+# Compares two MAC addresses and returns true if they are equal
+sub is_mac_equal {
+	my $mac_1 = uc shift; # convert to upper case
+	my $mac_2 = uc shift;
+
+	if(valid_mac($mac_1) && valid_mac($mac_2) && ($mac_1 eq $mac_2)) {
+		return 1;
+	}
+
+	return 0;
+}
+
 sub random_mac {
 	my $address = "02";
 
-- 
2.27.0.windows.1

[PATCH 2/2] zoneconf.cgi: Avoid unnecessary MAC address changes

[Leo-Andres Hofmann]

[-- Attachment #1: Type: text/plain, Size: 4497 bytes --]

Ensure that a bridge always has a MAC address configured, to prevent
udev/network-hotplug-bridges assigning random addresses at each start.
Cache previously generated MAC addresses so that they are not
regenerated each time the configuration is saved by the user.

Add more comments to existing code.

Fixes: #12583

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/zoneconf.cgi | 47 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index c0d44764f..ad0ec85fa 100644
--- a/html/cgi-bin/zoneconf.cgi
+++ b/html/cgi-bin/zoneconf.cgi
@@ -195,17 +195,23 @@ foreach (@nics) {
 ### Evaluate POST parameters ###
 
 if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
-	my %VALIDATE_nic_check = ();
-	my $VALIDATE_error = "";
+	my %VALIDATE_nic_check = (); # array of flags (assigned, restricted/pppoe, vlan, ...) per NIC
+	my $VALIDATE_error = ""; # contains an error message if the config validation failed
 
 	# 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 $slave_string = ""; # list of interfaces attached to the bridge
 		my $zone_mode = $cgiparams{"MODE $uc"};
 		my $VALIDATE_vlancount = 0;
 		my $VALIDATE_zoneslaves = 0;
 
+		# Each zone can contain up to one bridge and up to one VLAN,
+		# cache their mac addresses to prevent unnecessary changes
+		my $bridge_mac = $ethsettings{"${uc}_MACADDR"};
+		my $vlan_mac = $vlansettings{"${uc}_MAC_ADDRESS"};
+
+		# Clear old configuration
 		$ethsettings{"${uc}_MACADDR"} = "";
 		$ethsettings{"${uc}_MODE"} = "";
 		$ethsettings{"${uc}_SLAVES"} = "";
@@ -236,23 +242,47 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 			next;
 		}
 
+		# Zone in bridge mode: Always assign a MAC to the bridge
+		if($zone_mode eq "BRIDGE") {
+			# Ensure that the bridge's cached MAC does not come from a real NIC
+			# (this could happen if the zone was in default mode before)
+			foreach (@nics) {
+				my $nic_mac = $_->[0];
+				if(Network::is_mac_equal($bridge_mac, $nic_mac)) {
+					$bridge_mac = "";
+					last;
+				}
+			}
+
+			# Generate random MAC if none was configured
+			if(! Network::valid_mac($bridge_mac)) {
+				$bridge_mac = Network::random_mac();
+			}
+
+			# Assign the address to the bridge
+			$ethsettings{"${uc}_MACADDR"} = $bridge_mac;
+		}
+
 		foreach (@nics) {
 			my $mac = $_->[0];
 			my $nic_access = $cgiparams{"ACCESS $uc $mac"};
 
 			next unless ($nic_access);
 
+			# This NIC is to be assigned: check preconditions
 			if ($nic_access ne "NONE") {
 				if ($VALIDATE_nic_check{"RESTRICT $mac"}) { # If this interface is already assigned to RED in PPP mode, throw an error
 					$VALIDATE_error = $Lang::tr{"zoneconf val ppp assignment error"};
 					last;
 				}
 
+				# Enforce bridge mode when you try to assign multiple NICs to a zone
 				if ($zone_mode ne "BRIDGE" && $VALIDATE_zoneslaves > 0 && $nic_access ne "") {
 					$VALIDATE_error = $Lang::tr{"zoneconf val zoneslave amount error"};
 					last;
 				}
 
+				# Mark this NIC as "accessed by zone"
 				$VALIDATE_nic_check{"ACC $mac"} = 1;
 				$VALIDATE_zoneslaves++;
 			}
@@ -265,6 +295,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 
 				$VALIDATE_nic_check{"NATIVE $mac"} = 1;
 
+				# Zone in bridge mode: Add NIC to slave list. Otherwise access NIC directly
 				if ($zone_mode eq "BRIDGE") {
 					$slave_string = "${slave_string}${mac} ";
 				} else {
@@ -286,14 +317,18 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 					last;
 				}
 
-				my $rnd_mac = &Network::random_mac();
+				# Generate random MAC if none was configured
+				if(! Network::valid_mac($vlan_mac)) {
+					$vlan_mac = Network::random_mac();
+				}
 
 				$vlansettings{"${uc}_PARENT_DEV"} = $mac;
 				$vlansettings{"${uc}_VLAN_ID"} = $vlan_tag;
-				$vlansettings{"${uc}_MAC_ADDRESS"} = $rnd_mac;
+				$vlansettings{"${uc}_MAC_ADDRESS"} = $vlan_mac; # Generated MAC
 
+				# Zone in bridge mode: Add VLAN to slave list
 				if ($zone_mode eq "BRIDGE") {
-					$slave_string = "${slave_string}${rnd_mac} ";
+					$slave_string = "${slave_string}${vlan_mac} ";
 				}
 
 				$VALIDATE_vlancount++; # We can't allow more than one VLAN per zone
-- 
2.27.0.windows.1