From b0c8c7e226c70bab77eef878c9b0232970dc3b90 Mon Sep 17 00:00:00 2001 From: Pierre Rognant Date: Wed, 28 May 2014 14:18:45 -0400 Subject: [PATCH] Add an option to turn off DF for GRE and VXLAN tunnels Modifications included allow to set a new option (dont_fragment) in the ovs agent configuration file that can be used for (un-)setting the DF bit on GRE or VXLAN tunnels. The default behaviour is not altered (DF on). Change-Id: I17ecb00165990b72ab121c2688097139b3f2f157 Implements: blueprint neutron-ovs-agent-df-gre-vxlan --- .../openvswitch/ovs_neutron_plugin.ini | 5 +++ neutron/agent/linux/ovs_lib.py | 5 ++- .../ofagent/agent/ofa_neutron_agent.py | 4 +- .../openvswitch/agent/ovs_neutron_agent.py | 4 +- neutron/plugins/openvswitch/common/config.py | 3 ++ .../tests/unit/agent/linux/test_ovs_lib.py | 39 ++++++++++++++++++- .../unit/ofagent/test_ofa_neutron_agent.py | 4 +- .../openvswitch/test_ovs_neutron_agent.py | 21 +++++++++- .../tests/unit/openvswitch/test_ovs_tunnel.py | 2 +- 9 files changed, 78 insertions(+), 9 deletions(-) diff --git a/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini b/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini index 1718d4dd60..4beee58fa8 100644 --- a/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini +++ b/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini @@ -133,6 +133,11 @@ # # arp_responder = False +# (BoolOpt) Set or un-set the don't fragment (DF) bit on outgoing IP packet +# carrying GRE/VXLAN tunnel. The default value is True. +# +# dont_fragment = True + [securitygroup] # Firewall driver for realizing neutron security group function. # firewall_driver = neutron.agent.firewall.NoopFirewallDriver diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index 36b4faea45..2827dba194 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -239,7 +239,8 @@ class OVSBridge(BaseOVS): def add_tunnel_port(self, port_name, remote_ip, local_ip, tunnel_type=p_const.TYPE_GRE, - vxlan_udp_port=constants.VXLAN_UDP_PORT): + vxlan_udp_port=constants.VXLAN_UDP_PORT, + dont_fragment=True): vsctl_command = ["--", "--may-exist", "add-port", self.br_name, port_name] vsctl_command.extend(["--", "set", "Interface", port_name, @@ -248,6 +249,8 @@ class OVSBridge(BaseOVS): # Only set the VXLAN UDP port if it's not the default if vxlan_udp_port != constants.VXLAN_UDP_PORT: vsctl_command.append("options:dst_port=%s" % vxlan_udp_port) + vsctl_command.append(("options:df_default=%s" % + bool(dont_fragment)).lower()) vsctl_command.extend(["options:remote_ip=%s" % remote_ip, "options:local_ip=%s" % local_ip, "options:in_key=flow", diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py index 16348e20d8..92f66c81ef 100644 --- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py +++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py @@ -267,6 +267,7 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self.local_ip = local_ip self.tunnel_count = 0 self.vxlan_udp_port = cfg.CONF.AGENT.vxlan_udp_port + self.dont_fragment = cfg.CONF.AGENT.dont_fragment if self.enable_tunneling: self.setup_tunnel_br(tun_br) # Collect additional bridges to monitor @@ -1030,7 +1031,8 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): remote_ip, self.local_ip, tunnel_type, - self.vxlan_udp_port) + self.vxlan_udp_port, + self.dont_fragment) ofport_int = -1 try: ofport_int = int(ofport) diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 9c2f146eff..f654bfab1a 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -223,6 +223,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.local_ip = local_ip self.tunnel_count = 0 self.vxlan_udp_port = cfg.CONF.AGENT.vxlan_udp_port + self.dont_fragment = cfg.CONF.AGENT.dont_fragment self.tun_br = None if self.enable_tunneling: self.setup_tunnel_br(tun_br) @@ -1035,7 +1036,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, remote_ip, self.local_ip, tunnel_type, - self.vxlan_udp_port) + self.vxlan_udp_port, + self.dont_fragment) ofport_int = -1 try: ofport_int = int(ofport) diff --git a/neutron/plugins/openvswitch/common/config.py b/neutron/plugins/openvswitch/common/config.py index f27be19b17..07ba941689 100644 --- a/neutron/plugins/openvswitch/common/config.py +++ b/neutron/plugins/openvswitch/common/config.py @@ -82,6 +82,9 @@ agent_opts = [ "remote mac and IPs and improve tunnel scalability")), cfg.BoolOpt('arp_responder', default=False, help=_("Enable local ARP responder if it is supported")), + cfg.BoolOpt('dont_fragment', default=True, + help=_("Set or un-set the don't fragment (DF) bit on " + "outgoing IP packet carrying GRE/VXLAN tunnel")), ] diff --git a/neutron/tests/unit/agent/linux/test_ovs_lib.py b/neutron/tests/unit/agent/linux/test_ovs_lib.py index 951dd27bd9..81671bc011 100644 --- a/neutron/tests/unit/agent/linux/test_ovs_lib.py +++ b/neutron/tests/unit/agent/linux/test_ovs_lib.py @@ -22,6 +22,7 @@ from neutron.agent.linux import utils from neutron.common import exceptions from neutron.openstack.common import jsonutils from neutron.openstack.common import uuidutils +from neutron.plugins.common import constants as p_const from neutron.plugins.openvswitch.common import constants as const from neutron.tests import base from neutron.tests import tools @@ -496,7 +497,8 @@ class OVS_Lib_Test(base.BaseTestCase): command = ["ovs-vsctl", self.TO, '--', "--may-exist", "add-port", self.BR_NAME, pname] command.extend(["--", "set", "Interface", pname]) - command.extend(["type=gre", "options:remote_ip=" + remote_ip, + command.extend(["type=gre", "options:df_default=true", + "options:remote_ip=" + remote_ip, "options:local_ip=" + local_ip, "options:in_key=flow", "options:out_key=flow"]) @@ -516,6 +518,41 @@ class OVS_Lib_Test(base.BaseTestCase): tools.verify_mock_calls(self.execute, expected_calls_and_values) + def test_add_vxlan_fragmented_tunnel_port(self): + pname = "tap99" + local_ip = "1.1.1.1" + remote_ip = "9.9.9.9" + ofport = "6" + vxlan_udp_port = "9999" + dont_fragment = False + command = ["ovs-vsctl", self.TO, '--', "--may-exist", "add-port", + self.BR_NAME, pname] + command.extend(["--", "set", "Interface", pname]) + command.extend(["type=" + p_const.TYPE_VXLAN, + "options:dst_port=" + vxlan_udp_port, + "options:df_default=false", + "options:remote_ip=" + remote_ip, + "options:local_ip=" + local_ip, + "options:in_key=flow", + "options:out_key=flow"]) + # Each element is a tuple of (expected mock call, return_value) + expected_calls_and_values = [ + (mock.call(command, root_helper=self.root_helper), None), + (mock.call(["ovs-vsctl", self.TO, "get", + "Interface", pname, "ofport"], + root_helper=self.root_helper), + ofport), + ] + tools.setup_mock_calls(self.execute, expected_calls_and_values) + + self.assertEqual( + self.br.add_tunnel_port(pname, remote_ip, local_ip, + p_const.TYPE_VXLAN, vxlan_udp_port, + dont_fragment), + ofport) + + tools.verify_mock_calls(self.execute, expected_calls_and_values) + def test_add_patch_port(self): pname = "tap99" peer = "bar10" diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py index 97b48d45b3..b8d8569f5d 100644 --- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py +++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py @@ -643,7 +643,7 @@ class TestOFANeutronAgent(OFAAgentTestCase): 'gre-1', 'remote_ip', p_const.TYPE_GRE) add_tunnel_port_fn.assert_called_once_with( 'gre-1', 'remote_ip', self.agent.local_ip, p_const.TYPE_GRE, - self.agent.vxlan_udp_port) + self.agent.vxlan_udp_port, self.agent.dont_fragment) log_error_fn.assert_called_once_with( _("Failed to set-up %(type)s tunnel port to %(ip)s"), {'type': p_const.TYPE_GRE, 'ip': 'remote_ip'}) @@ -660,7 +660,7 @@ class TestOFANeutronAgent(OFAAgentTestCase): 'gre-1', 'remote_ip', p_const.TYPE_GRE) add_tunnel_port_fn.assert_called_once_with( 'gre-1', 'remote_ip', self.agent.local_ip, p_const.TYPE_GRE, - self.agent.vxlan_udp_port) + self.agent.vxlan_udp_port, self.agent.dont_fragment) log_exc_fn.assert_called_once_with( _("ofport should have a value that can be " "interpreted as an integer")) diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index eb2b536be8..7dc65bb20a 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -729,7 +729,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): 'gre-1', 'remote_ip', p_const.TYPE_GRE) add_tunnel_port_fn.assert_called_once_with( 'gre-1', 'remote_ip', self.agent.local_ip, p_const.TYPE_GRE, - self.agent.vxlan_udp_port) + self.agent.vxlan_udp_port, self.agent.dont_fragment) log_error_fn.assert_called_once_with( _("Failed to set-up %(type)s tunnel port to %(ip)s"), {'type': p_const.TYPE_GRE, 'ip': 'remote_ip'}) @@ -746,7 +746,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): 'gre-1', 'remote_ip', p_const.TYPE_GRE) add_tunnel_port_fn.assert_called_once_with( 'gre-1', 'remote_ip', self.agent.local_ip, p_const.TYPE_GRE, - self.agent.vxlan_udp_port) + self.agent.vxlan_udp_port, self.agent.dont_fragment) log_exc_fn.assert_called_once_with( _("ofport should have a value that can be " "interpreted as an integer")) @@ -755,6 +755,23 @@ class TestOvsNeutronAgent(base.BaseTestCase): {'type': p_const.TYPE_GRE, 'ip': 'remote_ip'}) self.assertEqual(ofport, 0) + def test_setup_tunnel_port_error_negative_df_disabled(self): + with contextlib.nested( + mock.patch.object(self.agent.tun_br, 'add_tunnel_port', + return_value='-1'), + mock.patch.object(ovs_neutron_agent.LOG, 'error') + ) as (add_tunnel_port_fn, log_error_fn): + self.agent.dont_fragment = False + ofport = self.agent.setup_tunnel_port( + 'gre-1', 'remote_ip', p_const.TYPE_GRE) + add_tunnel_port_fn.assert_called_once_with( + 'gre-1', 'remote_ip', self.agent.local_ip, p_const.TYPE_GRE, + self.agent.vxlan_udp_port, self.agent.dont_fragment) + log_error_fn.assert_called_once_with( + _("Failed to set-up %(type)s tunnel port to %(ip)s"), + {'type': p_const.TYPE_GRE, 'ip': 'remote_ip'}) + self.assertEqual(ofport, 0) + def test_tunnel_sync_with_ovs_plugin(self): fake_tunnel_details = {'tunnels': [{'id': '42', 'ip_address': '100.101.102.103'}]} diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index 55afce3aa7..c192cae650 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -508,7 +508,7 @@ class TunnelTest(base.BaseTestCase): self.mock_tun_bridge.add_tunnel_port.return_value = tunnel_port self.mock_tun_bridge_expected += [ mock.call.add_tunnel_port('gre-1', '10.0.10.1', '10.0.0.1', - 'gre', 4789), + 'gre', 4789, True), mock.call.add_flow(priority=1, in_port=tunnel_port, actions='resubmit(,2)') ]