From 89f6b2ccefa67d6a5b2d1c518d2361580f83fcc1 Mon Sep 17 00:00:00 2001 From: He Qing Date: Tue, 27 Sep 2016 09:41:18 +0800 Subject: [PATCH] Add MTU configuraion when plugging vip or network MTU must be set properly because if the tenant network is some kind of tunnels, the default mtu may cause packets loss. Change-Id: Ife10cb8b5ad8e5066f2e7a1565ad72a3e1916688 Closes-Bug: #1627687 --- .../backends/agent/api_server/plug.py | 14 ++++--- .../backends/agent/api_server/server.py | 6 ++- .../templates/plug_port_ethX.conf.j2 | 3 ++ .../templates/plug_vip_ethX.conf.j2 | 3 ++ .../drivers/haproxy/rest_api_driver.py | 4 +- .../drivers/neutron/allowed_address_pairs.py | 1 + .../backend/agent/api_server/test_server.py | 16 +++++--- .../backends/agent/api_server/test_plug.py | 39 +++++++++++-------- .../drivers/haproxy/test_rest_api_driver.py | 23 ++++++++--- 9 files changed, 73 insertions(+), 36 deletions(-) diff --git a/octavia/amphorae/backends/agent/api_server/plug.py b/octavia/amphorae/backends/agent/api_server/plug.py index 98928d8365..ba7d6a8025 100644 --- a/octavia/amphorae/backends/agent/api_server/plug.py +++ b/octavia/amphorae/backends/agent/api_server/plug.py @@ -51,7 +51,7 @@ template_vip = j2_env.get_template(ETH_X_VIP_CONF) class Plug(object): def plug_vip(self, vip, subnet_cidr, gateway, - mac_address, vrrp_ip=None, host_routes=None): + mac_address, mtu=None, vrrp_ip=None, host_routes=None): # Validate vip and subnet_cidr, calculate broadcast address and netmask try: render_host_routes = [] @@ -143,6 +143,7 @@ class Plug(object): broadcast=broadcast, netmask=netmask, gateway=gateway, + mtu=mtu, vrrp_ip=vrrp_ip, vrrp_ipv6=vrrp_version is 6, host_routes=render_host_routes, @@ -174,7 +175,7 @@ class Plug(object): details="VIP {vip} plugged on interface {interface}".format( vip=vip, interface=primary_interface))), 202) - def _generate_network_file_text(self, netns_interface, fixed_ips): + def _generate_network_file_text(self, netns_interface, fixed_ips, mtu): text = '' if fixed_ips is None: text = template_port.render(interface=netns_interface) @@ -213,6 +214,7 @@ class Plug(object): ip_address=ip.exploded, broadcast=broadcast, netmask=netmask, + mtu=mtu, host_routes=host_routes) text = '\n'.join([text, new_text]) return text @@ -225,7 +227,7 @@ class Plug(object): except socket.error: socket.inet_pton(socket.AF_INET6, ip.get('ip_address')) - def plug_network(self, mac_address, fixed_ips): + def plug_network(self, mac_address, fixed_ips, mtu=None): # Check if the interface is already in the network namespace # Do not attempt to re-plug the network if it is already in the # network namespace @@ -275,7 +277,9 @@ class Plug(object): with os.fdopen(os.open(interface_file_path, flags, mode), 'w') as text_file: - text = self._generate_network_file_text(netns_interface, fixed_ips) + text = self._generate_network_file_text(netns_interface, + fixed_ips, + mtu) text_file.write(text) # Update the list of interfaces to add to the namespace @@ -351,4 +355,4 @@ class Plug(object): for attr in link['attrs']: if attr[0] == 'IFLA_ADDRESS' and attr[1] == mac_address: return True - return False + return False \ No newline at end of file diff --git a/octavia/amphorae/backends/agent/api_server/server.py b/octavia/amphorae/backends/agent/api_server/server.py index d867252826..3a2fa5014f 100644 --- a/octavia/amphorae/backends/agent/api_server/server.py +++ b/octavia/amphorae/backends/agent/api_server/server.py @@ -155,6 +155,7 @@ class Server(object): net_info['subnet_cidr'], net_info['gateway'], net_info['mac_address'], + net_info.get('mtu'), net_info.get('vrrp_ip'), net_info.get('host_routes')) @@ -166,7 +167,8 @@ class Server(object): except Exception: raise exceptions.BadRequest(description='Invalid port information') return self._plug.plug_network(port_info['mac_address'], - port_info.get('fixed_ips')) + port_info.get('fixed_ips'), + port_info.get('mtu')) def upload_cert(self): return certificate_update.upload_server_cert() @@ -178,4 +180,4 @@ class Server(object): return self._keepalived.manager_keepalived_service(action) def get_interface(self, ip_addr): - return amphora_info.get_interface(ip_addr) + return amphora_info.get_interface(ip_addr) \ No newline at end of file diff --git a/octavia/amphorae/backends/agent/api_server/templates/plug_port_ethX.conf.j2 b/octavia/amphorae/backends/agent/api_server/templates/plug_port_ethX.conf.j2 index 55ecbb637f..d10f965e16 100644 --- a/octavia/amphorae/backends/agent/api_server/templates/plug_port_ethX.conf.j2 +++ b/octavia/amphorae/backends/agent/api_server/templates/plug_port_ethX.conf.j2 @@ -20,6 +20,9 @@ iface {{ interface }} inet{{ '6' if ipv6 }} static address {{ ip_address }} broadcast {{ broadcast }} netmask {{ netmask }} +{%- if mtu %} +mtu {{ mtu }} +{%- endif %} {%- for hr in host_routes %} up route add -net {{ hr.network }} gw {{ hr.gw }} dev {{ interface }} down route del -net {{ hr.network }} gw {{ hr.gw }} dev {{ interface }} diff --git a/octavia/amphorae/backends/agent/api_server/templates/plug_vip_ethX.conf.j2 b/octavia/amphorae/backends/agent/api_server/templates/plug_vip_ethX.conf.j2 index 1b8785fca1..a1684a6f55 100644 --- a/octavia/amphorae/backends/agent/api_server/templates/plug_vip_ethX.conf.j2 +++ b/octavia/amphorae/backends/agent/api_server/templates/plug_vip_ethX.conf.j2 @@ -23,6 +23,9 @@ address {{ vrrp_ip }} broadcast {{ broadcast }} netmask {{ netmask }} gateway {{ gateway }} +{%- if mtu %} +mtu {{ mtu }} +{%- endif %} {%- for hr in host_routes %} up route add -net {{ hr.network }} gw {{ hr.gw }} dev {{ interface }} down route del -net {{ hr.network }} gw {{ hr.gw }} dev {{ interface }} diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index eac411b55f..fc4beec15c 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -126,6 +126,7 @@ class HaproxyAmphoraLoadBalancerDriver( 'gateway': subnet.gateway_ip, 'mac_address': port.mac_address, 'vrrp_ip': amphora.vrrp_ip, + 'mtu': port.network.mtu, 'host_routes': host_routes} try: self.client.plug_vip(amphora, @@ -147,7 +148,8 @@ class HaproxyAmphoraLoadBalancerDriver( 'host_routes': host_routes} fixed_ips.append(ip) port_info = {'mac_address': port.mac_address, - 'fixed_ips': fixed_ips} + 'fixed_ips': fixed_ips, + 'mtu': port.network.mtu} try: self.client.plug_network(amphora, port_info) except exc.Conflict: diff --git a/octavia/network/drivers/neutron/allowed_address_pairs.py b/octavia/network/drivers/neutron/allowed_address_pairs.py index a3a2b2ada5..fb1690441d 100644 --- a/octavia/network/drivers/neutron/allowed_address_pairs.py +++ b/octavia/network/drivers/neutron/allowed_address_pairs.py @@ -531,6 +531,7 @@ class AllowedAddressPairsDriver(neutron_base.BaseNeutronDriver): vrrp_port = self.get_port(amp.vrrp_port_id) vrrp_subnet = self.get_subnet( vrrp_port.get_subnet_id(amp.vrrp_ip)) + vrrp_port.network = self.get_network(vrrp_port.network_id) ha_port = self.get_port(amp.ha_port_id) ha_subnet = self.get_subnet( ha_port.get_subnet_id(amp.ha_ip)) diff --git a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py index 48418c1f69..72af3ddd5f 100644 --- a/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py +++ b/octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py @@ -608,7 +608,7 @@ class TestServerTestCase(base.TestCase): 'ifup', 'eth' + test_int_num], stderr=-2) # fixed IPs happy path - port_info = {'mac_address': '123', 'fixed_ips': [ + port_info = {'mac_address': '123', 'mtu': 1450, 'fixed_ips': [ {'ip_address': '10.0.0.5', 'subnet_cidr': '10.0.0.0/24'}]} mock_interfaces.side_effect = [['blah']] mock_ifaddress.side_effect = [[netifaces.AF_LINK], @@ -642,13 +642,13 @@ class TestServerTestCase(base.TestCase): 'auto eth' + test_int_num + '\niface eth' + test_int_num + ' inet static\n' + 'address 10.0.0.5\nbroadcast 10.0.0.255\n' + - 'netmask 255.255.255.0\n') + 'netmask 255.255.255.0\nmtu 1450\n') mock_check_output.assert_called_with( ['ip', 'netns', 'exec', consts.AMPHORA_NAMESPACE, 'ifup', 'eth' + test_int_num], stderr=-2) # fixed IPs happy path IPv6 - port_info = {'mac_address': '123', 'fixed_ips': [ + port_info = {'mac_address': '123', 'mtu': 1450, 'fixed_ips': [ {'ip_address': '2001:db8::2', 'subnet_cidr': '2001:db8::/32'}]} mock_interfaces.side_effect = [['blah']] mock_ifaddress.side_effect = [[netifaces.AF_LINK], @@ -683,7 +683,7 @@ class TestServerTestCase(base.TestCase): '\niface eth' + test_int_num + ' inet6 static\n' + 'address 2001:0db8:0000:0000:0000:0000:0000:0002\n' 'broadcast 2001:0db8:ffff:ffff:ffff:ffff:ffff:ffff\n' + - 'netmask 32\n') + 'netmask 32\nmtu 1450\n') mock_check_output.assert_called_with( ['ip', 'netns', 'exec', consts.AMPHORA_NAMESPACE, 'ifup', 'eth' + test_int_num], stderr=-2) @@ -750,7 +750,7 @@ class TestServerTestCase(base.TestCase): netns_handle.get_links.return_value = [{ 'attrs': [['IFLA_IFNAME', consts.NETNS_PRIMARY_INTERFACE]]}] - port_info = {'mac_address': MAC, 'fixed_ips': [ + port_info = {'mac_address': MAC, 'mtu': 1450, 'fixed_ips': [ {'ip_address': IP, 'subnet_cidr': SUBNET_CIDR, 'host_routes': [{'destination': DEST1, 'nexthop': NEXTHOP}, {'destination': DEST2, 'nexthop': NEXTHOP}]}]} @@ -787,7 +787,7 @@ class TestServerTestCase(base.TestCase): '\niface ' + consts.NETNS_PRIMARY_INTERFACE + ' inet static\n' + 'address ' + IP + '\nbroadcast ' + BROADCAST + '\n' + - 'netmask ' + NETMASK + '\n' + + 'netmask ' + NETMASK + '\n' + 'mtu 1450\n' + 'up route add -net ' + DEST1 + ' gw ' + NEXTHOP + ' dev ' + consts.NETNS_PRIMARY_INTERFACE + '\n' 'down route del -net ' + DEST1 + ' gw ' + NEXTHOP + @@ -866,6 +866,7 @@ class TestServerTestCase(base.TestCase): 'gateway': '203.0.113.1', 'mac_address': '123', 'vrrp_ip': '203.0.113.4', + 'mtu': 1450, 'host_routes': [{'destination': '203.0.114.0/24', 'nexthop': '203.0.113.5'}, {'destination': '203.0.115.0/24', @@ -910,6 +911,7 @@ class TestServerTestCase(base.TestCase): 'broadcast 203.0.113.255\n' 'netmask 255.255.255.0\n' 'gateway 203.0.113.1\n' + 'mtu 1450\n' 'up route add -net 203.0.114.0/24 gw 203.0.113.5 ' 'dev {netns_int}\n' 'down route del -net 203.0.114.0/24 gw 203.0.113.5 ' @@ -1048,6 +1050,7 @@ class TestServerTestCase(base.TestCase): 'gateway': '2001:db8::1', 'mac_address': '123', 'vrrp_ip': '2001:db8::4', + 'mtu': 1450, 'host_routes': [{'destination': '2001:db9::/32', 'nexthop': '2001:db8::5'}, {'destination': '2001:db9::/32', @@ -1091,6 +1094,7 @@ class TestServerTestCase(base.TestCase): 'broadcast 2001:0db8:ffff:ffff:ffff:ffff:ffff:ffff\n' 'netmask 32\n' 'gateway 2001:db8::1\n' + 'mtu 1450\n' 'up route add -net 2001:db9::/32 gw 2001:db8::5 ' 'dev {netns_int}\n' 'down route del -net 2001:db9::/32 gw 2001:db8::5 ' diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py index 9cbfe804a9..f68da13e38 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py @@ -148,27 +148,34 @@ class TestPlugNetwork(base.TestCase): DEST1 = '198.51.100.0/24' DEST2 = '203.0.113.0/24' NEXTHOP = '192.0.2.1' + MTU = 1450 fixed_ips = [{'ip_address': FIXED_IP, 'subnet_cidr': SUBNET_CIDR, 'host_routes': [ {'destination': DEST1, 'nexthop': NEXTHOP}, {'destination': DEST2, 'nexthop': NEXTHOP} ]}] - text = self.test_plug._generate_network_file_text(netns_interface, - fixed_ips) - expected_text = ( + format_text = ( '\n\n# Generated by Octavia agent\n' - 'auto ' + netns_interface + '\n' - 'iface ' + netns_interface + ' inet static\n' - 'address ' + FIXED_IP + '\n' - 'broadcast ' + BROADCAST + '\n' - 'netmask ' + NETMASK + '\n' - 'up route add -net ' + DEST1 + ' gw ' + NEXTHOP + - ' dev ' + netns_interface + '\n' - 'down route del -net ' + DEST1 + ' gw ' + NEXTHOP + - ' dev ' + netns_interface + '\n' - 'up route add -net ' + DEST2 + ' gw ' + NEXTHOP + - ' dev ' + netns_interface + '\n' - 'down route del -net ' + DEST2 + ' gw ' + NEXTHOP + - ' dev ' + netns_interface + '\n') + 'auto {netns_interface}\n' + 'iface {netns_interface} inet static\n' + 'address {fixed_ip}\n' + 'broadcast {broadcast}\n' + 'netmask {netmask}\n' + 'mtu {mtu}\n' + 'up route add -net {dest1} gw {nexthop} dev {netns_interface}\n' + 'down route del -net {dest1} gw {nexthop} dev {netns_interface}\n' + 'up route add -net {dest2} gw {nexthop} dev {netns_interface}\n' + 'down route del -net {dest2} gw {nexthop} dev {netns_interface}\n') + + text = self.test_plug._generate_network_file_text(netns_interface, + fixed_ips, MTU) + expected_text = format_text.format(netns_interface=netns_interface, + fixed_ip=FIXED_IP, + broadcast=BROADCAST, + netmask=NETMASK, + mtu=MTU, + dest1=DEST1, + dest2=DEST2, + nexthop=NEXTHOP) self.assertEqual(expected_text, text) diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py index deb2dad34a..0eefa08383 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py @@ -36,6 +36,7 @@ FAKE_PEM_FILENAME = "file_name" FAKE_UUID_1 = uuidutils.generate_uuid() FAKE_VRRP_IP = '10.1.0.1' FAKE_MAC_ADDRESS = '123' +FAKE_MTU = 1450 class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): @@ -62,8 +63,10 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): self.fixed_ip = mock.MagicMock() self.fixed_ip.ip_address = '198.51.100.5' self.fixed_ip.subnet.cidr = '198.51.100.0/24' + self.network = network_models.Network(mtu=FAKE_MTU) self.port = network_models.Port(mac_address=FAKE_MAC_ADDRESS, - fixed_ips=[self.fixed_ip]) + fixed_ips=[self.fixed_ip], + network=self.network) self.host_routes = [network_models.HostRoute(destination=DEST1, nexthop=NEXTHOP), @@ -75,6 +78,7 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): 'gateway': FAKE_GATEWAY, 'mac_address': FAKE_MAC_ADDRESS, 'vrrp_ip': self.amp.vrrp_ip, + 'mtu': FAKE_MTU, 'host_routes': host_routes_data} @mock.patch('octavia.common.tls_utils.cert_parser.load_certificates_data') @@ -173,10 +177,14 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): def test_post_network_plug(self): # Test dhcp path - port = network_models.Port(mac_address=FAKE_MAC_ADDRESS, fixed_ips=[]) + port = network_models.Port(mac_address=FAKE_MAC_ADDRESS, + fixed_ips=[], + network=self.network) self.driver.post_network_plug(self.amp, port) self.driver.client.plug_network.assert_called_once_with( - self.amp, dict(mac_address=FAKE_MAC_ADDRESS, fixed_ips=[])) + self.amp, dict(mac_address=FAKE_MAC_ADDRESS, + fixed_ips=[], + mtu=FAKE_MTU)) self.driver.client.plug_network.reset_mock() @@ -186,7 +194,8 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): self.amp, dict(mac_address=FAKE_MAC_ADDRESS, fixed_ips=[dict(ip_address='198.51.100.5', subnet_cidr='198.51.100.0/24', - host_routes=[])])) + host_routes=[])], + mtu=FAKE_MTU)) def test_post_network_plug_with_host_routes(self): SUBNET_ID = 'SUBNET_ID' @@ -209,7 +218,8 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): subnet=subnet) ] port = network_models.Port(mac_address=FAKE_MAC_ADDRESS, - fixed_ips=fixed_ips) + fixed_ips=fixed_ips, + network=self.network) self.driver.post_network_plug(self.amp, port) expected_fixed_ips = [ {'ip_address': FIXED_IP1, 'subnet_cidr': SUBNET_CIDR, @@ -221,7 +231,8 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): ] self.driver.client.plug_network.assert_called_once_with( self.amp, dict(mac_address=FAKE_MAC_ADDRESS, - fixed_ips=expected_fixed_ips)) + fixed_ips=expected_fixed_ips, + mtu=FAKE_MTU)) def test_get_vrrp_interface(self): self.driver.get_vrrp_interface(self.amp)