From 2f17a30ba04082889f3a703aca1884b031767942 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Fri, 29 Apr 2016 18:01:51 -0700 Subject: [PATCH] OVS Mech: Set hybrid plug based on agent config This adjusts the logic in the OVS mechanism driver to determine what the ovs_hybrid_plug value should be set to in the VIF details. Previously it was based purely on the firewall driver configured on the server side. This prevented a mixed environment where some agents might be running a native OVS firewall driver while others are still based on the IPTables hybrid driver. This patch has the OVS agents report back whether they want hybrid plugging in their configuration dictionary sent during report_state. The OVS agent sets this based on an explicit attribute on the firewall driver requesting OVS hybrid plugging. To maintain backward compat, if an agent doesn't report this, the old logic of basing it off of the server-side config is applied. DocImpact: The server no longer needs to be configured with a firewall driver for OVS. It will read config from agent state reports. Closes-Bug: #1560957 Change-Id: Ie554c2d37ce036e7b51818048153b466eee02913 --- neutron/agent/linux/iptables_firewall.py | 1 + .../openvswitch/agent/ovs_neutron_agent.py | 55 +++++++++++-------- .../mech_driver/mech_openvswitch.py | 14 +++-- .../agent/test_ovs_neutron_agent.py | 39 +++++++++++-- .../mech_driver/test_mech_openvswitch.py | 44 +++++++++++++++ ...not_needed_on_server-4159669ad834dea6.yaml | 12 ++++ 6 files changed, 132 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/firewall_driver_not_needed_on_server-4159669ad834dea6.yaml diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 00d4535e8aa..b59ac786016 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -896,6 +896,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver): OVS_HYBRID_TAP_PREFIX = constants.TAP_DEVICE_PREFIX + OVS_HYBRID_PLUG_REQUIRED = True def _port_chain_name(self, port, direction): return iptables_manager.get_chain_name( 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 bce4fd7f4b9..1eb9547fa3b 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -44,6 +44,7 @@ from neutron.common import ipv6_utils as ipv6 from neutron.common import topics from neutron.common import utils as n_utils from neutron import context +from neutron.extensions import portbindings from neutron.plugins.common import constants as p_const from neutron.plugins.common import utils as p_utils from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc @@ -228,6 +229,34 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.enable_tunneling, self.enable_distributed_routing) + if self.enable_tunneling: + self.setup_tunnel_br_flows() + + self.dvr_agent.setup_dvr_flows() + + # Collect additional bridges to monitor + self.ancillary_brs = self.setup_ancillary_bridges( + ovs_conf.integration_bridge, ovs_conf.tunnel_bridge) + + # In order to keep existed device's local vlan unchanged, + # restore local vlan mapping at start + self._restore_local_vlan_map() + + # Security group agent support + self.sg_agent = sg_rpc.SecurityGroupAgentRpc(self.context, + self.sg_plugin_rpc, self.local_vlan_map, + defer_refresh_firewall=True, integration_bridge=self.int_br) + + # we default to False to provide backward compat with out of tree + # firewall drivers that expect the logic that existed on the Neutron + # server which only enabled hybrid plugging based on the use of the + # hybrid driver. + hybrid_plug = getattr(self.sg_agent.firewall, + 'OVS_HYBRID_PLUG_REQUIRED', False) + self.prevent_arp_spoofing = ( + agent_conf.prevent_arp_spoofing and + not self.sg_agent.firewall.provides_arp_spoofing_protection) + #TODO(mangelajo): optimize resource_versions to only report # versions about resources which are common, # or which are used by specific extensions. @@ -249,7 +278,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, 'datapath_type': ovs_conf.datapath_type, 'ovs_capabilities': self.ovs.capabilities, 'vhostuser_socket_dir': - ovs_conf.vhostuser_socket_dir}, + ovs_conf.vhostuser_socket_dir, + portbindings.OVS_HYBRID_PLUG: hybrid_plug}, 'resource_versions': resources.LOCAL_RESOURCE_VERSIONS, 'agent_type': agent_conf.agent_type, 'start_flag': True} @@ -259,29 +289,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, heartbeat = loopingcall.FixedIntervalLoopingCall( self._report_state) heartbeat.start(interval=report_interval) - - if self.enable_tunneling: - self.setup_tunnel_br_flows() - - self.dvr_agent.setup_dvr_flows() - - # Collect additional bridges to monitor - self.ancillary_brs = self.setup_ancillary_bridges( - ovs_conf.integration_bridge, ovs_conf.tunnel_bridge) - - # In order to keep existed device's local vlan unchanged, - # restore local vlan mapping at start - self._restore_local_vlan_map() - - # Security group agent support - self.sg_agent = sg_rpc.SecurityGroupAgentRpc(self.context, - self.sg_plugin_rpc, self.local_vlan_map, - defer_refresh_firewall=True, integration_bridge=self.int_br) - - self.prevent_arp_spoofing = ( - agent_conf.prevent_arp_spoofing and - not self.sg_agent.firewall.provides_arp_spoofing_protection) - # Initialize iteration counter self.iter_num = 0 self.run_daemon_loop = True diff --git a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py index 5ac14f0a6a8..339c8c5c9e3 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py @@ -85,10 +85,16 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): return self.vif_type def get_vif_details(self, agent, context): - if (agent['configurations'].get('datapath_type') != - a_const.OVS_DATAPATH_NETDEV): - return self.vif_details - caps = agent['configurations'].get('ovs_capabilities', {}) + a_config = agent['configurations'] + if a_config.get('datapath_type') != a_const.OVS_DATAPATH_NETDEV: + details = dict(self.vif_details) + hybrid = portbindings.OVS_HYBRID_PLUG + if hybrid in a_config: + # we only override the vif_details for hybrid pluggin set + # in the constuctor if the agent specifically requests it + details[hybrid] = a_config[hybrid] + return details + caps = a_config.get('ovs_capabilities', {}) if a_const.OVS_DPDK_VHOST_USER in caps.get('iface_types', []): sock_path = self.agent_vhu_sockpath(agent, context.current['id']) return { diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index bd98ed61311..a7a98f9c15f 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -115,6 +115,10 @@ class TestOvsNeutronAgent(object): mock.patch('neutron.agent.common.ovs_lib.BaseOVS.config', new_callable=mock.PropertyMock, return_value={}).start() + self.agent = self._make_agent() + self.agent.sg_agent = mock.Mock() + + def _make_agent(self): with mock.patch.object(self.mod_agent.OVSNeutronAgent, 'setup_integration_br'),\ mock.patch.object(self.mod_agent.OVSNeutronAgent, @@ -129,10 +133,10 @@ class TestOvsNeutronAgent(object): mock.patch( 'neutron.agent.common.ovs_lib.OVSBridge.' 'get_vif_ports', return_value=[]): - self.agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(), - cfg.CONF) - self.agent.tun_br = self.br_tun_cls(br_name='br-tun') - self.agent.sg_agent = mock.Mock() + agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(), + cfg.CONF) + agent.tun_br = self.br_tun_cls(br_name='br-tun') + return agent def _mock_port_bound(self, ofport=None, new_local_vlan=None, old_local_vlan=None, db_get_val=None): @@ -933,6 +937,31 @@ class TestOvsNeutronAgent(object): setup_port_filters.assert_called_once_with( set(), port_info.get('updated', set())) + def test_hybrid_plug_flag_based_on_firewall(self): + cfg.CONF.set_default( + 'firewall_driver', + 'neutron.agent.firewall.NoopFirewallDriver', + group='SECURITYGROUP') + agt = self._make_agent() + self.assertFalse(agt.agent_state['configurations']['ovs_hybrid_plug']) + cfg.CONF.set_default( + 'firewall_driver', + 'neutron.agent.linux.openvswitch_firewall.OVSFirewallDriver', + group='SECURITYGROUP') + with mock.patch('neutron.agent.linux.openvswitch_firewall.' + 'OVSFirewallDriver.initialize_bridge'): + agt = self._make_agent() + self.assertFalse(agt.agent_state['configurations']['ovs_hybrid_plug']) + cfg.CONF.set_default( + 'firewall_driver', + 'neutron.agent.linux.iptables_firewall.' + 'OVSHybridIptablesFirewallDriver', + group='SECURITYGROUP') + with mock.patch('neutron.agent.linux.iptables_firewall.' + 'IptablesFirewallDriver._populate_initial_zone_map'): + agt = self._make_agent() + self.assertTrue(agt.agent_state['configurations']['ovs_hybrid_plug']) + def test_report_state(self): with mock.patch.object(self.agent.state_rpc, "report_state") as report_st: @@ -2077,7 +2106,7 @@ class TestOvsDvrNeutronAgent(object): 'neutron.agent.common.ovs_lib.OVSBridge.' 'get_vif_ports', return_value=[]): self.agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(), - cfg.CONF) + cfg.CONF) self.agent.tun_br = self.br_tun_cls(br_name='br-tun') self.agent.sg_agent = mock.Mock() diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py index 801479a109d..9ccc900e641 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py @@ -17,6 +17,7 @@ from oslo_config import cfg from neutron.common import constants from neutron.extensions import portbindings +from neutron.plugins.ml2 import driver_api as api from neutron.plugins.ml2.drivers.openvswitch.mech_driver \ import mech_openvswitch from neutron.tests.unit.plugins.ml2 import _test_mech_agent as base @@ -71,6 +72,49 @@ class OpenvswitchMechanismSGDisabledBaseTestCase( super(OpenvswitchMechanismSGDisabledBaseTestCase, self).setUp() +class OpenvswitchMechanismHybridPlugTestCase(OpenvswitchMechanismBaseTestCase): + + def _make_port_ctx(self, agents): + segments = [{api.ID: 'local_segment_id', api.NETWORK_TYPE: 'local'}] + return base.FakePortContext(self.AGENT_TYPE, agents, segments, + vnic_type=self.VNIC_TYPE) + + def test_backward_compat_with_unreporting_agent(self): + hybrid = portbindings.OVS_HYBRID_PLUG + # agent didn't report so it should be hybrid based on server config + context = self._make_port_ctx(self.AGENTS) + self.driver.bind_port(context) + self.assertTrue(context._bound_vif_details[hybrid]) + self.driver.vif_details[hybrid] = False + context = self._make_port_ctx(self.AGENTS) + self.driver.bind_port(context) + self.assertFalse(context._bound_vif_details[hybrid]) + + def test_hybrid_plug_true_if_agent_requests(self): + hybrid = portbindings.OVS_HYBRID_PLUG + # set server side default to false and ensure that hybrid becomes + # true if requested by the agent + self.driver.vif_details[hybrid] = False + agents = [{'alive': True, + 'configurations': {hybrid: True}, + 'host': 'host'}] + context = self._make_port_ctx(agents) + self.driver.bind_port(context) + self.assertTrue(context._bound_vif_details[hybrid]) + + def test_hybrid_plug_false_if_agent_requests(self): + hybrid = portbindings.OVS_HYBRID_PLUG + # set server side default to true and ensure that hybrid becomes + # false if requested by the agent + self.driver.vif_details[hybrid] = True + agents = [{'alive': True, + 'configurations': {hybrid: False}, + 'host': 'host'}] + context = self._make_port_ctx(agents) + self.driver.bind_port(context) + self.assertFalse(context._bound_vif_details[hybrid]) + + class OpenvswitchMechanismGenericTestCase(OpenvswitchMechanismBaseTestCase, base.AgentMechanismGenericTestCase): pass diff --git a/releasenotes/notes/firewall_driver_not_needed_on_server-4159669ad834dea6.yaml b/releasenotes/notes/firewall_driver_not_needed_on_server-4159669ad834dea6.yaml new file mode 100644 index 00000000000..e7219091358 --- /dev/null +++ b/releasenotes/notes/firewall_driver_not_needed_on_server-4159669ad834dea6.yaml @@ -0,0 +1,12 @@ +--- +prelude: > + The Neutron server no longer needs to be configured + with a firewall driver and it can support mixed + environments of hybrid iptables firewalls and the + pure OVS firewall. +features: + - The Neutron server now learns the appropriate firewall + wiring behavior from each OVS agent so it no longer needs + to be configured with the firewall_driver. This means + it also supports multiple agents with different types + of firewalls.