Avoid agents adding ports as trunk by default.
Agent OVS interface code adds ports without a vlan tag, if neutron-openvswitch-agent fails to set the tag, or takes too long, the port will be a trunk port, receiving traffic from the external network or any other port sending traffic on br-int. Also, those kinds of ports are triggering a code path on the ovs-vswitchd revalidator thread which can eventually hog the CPU of the host (that's a bug under investigation [1]) [1] https://bugzilla.redhat.com/show_bug.cgi?id=1558336 Co-Authored-By: Slawek Kaplonski <skaplons@redhat.com> Change-Id: I024bbbdf7059835b2f23c264b48478c71633a43c Closes-Bug: 1767422
This commit is contained in:
parent
06b9603f0c
commit
88f5e11d8b
|
@ -296,6 +296,19 @@ class OVSBridge(BaseOVS):
|
||||||
with self.ovsdb.transaction() as txn:
|
with self.ovsdb.transaction() as txn:
|
||||||
txn.add(self.ovsdb.add_port(self.br_name, port_name,
|
txn.add(self.ovsdb.add_port(self.br_name, port_name,
|
||||||
may_exist=False))
|
may_exist=False))
|
||||||
|
# NOTE(mangelajo): Port is added to dead vlan (4095) by default
|
||||||
|
# until it's handled by the neutron-openvswitch-agent. Otherwise it
|
||||||
|
# becomes a trunk port on br-int (receiving traffic for all vlans),
|
||||||
|
# and also triggers issues on ovs-vswitchd related to the
|
||||||
|
# datapath flow revalidator thread, see lp#1767422
|
||||||
|
txn.add(self.ovsdb.db_set(
|
||||||
|
'Port', port_name, ('tag', constants.DEAD_VLAN_TAG)))
|
||||||
|
|
||||||
|
# TODO(mangelajo): We could accept attr tuples for the Port too
|
||||||
|
# but, that could potentially break usage of this function in
|
||||||
|
# stable branches (where we need to backport).
|
||||||
|
# https://review.openstack.org/#/c/564825/4/neutron/agent/common/
|
||||||
|
# ovs_lib.py@289
|
||||||
if interface_attr_tuples:
|
if interface_attr_tuples:
|
||||||
txn.add(self.ovsdb.db_set('Interface', port_name,
|
txn.add(self.ovsdb.db_set('Interface', port_name,
|
||||||
*interface_attr_tuples))
|
*interface_attr_tuples))
|
||||||
|
|
|
@ -640,6 +640,15 @@ class RouterInfo(object):
|
||||||
namespace=ns_name,
|
namespace=ns_name,
|
||||||
prefix=EXTERNAL_DEV_PREFIX,
|
prefix=EXTERNAL_DEV_PREFIX,
|
||||||
mtu=ex_gw_port.get('mtu'))
|
mtu=ex_gw_port.get('mtu'))
|
||||||
|
if self.agent_conf.external_network_bridge:
|
||||||
|
# NOTE(slaweq): for OVS implementations remove the DEAD VLAN tag
|
||||||
|
# on ports. DEAD VLAN tag is added to each newly created port
|
||||||
|
# and should be removed by L2 agent but if
|
||||||
|
# external_network_bridge is set than external gateway port is
|
||||||
|
# created in this bridge and will not be touched by L2 agent.
|
||||||
|
# This is related to lp#1767422
|
||||||
|
self.driver.remove_vlan_tag(
|
||||||
|
self.agent_conf.external_network_bridge, interface_name)
|
||||||
|
|
||||||
def _get_external_gw_ips(self, ex_gw_port):
|
def _get_external_gw_ips(self, ex_gw_port):
|
||||||
gateway_ips = []
|
gateway_ips = []
|
||||||
|
|
|
@ -223,6 +223,17 @@ class LinuxInterfaceDriver(object):
|
||||||
def get_device_name(self, port):
|
def get_device_name(self, port):
|
||||||
return (self.DEV_NAME_PREFIX + port.id)[:self.DEV_NAME_LEN]
|
return (self.DEV_NAME_PREFIX + port.id)[:self.DEV_NAME_LEN]
|
||||||
|
|
||||||
|
def remove_vlan_tag(self, bridge, interface_name):
|
||||||
|
"""Remove vlan tag from given interface.
|
||||||
|
|
||||||
|
This method is necessary only for the case when deprecated
|
||||||
|
option 'external_network_bridge' is used in L3 agent as
|
||||||
|
external gateway port is then created in this external bridge
|
||||||
|
directly and it will have DEAD_VLAN_TAG added by default.
|
||||||
|
"""
|
||||||
|
# TODO(slaweq): remove it when external_network_bridge option will be
|
||||||
|
# removed
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def configure_ipv6_ra(namespace, dev_name, value):
|
def configure_ipv6_ra(namespace, dev_name, value):
|
||||||
"""Configure handling of IPv6 Router Advertisements on an
|
"""Configure handling of IPv6 Router Advertisements on an
|
||||||
|
@ -322,6 +333,10 @@ class OVSInterfaceDriver(LinuxInterfaceDriver):
|
||||||
ovs = ovs_lib.OVSBridge(bridge)
|
ovs = ovs_lib.OVSBridge(bridge)
|
||||||
ovs.replace_port(device_name, *attrs)
|
ovs.replace_port(device_name, *attrs)
|
||||||
|
|
||||||
|
def remove_vlan_tag(self, bridge, interface):
|
||||||
|
ovs = ovs_lib.OVSBridge(bridge)
|
||||||
|
ovs.clear_db_attribute("Port", interface, "tag")
|
||||||
|
|
||||||
def plug_new(self, network_id, port_id, device_name, mac_address,
|
def plug_new(self, network_id, port_id, device_name, mac_address,
|
||||||
bridge=None, namespace=None, prefix=None, mtu=None):
|
bridge=None, namespace=None, prefix=None, mtu=None):
|
||||||
"""Plug in the interface."""
|
"""Plug in the interface."""
|
||||||
|
|
|
@ -782,6 +782,10 @@ class OVSPortFixture(PortFixture):
|
||||||
self.mac,
|
self.mac,
|
||||||
bridge=self.bridge.br_name,
|
bridge=self.bridge.br_name,
|
||||||
namespace=self.namespace)
|
namespace=self.namespace)
|
||||||
|
# NOTE(mangelajo): for OVS implementations remove the DEAD VLAN tag
|
||||||
|
# on ports that we intend to use as fake vm interfaces, they
|
||||||
|
# need to be flat. This is related to lp#1767422
|
||||||
|
self.bridge.clear_db_attribute("Port", port_name, "tag")
|
||||||
self.addCleanup(self.bridge.delete_port, port_name)
|
self.addCleanup(self.bridge.delete_port, port_name)
|
||||||
self.port = ip_lib.IPDevice(port_name, self.namespace)
|
self.port = ip_lib.IPDevice(port_name, self.namespace)
|
||||||
|
|
||||||
|
|
|
@ -395,7 +395,13 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
|
||||||
self._plug_ports(network, ports, self.agent, bridge=phys_br,
|
self._plug_ports(network, ports, self.agent, bridge=phys_br,
|
||||||
namespace=namespace)
|
namespace=namespace)
|
||||||
|
|
||||||
if phys_segmentation_id and network_type == 'vlan':
|
if network_type == 'flat':
|
||||||
|
# NOTE(slaweq): for OVS implementations remove the DEAD VLAN tag
|
||||||
|
# on ports that belongs to flat network. DEAD VLAN tag is added
|
||||||
|
# to each newly created port. This is related to lp#1767422
|
||||||
|
for port in ports:
|
||||||
|
phys_br.clear_db_attribute("Port", port['vif_name'], "tag")
|
||||||
|
elif phys_segmentation_id and network_type == 'vlan':
|
||||||
for port in ports:
|
for port in ports:
|
||||||
phys_br.set_db_attribute(
|
phys_br.set_db_attribute(
|
||||||
"Port", port['vif_name'], "tag", phys_segmentation_id)
|
"Port", port['vif_name'], "tag", phys_segmentation_id)
|
||||||
|
|
|
@ -31,6 +31,7 @@ from neutron.agent.l3 import namespaces
|
||||||
from neutron.agent.l3 import router_info as l3_router_info
|
from neutron.agent.l3 import router_info as l3_router_info
|
||||||
from neutron.agent import l3_agent as l3_agent_main
|
from neutron.agent import l3_agent as l3_agent_main
|
||||||
from neutron.agent.linux import external_process
|
from neutron.agent.linux import external_process
|
||||||
|
from neutron.agent.linux import interface
|
||||||
from neutron.agent.linux import ip_lib
|
from neutron.agent.linux import ip_lib
|
||||||
from neutron.agent.linux import keepalived
|
from neutron.agent.linux import keepalived
|
||||||
from neutron.common import constants as n_const
|
from neutron.common import constants as n_const
|
||||||
|
@ -44,13 +45,15 @@ from neutron.tests.functional import base
|
||||||
|
|
||||||
_uuid = uuidutils.generate_uuid
|
_uuid = uuidutils.generate_uuid
|
||||||
|
|
||||||
|
OVS_INTERFACE_DRIVER = 'neutron.agent.linux.interface.OVSInterfaceDriver'
|
||||||
|
|
||||||
|
|
||||||
def get_ovs_bridge(br_name):
|
def get_ovs_bridge(br_name):
|
||||||
return ovs_lib.OVSBridge(br_name)
|
return ovs_lib.OVSBridge(br_name)
|
||||||
|
|
||||||
|
|
||||||
class L3AgentTestFramework(base.BaseSudoTestCase):
|
class L3AgentTestFramework(base.BaseSudoTestCase):
|
||||||
INTERFACE_DRIVER = 'neutron.agent.linux.interface.OVSInterfaceDriver'
|
INTERFACE_DRIVER = OVS_INTERFACE_DRIVER
|
||||||
NESTED_NAMESPACE_SEPARATOR = '@'
|
NESTED_NAMESPACE_SEPARATOR = '@'
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
|
@ -318,7 +321,28 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
|
||||||
|
|
||||||
def manage_router(self, agent, router):
|
def manage_router(self, agent, router):
|
||||||
self.addCleanup(agent._safe_router_removed, router['id'])
|
self.addCleanup(agent._safe_router_removed, router['id'])
|
||||||
|
|
||||||
|
# NOTE(mangelajo): Neutron functional for l3 don't rely on openvswitch
|
||||||
|
# agent tagging ports, and all ports remain untagged
|
||||||
|
# during test execution.
|
||||||
|
# Workaround related to lp#1767422 plugs new ports as
|
||||||
|
# dead vlan (4095) to avoid issues, we need to remove
|
||||||
|
# such tag during functional l3 testing.
|
||||||
|
original_plug_new = interface.OVSInterfaceDriver.plug_new
|
||||||
|
|
||||||
|
def new_ovs_plug(self, *args, **kwargs):
|
||||||
|
original_plug_new(self, *args, **kwargs)
|
||||||
|
bridge = (kwargs.get('bridge') or args[4] or
|
||||||
|
self.conf.ovs_integration_bridge)
|
||||||
|
device_name = kwargs.get('device_name') or args[2]
|
||||||
|
ovsbr = ovs_lib.OVSBridge(bridge)
|
||||||
|
ovsbr.clear_db_attribute('Port', device_name, 'tag')
|
||||||
|
|
||||||
|
with mock.patch(OVS_INTERFACE_DRIVER + '.plug_new', autospec=True) as (
|
||||||
|
ovs_plug):
|
||||||
|
ovs_plug.side_effect = new_ovs_plug
|
||||||
agent._process_added_router(router)
|
agent._process_added_router(router)
|
||||||
|
|
||||||
return agent.router_info[router['id']]
|
return agent.router_info[router['id']]
|
||||||
|
|
||||||
def _delete_router(self, agent, router_id):
|
def _delete_router(self, agent, router_id):
|
||||||
|
|
|
@ -90,6 +90,8 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
|
||||||
self.assertTrue(self.br.port_exists(port_name))
|
self.assertTrue(self.br.port_exists(port_name))
|
||||||
self.assertEqual('test', self.br.db_get_val('Interface', port_name,
|
self.assertEqual('test', self.br.db_get_val('Interface', port_name,
|
||||||
'external_ids')['test'])
|
'external_ids')['test'])
|
||||||
|
self.assertEqual(agent_const.DEAD_VLAN_TAG,
|
||||||
|
self.br.db_get_val('Port', port_name, 'tag'))
|
||||||
|
|
||||||
def test_attribute_lifecycle(self):
|
def test_attribute_lifecycle(self):
|
||||||
(port_name, ofport) = self.create_ovs_port()
|
(port_name, ofport) = self.create_ovs_port()
|
||||||
|
|
Loading…
Reference in New Issue