interfaces: improve bridge code a bit

* Allow the bridge to come up without members.  While the GUI made sure
  to avoid empty bridges there is no reason for it and underneath if the
  interface was disabled the GUI made the interface disappear from the
  selection.  So now allow an empty bridge (may be nice for migration)
  and show disabled interfaces in the edit page.  Fix all callers to not
  assume the member property is always set.  Can probably go away once
  bridges are moved to MVC.
* Inline interface_bridge_add_member() since link_interface_to_bridge()
  is the only caller.  Improve the parameter passing a bit too.
* Add bridge interface return code to (_)interfaces_bridge_configure().
* Improve device resolution and a few mwexecf() replacements.
* Log the reason why a device could not be attached to bridge when one
  device is not there as expected.
This commit is contained in:
Franco Fichtner 2023-09-13 10:09:26 +02:00
parent 818f729379
commit 189e3af29e
5 changed files with 89 additions and 106 deletions

View File

@ -652,7 +652,7 @@ function filter_rules_legacy(&$FilterIflist)
$bridge_interfaces = [];
if (!empty($config['bridges']['bridged'])) {
foreach ($config['bridges']['bridged'] as $bridge) {
$bridge_interfaces = array_merge(explode(',', $bridge['members']), $bridge_interfaces);
$bridge_interfaces = array_merge(explode(',', $bridge['members'] ?? ''), $bridge_interfaces);
}
}
foreach ($FilterIflist as $on => $oc) {

View File

@ -213,73 +213,66 @@ function interfaces_bridge_configure($device)
global $config;
if (!isset($config['bridges']['bridged'])) {
return;
return null;
}
foreach ($config['bridges']['bridged'] as $bridge) {
if ($bridge['bridgeif'] == $device) {
_interfaces_bridge_configure($bridge);
return;
return _interfaces_bridge_configure($bridge);
}
}
}
function _interfaces_bridge_configure($bridge)
{
if (empty($bridge['members'])) {
/* XXX really necessary? we would like our bridge anyway */
log_msg("No members found on {$bridge['bridgeif']}", LOG_WARNING);
return;
/* XXX avoid destroy/create */
legacy_interface_destroy($bridge['bridgeif']);
legacy_interface_create($bridge['bridgeif']);
$checklist = get_configured_interface_with_descr();
$members = [];
/* find all required members */
foreach (explode(',', $bridge['members'] ?? '') as $member) {
if (empty($checklist[$member])) {
/* ignores disabled ones */
continue;
}
$device = get_real_interface($member);
if (!does_interface_exist($device)) {
log_msg("Device {$bridge['bridgeif']} cannot attach non-existent member {$device}, skipping now.");
continue;
}
$members[$member] = $device;
}
$members = explode(',', $bridge['members']);
/* Calculate smaller mtu and enforce it */
/* calculate smaller mtu and enforce it */
$mtu = null;
foreach ($members as $member) {
$opts = legacy_interface_stats(get_real_interface($member));
foreach ($members as $member => $device) {
$opts = legacy_interface_stats($device);
if (!empty($opts['mtu']) && ($mtu == null || $opts['mtu'] < $mtu)) {
$mtu = $opts['mtu'];
}
}
/* XXX avoid destroy/create */
legacy_interface_destroy($bridge['bridgeif']);
legacy_interface_create($bridge['bridgeif']);
mwexecf('/sbin/ifconfig %s inet6 %sauto_linklocal', [$bridge['bridgeif'], isset($bridge['linklocal']) ? '' : '-']);
$checklist = get_configured_interface_with_descr();
/* Add interfaces to bridge */
foreach ($members as $member) {
if (empty($checklist[$member])) {
continue;
}
/* XXX why can we not iterate this once for MTU check and interface add */
$realif = get_real_interface($member);
if (!does_interface_exist($realif)) {
log_msg("interface '{$realif}' not defined in interfaces bridge - up", LOG_WARNING);
continue;
}
/* make sure the parent interface is up */
configure_interface_hardware($realif);
legacy_interface_mtu($realif, $mtu);
interfaces_bring_up($realif);
legacy_bridge_member($bridge['bridgeif'], $realif);
/* add member interfaces to bridge */
foreach ($members as $member => $device) {
configure_interface_hardware($device);
legacy_interface_mtu($device, $mtu);
interfaces_bring_up($device);
legacy_bridge_member($bridge['bridgeif'], $device);
}
if (isset($bridge['enablestp'])) {
/* Choose spanning tree proto */
mwexec("/sbin/ifconfig {$bridge['bridgeif']} proto " . escapeshellarg($bridge['proto']));
mwexecf('/sbin/ifconfig %s proto %s', [$bridge['bridgeif'], $bridge['proto']]);
if (!empty($bridge['stp'])) {
$stpifs = explode(',', $bridge['stp']);
foreach ($stpifs as $stpif) {
$realif = get_real_interface($stpif);
mwexec("/sbin/ifconfig {$bridge['bridgeif']} stp {$realif}");
mwexecf('/sbin/ifconfig %s stp %s', [$bridge['bridgeif'], get_real_interface($stpif)]);
}
}
if (!empty($bridge['maxage'])) {
@ -381,29 +374,8 @@ function _interfaces_bridge_configure($bridge)
}
interfaces_bring_up($bridge['bridgeif']);
}
function interface_bridge_add_member($bridgeif, $realif)
{
$ifconfig_details = legacy_interfaces_details();
if (empty($ifconfig_details[$bridgeif]) || empty($ifconfig_details[$realif])) {
return;
}
configure_interface_hardware($realif);
/*
* The MTU of the first member interface to be added is used as the bridge
* MTU. All additional members are required to have exactly the same MTU
* value.
*/
if ($ifconfig_details[$bridgeif]['mtu'] != $ifconfig_details[$realif]['mtu']) {
legacy_interface_mtu($realif, $ifconfig_details[$bridgeif]['mtu']);
}
interfaces_bring_up($realif);
legacy_bridge_member($bridgeif, $realif);
return $bridge['bridgeif'];
}
function interfaces_lagg_configure($verbose = false)
@ -2353,12 +2325,12 @@ function interface_configure($verbose = false, $interface = 'wan', $reload = fal
interfaces_vips_configure($interface);
/* XXX device plugin hook */
/* XXX device dependency plugin hook */
$loaded = array_merge($loaded, link_interface_to_gre($interface, true));
$loaded = array_merge($loaded, link_interface_to_gif($interface, true));
/* XXX bridge hook does "something" else -- should we attach $realifv6 too? ;) */
link_interface_to_bridge($interface, $realif);
/* XXX device member plugin hook */
link_interface_to_bridge($interface, $realif, $ifconfig_details);
/* XXX reload routes for linked devices -- not great but also not avoidable at the moment */
interfaces_restart_by_device($verbose, $loaded, false);
@ -3428,7 +3400,7 @@ function interface_parent_devices($device)
} elseif (strstr($device, 'bridge')) {
foreach (config_read_array('bridges', 'bridged') as $bridge) {
if ($device == $bridge['bridgeif']) {
foreach (explode(',', $bridge['members']) as $member) {
foreach (explode(',', $bridge['members'] ?? '') as $member) {
/* bridge stores members as configured interfaces */
$parents[] = get_real_interface($member);
}
@ -3638,25 +3610,41 @@ function interfaces_restart_by_device($verbose, $devices, $reconfigure = true)
}
}
function link_interface_to_bridge($interface, $attach_device = null)
function link_interface_to_bridge($interface, $attach_device = null, $ifconfig_details = [/* must be set for attach to work */])
{
global $config;
if (isset($config['bridges']['bridged'])) {
foreach ($config['bridges']['bridged'] as $bridge) {
if (!in_array($interface, explode(',', $bridge['members']))) {
continue;
}
if ($attach_device) {
/* XXX _interfaces_bridge_configure() is probably cleaner */
interface_bridge_add_member($bridge, $attach_device);
/* XXX bridges can only load if all interfaces are there */
}
return $bridge['bridgeif'];
foreach (config_read_array('bridges', 'bridged') as $bridge) {
if (!in_array($interface, explode(',', $bridge['members'] ?? ''))) {
continue;
}
if ($attach_device) {
if (empty($ifconfig_details[$bridge['bridgeif']])) {
log_msg("Device {$attach_device} cannot be added to non-existent {$bridge['bridgeif']}, skipping now.");
break;
} elseif (empty($ifconfig_details[$attach_device])) {
log_msg("Device {$attach_device} was not found so it cannot be added to {$bridge['bridgeif']}, skipping now.");
break;
}
configure_interface_hardware($attach_device);
/*
* The MTU of the first member interface to be added is used as the bridge
* MTU. All additional members are required to have exactly the same MTU
* value.
*/
if ($ifconfig_details[$bridge['bridgeif']]['mtu'] != $ifconfig_details[$attach_device]['mtu']) {
legacy_interface_mtu($attach_device, $ifconfig_details[$bridge['bridgeif']]['mtu']);
}
interfaces_bring_up($attach_device);
legacy_bridge_member($bridge['bridgeif'], $attach_device);
}
return $bridge['bridgeif'];
}
return null;
}
function link_interface_to_gre($interface, $update = false, $family = null)

View File

@ -219,7 +219,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
continue;
}
$members = explode(",", strtoupper($bridge['members']));
$members = explode(',', strtoupper($bridge['members'] ?? ''));
foreach ($members as $member) {
if ($member == $ifnames[0]) {
$input_errors[] = sprintf(gettext("You cannot set port %s to interface %s because this interface is a member of %s."), $portname, $member, $portname);

View File

@ -120,17 +120,15 @@ legacy_html_escape_form_data($a_bridges);
<td><?= $bridge['bridgeif'] ?></td>
<td>
<?php
$members = explode(',', $bridge['members']);
$j = 0;
foreach ($members as $member) {
if (isset($ifdescrs[$member])) {
echo htmlspecialchars($ifdescrs[$member]);
$j++;
$members = explode(',', $bridge['members'] ?? '');
foreach ($members as $key => $member) {
if (!isset($ifdescrs[$member])) {
unset($members[$key]);
} else {
$members[$key] = $ifdescrs[$member];
}
if ($j > 0 && $j < count($members)) {
echo ", ";
}
}?>
}
echo implode(', ', $members); ?>
</td>
<td><?=$bridge['descr'];?></td>
<td><?= !empty($bridge['linklocal']) ? gettext('On') : gettext('Off') ?></td>

View File

@ -36,9 +36,12 @@ $a_bridges = &config_read_array('bridges', 'bridged');
// interface list
$ifacelist = [];
foreach (legacy_config_get_interfaces(['virtual' => false, 'enable' => true]) as $intf => $intfdata) {
foreach (legacy_config_get_interfaces(['virtual' => false]) as $intf => $intfdata) {
if (substr($intfdata['if'], 0, 3) != 'gre' && substr($intfdata['if'], 0, 2) != 'lo') {
$ifacelist[$intf] = $intfdata['descr'];
if (!isset($intfdata['enable'])) {
$ifacelist[$intf] .= ' (' . gettext('disabled') . ')';
}
}
}
@ -93,12 +96,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
$input_errors = [];
$pconfig = $_POST;
/* input validation */
$reqdfields = explode(" ", "members");
$reqdfieldsn = [gettext('Member Interfaces')];
do_input_validation($pconfig, $reqdfields, $reqdfieldsn, $input_errors);
$not_between_int = function($val, $min, $max) {
return !is_numeric($val) || $val < $min || $val > $max;
};
@ -133,8 +130,9 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
}
}
if (is_array($pconfig['members'])) {
foreach($pconfig['members'] as $ifmembers) {
$members = !empty($pconfig['members']) ? $pconfig['members'] : [];
if (!empty($members)) {
foreach($members as $ifmembers) {
if (empty($config['interfaces'][$ifmembers])) {
$input_errors[] = gettext("A member interface passed does not exist in configuration");
}
@ -147,7 +145,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
}
}
$members = !empty($pconfig['members']) ? $pconfig['members'] : [];
foreach (['stp', 'edge', 'autoedge', 'ptp', 'autoptp', 'static', 'private'] as $section) {
if (!empty($pconfig[$section])) {
foreach ($pconfig[$section] as $if) {
@ -277,7 +274,7 @@ $(document).ready(function() {
$bridge_interfaces = [];
foreach ($a_bridges as $idx => $bridge_item) {
if (!isset($id) || $idx != $id) {
$bridge_interfaces = array_merge(explode(',', $bridge_item['members']), $bridge_interfaces);
$bridge_interfaces = array_merge(explode(',', $bridge_item['members'] ?? ''), $bridge_interfaces);
}
}
foreach ($ifacelist as $ifn => $ifinfo):