From 2637e6ebca1a7d26daa4ec28f6bfcc7f15a340eb Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Wed, 23 Mar 2022 08:10:10 +0100 Subject: [PATCH] interfaces: use consistent "vlan" or "qinq" prefix, start a 0 #5560 Since we also change the vlan names here for new devices to eventually avoid overlong vlan interface names (#3222) we need to make sure the rest of the system knows the new prefixes. Some related style changes in code and text. --- src/etc/inc/console.inc | 2 +- src/etc/inc/interfaces.inc | 5 ++-- src/etc/inc/interfaces.lib.inc | 4 +-- src/etc/inc/plugins.inc.d/core.inc | 2 +- src/etc/inc/util.inc | 2 ++ .../Interfaces/Api/VlanSettingsController.php | 30 ++++++++++--------- .../Base/FieldTypes/InterfaceField.php | 4 +-- src/www/interfaces.php | 6 ++-- src/www/interfaces_lagg_edit.php | 2 +- 9 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/etc/inc/console.inc b/src/etc/inc/console.inc index 0942bdee8..77d856c83 100644 --- a/src/etc/inc/console.inc +++ b/src/etc/inc/console.inc @@ -165,7 +165,7 @@ EOD; if (isset($config['vlans']['vlan'][0])) { foreach ($config['vlans']['vlan'] as $vlan) { - $iflist_all[$vlan['if'] . '_vlan' . $vlan['tag']] = [ + $iflist_all[$vlan['vlanif']] = [ 'dmesg' => "VLAN tag {$vlan['tag']}, parent interface {$vlan['if']}", 'mac' => '00:00:00:00:00:00', ]; diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc index 5b50577c2..bf37ea02a 100644 --- a/src/etc/inc/interfaces.inc +++ b/src/etc/inc/interfaces.inc @@ -2289,7 +2289,7 @@ function interface_configure($verbose = false, $interface = 'wan', $reload = fal $intf_details = $ifconfig_details[$realhwif]; $mtu = $wancfg['mtu']; - if (strstr($realif, '_vlan')) { + if (strstr($realif, 'vlan') || strstr($realif, 'qinq')) { $parent_realif = interface_parent_devices($interface)[0]; $parent_details = $ifconfig_details[$parent_realif]; @@ -3379,7 +3379,8 @@ function interface_parent_devices($interface) $realif = get_real_interface($interface); - if (strstr($realif, '_vlan')) { + if (strstr($realif, 'vlan') || strstr($realif, 'qinq')) { + /* XXX maybe if we have a qinq type return both parents? */ foreach (config_read_array('vlans', 'vlan') as $vlan) { if ($realif == $vlan['vlanif']) { $parents[] = $vlan['if']; diff --git a/src/etc/inc/interfaces.lib.inc b/src/etc/inc/interfaces.lib.inc index 9f9d608cd..90d769b6d 100644 --- a/src/etc/inc/interfaces.lib.inc +++ b/src/etc/inc/interfaces.lib.inc @@ -448,7 +448,7 @@ function configure_interface_hardware($ifs, $intf_details = null) $hwsettings = $config['system']; - if (strstr($ifs, '_vlan') || strpos($ifs, '/') === 0) { + if (strstr($ifs, 'vlan') || strstr($ifs, 'qinq') || strpos($ifs, '/') === 0) { /* skip checksumming */ return; } @@ -502,7 +502,7 @@ function configure_interface_hardware($ifs, $intf_details = null) legacy_interface_flags($ifs, 'lro', false); } - // disable/enable hardware vlan tags, will be skipped when "Leave default" (option 2) is selected + // disable/enable hardware VLAN tags, will be skipped when "Leave default" (option 2) is selected if (!isset($hwsettings['disablevlanhwfilter']) || $hwsettings['disablevlanhwfilter'] == 1) { // probe already selected options $selected_opts = []; diff --git a/src/etc/inc/plugins.inc.d/core.inc b/src/etc/inc/plugins.inc.d/core.inc index fefd0d73f..5e6b9a534 100644 --- a/src/etc/inc/plugins.inc.d/core.inc +++ b/src/etc/inc/plugins.inc.d/core.inc @@ -185,7 +185,7 @@ function core_devices() $devices[] = array('pattern' => '^wg', 'volatile' => true); $devices[] = array('pattern' => '^zt', 'volatile' => true); $devices[] = array('pattern' => '_stf', 'volatile' => true); - $devices[] = array('pattern' => '_vlan', 'volatile' => true); + $devices[] = array('pattern' => '_vlan|^vlan|^qinq', 'volatile' => true); $devices[] = array('pattern' => '_wlan', 'volatile' => true); return $devices; diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc index d2e465c12..335f6a799 100644 --- a/src/etc/inc/util.inc +++ b/src/etc/inc/util.inc @@ -869,9 +869,11 @@ function get_interface_list($only_active = false, $include_dmesg = false) 'ppp', 'pppoe', 'pptp', + 'qinq', 'sl', 'tap', 'tun', + 'vlan', ]; $ifnames_up = legacy_interface_listget('up'); diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Interfaces/Api/VlanSettingsController.php b/src/opnsense/mvc/app/controllers/OPNsense/Interfaces/Api/VlanSettingsController.php index 8f3fb3ac2..f6bf7918b 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Interfaces/Api/VlanSettingsController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Interfaces/Api/VlanSettingsController.php @@ -40,8 +40,8 @@ class VlanSettingsController extends ApiMutableModelControllerBase private function generateVlanIfName($current = null) { - $tmp = $this->request->getPost("vlan"); - $prefix = (strpos($tmp['if'], 'vlan') === false ? "vlan" : "qinq"); + $tmp = $this->request->getPost('vlan'); + $prefix = (strpos($tmp['if'], 'vlan') === false ? 'vlan' : 'qinq'); if ($current != null && (string)$current->vlanif == "{$tmp['if']}_vlan{$tmp['tag']}") { // keep legacy naming return "{$tmp['if']}_vlan{$tmp['tag']}"; @@ -53,13 +53,13 @@ class VlanSettingsController extends ApiMutableModelControllerBase return (string)$current->vlanif; } else { // autonumber new - $ifid = 0; + $ifid = -1; foreach ($this->getModel()->vlan->iterateItems() as $node) { - if (strpos((string)$node->vlanif . "_", $prefix) === 0) { - $ifid = max($ifid, (int)explode("_", (string)$node->vlanif)[1]); + if (strpos((string)$node->vlanif, $prefix) === 0) { + $ifid = max($ifid, (int)filter_var((string)$node->vlanif, FILTER_SANITIZE_NUMBER_INT)); } } - return $prefix . "_" . ($ifid + 1); + return $prefix . ($ifid + 1); } } @@ -78,7 +78,7 @@ class VlanSettingsController extends ApiMutableModelControllerBase public function searchItemAction() { - return $this->searchBase("vlan", ['vlanif','if','tag','pcp','descr'], "vlanif"); + return $this->searchBase('vlan', ['vlanif', 'if', 'tag', 'pcp', 'descr'], 'vlanif'); } public function setItemAction($uuid) @@ -100,9 +100,11 @@ class VlanSettingsController extends ApiMutableModelControllerBase ] ]; } elseif ($old_vlanif != null && $old_vlanif != $new_vlanif && $this->interfaceAssigned($old_vlanif)) { - // Reassignment is only an issue when naming changes. These additional validations only apply - // for legacy interface nameing (e.g. _vlan_) and type changes vlan verses qinq. - $tmp = $this->request->getPost("vlan"); + /* + * Reassignment is only an issue when naming changes. These additional validations only apply + * for legacy interface naming (e.g. _vlan_) and type changes "vlan" versus "qinq". + */ + $tmp = $this->request->getPost('vlan'); if ($tmp['tag'] != (string)$node->tag) { $result = [ "result" => "failed", @@ -119,7 +121,7 @@ class VlanSettingsController extends ApiMutableModelControllerBase ]; } } else { - $result = $this->setBase("vlan", "vlan", $uuid, ["vlanif" => $new_vlanif]); + $result = $this->setBase('vlan', 'vlan', $uuid, ['vlanif' => $new_vlanif]); // store interface name for apply action if ($result['result'] != 'failed' && $old_vlanif != $new_vlanif) { file_put_contents("/tmp/.vlans.removed", "{$old_vlanif}\n", FILE_APPEND | LOCK_EX); @@ -130,14 +132,14 @@ class VlanSettingsController extends ApiMutableModelControllerBase public function addItemAction() { - return $this->addBase("vlan", "vlan", [ + return $this->addBase('vlan', 'vlan', [ "vlanif" => $this->generateVlanIfName() ]); } public function getItemAction($uuid = null) { - return $this->getBase("vlan", "vlan", $uuid); + return $this->getBase('vlan', 'vlan', $uuid); } public function delItemAction($uuid) @@ -155,7 +157,7 @@ class VlanSettingsController extends ApiMutableModelControllerBase } elseif ($old_vlanif != null && $this->interfaceAssigned($old_vlanif)) { throw new UserException(gettext("This VLAN cannot be deleted because it is assigned as an interface.")); } else { - $result = $this->delBase("vlan", $uuid); + $result = $this->delBase('vlan', $uuid); // store interface name for apply action if ($result['result'] != 'failed') { file_put_contents("/tmp/.vlans.removed", "{$old_vlanif}\n", FILE_APPEND | LOCK_EX); diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php index 6012951e3..ddd2bda76 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php +++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php @@ -86,8 +86,8 @@ class InterfaceField extends BaseListField } /** - * collect parents for vlan interfaces - * @return array named array containing device and vlan interfaces + * collect parents for VLAN interfaces + * @return array named array containing device and VLAN interfaces */ private function getConfigVLANInterfaces() { diff --git a/src/www/interfaces.php b/src/www/interfaces.php index 68e774dab..291504c50 100644 --- a/src/www/interfaces.php +++ b/src/www/interfaces.php @@ -866,15 +866,15 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $input_errors[] = sprintf(gettext('The MTU must be greater than %s bytes and less than %s.'), $mtu_low, $mtu_high); } - if (strstr($a_interfaces[$if]['if'], '_vlan')) { + if (strstr($a_interfaces[$if]['if'], 'vlan') || strstr($a_interfaces[$if]['if'], 'qinq')) { list ($parentif) = interface_parent_devices($if); $intf_details = legacy_interface_details($parentif); if ($intf_details['mtu'] < $pconfig['mtu']) { - $input_errors[] = gettext("MTU of a vlan should not be bigger than parent interface."); + $input_errors[] = gettext("MTU of a VLAN should not be bigger than parent interface."); } } else { foreach ($config['interfaces'] as $idx => $ifdata) { - if ($idx == $if || !strstr($ifdata['if'], '_vlan')) { + if ($idx == $if || !strstr($ifdata['if'], 'vlan') || !strstr($ifdata['if'], 'qinq')) { continue; } diff --git a/src/www/interfaces_lagg_edit.php b/src/www/interfaces_lagg_edit.php index 27f8d7cf5..c4c544012 100644 --- a/src/www/interfaces_lagg_edit.php +++ b/src/www/interfaces_lagg_edit.php @@ -331,7 +331,7 @@ legacy_html_escape_form_data($pconfig);