From 959d8b6d73e2a6ab1a45c9a7b0b05ae163e650fc Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Fri, 10 Jul 2020 17:25:15 +0800 Subject: [PATCH] Local mac direct flow for non-openflow firewall When there is no openflow firewall, aka the ovs agent security group is disabled or Noop/HybridIptable, this patch will introduce a different ingress pipeline for bridge ports which will avoid ingress flood: (1) table=0, in_port=patch_bridge,dl_vlan=physical_vlan action=mod_vlan:local_vlan,goto:60 (original) (2) table=60, in_port=patch_bridge action=goto:61 (new) (3) table=61, dl_dst=local_port_mac,dl_vlan=local_vlan, action=strip_vlan,output: (changes) And changes the local ports pipeline: (1) table=0, in_port=local_ofport action=goto:25 (original) (2) table=25, in_port=local_ofport,dl_src=local_port_mac action=goto:60 (original) (3) table=60, in_port=local_ofport,dl_src=local_port_mac action=local_vlan->reg6,goto:61 (changes) (4) table=61, dl_dst=local_port_mac,reg6=local_vlan, action=output: (changes) Closes-Bug: #1884708 Closes-Bug: #1881070 Related-Bug: #1732067 Related-Bug: #1866445 Related-Bug: #1883321 Change-Id: Iecf9cffaf02616342f1727ad7db85545d8adbec2 --- .../openvswitch/agent/common/constants.py | 4 +- .../openvswitch/agent/ovs_neutron_agent.py | 162 ++++++++++++------ .../agent/openflow/native/test_br_int.py | 2 +- .../openvswitch/agent/test_ovs_tunnel.py | 28 +++ 4 files changed, 144 insertions(+), 52 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index 060ab85beb1..dfa37e804b8 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -59,7 +59,8 @@ MAC_SPOOF_TABLE = 25 # Table to decide whether further filtering is needed TRANSIENT_TABLE = 60 -TRANSIENT_EGRESS_TABLE = 61 +LOCAL_MAC_DIRECT = 61 +TRANSIENT_EGRESS_TABLE = 62 # Tables used for ovs firewall BASE_EGRESS_TABLE = 71 @@ -89,6 +90,7 @@ INT_BR_ALL_TABLES = ( CANARY_TABLE, ARP_SPOOF_TABLE, MAC_SPOOF_TABLE, + LOCAL_MAC_DIRECT, TRANSIENT_TABLE, TRANSIENT_EGRESS_TABLE, BASE_EGRESS_TABLE, diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 5aa231d47f5..21a44fcdb28 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -55,6 +55,7 @@ from neutron.agent.common import polling from neutron.agent.common import utils from neutron.agent import firewall as agent_firewall from neutron.agent.l2 import l2_agent_extensions_manager as ext_manager +from neutron.agent.linux import iptables_firewall from neutron.agent.linux import xenapi_root_helper from neutron.agent import rpc as agent_rpc from neutron.agent import securitygroups_rpc as agent_sg_rpc @@ -373,6 +374,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.quitting_rpc_timeout = agent_conf.quitting_rpc_timeout + self.install_ingress_direct_goto_flows() + def _parse_bridge_mappings(self, bridge_mappings): try: return helpers.parse_mappings(bridge_mappings) @@ -669,6 +672,23 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, lvm = self.vlan_manager.get(network_id) return lvm.vlan + def _deferred_delete_direct_flows(self, ports): + if not self.direct_for_non_openflow_firewall: + return + with self.int_br.deferred(full_ordered=True, + use_bundle=True) as int_br: + for port_id in ports: + lvm, vif_port, _net_id = self._get_port_lvm_and_vif(port_id) + try: + self.delete_accepted_egress_direct_flow( + int_br, + vif_port.ofport, + vif_port.vif_mac, + lvm.vlan) + except Exception as err: + LOG.debug("Failed to remove accepted egress flows " + "for port %s, error: %s", port_id, err) + def process_deleted_ports(self, port_info): # don't try to process removed ports as deleted ports since # they are already gone @@ -676,35 +696,23 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.deleted_ports -= port_info['removed'] deleted_ports = list(self.deleted_ports) - with self.int_br.deferred(full_ordered=True, - use_bundle=True) as int_br: - while self.deleted_ports: - port_id = self.deleted_ports.pop() - port = self.int_br.get_vif_port_by_id(port_id) + self._deferred_delete_direct_flows(self.deleted_ports) - if (isinstance(self.sg_agent.firewall, - agent_firewall.NoopFirewallDriver) or - not agent_sg_rpc.is_firewall_enabled()): - try: - self.delete_accepted_egress_direct_flow( - int_br, - port.ofport, - port.mac, self._get_port_local_vlan(port_id)) - except Exception as err: - LOG.debug("Failed to remove accepted egress flows " - "for port %s, error: %s", port_id, err) + while self.deleted_ports: + port_id = self.deleted_ports.pop() + port = self.int_br.get_vif_port_by_id(port_id) - self._clean_network_ports(port_id) - self.ext_manager.delete_port(self.context, - {"vif_port": port, - "port_id": port_id}) - # move to dead VLAN so deleted ports no - # longer have access to the network - if port: - # don't log errors since there is a chance someone will be - # removing the port from the bridge at the same time - self.port_dead(port, log_errors=False) - self.port_unbound(port_id) + self._clean_network_ports(port_id) + self.ext_manager.delete_port(self.context, + {"vif_port": port, + "port_id": port_id}) + # move to dead VLAN so deleted ports no + # longer have access to the network + if port: + # don't log errors since there is a chance someone will be + # removing the port from the bridge at the same time + self.port_dead(port, log_errors=False) + self.port_unbound(port_id) # Flush firewall rules after ports are put on dead VLAN to be # more secure @@ -1273,6 +1281,22 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, else: bridge.delete_arp_spoofing_protection(port=vif.ofport) + def _get_port_lvm_and_vif(self, vif_id, net_uuid=None): + try: + net_uuid = net_uuid or self.vlan_manager.get_net_uuid(vif_id) + except vlanmanager.VifIdNotFound: + LOG.info('net_uuid %s not managed by VLAN manager', + net_uuid) + return None, None, None + + lvm = self.vlan_manager.get(net_uuid) + + if vif_id in lvm.vif_ports: + vif_port = lvm.vif_ports[vif_id] + return lvm, vif_port, net_uuid + + return lvm, None, net_uuid + def port_unbound(self, vif_id, net_uuid=None): '''Unbind port. @@ -1282,18 +1306,11 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, :param vif_id: the id of the vif :param net_uuid: the net_uuid this port is associated with. ''' - try: - net_uuid = net_uuid or self.vlan_manager.get_net_uuid(vif_id) - except vlanmanager.VifIdNotFound: - LOG.info( - 'port_unbound(): net_uuid %s not managed by VLAN manager', - net_uuid) + lvm, vif_port, net_uuid = self._get_port_lvm_and_vif(vif_id, net_uuid) + if not lvm: return - lvm = self.vlan_manager.get(net_uuid) - - if vif_id in lvm.vif_ports: - vif_port = lvm.vif_ports[vif_id] + if vif_port and vif_id in lvm.vif_ports: self.dvr_agent.unbind_port_from_dvr(vif_port, lvm) lvm.vif_ports.pop(vif_id, None) @@ -2003,6 +2020,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.conf.host) failed_devices = set(devices_down.get('failed_devices_down')) LOG.debug("Port removal failed for %s", failed_devices) + self._deferred_delete_direct_flows(devices) for device in devices: self.ext_manager.delete_port(self.context, {'port_id': device}) self.port_unbound(device) @@ -2115,13 +2133,38 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, 'elapsed': time.time() - start}) return failed_devices - def process_install_ports_egress_flows(self, ports): - if not self.conf.AGENT.explicitly_egress_direct: - return + @property + def direct_for_non_openflow_firewall(self): + return ((isinstance(self.sg_agent.firewall, + agent_firewall.NoopFirewallDriver) or + isinstance( + self.sg_agent.firewall, + iptables_firewall.OVSHybridIptablesFirewallDriver) or + not agent_sg_rpc.is_firewall_enabled()) and + self.conf.AGENT.explicitly_egress_direct) - if (isinstance(self.sg_agent.firewall, - agent_firewall.NoopFirewallDriver) or - not agent_sg_rpc.is_firewall_enabled()): + def install_ingress_direct_goto_flows(self): + if self.direct_for_non_openflow_firewall: + for physical_network in self.bridge_mappings: + self.int_br.install_goto( + table_id=constants.TRANSIENT_TABLE, + dest_table_id=constants.LOCAL_MAC_DIRECT, + priority=4, # a bit higher than NORMAL + in_port=self.int_ofports[physical_network]) + + if self.enable_tunneling: + self.int_br.install_goto( + table_id=constants.TRANSIENT_TABLE, + dest_table_id=constants.LOCAL_MAC_DIRECT, + priority=4, # a bit higher than NORMAL + in_port=self.patch_tun_ofport) + + self.int_br.install_goto( + table_id=constants.LOCAL_MAC_DIRECT, + dest_table_id=constants.TRANSIENT_EGRESS_TABLE) + + def process_install_ports_egress_flows(self, ports): + if self.direct_for_non_openflow_firewall: with self.int_br.deferred(full_ordered=True, use_bundle=True) as int_br: for port in ports: @@ -2138,17 +2181,31 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, lvm = self.vlan_manager.get(port_detail['network_id']) port = port_detail['vif_port'] + # Adding the local vlan to register 6 in case of MAC overlapping + # in different networks. br_int.add_flow( table=constants.TRANSIENT_TABLE, priority=9, in_port=port.ofport, dl_src=port_detail['mac_address'], - actions='resubmit(,{:d})'.format( - constants.TRANSIENT_EGRESS_TABLE)) + actions='set_field:{:d}->reg6,' + 'resubmit(,{:d})'.format( + lvm.vlan, + constants.LOCAL_MAC_DIRECT)) + # For packets from patch ports. br_int.add_flow( - table=constants.TRANSIENT_EGRESS_TABLE, + table=constants.LOCAL_MAC_DIRECT, priority=12, + dl_vlan=lvm.vlan, + dl_dst=port_detail['mac_address'], + actions='strip_vlan,output:{:d}'.format(port.ofport)) + + # For packets from internal ports or VM ports. + br_int.add_flow( + table=constants.LOCAL_MAC_DIRECT, + priority=12, + reg6=lvm.vlan, dl_dst=port_detail['mac_address'], actions='output:{:d}'.format(port.ofport)) @@ -2176,18 +2233,23 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, patch_ofport)) def delete_accepted_egress_direct_flow(self, br_int, ofport, mac, vlan): - if not self.conf.AGENT.explicitly_egress_direct: + if not self.direct_for_non_openflow_firewall: return br_int.delete_flows( table=constants.TRANSIENT_TABLE, in_port=ofport, dl_src=mac) - self.delete_flows( - table=constants.TRANSIENT_EGRESS_TABLE, + br_int.delete_flows( + table=constants.LOCAL_MAC_DIRECT, + dl_vlan=vlan, + dl_dst=mac) + br_int.delete_flows( + table=constants.LOCAL_MAC_DIRECT, + reg6=vlan, dl_dst=mac) - self.delete_flows( + br_int.delete_flows( table=constants.TRANSIENT_EGRESS_TABLE, dl_src=mac, in_port=ofport) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py index 4ce50c8e06c..0c479dd656e 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py @@ -88,7 +88,7 @@ class OVSIntegrationBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase): ], match=ofpp.OFPMatch(), priority=3, - table_id=61), + table_id=constants.TRANSIENT_EGRESS_TABLE), active_bundle=None), ] self.assertEqual(expected, self.mock.mock_calls) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 914bf7d231d..edccc6beb97 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -279,6 +279,20 @@ class TunnelTest(object): self.intb_expected = [] self.execute_expected = [] + self.mock_int_bridge_expected += [ + mock.call.install_goto( + dest_table_id=constants.LOCAL_MAC_DIRECT, + in_port=self.MAP_TUN_INT_OFPORT, + priority=4, table_id=constants.TRANSIENT_TABLE), + mock.call.install_goto( + dest_table_id=constants.LOCAL_MAC_DIRECT, + in_port=self.TUN_OFPORT, + priority=4, table_id=constants.TRANSIENT_TABLE), + mock.call.install_goto( + dest_table_id=constants.TRANSIENT_EGRESS_TABLE, + table_id=constants.LOCAL_MAC_DIRECT), + ] + def _build_agent(self, **config_opts_agent): """Configure and initialize OVS agent. @@ -750,6 +764,20 @@ class TunnelTestUseVethInterco(TunnelTest): self.execute_expected = [mock.call(['udevadm', 'settle', '--timeout=10'])] + self.mock_int_bridge_expected += [ + mock.call.install_goto( + dest_table_id=constants.LOCAL_MAC_DIRECT, + in_port=self.MAP_TUN_INT_OFPORT, + priority=4, table_id=constants.TRANSIENT_TABLE), + mock.call.install_goto( + dest_table_id=constants.LOCAL_MAC_DIRECT, + in_port=self.TUN_OFPORT, + priority=4, table_id=constants.TRANSIENT_TABLE), + mock.call.install_goto( + dest_table_id=constants.TRANSIENT_EGRESS_TABLE, + table_id=constants.LOCAL_MAC_DIRECT), + ] + class TunnelTestUseVethIntercoOSKen(TunnelTestUseVethInterco, ovs_test_base.OVSOSKenTestBase):