Commit
d6a55c17360d1aa8ca91849199987ae71e8600ee added
logic to the OVS agent to delete a port from the integration
bridge when a port was deleted on the Neutron side. However,
this led to several races where whoever created the initial
port (e.g. Nova, L3 agent, DHCP agent) would be trying to
remove the port from the bridge at the same time. These
would result in ugly exceptions on one side or the other.
The original commit was trying to address the problem where
the port would maintain connectivity even though it was removed
from the integration bridge.
This patch addresses both cases by removing the iptables rules
for the deleted port and putting it in the dead VLAN so it loses
connectivity. However, it still leaves the port attached to the
integration bridge so the original creator can delete it.
Conflicts:
neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
neutron/tests/unit/plugins/openvswitch/test_ovs_tunnel.py
Related-Bug: #
1333365
Closes-Bug: #
1448148
Change-Id: I7ae7750b7ac7d15325ed9f2d517ca171543b53be
(cherry picked from commit
e007167a700aa5b80ecb48adff0ac36bb330591d)
return self.ovsdb.br_get_external_id(bridge, 'bridge-id').execute()
def set_db_attribute(self, table_name, record, column, value,
- check_error=False):
+ check_error=False, log_errors=True):
self.ovsdb.db_set(table_name, record, (column, value)).execute(
- check_error=check_error)
+ check_error=check_error, log_errors=log_errors)
def clear_db_attribute(self, table_name, record, column):
self.ovsdb.db_clear(table_name, record, column).execute()
- def db_get_val(self, table, record, column, check_error=False):
+ def db_get_val(self, table, record, column, check_error=False,
+ log_errors=True):
return self.ovsdb.db_get(table, record, column).execute(
- check_error=check_error)
+ check_error=check_error, log_errors=log_errors)
class OVSBridge(BaseOVS):
self.setup_integration_br()
# Stores port update notifications for processing in main rpc loop
self.updated_ports = set()
+ # Stores port delete notifications
+ self.deleted_ports = set()
# keeps association between ports and ofports to detect ofport change
self.vifname_to_ofport_map = {}
self.setup_rpc()
def port_delete(self, context, **kwargs):
port_id = kwargs.get('port_id')
- port = self.int_br.get_vif_port_by_id(port_id)
- # If port exists, delete it
- if port:
- self.int_br.delete_port(port.port_name)
+ self.deleted_ports.add(port_id)
+ LOG.debug("port_delete message processed for port %s", port_id)
+
+ def process_deleted_ports(self):
+ while self.deleted_ports:
+ port_id = self.deleted_ports.pop()
+ # Flush firewall rules and move to dead VLAN so deleted ports no
+ # longer have access to the network
+ self.sg_agent.remove_devices_filter([port_id])
+ port = self.int_br.get_vif_port_by_id(port_id)
+ if port:
+ # don't log errors since there is a chance someone will be
+ # removing the port from the bridge at the same time
+ self.port_dead(port, log_errors=False)
+ self.port_unbound(port_id)
def tunnel_update(self, context, **kwargs):
LOG.debug("tunnel_update received")
if not lvm.vif_ports:
self.reclaim_local_vlan(net_uuid)
- def port_dead(self, port):
+ def port_dead(self, port, log_errors=True):
'''Once a port has no binding, put it on the "dead vlan".
:param port: a ovs_lib.VifPort object.
'''
# Don't kill a port if it's already dead
- cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag")
+ cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag",
+ log_errors=log_errors)
if cur_tag != DEAD_VLAN_TAG:
self.int_br.set_db_attribute("Port", port.port_name, "tag",
- DEAD_VLAN_TAG)
+ DEAD_VLAN_TAG, log_errors=log_errors)
self.int_br.add_flow(priority=2, in_port=port.ofport,
actions="drop")
self.updated_ports = set()
reg_ports = (set() if ovs_restarted else ports)
port_info = self.scan_ports(reg_ports, updated_ports_copy)
+ # don't try to process removed ports as deleted ports since
+ # they are already gone
+ self.deleted_ports -= port_info['removed']
+ self.process_deleted_ports()
self.update_stale_ofport_rules()
LOG.debug("Agent rpc_loop - iteration:%(iter_num)d - "
"port information retrieved. "
mock.patch.object(self.agent.int_br, 'add_flow')
) as (set_ovs_db_func, get_ovs_db_func, add_flow_func):
self.agent.port_dead(port)
- get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag")
+ get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag",
+ log_errors=True)
if cur_tag == ovs_neutron_agent.DEAD_VLAN_TAG:
self.assertFalse(set_ovs_db_func.called)
self.assertFalse(add_flow_func.called)
else:
set_ovs_db_func.assert_called_once_with(
- "Port", mock.ANY, "tag", ovs_neutron_agent.DEAD_VLAN_TAG)
+ "Port", mock.ANY, "tag", ovs_neutron_agent.DEAD_VLAN_TAG,
+ log_errors=True)
add_flow_func.assert_called_once_with(
priority=2, in_port=port.ofport, actions="drop")
self.assertEqual(set(['123']), self.agent.updated_ports)
def test_port_delete(self):
- port_id = "123"
- port_name = "foo"
- with contextlib.nested(
- mock.patch.object(self.agent.int_br, 'get_vif_port_by_id',
- return_value=mock.MagicMock(
- port_name=port_name)),
- mock.patch.object(self.agent.int_br, "delete_port")
- ) as (get_vif_func, del_port_func):
+ vif = FakeVif()
+ with mock.patch.object(self.agent, 'int_br') as int_br:
+ int_br.get_vif_by_port_id.return_value = vif.port_name
+ int_br.get_vif_port_by_id.return_value = vif
self.agent.port_delete("unused_context",
- port_id=port_id)
- self.assertTrue(get_vif_func.called)
- del_port_func.assert_called_once_with(port_name)
+ port_id='id')
+ self.agent.process_deleted_ports()
+ # the main things we care about are that it gets put in the
+ # dead vlan and gets blocked
+ int_br.set_db_attribute.assert_any_call(
+ 'Port', vif.port_name, 'tag', ovs_neutron_agent.DEAD_VLAN_TAG,
+ log_errors=False)
+ int_br.add_flow.assert_any_call(priority=mock.ANY,
+ in_port=vif.ofport,
+ actions='drop')
def test_setup_physical_bridges(self):
with contextlib.nested(
def test_port_dead(self):
self.mock_int_bridge_expected += [
- mock.call.db_get_val('Port', VIF_PORT.port_name, 'tag'),
+ mock.call.db_get_val('Port', VIF_PORT.port_name, 'tag',
+ log_errors=True),
mock.call.set_db_attribute(
'Port', VIF_PORT.port_name,
- 'tag', ovs_neutron_agent.DEAD_VLAN_TAG),
+ 'tag', ovs_neutron_agent.DEAD_VLAN_TAG,
+ log_errors=True),
mock.call.add_flow(priority=2, in_port=VIF_PORT.ofport,
actions='drop')
]
Code Review / openstack-build / neutron-build.git / commitdiff