Skip to content

Commit

Permalink
Add MTU configuraion when plugging vip or network
Browse files Browse the repository at this point in the history
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
  • Loading branch information
He Qing committed Oct 11, 2016
1 parent a97dbf3 commit 89f6b2c
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 36 deletions.
14 changes: 9 additions & 5 deletions octavia/amphorae/backends/agent/api_server/plug.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
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 = []
Expand Down Expand Up @@ -143,6 +143,7 @@ def plug_vip(self, vip, subnet_cidr, gateway,
broadcast=broadcast,
netmask=netmask,
gateway=gateway,
mtu=mtu,
vrrp_ip=vrrp_ip,
vrrp_ipv6=vrrp_version is 6,
host_routes=render_host_routes,
Expand Down Expand Up @@ -174,7 +175,7 @@ def plug_vip(self, vip, subnet_cidr, gateway,
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)
Expand Down Expand Up @@ -213,6 +214,7 @@ def _generate_network_file_text(self, netns_interface, fixed_ips):
ip_address=ip.exploded,
broadcast=broadcast,
netmask=netmask,
mtu=mtu,
host_routes=host_routes)
text = '\n'.join([text, new_text])
return text
Expand All @@ -225,7 +227,7 @@ def _check_ip_addresses(self, fixed_ips):
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
Expand Down Expand Up @@ -275,7 +277,9 @@ def plug_network(self, mac_address, fixed_ips):

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
Expand Down Expand Up @@ -351,4 +355,4 @@ def _netns_interface_exists(self, mac_address):
for attr in link['attrs']:
if attr[0] == 'IFLA_ADDRESS' and attr[1] == mac_address:
return True
return False
return False
6 changes: 4 additions & 2 deletions octavia/amphorae/backends/agent/api_server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ def plug_vip(self, vip):
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'))

Expand All @@ -166,7 +167,8 @@ def plug_network(self):
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()
Expand All @@ -178,4 +180,4 @@ def manage_service_vrrp(self, action):
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)
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
4 changes: 3 additions & 1 deletion octavia/amphorae/drivers/haproxy/rest_api_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def post_vip_plug(self, amphora, load_balancer, amphorae_network_config):
'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,
Expand All @@ -147,7 +148,8 @@ def post_network_plug(self, amphora, port):
'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:
Expand Down
1 change: 1 addition & 0 deletions octavia/network/drivers/neutron/allowed_address_pairs.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ def get_network_configs(self, loadbalancer):
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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ def test_plug_network(self, mock_int_exists, mock_check_output, mock_netns,
'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],
Expand Down Expand Up @@ -642,13 +642,13 @@ def test_plug_network(self, mock_int_exists, mock_check_output, mock_netns,
'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],
Expand Down Expand Up @@ -683,7 +683,7 @@ def test_plug_network(self, mock_int_exists, mock_check_output, mock_netns,
'\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)
Expand Down Expand Up @@ -750,7 +750,7 @@ def test_plug_network_host_routes(self, mock_check_output, mock_netns,
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}]}]}
Expand Down Expand Up @@ -787,7 +787,7 @@ def test_plug_network_host_routes(self, mock_check_output, mock_netns,
'\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 +
Expand Down Expand Up @@ -866,6 +866,7 @@ def test_plug_VIP4(self, mock_makedirs, mock_copytree, mock_check_output,
'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',
Expand Down Expand Up @@ -910,6 +911,7 @@ def test_plug_VIP4(self, mock_makedirs, mock_copytree, mock_check_output,
'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 '
Expand Down Expand Up @@ -1048,6 +1050,7 @@ def test_plug_vip6(self, mock_makedirs, mock_copytree, mock_check_output,
'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',
Expand Down Expand Up @@ -1091,6 +1094,7 @@ def test_plug_vip6(self, mock_makedirs, mock_copytree, mock_check_output,
'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 '
Expand Down
39 changes: 23 additions & 16 deletions octavia/tests/unit/amphorae/backends/agent/api_server/test_plug.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,27 +148,34 @@ def test__generate_network_file_text_static_ip(self):
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)
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
FAKE_UUID_1 = uuidutils.generate_uuid()
FAKE_VRRP_IP = '10.1.0.1'
FAKE_MAC_ADDRESS = '123'
FAKE_MTU = 1450


class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase):
Expand All @@ -62,8 +63,10 @@ def setUp(self):
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),
Expand All @@ -75,6 +78,7 @@ def setUp(self):
'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')
Expand Down Expand Up @@ -173,10 +177,14 @@ def test_post_vip_plug(self):

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()

Expand All @@ -186,7 +194,8 @@ def test_post_network_plug(self):
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'
Expand All @@ -209,7 +218,8 @@ def test_post_network_plug_with_host_routes(self):
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,
Expand All @@ -221,7 +231,8 @@ def test_post_network_plug_with_host_routes(self):
]
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)
Expand Down

0 comments on commit 89f6b2c

Please sign in to comment.