Browse Source

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>
Daniel Alvarez 5 months ago
parent
commit
5181f1106f

+ 7
- 0
networking_ovn/ml2/mech_driver.py View File

@@ -801,6 +801,13 @@ class OVNMechanismDriver(api.MechanismDriver):
801 801
             LOG.debug("Port not found during OVN status down report: %s",
802 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 811
     def update_segment_host_mapping(self, host, phy_nets):
805 812
         """Update SegmentHostMapping in DB"""
806 813
         if not host:

+ 25
- 1
networking_ovn/ovsdb/ovsdb_monitor.py View File

@@ -199,6 +199,27 @@ class LogicalSwitchPortUpdateDownEvent(row_event.RowEvent):
199 199
         self.driver.set_port_status_down(row.name)
200 200
 
201 201
 
202
+class 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
+
202 223
 class OvnDbNotifyHandler(event.RowEventHandler):
203 224
     def __init__(self, driver):
204 225
         super(OvnDbNotifyHandler, self).__init__()
@@ -279,11 +300,13 @@ class OvnNbIdl(OvnIdl):
279 300
         self._lsp_update_down_event = LogicalSwitchPortUpdateDownEvent(driver)
280 301
         self._lsp_create_up_event = LogicalSwitchPortCreateUpEvent(driver)
281 302
         self._lsp_create_down_event = LogicalSwitchPortCreateDownEvent(driver)
303
+        self._fip_create_delete_event = FIPAddDeleteEvent(driver)
282 304
 
283 305
         self.notify_handler.watch_events([self._lsp_create_up_event,
284 306
                                           self._lsp_create_down_event,
285 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 311
     @classmethod
289 312
     def from_server(cls, connection_string, schema_name, driver):
@@ -323,6 +346,7 @@ class OvnSbIdl(OvnIdl):
323 346
         helper.register_table('Encap')
324 347
         helper.register_table('Port_Binding')
325 348
         helper.register_table('Datapath_Binding')
349
+        helper.register_table('MAC_Binding')
326 350
         _idl = cls(driver, connection_string, helper)
327 351
         _idl.set_lock(_idl.event_lock_name)
328 352
         return _idl

+ 103
- 0
networking_ovn/tests/functional/test_ovsdb_monitor.py View File

@@ -12,6 +12,8 @@
12 12
 #    License for the specific language governing permissions and limitations
13 13
 #    under the License.
14 14
 
15
+import threading
16
+
15 17
 import mock
16 18
 from oslo_utils import uuidutils
17 19
 
@@ -19,7 +21,31 @@ from networking_ovn.ovsdb import ovsdb_monitor
19 21
 from networking_ovn.tests.functional import base
20 22
 from neutron.common import utils as n_utils
21 23
 from neutron_lib.api.definitions import portbindings
24
+from neutron_lib.plugins import constants as plugin_constants
22 25
 from neutron_lib.plugins import directory
26
+from ovsdbapp.backend.ovs_idl import event
27
+
28
+
29
+class 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
 
25 51
 class TestNBDbMonitor(base.TestOVNFunctionalBase):
@@ -27,6 +53,7 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
27 53
     def setUp(self):
28 54
         super(TestNBDbMonitor, self).setUp(ovn_worker=True)
29 55
         self.chassis = self.add_fake_chassis('ovs-host1')
56
+        self.l3_plugin = directory.get_plugin(plugin_constants.L3)
30 57
 
31 58
     def create_port(self):
32 59
         net = self._make_network(self.fmt, 'net1', True)
@@ -41,6 +68,82 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
41 68
         port = self.deserialize(self.fmt, port_res)['port']
42 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 147
     def _test_port_binding_and_status(self, port_id, action, status):
45 148
         # This function binds or unbinds port to chassis and
46 149
         # checks if port status matches with input status

Loading…
Cancel
Save