From e738e3ca059f6dafb7917fdef66e05c8b695567a Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Fri, 13 Mar 2020 08:24:37 +0100 Subject: [PATCH] ifgroups: simplify group maintenance for https://github.com/opnsense/core/issues/3780 (#3977) It doesn't really seem to make sense to try to link groups on different places, if they're only usable for pf which is always reloaded when new interfaces are connected. The new ifgroup_setup() synchonizes attached interfaces for all configured groups, only rename and delete are still being served from the management pages (to avoid dropping static groups like openvpn). --- src/etc/inc/filter.inc | 67 ++++++++++++++++++++++++++++++ src/etc/inc/interfaces.inc | 51 ----------------------- src/etc/inc/plugins.inc.d/pf.inc | 9 ---- src/etc/rc.newwanip | 6 --- src/etc/rc.newwanipv6 | 5 --- src/www/interfaces_groups_edit.php | 11 +---- 6 files changed, 68 insertions(+), 81 deletions(-) diff --git a/src/etc/inc/filter.inc b/src/etc/inc/filter.inc index 491fe249c..ca679485d 100644 --- a/src/etc/inc/filter.inc +++ b/src/etc/inc/filter.inc @@ -147,6 +147,67 @@ function filter_delete_states_for_down_gateways() } } +/** + * sync interface groups, but leave the ones not managed by us intact. + */ +function ifgroup_setup() +{ + global $config; + $all_ifgroups = array(); + $all_ifs = array(); + $interface_details = legacy_interfaces_details(); + if (isset($config['ifgroups']['ifgroupentry'])) { + foreach ($config['ifgroups']['ifgroupentry'] as $group) { + $all_ifgroups[$group['ifname']] = array(); + foreach (explode(" ", $group['members']) as $member) { + if (!empty($config['interfaces'][$member])) { + $if = $config['interfaces'][$member]['if']; + if (!isset($all_ifs[$if])) { + $all_ifs[$if] = array(); + } + $all_ifs[$if][] = $group['ifname']; + $all_ifgroups[$group['ifname']][] = $if; + } + } + } + } + foreach ($interface_details as $intf => $details) { + $thisifgroups = !empty($details['groups']) ? $details['groups'] : array(); + foreach ($thisifgroups as $ifgroup) { + if (isset($all_ifgroups[$ifgroup]) && !in_array($intf, $all_ifgroups[$ifgroup])) { + // detach + mwexecf('/sbin/ifconfig %s -group %s', array($intf, $ifgroup)); + } + } + if (!empty($all_ifs[$intf])) { + foreach ($all_ifs[$intf] as $ifgroup) { + if (!in_array($ifgroup, $thisifgroups)) { + // attach + mwexecf('/sbin/ifconfig %s group %s', array($intf, $ifgroup)); + } + } + } + } +} + +/** + * XXX: replace with check on interfaces section (see pf_interfaces) + */ +function is_interface_group($if) +{ + global $config; + + if (isset($config['ifgroups']['ifgroupentry'])) { + foreach ($config['ifgroups']['ifgroupentry'] as $groupentry) { + if ($groupentry['ifname'] === $if) { + return true; + } + } + } + + return false; +} + function filter_configure_sync($verbose = false, $flush_states = false, $load_aliases = true) { global $config; @@ -160,6 +221,12 @@ function filter_configure_sync($verbose = false, $flush_states = false, $load_al /* Use filter lock to not allow concurrent filter reloads during this run. */ $filterlck = lock('filter', LOCK_EX); + ifgroup_setup(); + if ($verbose) { + echo '.'; + flush(); + } + // initialize fw plugin object $fw = filter_core_get_initialized_plugin_system(); filter_core_bootstrap($fw); diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc index d749476a6..de0c382b7 100644 --- a/src/etc/inc/interfaces.inc +++ b/src/etc/inc/interfaces.inc @@ -962,8 +962,6 @@ function interfaces_configure($verbose = false) plugins_configure('ipsec_prepare', $verbose); plugins_configure('openvpn_prepare', $verbose); plugins_configure('vxlan_prepare', $verbose); - - interfaces_group_setup(); } function interface_vip_bring_down($vip) @@ -2561,11 +2559,6 @@ function interface_configure($verbose = false, $interface = 'wan', $reload = fal interface_bridge_add_member($bridgetmp, $realif); } } - - $grouptmp = link_interface_to_group($interface); - if (!empty($grouptmp)) { - array_walk($grouptmp, 'interface_group_add_member'); - } } if ($verbose) { @@ -3447,50 +3440,6 @@ function DHCP_Config_File_Substitutions($wancfg, $wanif, $dhclientconf) return $dhclientconf; } -function interfaces_group_setup() -{ - global $config; - - if (!isset($config['ifgroups']['ifgroupentry'])) { - return; - } - - foreach ($config['ifgroups']['ifgroupentry'] as $groupar) { - interface_group_setup($groupar); - } -} - -function interface_group_setup(&$groupname) -{ - if (!is_array($groupname)) { - return; - } - - $members = explode(' ', $groupname['members']); - foreach ($members as $ifs) { - interface_group_add_member($ifs, $groupname['ifname']); - } -} - -function is_interface_group($if) -{ - global $config; - - if (isset($config['ifgroups']['ifgroupentry'])) { - foreach ($config['ifgroups']['ifgroupentry'] as $groupentry) { - if ($groupentry['ifname'] === $if) { - return true; - } - } - } - - return false; -} - -function interface_group_add_member($interface, $groupname) -{ - mwexecf('/sbin/ifconfig %s group %s', array(get_real_interface($interface), $groupname)); -} /* convert fxp0 -> wan, etc. */ function convert_real_interface_to_friendly_interface_name($interface = 'wan') diff --git a/src/etc/inc/plugins.inc.d/pf.inc b/src/etc/inc/plugins.inc.d/pf.inc index c7e57bf56..47b24ebb3 100644 --- a/src/etc/inc/plugins.inc.d/pf.inc +++ b/src/etc/inc/plugins.inc.d/pf.inc @@ -42,15 +42,6 @@ function pf_services() 'nocheck' => true, 'name' => 'pf', ); - $services[] = array( - 'description' => gettext('Interface groups'), - 'nocheck' => true, - 'php' => array( - 'start' => array('interfaces_group_setup'), - 'restart' => array('interfaces_group_setup'), - ), - 'name' => 'ifgroups', - ); } return $services; diff --git a/src/etc/rc.newwanip b/src/etc/rc.newwanip index 7cd1e0c9a..7288b8e56 100755 --- a/src/etc/rc.newwanip +++ b/src/etc/rc.newwanip @@ -99,12 +99,6 @@ if (!empty($gif)) { array_walk($gif, 'interface_gif_configure'); } -$grouptmp = link_interface_to_group($interface); -if (!empty($grouptmp)) { - array_walk($grouptmp, 'interface_group_add_member'); -} - -unset($bridgetmp); $bridgetmp = link_interface_to_bridge($interface); if (!empty($bridgetmp)) { interface_bridge_add_member($bridgetmp, $interface_real); diff --git a/src/etc/rc.newwanipv6 b/src/etc/rc.newwanipv6 index a7562d147..ce54a4244 100755 --- a/src/etc/rc.newwanipv6 +++ b/src/etc/rc.newwanipv6 @@ -109,11 +109,6 @@ if (is_ipaddr($ip)) { interfaces_vips_configure(false, $interface); -$grouptmp = link_interface_to_group($interface); -if (!empty($grouptmp)) { - array_walk($grouptmp, 'interface_group_add_member'); -} - if (count(link_interface_to_track6($interface, true))) { plugins_configure('dhcp', false, array('inet6')); } diff --git a/src/www/interfaces_groups_edit.php b/src/www/interfaces_groups_edit.php index a0009c34c..ff6bc23f0 100644 --- a/src/www/interfaces_groups_edit.php +++ b/src/www/interfaces_groups_edit.php @@ -113,28 +113,19 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } } } - mark_subsystem_dirty('filter'); - } - $old_ifname = isset($id) ? $a_ifgroups[$id]['ifname'] : $pconfig['ifname']; - // remove group members - foreach (explode(" ", $a_ifgroups[$id]['members']) as $old_member) { - if (!in_array($old_member, $pconfig['members']) || $old_ifname != $pconfig['ifname']) { - mwexecf('/sbin/ifconfig %s -group %s', array(get_real_interface($old_member), $a_ifgroups[$id]['ifname'])); - } } // update item $a_ifgroups[$id] = $ifgroupentry; } else { - mark_subsystem_dirty('filter'); // add new item $a_ifgroups[] = $ifgroupentry; } + mark_subsystem_dirty('filter'); usort($a_ifgroups, function($a, $b) { return strnatcmp($a['ifname'], $b['ifname']); }); filter_rules_sort(); write_config(); - interface_group_setup($ifgroupentry); header(url_safe('Location: /interfaces_groups.php')); exit; }