summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Alvarez <dalvarez@redhat.com>2018-10-26 13:42:38 +0000
committerDaniel Alvarez <dalvarez@redhat.com>2018-11-15 09:32:36 +0000
commit5181f1106ff839d08152623c25c9a5f6797aa2d7 (patch)
treee8bdbccd34d838b9eafc2da75168edf1eee6f6c4
parent64ec23ab39cf0cfd93b6f37f4259bc8068388f45 (diff)
Clean MAC_Binding entries when (dis)associating a FIP
When a FIP is assigned to a port, it may happen that it was previously used by another FIP or gateway port and an entry in MAC_Binding table exits. If that's the case, the ARP responder will answer with the wrong MAC address and the FIP will then be unreachable. In order to be on the safe side, this patch is deleting all MAC_Binding entries upon association or disassociation of a FIP as a workaround. The proper way to fix this should be making ovn-northd or ovn-controller clearing/updating the stale entries upon new NAT rules creation. Details about the bug reported in OVS ML at [0]. [0] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047604.html Change-Id: Ic04fcbe332adf030ab0ee3aad3a94ad874b18c9a Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
Notes
Notes (review): Code-Review+2: Lucas Alvares Gomes <lucasagomes@gmail.com> Code-Review+2: Numan Siddique <nusiddiq@redhat.com> Workflow+1: Numan Siddique <nusiddiq@redhat.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Wed, 21 Nov 2018 15:10:46 +0000 Reviewed-on: https://review.openstack.org/613584 Project: openstack/networking-ovn Branch: refs/heads/master
-rw-r--r--networking_ovn/ml2/mech_driver.py7
-rw-r--r--networking_ovn/ovsdb/ovsdb_monitor.py26
-rw-r--r--networking_ovn/tests/functional/test_ovsdb_monitor.py103
3 files changed, 135 insertions, 1 deletions
diff --git a/networking_ovn/ml2/mech_driver.py b/networking_ovn/ml2/mech_driver.py
index 17b7d7a..92b3d0c 100644
--- a/networking_ovn/ml2/mech_driver.py
+++ b/networking_ovn/ml2/mech_driver.py
@@ -801,6 +801,13 @@ class OVNMechanismDriver(api.MechanismDriver):
801 LOG.debug("Port not found during OVN status down report: %s", 801 LOG.debug("Port not found during OVN status down report: %s",
802 port_id) 802 port_id)
803 803
804 def delete_mac_binding_entries(self, external_ip):
805 """Delete all MAC_Binding entries associated to this IP address"""
806 mac_binds = self._sb_ovn.db_find_rows(
807 'MAC_Binding', ('ip', '=', external_ip)).execute() or []
808 for entry in mac_binds:
809 self._sb_ovn.db_destroy('MAC_Binding', entry.uuid).execute()
810
804 def update_segment_host_mapping(self, host, phy_nets): 811 def update_segment_host_mapping(self, host, phy_nets):
805 """Update SegmentHostMapping in DB""" 812 """Update SegmentHostMapping in DB"""
806 if not host: 813 if not host:
diff --git a/networking_ovn/ovsdb/ovsdb_monitor.py b/networking_ovn/ovsdb/ovsdb_monitor.py
index 7790cd7..eb99b14 100644
--- a/networking_ovn/ovsdb/ovsdb_monitor.py
+++ b/networking_ovn/ovsdb/ovsdb_monitor.py
@@ -199,6 +199,27 @@ class LogicalSwitchPortUpdateDownEvent(row_event.RowEvent):
199 self.driver.set_port_status_down(row.name) 199 self.driver.set_port_status_down(row.name)
200 200
201 201
202class FIPAddDeleteEvent(row_event.RowEvent):
203 """Row event - NAT 'dnat_and_snat' entry added or deleted
204
205 This happens when a FIP is created or removed.
206 """
207 def __init__(self, driver):
208 self.driver = driver
209 table = 'NAT'
210 events = (self.ROW_CREATE, self.ROW_DELETE)
211 super(FIPAddDeleteEvent, self).__init__(
212 events, table, (('type', '=', 'dnat_and_snat'),))
213 self.event_name = 'FIPAddDeleteEvent'
214
215 def run(self, event, row, old):
216 # When a FIP is added or deleted, we will delete all entries in the
217 # MAC_Binding table of SB OVSDB corresponding to that IP Address.
218 # TODO(dalvarez): Remove this workaround once fixed in core OVN:
219 # https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047604.html
220 self.driver.delete_mac_binding_entries(row.external_ip)
221
222
202class OvnDbNotifyHandler(event.RowEventHandler): 223class OvnDbNotifyHandler(event.RowEventHandler):
203 def __init__(self, driver): 224 def __init__(self, driver):
204 super(OvnDbNotifyHandler, self).__init__() 225 super(OvnDbNotifyHandler, self).__init__()
@@ -279,11 +300,13 @@ class OvnNbIdl(OvnIdl):
279 self._lsp_update_down_event = LogicalSwitchPortUpdateDownEvent(driver) 300 self._lsp_update_down_event = LogicalSwitchPortUpdateDownEvent(driver)
280 self._lsp_create_up_event = LogicalSwitchPortCreateUpEvent(driver) 301 self._lsp_create_up_event = LogicalSwitchPortCreateUpEvent(driver)
281 self._lsp_create_down_event = LogicalSwitchPortCreateDownEvent(driver) 302 self._lsp_create_down_event = LogicalSwitchPortCreateDownEvent(driver)
303 self._fip_create_delete_event = FIPAddDeleteEvent(driver)
282 304
283 self.notify_handler.watch_events([self._lsp_create_up_event, 305 self.notify_handler.watch_events([self._lsp_create_up_event,
284 self._lsp_create_down_event, 306 self._lsp_create_down_event,
285 self._lsp_update_up_event, 307 self._lsp_update_up_event,
286 self._lsp_update_down_event]) 308 self._lsp_update_down_event,
309 self._fip_create_delete_event])
287 310
288 @classmethod 311 @classmethod
289 def from_server(cls, connection_string, schema_name, driver): 312 def from_server(cls, connection_string, schema_name, driver):
@@ -323,6 +346,7 @@ class OvnSbIdl(OvnIdl):
323 helper.register_table('Encap') 346 helper.register_table('Encap')
324 helper.register_table('Port_Binding') 347 helper.register_table('Port_Binding')
325 helper.register_table('Datapath_Binding') 348 helper.register_table('Datapath_Binding')
349 helper.register_table('MAC_Binding')
326 _idl = cls(driver, connection_string, helper) 350 _idl = cls(driver, connection_string, helper)
327 _idl.set_lock(_idl.event_lock_name) 351 _idl.set_lock(_idl.event_lock_name)
328 return _idl 352 return _idl
diff --git a/networking_ovn/tests/functional/test_ovsdb_monitor.py b/networking_ovn/tests/functional/test_ovsdb_monitor.py
index 0e61248..c8bde1f 100644
--- a/networking_ovn/tests/functional/test_ovsdb_monitor.py
+++ b/networking_ovn/tests/functional/test_ovsdb_monitor.py
@@ -12,6 +12,8 @@
12# License for the specific language governing permissions and limitations 12# License for the specific language governing permissions and limitations
13# under the License. 13# under the License.
14 14
15import threading
16
15import mock 17import mock
16from oslo_utils import uuidutils 18from oslo_utils import uuidutils
17 19
@@ -19,7 +21,31 @@ from networking_ovn.ovsdb import ovsdb_monitor
19from networking_ovn.tests.functional import base 21from networking_ovn.tests.functional import base
20from neutron.common import utils as n_utils 22from neutron.common import utils as n_utils
21from neutron_lib.api.definitions import portbindings 23from neutron_lib.api.definitions import portbindings
24from neutron_lib.plugins import constants as plugin_constants
22from neutron_lib.plugins import directory 25from neutron_lib.plugins import directory
26from ovsdbapp.backend.ovs_idl import event
27
28
29class WaitForMACBindingDeleteEvent(event.RowEvent):
30 # TODO(dalvarez): Use WaitEvent from ovsdbapp once this patch
31 # https://review.openstack.org/#/c/613121 merges.
32 event_name = 'WaitForMACBindingDeleteEvent'
33 ONETIME = True
34
35 def __init__(self, entry):
36 self.event = threading.Event()
37 self.timeout = 15
38 table = 'MAC_Binding'
39 events = (self.ROW_DELETE)
40 conditions = (('_uuid', '=', entry),)
41 super(WaitForMACBindingDeleteEvent, self).__init__(
42 events, table, conditions)
43
44 def run(self, event, row, old):
45 self.event.set()
46
47 def wait(self):
48 return self.event.wait(self.timeout)
23 49
24 50
25class TestNBDbMonitor(base.TestOVNFunctionalBase): 51class TestNBDbMonitor(base.TestOVNFunctionalBase):
@@ -27,6 +53,7 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
27 def setUp(self): 53 def setUp(self):
28 super(TestNBDbMonitor, self).setUp(ovn_worker=True) 54 super(TestNBDbMonitor, self).setUp(ovn_worker=True)
29 self.chassis = self.add_fake_chassis('ovs-host1') 55 self.chassis = self.add_fake_chassis('ovs-host1')
56 self.l3_plugin = directory.get_plugin(plugin_constants.L3)
30 57
31 def create_port(self): 58 def create_port(self):
32 net = self._make_network(self.fmt, 'net1', True) 59 net = self._make_network(self.fmt, 'net1', True)
@@ -41,6 +68,82 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
41 port = self.deserialize(self.fmt, port_res)['port'] 68 port = self.deserialize(self.fmt, port_res)['port']
42 return port 69 return port
43 70
71 def _create_fip(self, port, fip_address):
72 e1 = self._make_network(self.fmt, 'e1', True,
73 arg_list=('router:external',
74 'provider:network_type',
75 'provider:physical_network'),
76 **{'router:external': True,
77 'provider:network_type': 'flat',
78 'provider:physical_network': 'public'})
79 res = self._create_subnet(self.fmt, e1['network']['id'],
80 '100.0.0.0/24', gateway_ip='100.0.0.254',
81 allocation_pools=[{'start': '100.0.0.2',
82 'end': '100.0.0.253'}],
83 enable_dhcp=False)
84 e1_s1 = self.deserialize(self.fmt, res)
85 r1 = self.l3_plugin.create_router(
86 self.context,
87 {'router': {
88 'name': 'r1', 'admin_state_up': True,
89 'tenant_id': self._tenant_id,
90 'external_gateway_info': {
91 'enable_snat': True,
92 'network_id': e1['network']['id'],
93 'external_fixed_ips': [
94 {'ip_address': '100.0.0.2',
95 'subnet_id': e1_s1['subnet']['id']}]}}})
96 self.l3_plugin.add_router_interface(
97 self.context, r1['id'],
98 {'subnet_id': port['fixed_ips'][0]['subnet_id']})
99 r1_f2 = self.l3_plugin.create_floatingip(
100 self.context, {'floatingip': {
101 'tenant_id': self._tenant_id,
102 'floating_network_id': e1['network']['id'],
103 'subnet_id': None,
104 'floating_ip_address': fip_address,
105 'port_id': port['id']}})
106 return r1_f2
107
108 def test_floatingip_mac_bindings(self):
109 """Check that MAC_Binding entries are cleared on FIP add/removal
110
111 This test will:
112 * Create a MAC_Binding entry for an IP address on the
113 'network1' datapath.
114 * Create a FIP with that same IP address on an external.
115 network and associate it to a Neutron port on a private network.
116 * Check that the MAC_Binding entry gets deleted.
117 * Create a new MAC_Binding entry for the same IP address.
118 * Delete the FIP.
119 * Check that the MAC_Binding entry gets deleted.
120 """
121 self._make_network(self.fmt, 'network1', True)
122 dp = self.sb_api.db_find(
123 'Datapath_Binding',
124 ('external_ids', '=', {'name2': 'network1'})).execute()
125 macb_id = self.sb_api.db_create('MAC_Binding', datapath=dp[0]['_uuid'],
126 ip='100.0.0.21').execute()
127 port = self.create_port()
128
129 # Ensure that the MAC_Binding entry gets deleted after creating a FIP
130 row_event = WaitForMACBindingDeleteEvent(macb_id)
131 self.mech_driver._sb_ovn.idl.notify_handler.watch_event(row_event)
132 fip = self._create_fip(port, '100.0.0.21')
133 self.assertTrue(row_event.wait())
134
135 # Now that the FIP is created, add a new MAC_Binding entry with the
136 # same IP address
137
138 macb_id = self.sb_api.db_create('MAC_Binding', datapath=dp[0]['_uuid'],
139 ip='100.0.0.21').execute()
140
141 # Ensure that the MAC_Binding entry gets deleted after deleting the FIP
142 row_event = WaitForMACBindingDeleteEvent(macb_id)
143 self.mech_driver._sb_ovn.idl.notify_handler.watch_event(row_event)
144 self.l3_plugin.delete_floatingip(self.context, fip['id'])
145 self.assertTrue(row_event.wait())
146
44 def _test_port_binding_and_status(self, port_id, action, status): 147 def _test_port_binding_and_status(self, port_id, action, status):
45 # This function binds or unbinds port to chassis and 148 # This function binds or unbinds port to chassis and
46 # checks if port status matches with input status 149 # checks if port status matches with input status