From 268bdb8ab2fda1f92df67fc9a2b3d9bdd79dc675 Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Thu, 10 Feb 2022 10:07:57 +0100 Subject: [PATCH] interfaces: kill creation side effect for bridges #5540 Ironically, brigdes were the only user of the "&" reference trick removed from LAGG, GIF, GRE and VLAN. --- src/etc/inc/interfaces.inc | 66 +++++++++++------------------- src/www/interfaces_bridge_edit.php | 30 ++++++++------ 2 files changed, 42 insertions(+), 54 deletions(-) diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc index 66f325a0e..c8edf1377 100644 --- a/src/etc/inc/interfaces.inc +++ b/src/etc/inc/interfaces.inc @@ -280,21 +280,15 @@ function interfaces_bridge_configure($verbose = false) } } -function interface_bridge_configure(&$bridge) +function interface_bridge_configure($bridge) { - if (!is_array($bridge)) { - return; - } - if (empty($bridge['members'])) { + /* XXX really necessary? we would like our bridge anyway */ log_error("No members found on {$bridge['bridgeif']}"); return; } $members = explode(',', $bridge['members']); - if (!count($members)) { - return; - } /* Calculate smaller mtu and enforce it */ $mtu = null; @@ -311,17 +305,11 @@ function interface_bridge_configure(&$bridge) } } - if (file_exists("/var/run/booting") || !empty($bridge['bridgeif'])) { - /* XXX avoid destroy/create */ - legacy_interface_destroy($bridge['bridgeif']); - legacy_interface_create($bridge['bridgeif']); - $bridgeif = $bridge['bridgeif']; - } else { - $bridgeif = legacy_interface_create('bridge'); - $bridge['bridgeif'] = $bridgeif; - } + /* XXX avoid destroy/create */ + legacy_interface_destroy($bridge['bridgeif']); + legacy_interface_create($bridge['bridgeif']); - mwexecf('/sbin/ifconfig %s inet6 %sauto_linklocal', array($bridgeif, isset($bridge['linklocal']) ? '' : '-')); + mwexecf('/sbin/ifconfig %s inet6 %sauto_linklocal', [$bridge['bridgeif'], isset($bridge['linklocal']) ? '' : '-']); $checklist = get_configured_interface_with_descr(); @@ -346,33 +334,33 @@ function interface_bridge_configure(&$bridge) if (isset($bridge['enablestp'])) { /* Choose spanning tree proto */ - mwexec("/sbin/ifconfig {$bridgeif} proto " . escapeshellarg($bridge['proto'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} proto " . escapeshellarg($bridge['proto'])); if (!empty($bridge['stp'])) { $stpifs = explode(',', $bridge['stp']); foreach ($stpifs as $stpif) { $realif = get_real_interface($stpif); - mwexec("/sbin/ifconfig {$bridgeif} stp {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} stp {$realif}"); } } if (!empty($bridge['maxage'])) { - mwexec("/sbin/ifconfig {$bridgeif} maxage " . escapeshellarg($bridge['maxage'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} maxage " . escapeshellarg($bridge['maxage'])); } if (!empty($bridge['fwdelay'])) { - mwexec("/sbin/ifconfig {$bridgeif} fwddelay " . escapeshellarg($bridge['fwdelay'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} fwddelay " . escapeshellarg($bridge['fwdelay'])); } if (!empty($bridge['hellotime'])) { - mwexec("/sbin/ifconfig {$bridgeif} hellotime " . escapeshellarg($bridge['hellotime'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} hellotime " . escapeshellarg($bridge['hellotime'])); } if (!empty($bridge['priority'])) { - mwexec("/sbin/ifconfig {$bridgeif} priority " . escapeshellarg($bridge['priority'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} priority " . escapeshellarg($bridge['priority'])); } if (!empty($bridge['holdcnt'])) { - mwexec("/sbin/ifconfig {$bridgeif} holdcnt " . escapeshellarg($bridge['holdcnt'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} holdcnt " . escapeshellarg($bridge['holdcnt'])); } if (!empty($bridge['ifpriority'])) { $pconfig = explode(",", $bridge['ifpriority']); - $ifpriority = array(); + $ifpriority = []; foreach ($pconfig as $cfg) { $embcfg = explode_assoc(":", $cfg); foreach ($embcfg as $key => $value) { @@ -386,7 +374,7 @@ function interface_bridge_configure(&$bridge) } if (!empty($bridge['ifpathcost'])) { $pconfig = explode(",", $bridge['ifpathcost']); - $ifpathcost = array(); + $ifpathcost = []; foreach ($pconfig as $cfg) { $embcfg = explode_assoc(":", $cfg); foreach ($embcfg as $key => $value) { @@ -401,63 +389,59 @@ function interface_bridge_configure(&$bridge) } if ($bridge['maxaddr'] != '') { - mwexec("/sbin/ifconfig {$bridgeif} maxaddr " . escapeshellarg($bridge['maxaddr'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} maxaddr " . escapeshellarg($bridge['maxaddr'])); } if ($bridge['timeout'] != '') { - mwexec("/sbin/ifconfig {$bridgeif} timeout " . escapeshellarg($bridge['timeout'])); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} timeout " . escapeshellarg($bridge['timeout'])); } if (!empty($bridge['span'])) { $realif = get_real_interface($bridge['span']); - mwexec("/sbin/ifconfig {$bridgeif} span {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} span {$realif}"); } if (!empty($bridge['edge'])) { $edgeifs = explode(',', $bridge['edge']); foreach ($edgeifs as $edgeif) { $realif = get_real_interface($edgeif); - mwexec("/sbin/ifconfig {$bridgeif} edge {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} edge {$realif}"); } } if (!empty($bridge['autoedge'])) { $edgeifs = explode(',', $bridge['autoedge']); foreach ($edgeifs as $edgeif) { $realif = get_real_interface($edgeif); - mwexec("/sbin/ifconfig {$bridgeif} -autoedge {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} -autoedge {$realif}"); } } if (!empty($bridge['ptp'])) { $ptpifs = explode(',', $bridge['ptp']); foreach ($ptpifs as $ptpif) { $realif = get_real_interface($ptpif); - mwexec("/sbin/ifconfig {$bridgeif} ptp {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} ptp {$realif}"); } } if (!empty($bridge['autoptp'])) { $ptpifs = explode(',', $bridge['autoptp']); foreach ($ptpifs as $ptpif) { $realif = get_real_interface($ptpif); - mwexec("/sbin/ifconfig {$bridgeif} -autoptp {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} -autoptp {$realif}"); } } if (!empty($bridge['static'])) { $stickyifs = explode(',', $bridge['static']); foreach ($stickyifs as $stickyif) { $realif = get_real_interface($stickyif); - mwexec("/sbin/ifconfig {$bridgeif} sticky {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} sticky {$realif}"); } } if (!empty($bridge['private'])) { $privateifs = explode(',', $bridge['private']); foreach ($privateifs as $privateif) { $realif = get_real_interface($privateif); - mwexec("/sbin/ifconfig {$bridgeif} private {$realif}"); + mwexec("/sbin/ifconfig {$bridge['bridgeif']} private {$realif}"); } } - if ($bridge['bridgeif']) { - interfaces_bring_up($bridge['bridgeif']); - } else { - log_error('bridgeif not defined -- could not bring interface up'); - } + interfaces_bring_up($bridge['bridgeif']); } function interface_bridge_add_member($bridgeif, $interface) diff --git a/src/www/interfaces_bridge_edit.php b/src/www/interfaces_bridge_edit.php index 30a366576..b444362d3 100644 --- a/src/www/interfaces_bridge_edit.php +++ b/src/www/interfaces_bridge_edit.php @@ -35,8 +35,8 @@ require_once("filter.inc"); $a_bridges = &config_read_array('bridges', 'bridged'); // interface list -$ifacelist = array(); -foreach (legacy_config_get_interfaces(array('virtual' => false, "enable" => true)) as $intf => $intfdata) { +$ifacelist = []; +foreach (legacy_config_get_interfaces(['virtual' => false, 'enable' => true]) as $intf => $intfdata) { if (substr($intfdata['if'], 0, 3) != 'gre' && substr($intfdata['if'], 0, 2) != 'lo') { $ifacelist[$intf] = $intfdata['descr']; } @@ -48,7 +48,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $id = $_GET['id']; } // copy fields 1-on-1 - $copy_fields = array('descr', 'bridgeif', 'maxaddr', 'timeout', 'maxage','fwdelay', 'hellotime', 'priority', 'proto', 'holdcnt', 'span'); + $copy_fields = ['descr', 'bridgeif', 'maxaddr', 'timeout', 'maxage','fwdelay', 'hellotime', 'priority', 'proto', 'holdcnt', 'span']; foreach ($copy_fields as $fieldname) { if (isset($a_bridges[$id][$fieldname])) { $pconfig[$fieldname] = $a_bridges[$id][$fieldname]; @@ -67,7 +67,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { if (!empty($a_bridges[$id][$fieldname])) { $pconfig[$fieldname] = explode(',', $a_bridges[$id][$fieldname]); } else { - $pconfig[$fieldname] = array(); + $pconfig[$fieldname] = []; } } @@ -90,14 +90,14 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $id = $_POST['id']; } - $input_errors = array(); + $input_errors = []; $pconfig = $_POST; /* input validation */ $reqdfields = explode(" ", "members"); - $reqdfieldsn = array(gettext("Member Interfaces")); + $reqdfieldsn = [gettext('Member Interfaces')]; - do_input_validation($_POST, $reqdfields, $reqdfieldsn, $input_errors); + do_input_validation($pconfig, $reqdfields, $reqdfieldsn, $input_errors); $not_between_int = function($val, $min, $max) { return !is_numeric($val) || $val < $min || $val > $max; @@ -170,7 +170,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } // 1 on 1 copy - $copy_fields = array('descr', 'maxaddr', 'timeout', 'bridgeif', 'maxage','fwdelay', 'hellotime', 'priority', 'proto', 'holdcnt'); + $copy_fields = ['descr', 'maxaddr', 'timeout', 'bridgeif', 'maxage','fwdelay', 'hellotime', 'priority', 'proto', 'holdcnt']; foreach ($copy_fields as $fieldname) { if (isset($pconfig[$fieldname]) && $pconfig[$fieldname] != '') { $bridge[$fieldname] = $pconfig[$fieldname]; @@ -183,7 +183,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } // simple array fields - $array_fields = array('members', 'stp', 'edge', 'autoedge', 'ptp', 'autoptp', 'static', 'private'); + $array_fields = ['members', 'stp', 'edge', 'autoedge', 'ptp', 'autoptp', 'static', 'private']; foreach ($array_fields as $fieldname) { if(!empty($pconfig[$fieldname])) { $bridge[$fieldname] = implode(',', $pconfig[$fieldname]); @@ -208,9 +208,11 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } } - interface_bridge_configure($bridge); - ifgroup_setup(); - if ($bridge['bridgeif'] == "" || !stristr($bridge['bridgeif'], "bridge")) { + if (empty($bridge['bridgeif'])) { + $bridge['bridgeif'] = legacy_interface_create('bridge'); /* XXX find another strategy */ + } + + if (empty($bridge['bridgeif']) || strpos($bridge['bridgeif'], 'bridge') !== 0) { $input_errors[] = gettext("Error occurred creating interface, please retry."); } else { if (isset($id)) { @@ -219,6 +221,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $a_bridges[] = $bridge; } write_config(); + interface_bridge_configure($bridge); + ifgroup_setup(); $confif = convert_real_interface_to_friendly_interface_name($bridge['bridgeif']); if ($confif != '') { interface_configure(false, $confif); @@ -270,7 +274,7 @@ $(document).ready(function() {