Skip to content

Commit

Permalink
[OVN][L3] Optimize FIP update operation
Browse files Browse the repository at this point in the history
If the floating IP updates only the QoS policy, the method now
skips the OVN NAT rules update and updates only the QoS policy.
That avoids the OVN NAT rules deletion and creation and the
``FIPAddDeleteEvent`` event that deletes the MAC binding entries
for an active floating IP, causing a disruption.

Closes-Bug: #2025144

Conflicts:
    neutron/tests/unit/services/ovn_l3/test_plugin.py

Change-Id: Ib9ec45d643c6162c526cd5a02db270094b575e34
(cherry picked from commit 7b85f9c)
  • Loading branch information
ralonsoh committed Jul 6, 2023
1 parent 7972c1e commit 4f2de74
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 17 deletions.
28 changes: 17 additions & 11 deletions neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
Expand Up @@ -32,6 +32,7 @@
from neutron_lib.plugins import directory
from neutron_lib.plugins import utils as p_utils
from neutron_lib.services.logapi import constants as log_const
from neutron_lib.services.qos import constants as qos_consts
from neutron_lib.utils import helpers
from neutron_lib.utils import net as n_net
from oslo_config import cfg
Expand Down Expand Up @@ -1124,25 +1125,30 @@ def create_floatingip(self, context, floatingip):
n_context.get_admin_context(), floatingip['id'],
const.FLOATINGIP_STATUS_ACTIVE)

def update_floatingip(self, context, floatingip):
def update_floatingip(self, context, floatingip, fip_request=None):
fip_status = None
router_id = None
ovn_fip = self._nb_idl.get_floatingip(floatingip['id'])
fip_request = fip_request[l3.FLOATINGIP] if fip_request else {}
qos_update_only = (len(fip_request.keys()) == 1 and
qos_consts.QOS_POLICY_ID in fip_request)

check_rev_cmd = self._nb_idl.check_revision_number(
floatingip['id'], floatingip, ovn_const.TYPE_FLOATINGIPS)
with self._nb_idl.transaction(check_error=True) as txn:
txn.add(check_rev_cmd)
if ovn_fip:
lrouter = ovn_fip['external_ids'].get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY,
utils.ovn_name(router_id))
self._delete_floatingip(ovn_fip, lrouter, txn=txn)
fip_status = const.FLOATINGIP_STATUS_DOWN

if floatingip.get('port_id'):
self._create_or_update_floatingip(floatingip, txn=txn)
fip_status = const.FLOATINGIP_STATUS_ACTIVE
# If FIP updates the QoS policy only, skip the OVN NAT rules update
if not qos_update_only:
if ovn_fip:
lrouter = ovn_fip['external_ids'].get(
ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY,
utils.ovn_name(router_id))
self._delete_floatingip(ovn_fip, lrouter, txn=txn)
fip_status = const.FLOATINGIP_STATUS_DOWN

if floatingip.get('port_id'):
self._create_or_update_floatingip(floatingip, txn=txn)
fip_status = const.FLOATINGIP_STATUS_ACTIVE

self._qos_driver.update_floatingip(txn, floatingip)

Expand Down
2 changes: 1 addition & 1 deletion neutron/services/ovn_l3/plugin.py
Expand Up @@ -285,7 +285,7 @@ def delete_floatingip(self, context, id):
def update_floatingip(self, context, id, floatingip):
fip = super(OVNL3RouterPlugin, self).update_floatingip(context, id,
floatingip)
self._ovn_client.update_floatingip(context, fip)
self._ovn_client.update_floatingip(context, fip, floatingip)
return fip

def update_floatingip_status(self, context, floatingip_id, status):
Expand Down
34 changes: 29 additions & 5 deletions neutron/tests/unit/services/ovn_l3/test_plugin.py
Expand Up @@ -16,6 +16,7 @@
from unittest import mock

from neutron_lib.api.definitions import external_net
from neutron_lib.api.definitions import l3 as l3_def
from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import provider_net as pnet
from neutron_lib.callbacks import events
Expand All @@ -26,12 +27,14 @@
from neutron_lib.exceptions import l3 as l3_exc
from neutron_lib.plugins import constants as plugin_constants
from neutron_lib.plugins import directory
from neutron_lib.services.qos import constants as qos_consts
from oslo_config import cfg
from oslo_utils import uuidutils

from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config
from neutron.db import extraroute_db
from neutron.services.revisions import revision_plugin
from neutron.tests.unit.api import test_extensions
from neutron.tests.unit.extensions import test_extraroute
Expand Down Expand Up @@ -1216,7 +1219,8 @@ def test_update_floatingip(self, uf):
uf.return_value = self.fake_floating_ip_new
nb_ovn.get_floatingip.return_value = (
self.fake_ovn_nat_rule)
self.l3_inst.update_floatingip(self.context, 'id', 'floatingip')
fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}}
self.l3_inst.update_floatingip(self.context, 'id', fip)
nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id',
type='dnat_and_snat',
Expand All @@ -1240,13 +1244,30 @@ def test_update_floatingip(self, uf):
logical_port='new-port_id',
external_ids=expected_ext_ids)

@mock.patch.object(extraroute_db.ExtraRoute_dbonly_mixin,
'update_floatingip')
def test_update_floatingip_qos(self, uf):
nb_ovn = self.l3_inst._nb_ovn
nb_ovn.is_col_present.return_value = True
uf.return_value = self.fake_floating_ip_new
nb_ovn.get_floatingip.return_value = (
self.fake_ovn_nat_rule)
fip = {l3_def.FLOATINGIP: {qos_consts.QOS_POLICY_ID: 'qos_id_1'}}
with mock.patch.object(self.l3_inst._ovn_client._qos_driver,
'update_floatingip') as ufip:
self.l3_inst.update_floatingip(self.context, 'id', fip)
nb_ovn.delete_nat_rule_in_lrouter.assert_not_called()
nb_ovn.add_nat_rule_in_lrouter.assert_not_called()
ufip.assert_called_once_with(mock.ANY, self.fake_floating_ip_new)

@mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.'
'update_floatingip')
def test_update_floatingip_associate(self, uf):
self.l3_inst._nb_ovn.is_col_present.return_value = True
self.fake_floating_ip.update({'fixed_port_id': None})
uf.return_value = self.fake_floating_ip_new
self.l3_inst.update_floatingip(self.context, 'id', 'floatingip')
fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}}
self.l3_inst.update_floatingip(self.context, 'id', fip)
self.l3_inst._nb_ovn.delete_nat_rule_in_lrouter.assert_not_called()
expected_ext_ids = {
ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip_new['id'],
Expand Down Expand Up @@ -1282,7 +1303,8 @@ def test_update_floatingip_associate_distributed(self, uf, gn):

config.cfg.CONF.set_override(
'enable_distributed_floating_ip', True, group='ovn')
self.l3_inst.update_floatingip(self.context, 'id', 'floatingip')
fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}}
self.l3_inst.update_floatingip(self.context, 'id', fip)
self.l3_inst._nb_ovn.delete_nat_rule_in_lrouter.assert_not_called()
expected_ext_ids = {
ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip_new['id'],
Expand Down Expand Up @@ -1310,7 +1332,8 @@ def test_update_floatingip_association_empty_update(self, uf):
self.fake_floating_ip.update({'fixed_port_id': 'foo'})
self.fake_floating_ip_new.update({'port_id': 'foo'})
uf.return_value = self.fake_floating_ip_new
self.l3_inst.update_floatingip(self.context, 'id', 'floatingip')
fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}}
self.l3_inst.update_floatingip(self.context, 'id', fip)
nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id',
type='dnat_and_snat',
Expand Down Expand Up @@ -1345,7 +1368,8 @@ def test_update_floatingip_reassociate_to_same_port_diff_fixed_ip(
self.fake_floating_ip_new.update({'port_id': 'port_id',
'fixed_port_id': 'port_id'})
uf.return_value = self.fake_floating_ip_new
self.l3_inst.update_floatingip(self.context, 'id', 'floatingip')
fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}}
self.l3_inst.update_floatingip(self.context, 'id', fip)

nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id',
Expand Down

0 comments on commit 4f2de74

Please sign in to comment.