Browse Source

Merge "ovn: Remove is_port_groups_supported() code" into stable/train

changes/67/730567/5
Zuul 3 weeks ago
committed by Gerrit Code Review
parent
commit
8a435757f3
19 changed files with 258 additions and 1776 deletions
  1. +56
    -27
      doc/source/admin/refarch/refarch.rst
  2. +0
    -184
      doc/source/contributor/design/acl_optimizations.rst
  3. +0
    -1
      doc/source/contributor/design/index.rst
  4. +16
    -162
      networking_ovn/common/acl.py
  5. +2
    -7
      networking_ovn/common/maintenance.py
  6. +35
    -176
      networking_ovn/common/ovn_client.py
  7. +15
    -195
      networking_ovn/ovn_db_sync.py
  8. +0
    -69
      networking_ovn/ovsdb/commands.py
  9. +0
    -23
      networking_ovn/ovsdb/impl_idl_ovn.py
  10. +0
    -54
      networking_ovn/ovsdb/ovn_api.py
  11. +3
    -9
      networking_ovn/tests/functional/test_maintenance.py
  12. +1
    -94
      networking_ovn/tests/functional/test_ovn_db_resources.py
  13. +22
    -125
      networking_ovn/tests/functional/test_ovn_db_sync.py
  14. +3
    -102
      networking_ovn/tests/unit/common/test_acl.py
  15. +7
    -19
      networking_ovn/tests/unit/common/test_maintenance.py
  16. +0
    -10
      networking_ovn/tests/unit/fakes.py
  17. +88
    -233
      networking_ovn/tests/unit/ml2/test_mech_driver.py
  18. +0
    -165
      networking_ovn/tests/unit/ovsdb/test_commands.py
  19. +10
    -121
      networking_ovn/tests/unit/test_ovn_db_sync.py

+ 56
- 27
doc/source/admin/refarch/refarch.rst View File

@@ -216,10 +216,23 @@ for the compute node.
Security Groups/Rules
---------------------

Each security group will map to 2 Address_Sets in the OVN NB and SB
tables, one for ipv4 and another for ipv6, which will be used to hold ip
addresses for the ports that belong to the security group, so that rules
with remote_group_id can be efficiently applied.
When a Neutron Security Group is created, the equivalent Port Group in OVN
(pg-<security_group_id> is created). This Port Group references Neutron SG id
in its external_ids column.

When a Neutron Port is created, the equivalent Logical Port in OVN is added to
those Port Groups associated to the Neutron Security Groups this port belongs
to.

When a Neutron Port is deleted, the associated Logical Port in OVN is deleted.
Since the schema includes a weak reference to the port, when the LSP gets
deleted, it is automatically deleted from any Port Group entry where it was
previously present.

Every time a security group rule is created, instead of figuring out the ports
affected by its SG and inserting an ACL row which will be referenced by
different Logical Switches, we just reference it from the associated
Port Group.

.. todo: add block with openstack security group rule example

@@ -227,36 +240,52 @@ OVN operations
~~~~~~~~~~~~~~

#. Creating a security group will cause the OVN mechanism driver to create
2 new entries in the Address Set table of the northbound DB:
a port group in the Port_Group table of the northbound DB:

.. code-block:: console

_uuid : 9a9d01bd-4afc-4d12-853a-cd21b547911d
addresses : []
external_ids : {"neutron:security_group_name"=default}
name : "as_ip4_90a78a43_b549_4bee_8822_21fcccab58dc"
_uuid : e96c5994-695d-4b9c-a17b-c7375ad281e2
acls : [33c3c2d0-bc7b-421b-ace9-10884851521a, c22170ec-da5d-4a59-b118-f7f0e370ebc4]
external_ids : {"neutron:security_group_id"="ccbeffee-7b98-4b6f-adf7-d42027ca6447"}
name : pg_ccbeffee_7b98_4b6f_adf7_d42027ca6447
ports : []

_uuid : 27a91327-636e-4125-99f0-6f2937a3b6d8
addresses : []
external_ids : {"neutron:security_group_name"=default}
name : "as_ip6_90a78a43_b549_4bee_8822_21fcccab58dc"

In the above entries, the address set name include the protocol (IPv4
or IPv6, written as ip4 or ip6) and the UUID of the Openstack security
group, dashes translated to underscores.

#. In turn, these new entries will be translated by the OVN northd daemon
into entries in the southbound DB:
And it also creates the default ACLs for egress traffic in the ACL table of
the northbound DB:

.. code-block:: console

_uuid : 886d7b3a-e460-470f-8af2-7c7d88ce45d2
addresses : []
name : "as_ip4_90a78a43_b549_4bee_8822_21fcccab58dc"

_uuid : 355ddcba-941d-4f1c-b823-dc811cec59ca
addresses : []
name : "as_ip6_90a78a43_b549_4bee_8822_21fcccab58dc"
_uuid : 33c3c2d0-bc7b-421b-ace9-10884851521a
action : allow-related
direction : from-lport
external_ids : {"neutron:security_group_rule_id"="655b0d7e-144e-4bd8-9243-10a261b91041"}
log : false
match : "inport == @pg_ccbeffee_7b98_4b6f_adf7_d42027ca6447 && ip4"
meter : []
name : []
priority : 1002
severity : []

_uuid : c22170ec-da5d-4a59-b118-f7f0e370ebc4
action : allow-related
direction : from-lport
external_ids : {"neutron:security_group_rule_id"="a303a34f-5f19-494f-a9e2-e23f246bfcad"}
log : false
match : "inport == @pg_ccbeffee_7b98_4b6f_adf7_d42027ca6447 && ip6"
meter : []
name : []
priority : 1002
severity : []

Ports with no security groups
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When a port doesn't belong to any Security Group and port security is enabled,
we, by default, drop all the traffic to/from that port. In order to implement
this through Port Groups, we'll create a special Port Group with a fixed name
(``neutron_pg_drop``) which holds the ACLs to drop all the traffic.

This PG is created automatically once before neutron-server forks into workers.

Networks
--------


+ 0
- 184
doc/source/contributor/design/acl_optimizations.rst View File

@@ -1,184 +0,0 @@
============================================
ACL Handling optimizations in networking-ovn
============================================

This document presents the current problem with ACLs and the design changes
proposed to core OVN as well as the necessary modifications to be made to
networking-ovn to improve their usage.

Problem description
===================

There is basically two problems being addressed in this spec:

1. While in Neutron, a ``Security Group Rule`` is tied to a
``Security Group``, in OVN ``ACLs`` are created per port. Therefore,
we'll typically have *many* more ACLs than Security Group Rules, resulting
in a performance hit as the number of ports grows.

2. An ACL in OVN is applied to a ``Logical Switch``. As a result,
``networking-ovn`` has to figure out which Logical Switches to apply the
generated ACLs per each Security Rule.

Let's highlight both problems with an example:

- Neutron Networks: NA, NB, NC
- Neutron Security Group: SG1
- Number of Neutron Security Group Rules in SG1: 10
- Neutron Ports in NA: 100
- Neutron Ports in NB: 100
- Neutron Ports in NC: 100
- All ports belong to SG1

When we implement the above scenario in OVN, this is what we'll get:

- OVN Logical Switches: NA, NB, NC
- Number of ACL rows in Northbound DB ACL table: 3000 (10 rules * 100 ports *
3 networks)
- Number of elements in acl column on each Logical_Switch row: 1000 (10 rules
* 100 ports).

And this is how, for example, the ACL match fields for the default Neutron
Security Group would look like::

outport == <port1_uuid> && ip4 && ip4.src == $as_ip4_<sg1_uuid>
outport == <port2_uuid> && ip4 && ip4.src == $as_ip4_<sg1_uuid>
outport == <port3_uuid> && ip4 && ip4.src == $as_ip4_<sg1_uuid>
...
outport == <port300_uuid> && ip4 && ip4.src == $as_ip4_<sg1_uuid>

As you can see, all of them look the same except for the outport field which
is clearly redundant and makes the NB database grow a lot at scale.
Also, ``networking-ovn`` had to figure out for each rule in SG1 which Logical
Switches it had to apply the ACLs on (NA, NB and NC). This can be really costly
when the number of networks and port grows.


Proposed optimization
=====================

In the OpenStack context, we'll be facing this scenario most of the time
where the majority of the ACLs will look the same except for the
outport/inport fields in the match column. It would make sense to be able to
substitute all those ACLs by a single one which references all the ports
affected by that SG rule::

outport == @port_group1 && ip4 && ip4.src == $port_group1_ip4


Implementation Details
======================

Core OVN
--------

There's a series of patches in Core OVN that will enable us to achieve this
optimization:

https://github.com/openvswitch/ovs/commit/3d2848bafa93a2b483a4504c5de801454671dccf
https://github.com/openvswitch/ovs/commit/1beb60afd25a64f1779903b22b37ed3d9956d47c
https://github.com/openvswitch/ovs/commit/689829d53612a573f810271a01561f7b0948c8c8


In summary, these patches are:

- Adding a new entity called Port_Group which will hold a list of weak
references to the Logical Switch ports that belong to it.
- Automatically creating/updating two Address Sets (_ip4 and _ip6) in
Southbound database every time a new port is added to the group.
- Support adding a list of ACLs to a Port Group. As the SG rules may
span across different Logical Switches, we used to insert the ACLs in
all the Logical Switches where we have ports in within a SG. Figuring this
out is expensive and this new feature is a huge gain in terms of
performance when creating/deleting ports.


networking-ovn
--------------

In the OpenStack integration driver, the following changes are required to
accomplish this optimization:

- When a Neutron Security Group is created, create the equivalent Port Group
in OVN (pg-<security_group_id>), instead of creating a pair of Adress Sets
for IPv4 and IPv6. This Port Group will reference Neutron SG id in its
``external_ids`` column.

- When a Neutron Port is created, the equivalent Logical Port in OVN will be
added to those Port Groups associated to the Neutron Security Groups this
port belongs to.

- When a Neutron Port is deleted, we'll delete the associated Logical Port in
OVN. Since the schema includes a weak reference to the port, when the LSP
gets deleted, it will also be automatically deleted from any Port Group
entry where it was previously present.

- Instead of handling SG rules per port, we now need to handle them per SG
referencing the associated Port Group in the outport/inport fields. This
will be the biggest gain in terms of processing since we don't need to
iterate through all the ports anymore. For example:

.. code-block:: python

-def acl_direction(r, port):
+def acl_direction(r):
if r['direction'] == 'ingress':
portdir = 'outport'
else:
portdir = 'inport'
- return '%s == "%s"' % (portdir, port['id'])
+ return '%s == "@%s"' % (portdir, utils.ovn_name(r['security_group_id'])

- Every time a SG rule is created, instead of figuring out the ports affected
by its SG and inserting an ACL row which will be referrenced by different
Logical Switches, we will just reference it from the associated Port Group.

- For Neutron remote security groups, we just need to reference the
automatically created Address_Set for that Port Group.

As a bonus, we are tackling the race conditions that could happen in
Address_Sets right now when we're deleting and creating a port at the same
time. This is thanks to the fact that the Address_Sets in the SB table are
generated automatically by ovn-northd from the Port_Group contents and
Port Group is referencing actual Logical Switch Ports. More info at:
https://bugs.launchpad.net/networking-ovn/+bug/1611852


Backwards compatibility considerations
--------------------------------------

- If the schema doesn't include the ``Port_Group`` table, keep the old
behavior(Address Sets) for backwards compatibility.

- If the schema supports Port Groups, then a migration task will be performed
from an OvnWorker. This way we'll ensure that it'll happen only once across
the cloud thanks the OVSDB lock. This will be done right at the beginning of
the ovn_db_sync process to make sure that when neutron-server starts,
everything is in place to work with Port Groups. This migration process will
perform the following steps:

* Create the default drop Port Group and add all ports with port
security enabled to it.
* Create a Port Group for every existing Neutron Security Group and
add all its Security Group Rules as ACLs to that Port Group.
* Delete all existing Address Sets in NorthBound database which correspond to
a Neutron Security Group.
* Delete all the ACLs in every Logical Switch (Neutron network).

We should eventually remove the backwards compatibility and migration path. At
that point we should require OVS >= 2.10 from networking-ovn.

Special cases
-------------

Ports with no security groups
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When a port doesn't belong to any Security Group and port security is enabled,
we, by default, drop all the traffic to/from that port. In order to implement
this through Port Groups, we'll create a special Port Group with a fixed name
(``neutron_pg_drop``) which holds the ACLs to drop all the traffic.

This PG will be created automatically when we first need it, avoiding the need
to create it beforehand or during deployment.


+ 0
- 1
doc/source/contributor/design/index.rst View File

@@ -10,7 +10,6 @@ Design Notes
ovn_worker
metadata_api
database_consistency
acl_optimizations
loadbalancer
distributed_ovsdb_events
dhcp_opts

+ 16
- 162
networking_ovn/common/acl.py View File

@@ -12,8 +12,6 @@
# under the License.
#

import netaddr

from neutron_lib import constants as const
from neutron_lib import exceptions as n_exceptions
from oslo_config import cfg
@@ -278,44 +276,17 @@ def _get_sg_from_cache(plugin, admin_context, sg_cache, sg_id):
return sg


def acl_remote_group_id(r, ip_version, ovn=None):
def acl_remote_group_id(r, ip_version):
if not r['remote_group_id']:
return ''

src_or_dst = 'src' if r['direction'] == 'ingress' else 'dst'
if (ovn and ovn.is_port_groups_supported()):
addrset_name = utils.ovn_pg_addrset_name(r['remote_group_id'],
ip_version)
else:
addrset_name = utils.ovn_addrset_name(r['remote_group_id'],
ip_version)
addrset_name = utils.ovn_pg_addrset_name(r['remote_group_id'],
ip_version)
return ' && %s.%s == $%s' % (ip_version, src_or_dst, addrset_name)


def _add_sg_rule_acl_for_port(port, r):
# Update the match based on which direction this rule is for (ingress
# or egress).
match = acl_direction(r, port)

# Update the match for IPv4 vs IPv6.
ip_match, ip_version, icmp = acl_ethertype(r)
match += ip_match

# Update the match if an IPv4 or IPv6 prefix was specified.
match += acl_remote_ip_prefix(r, ip_version)

# Update the match if remote group id was specified.
match += acl_remote_group_id(r, ip_version)

# Update the match for the protocol (tcp, udp, icmp) and port/type
# range if specified.
match += acl_protocol_and_ports(r, icmp)

# Finally, create the ACL entry for the direction specified.
return add_sg_rule_acl_for_port(port, r, match)


def _add_sg_rule_acl_for_port_group(port_group, r, ovn):
def _add_sg_rule_acl_for_port_group(port_group, r):
# Update the match based on which direction this rule is for (ingress
# or egress).
match = acl_direction(r, port_group=port_group)
@@ -328,7 +299,7 @@ def _add_sg_rule_acl_for_port_group(port_group, r, ovn):
match += acl_remote_ip_prefix(r, ip_version)

# Update the match if remote group id was specified.
match += acl_remote_group_id(r, ip_version, ovn)
match += acl_remote_group_id(r, ip_version)

# Update the match for the protocol (tcp, udp, icmp) and port/type
# range if specified.
@@ -346,7 +317,7 @@ def _acl_columns_name_severity_supported(nb_idl):
def add_acls_for_sg_port_group(ovn, security_group, txn):
for r in security_group['security_group_rules']:
acl = _add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(security_group['id']), r, ovn)
utils.ovn_port_group_name(security_group['id']), r)
txn.add(ovn.pg_acl_add(**acl))


@@ -355,7 +326,6 @@ def update_acls_for_security_group(plugin,
ovn,
security_group_id,
security_group_rule,
sg_ports_cache=None,
is_add_acl=True):

# Skip ACLs if security groups aren't enabled
@@ -365,135 +335,19 @@ def update_acls_for_security_group(plugin,
# Check if ACL log name and severity supported or not
keep_name_severity = _acl_columns_name_severity_supported(ovn)

# If we're using a Port Group for this SG, just update it.
# Otherwise, keep the old behavior.
if (ovn.is_port_groups_supported() and
not ovn.get_address_set(security_group_id)):
acl = _add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(security_group_id),
security_group_rule, ovn)
# Remove ACL log name and severity if not supported
if is_add_acl:
if not keep_name_severity:
acl.pop('name')
acl.pop('severity')
ovn.pg_acl_add(**acl).execute(check_error=True)
else:
ovn.pg_acl_del(acl['port_group'], acl['direction'],
acl['priority'], acl['match']).execute(
check_error=True)
return

# Get the security group ports.
sg_ports_cache = sg_ports_cache or {}
sg_ports = _get_sg_ports_from_cache(plugin,
admin_context,
sg_ports_cache,
security_group_id)

# ACLs associated with a security group may span logical switches
sg_port_ids = [binding['port_id'] for binding in sg_ports]
sg_port_ids = list(set(sg_port_ids))
port_list = plugin.get_ports(admin_context,
filters={'id': sg_port_ids})
if not port_list:
return

acl_new_values_dict = {}
update_port_list = []

# NOTE(lizk): We can directly locate the affected acl records,
# so no need to compare new acl values with existing acl objects.
for port in port_list:
# Skip trusted port
if utils.is_lsp_trusted(port):
continue

update_port_list.append(port)
acl = _add_sg_rule_acl_for_port(port, security_group_rule)
# Remove lport and lswitch since we don't need them
acl.pop('lport')
acl.pop('lswitch')
# Remove ACL log name and severity if not supported,
acl = _add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(security_group_id),
security_group_rule)
# Remove ACL log name and severity if not supported
if is_add_acl:
if not keep_name_severity:
acl.pop('name')
acl.pop('severity')
acl_new_values_dict[port['id']] = acl

if not update_port_list:
return
lswitch_names = {p['network_id'] for p in update_port_list}

ovn.update_acls(list(lswitch_names),
iter(update_port_list),
acl_new_values_dict,
need_compare=False,
is_add_acl=is_add_acl).execute(check_error=True)


def add_acls(plugin, admin_context, port, sg_cache, subnet_cache, ovn):
acl_list = []

# Skip ACLs if security groups aren't enabled
if not is_sg_enabled():
return acl_list

sec_groups = utils.get_lsp_security_groups(port)
if not sec_groups:
# If it is a trusted port or port security is disabled, allow all
# traffic. So don't add any ACLs.
if utils.is_lsp_trusted(port) or not utils.is_port_security_enabled(
port):
return acl_list

# if port security is enabled, drop all traffic.
return drop_all_ip_traffic_for_port(port)

# Drop all IP traffic to and from the logical port by default.
acl_list += drop_all_ip_traffic_for_port(port)

# Add DHCP ACLs.
port_subnet_ids = set()
for ip in port['fixed_ips']:
if netaddr.IPNetwork(ip['ip_address']).version != 4:
continue
subnet = _get_subnet_from_cache(plugin,
admin_context,
subnet_cache,
ip['subnet_id'])
# Ignore duplicate DHCP ACLs for the subnet.
if subnet['id'] not in port_subnet_ids:
acl_list += add_acl_dhcp(port, subnet, True)
port_subnet_ids.add(subnet['id'])

# We create an ACL entry for each rule on each security group applied
# to this port.
for sg_id in sec_groups:
sg = _get_sg_from_cache(plugin,
admin_context,
sg_cache,
sg_id)
for r in sg['security_group_rules']:
acl = _add_sg_rule_acl_for_port(port, r)
acl_list.append(acl)

# Remove ACL log name and severity if not supported,
if not _acl_columns_name_severity_supported(ovn):
for acl in acl_list:
acl.pop('name')
acl.pop('severity')

return acl_list


def acl_port_ips(port):
# Skip ACLs if security groups aren't enabled
if not is_sg_enabled():
return {'ip4': [], 'ip6': []}

ip_list = [x['ip_address'] for x in port.get('fixed_ips', [])]
ip_list.extend(utils.get_allowed_address_pairs_ip_addresses(port))
return utils.sort_ips_by_version(ip_list)
ovn.pg_acl_add(**acl).execute(check_error=True)
else:
ovn.pg_acl_del(acl['port_group'], acl['direction'],
acl['priority'], acl['match']).execute(
check_error=True)


def filter_acl_dict(acl, extra_fields=None):


+ 2
- 7
networking_ovn/common/maintenance.py View File

@@ -171,7 +171,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
},
ovn_const.TYPE_SECURITY_GROUPS: {
'neutron_get': self._ovn_client._plugin.get_security_group,
'ovn_get': self._get_security_group,
'ovn_get': self._nb_idl.get_port_group,
'ovn_create': self._ovn_client.create_security_group,
'ovn_delete': self._ovn_client.delete_security_group,
},
@@ -192,10 +192,6 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
},
}

def _get_security_group(self, uuid):
return (self._nb_idl.get_address_set(uuid) or
self._nb_idl.get_port_group(uuid))

@property
def has_lock(self):
return not self._idl.is_lock_contended
@@ -289,8 +285,7 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):

# If Port Groups are not supported or we've already migrated, we don't
# need to attempt to migrate again.
if (not self._nb_idl.is_port_groups_supported() or
not self._nb_idl.get_address_sets()):
if not self._nb_idl.get_address_sets():
raise periodics.NeverAgain()

# Only the worker holding a valid lock within OVSDB will perform the


+ 35
- 176
networking_ovn/common/ovn_client.py View File

@@ -324,8 +324,6 @@ class OVNClient(object):
port, ovn_const.TYPE_PORTS))}
lswitch_name = utils.ovn_name(port['network_id'])
admin_context = n_context.get_admin_context()
sg_cache = {}
subnet_cache = {}

# It's possible to have a network created on one controller and then a
# port created on a different controller quickly enough that the second
@@ -395,45 +393,17 @@ class OVNClient(object):
port_cmd = txn.add(self._nb_idl.create_lswitch_port(
**kwargs))

# Handle ACL's for this port. If we're not using Port Groups
# because either the schema doesn't support it or we didn't
# migrate old SGs from Address Sets to Port Groups, then we
# keep the old behavior. For those SGs this port belongs to
# that are modelled as a Port Group, we'll use it.
sg_ids = utils.get_lsp_security_groups(port)
if self._nb_idl.is_port_groups_supported():
# If this is not a trusted port or port security is enabled,
# add it to the default drop Port Group so that all traffic
# is dropped by default.
if not utils.is_lsp_trusted(port) or port_info.port_security:
self._add_port_to_drop_port_group(port_cmd, txn)
# For SGs modelled as OVN Port Groups, just add the port to
# its Port Group.
for sg in sg_ids:
txn.add(self._nb_idl.pg_add_ports(
utils.ovn_port_group_name(sg), port_cmd))
else:
# SGs modelled as Address Sets:
acls_new = ovn_acl.add_acls(self._plugin, admin_context,
port, sg_cache, subnet_cache,
self._nb_idl)
for acl in acls_new:
txn.add(self._nb_idl.add_acl(**acl))

if port.get('fixed_ips') and sg_ids:
addresses = ovn_acl.acl_port_ips(port)
# NOTE(rtheis): Fail port creation if the address set
# doesn't exist. This prevents ports from being created on
# any security groups out-of-sync between neutron and OVN.
for sg_id in sg_ids:
for ip_version in addresses:
if addresses[ip_version]:
txn.add(self._nb_idl.update_address_set(
name=utils.ovn_addrset_name(sg_id,
ip_version),
addrs_add=addresses[ip_version],
addrs_remove=None,
if_exists=False))
# If this is not a trusted port or port security is enabled,
# add it to the default drop Port Group so that all traffic
# is dropped by default.
if not utils.is_lsp_trusted(port) or port_info.port_security:
self._add_port_to_drop_port_group(port_cmd, txn)
# For SGs modelled as OVN Port Groups, just add the port to
# its Port Group.
for sg in sg_ids:
txn.add(self._nb_idl.pg_add_ports(
utils.ovn_port_group_name(sg), port_cmd))

if self.is_dns_required_for_port(port):
self.add_txns_to_sync_port_dns_records(txn, port)
@@ -497,8 +467,6 @@ class OVNClient(object):
utils.get_revision_number(
port, ovn_const.TYPE_PORTS))}
admin_context = n_context.get_admin_context()
sg_cache = {}
subnet_cache = {}

check_rev_cmd = self._nb_idl.check_revision_number(
port['id'], port, ovn_const.TYPE_PORTS)
@@ -577,96 +545,22 @@ class OVNClient(object):
detached_sg_ids = old_sg_ids - new_sg_ids
attached_sg_ids = new_sg_ids - old_sg_ids

if self._nb_idl.is_port_groups_supported():
for sg in detached_sg_ids:
txn.add(self._nb_idl.pg_del_ports(
utils.ovn_port_group_name(sg), port['id']))
for sg in attached_sg_ids:
txn.add(self._nb_idl.pg_add_ports(
utils.ovn_port_group_name(sg), port['id']))
if (not utils.is_lsp_trusted(port) and
utils.is_port_security_enabled(port)):
self._add_port_to_drop_port_group(port['id'], txn)
# If the port doesn't belong to any security group and
# port_security is disabled, or it's a trusted port, then
# allow all traffic.
elif ((not new_sg_ids and
not utils.is_port_security_enabled(port)) or
utils.is_lsp_trusted(port)):
self._del_port_from_drop_port_group(port['id'], txn)
else:

old_fixed_ips = utils.remove_macs_from_lsp_addresses(
ovn_port.addresses)
new_fixed_ips = [x['ip_address'] for x in
port.get('fixed_ips', [])]
is_fixed_ips_updated = (
sorted(old_fixed_ips) != sorted(new_fixed_ips))
port_security_changed = (
utils.is_port_security_enabled(port) !=
bool(ovn_port.port_security))
# Refresh ACLs for changed security groups or fixed IPs.
if (detached_sg_ids or attached_sg_ids or
is_fixed_ips_updated or port_security_changed):
# Note that update_acls will compare the port's ACLs to
# ensure only the necessary ACLs are added and deleted
# on the transaction.
acls_new = ovn_acl.add_acls(self._plugin,
admin_context,
port,
sg_cache,
subnet_cache,
self._nb_idl)
txn.add(self._nb_idl.update_acls([port['network_id']],
[port],
{port['id']: acls_new},
need_compare=True))

# Refresh address sets for changed security groups or fixed
# IPs.
if len(old_fixed_ips) != 0 or len(new_fixed_ips) != 0:
addresses = ovn_acl.acl_port_ips(port)
addresses_old = utils.sort_ips_by_version(
utils.get_ovn_port_addresses(ovn_port))
# Add current addresses to attached security groups.
for sg_id in attached_sg_ids:
for ip_version in addresses:
if addresses[ip_version]:
txn.add(self._nb_idl.update_address_set(
name=utils.ovn_addrset_name(sg_id,
ip_version),
addrs_add=addresses[ip_version],
addrs_remove=None))
# Remove old addresses from detached security groups.
for sg_id in detached_sg_ids:
for ip_version in addresses_old:
if addresses_old[ip_version]:
txn.add(self._nb_idl.update_address_set(
name=utils.ovn_addrset_name(sg_id,
ip_version),
addrs_add=None,
addrs_remove=addresses_old[ip_version]))

if is_fixed_ips_updated or addr_pairs_diff.changed:
# We have refreshed address sets for attached and
# detached security groups, so now we only need to take
# care of unchanged security groups.
unchanged_sg_ids = new_sg_ids & old_sg_ids
for sg_id in unchanged_sg_ids:
for ip_version in addresses:
addr_add = ((set(addresses[ip_version]) -
set(addresses_old[ip_version])) or
None)
addr_remove = (
(set(addresses_old[ip_version]) -
set(addresses[ip_version])) or None)

if addr_add or addr_remove:
txn.add(self._nb_idl.update_address_set(
name=utils.ovn_addrset_name(
sg_id, ip_version),
addrs_add=addr_add,
addrs_remove=addr_remove))
for sg in detached_sg_ids:
txn.add(self._nb_idl.pg_del_ports(
utils.ovn_port_group_name(sg), port['id']))
for sg in attached_sg_ids:
txn.add(self._nb_idl.pg_add_ports(
utils.ovn_port_group_name(sg), port['id']))
if (not utils.is_lsp_trusted(port) and
utils.is_port_security_enabled(port)):
self._add_port_to_drop_port_group(port['id'], txn)
# If the port doesn't belong to any security group and
# port_security is disabled, or it's a trusted port, then
# allow all traffic.
elif ((not new_sg_ids and
not utils.is_port_security_enabled(port)) or
utils.is_lsp_trusted(port)):
self._del_port_from_drop_port_group(port['id'], txn)

self._qos_driver.update_port(txn, port, port_object)

@@ -698,24 +592,6 @@ class OVNClient(object):
txn.add(self._nb_idl.delete_lswitch_port(
port_id, network_id))

if not self._nb_idl.is_port_groups_supported():
txn.add(self._nb_idl.delete_acl(
network_id, port_id, if_exists=True))

addresses = utils.sort_ips_by_version(
utils.get_ovn_port_addresses(ovn_port))
sec_groups = self._get_lsp_backward_compat_sgs(
ovn_port, port_object=port_object, skip_trusted_port=False)
for sg_id in sec_groups:
for ip_version, addr_list in addresses.items():
if not addr_list:
continue
txn.add(self._nb_idl.update_address_set(
name=utils.ovn_addrset_name(sg_id, ip_version),
addrs_add=None,
addrs_remove=addr_list,
if_exists=True))

p_object = ({'id': port_id, 'network_id': network_id}
if not port_object else port_object)
self._qos_driver.delete_port(txn, p_object)
@@ -2116,26 +1992,15 @@ class OVNClient(object):
db_rev.delete_revision(subnet_id, ovn_const.TYPE_SUBNETS)

def create_security_group(self, security_group):
# If the OVN schema supports Port Groups, we'll model security groups
# as such. Otherwise, for backwards compatibility, we'll keep creating
# two Address Sets for each Neutron SG (one for IPv4 and one for
# IPv6).
with self._nb_idl.transaction(check_error=True) as txn:
ext_ids = {ovn_const.OVN_SG_EXT_ID_KEY: security_group['id']}
if self._nb_idl.is_port_groups_supported():
name = utils.ovn_port_group_name(security_group['id'])
txn.add(self._nb_idl.pg_add(
name=name, acls=[], external_ids=ext_ids))
# When a SG is created, it comes with some default rules,
# so we'll apply them to the Port Group.
ovn_acl.add_acls_for_sg_port_group(self._nb_idl,
security_group, txn)
else:
for ip_version in ('ip4', 'ip6'):
name = utils.ovn_addrset_name(security_group['id'],
ip_version)
txn.add(self._nb_idl.create_address_set(
name=name, external_ids=ext_ids))
name = utils.ovn_port_group_name(security_group['id'])
txn.add(self._nb_idl.pg_add(
name=name, acls=[], external_ids=ext_ids))
# When a SG is created, it comes with some default rules,
# so we'll apply them to the Port Group.
ovn_acl.add_acls_for_sg_port_group(self._nb_idl,
security_group, txn)
db_rev.bump_revision(security_group, ovn_const.TYPE_SECURITY_GROUPS)

def create_default_drop_port_group(self, ports=None):
@@ -2166,14 +2031,8 @@ class OVNClient(object):

def delete_security_group(self, security_group_id):
with self._nb_idl.transaction(check_error=True) as txn:
if self._nb_idl.is_port_groups_supported():
name = utils.ovn_port_group_name(security_group_id)
txn.add(self._nb_idl.pg_del(name=name))
else:
for ip_version in ('ip4', 'ip6'):
name = utils.ovn_addrset_name(security_group_id,
ip_version)
txn.add(self._nb_idl.delete_address_set(name=name))
name = utils.ovn_port_group_name(security_group_id)
txn.add(self._nb_idl.pg_del(name=name))
db_rev.delete_revision(security_group_id,
ovn_const.TYPE_SECURITY_GROUPS)



+ 15
- 195
networking_ovn/ovn_db_sync.py View File

@@ -93,7 +93,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):

ctx = context.get_admin_context()

self.sync_address_sets(ctx)
self.sync_port_groups(ctx)
self.sync_networks_ports_and_dhcp_opts(ctx)
self.sync_port_dns_records(ctx)
@@ -125,24 +124,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
neutron_acls[port].remove(acl)
nb_acls[port].remove(acl)

def compute_address_set_difference(self, neutron_sgs, nb_sgs):
neutron_sgs_name_set = set(neutron_sgs.keys())
nb_sgs_name_set = set(nb_sgs.keys())
sgnames_to_add = list(neutron_sgs_name_set - nb_sgs_name_set)
sgnames_to_delete = list(nb_sgs_name_set - neutron_sgs_name_set)
sgs_common = list(neutron_sgs_name_set & nb_sgs_name_set)
sgs_to_update = {}
for sg_name in sgs_common:
neutron_addr_set = set(neutron_sgs[sg_name]['addresses'])
nb_addr_set = set(nb_sgs[sg_name]['addresses'])
addrs_to_add = list(neutron_addr_set - nb_addr_set)
addrs_to_delete = list(nb_addr_set - neutron_addr_set)
if addrs_to_add or addrs_to_delete:
sgs_to_update[sg_name] = {'name': sg_name,
'addrs_add': addrs_to_add,
'addrs_remove': addrs_to_delete}
return sgnames_to_add, sgnames_to_delete, sgs_to_update

def get_acls(self, context):
"""create the list of ACLS in OVN.

@@ -170,17 +151,12 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
acl_list_dict[key] = list([acl])
return acl_list_dict

def get_address_sets(self):
return self.ovn_api.get_address_sets()

def sync_port_groups(self, ctx):
"""Sync Port Groups between neutron and NB.

@param ctx: neutron_lib.context
@type ctx: object of type neutron_lib.context.Context
"""
if not self.ovn_api.is_port_groups_supported():
return

neutron_sgs = {}
neutron_pgs = set()
@@ -241,67 +217,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
LOG.debug('Port-Group-SYNC: transaction finished @ %s',
str(datetime.now()))

def sync_address_sets(self, ctx):
"""Sync Address Sets between neutron and NB.

@param ctx: neutron_lib.context
@type ctx: object of type neutron_lib.context.Context
@var db_ports: List of ports from neutron DB
"""
LOG.debug('Address-Set-SYNC: started @ %s', str(datetime.now()))

sgnames_to_add = sgnames_to_delete = []
sgs_to_update = {}
nb_sgs = self.get_address_sets()

if self.ovn_api.is_port_groups_supported():
# If Port Groups are supported, we just need to delete all Address
# Sets from NB database.
sgnames_to_delete = nb_sgs.keys()
else:
neutron_sgs = {}
with ctx.session.begin(subtransactions=True):
db_sgs = self.core_plugin.get_security_groups(ctx)
db_ports = self.core_plugin.get_ports(ctx)

for sg in db_sgs:
for ip_version in ['ip4', 'ip6']:
name = utils.ovn_addrset_name(sg['id'], ip_version)
neutron_sgs[name] = {
'name': name, 'addresses': [],
'external_ids': {const.OVN_SG_EXT_ID_KEY: sg['id']}}

for port in db_ports:
sg_ids = utils.get_lsp_security_groups(port)
if port.get('fixed_ips') and sg_ids:
addresses = acl_utils.acl_port_ips(port)
for sg_id in sg_ids:
for ip_version in addresses:
name = utils.ovn_addrset_name(sg_id, ip_version)
neutron_sgs[name]['addresses'].extend(
addresses[ip_version])

sgnames_to_add, sgnames_to_delete, sgs_to_update = (
self.compute_address_set_difference(neutron_sgs, nb_sgs))

LOG.debug('Address_Sets added %d, removed %d, updated %d',
len(sgnames_to_add), len(sgnames_to_delete),
len(sgs_to_update))

if self.mode == SYNC_MODE_REPAIR:
LOG.debug('Address-Set-SYNC: transaction started @ %s',
str(datetime.now()))
with self.ovn_api.transaction(check_error=True) as txn:
for sgname in sgnames_to_add:
sg = neutron_sgs[sgname]
txn.add(self.ovn_api.create_address_set(**sg))
for sgname, sg in sgs_to_update.items():
txn.add(self.ovn_api.update_address_set(**sg))
for sgname in sgnames_to_delete:
txn.add(self.ovn_api.delete_address_set(name=sgname))
LOG.debug('Address-Set-SYNC: transaction finished @ %s',
str(datetime.now()))

def _get_acls_from_port_groups(self):
ovn_acls = []
port_groups = self.ovn_api.db_list_rows('Port_Group').execute()
@@ -317,15 +232,20 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
ovn_acls.append(acl_string)
return ovn_acls

def _sync_acls_port_groups(self, ctx):
# If Port Groups are supported, the ACLs in the system will equal
# the number of SG rules plus the default drop rules as OVN would
# allow all traffic by default if those are not added.
def sync_acls(self, ctx):
"""Sync ACLs between neutron and NB.

@param ctx: neutron_lib.context
@type ctx: object of type neutron_lib.context.Context
@return: Nothing
"""
LOG.debug('ACL-SYNC: started @ %s', str(datetime.now()))

neutron_acls = []
for sgr in self.core_plugin.get_security_group_rules(ctx):
pg_name = utils.ovn_port_group_name(sgr['security_group_id'])
neutron_acls.append(acl_utils._add_sg_rule_acl_for_port_group(
pg_name, sgr, self.ovn_api))
pg_name, sgr))
neutron_acls += acl_utils.add_acls_for_drop_port_group(
const.OVN_DROP_PORT_GROUP_NAME)

@@ -379,88 +299,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
'Switch %s', aclr[0])
txn.add(self.ovn_api.acl_del(aclr[0]))

def _sync_acls(self, ctx):
"""Sync ACLs between neutron and NB when not using Port Groups.

@param ctx: neutron_lib.context
@type ctx: object of type neutron_lib.context.Context
@var db_ports: List of ports from neutron DB
@var neutron_acls: neutron dictionary of port
vs list-of-acls
@var nb_acls: NB dictionary of port
vs list-of-acls
@var subnet_cache: cache for subnets
@return: Nothing
"""
db_ports = {}
for port in self.core_plugin.get_ports(ctx):
db_ports[port['id']] = port

sg_cache = {}
subnet_cache = {}
neutron_acls = {}
for port_id, port in db_ports.items():
if utils.get_lsp_security_groups(port):
acl_list = acl_utils.add_acls(self.core_plugin,
ctx,
port,
sg_cache,
subnet_cache,
self.ovn_api)
if port_id in neutron_acls:
neutron_acls[port_id].extend(acl_list)
else:
neutron_acls[port_id] = acl_list

nb_acls = self.get_acls(ctx)

self.remove_common_acls(neutron_acls, nb_acls)

num_acls_to_add = len(list(itertools.chain(*neutron_acls.values())))
num_acls_to_remove = len(list(itertools.chain(*nb_acls.values())))
if 0 != num_acls_to_add or 0 != num_acls_to_remove:
LOG.warning('ACLs-to-be-added %(add)d '
'ACLs-to-be-removed %(remove)d',
{'add': num_acls_to_add,
'remove': num_acls_to_remove})

if self.mode == SYNC_MODE_REPAIR:
with self.ovn_api.transaction(check_error=True) as txn:
for acla in list(itertools.chain(*neutron_acls.values())):
LOG.warning('ACL found in Neutron but not in '
'OVN DB for port %s', acla['lport'])
txn.add(self.ovn_api.add_acl(**acla))

with self.ovn_api.transaction(check_error=True) as txn:
for aclr in list(itertools.chain(*nb_acls.values())):
# Both lswitch and lport aren't needed within the ACL.
lswitchr = aclr.pop('lswitch').replace('neutron-', '')
lportr = aclr.pop('lport')
aclr_dict = {lportr: aclr}
LOG.warning('ACLs found in OVN DB but not in '
'Neutron for port %s', lportr)
txn.add(self.ovn_api.update_acls(
[lswitchr],
[lportr],
aclr_dict,
need_compare=False,
is_add_acl=False
))

def sync_acls(self, ctx):
"""Sync ACLs between neutron and NB.

@param ctx: neutron_lib.context
@type ctx: object of type neutron_lib.context.Context
@return: Nothing
"""
LOG.debug('ACL-SYNC: started @ %s', str(datetime.now()))

if self.ovn_api.is_port_groups_supported():
self._sync_acls_port_groups(ctx)
else:
self._sync_acls(ctx)

LOG.debug('ACL-SYNC: finished @ %s', str(datetime.now()))

def _calculate_fips_differences(self, ovn_fips, db_fips):
@@ -1170,20 +1008,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
# all the ACLs belonging to that Logical Switch.
txn.add(self.ovn_api.acl_del(utils.ovn_name(net['id'])))

def _create_default_drop_port_group(self, db_ports):
with self.ovn_api.transaction(check_error=True) as txn:
pg_name = const.OVN_DROP_PORT_GROUP_NAME
if not self.ovn_api.get_port_group(pg_name):
# If drop Port Group doesn't exist yet, create it.
txn.add(self.ovn_api.pg_add(pg_name, acls=[]))
# Add ACLs to this Port Group so that all traffic is dropped.
acls = acl_utils.add_acls_for_drop_port_group(pg_name)
for acl in acls:
txn.add(self.ovn_api.pg_acl_add(**acl))
ports_ids = [port['id'] for port in db_ports]
# Add the ports to the default Port Group
txn.add(self.ovn_api.pg_add_ports(pg_name, ports_ids))

def _create_sg_port_groups_and_acls(self, ctx, db_ports):
# Create a Port Group per Neutron Security Group
with self.ovn_api.transaction(check_error=True) as txn:
@@ -1203,17 +1027,14 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
def migrate_to_port_groups(self, ctx):
# This routine is responsible for migrating the current Security
# Groups and SG Rules to the new Port Groups implementation.
# 1. Create the default drop Port Group and add all ports with port
# security enabled to it.
# 2. Create a Port Group for every existing Neutron Security Group and
# 1. Create a Port Group for every existing Neutron Security Group and
# add all its Security Group Rules as ACLs to that Port Group.
# 3. Delete all existing Address Sets in NorthBound database which
# 2. Delete all existing Address Sets in NorthBound database which
# correspond to a Neutron Security Group.
# 4. Delete all the ACLs in every Logical Switch (Neutron network).
# 3. Delete all the ACLs in every Logical Switch (Neutron network).

# If Port Groups are not supported or we've already migrated, return
if (not self.ovn_api.is_port_groups_supported() or
not self.ovn_api.get_address_sets()):
# If we've already migrated, return
if not self.ovn_api.get_address_sets():
return

LOG.debug('Port Groups Migration task started')
@@ -1226,7 +1047,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
utils.is_lsp_trusted(port) and
utils.is_port_security_enabled(port)]

self._create_default_drop_port_group(db_ports)
self._create_sg_port_groups_and_acls(ctx, db_ports)
self._delete_address_sets(ctx)
self._delete_acls_from_lswitches(ctx)


+ 0
- 69
networking_ovn/ovsdb/commands.py View File

@@ -673,25 +673,6 @@ class DelStaticRouteCommand(command.BaseCommand):
break


class AddAddrSetCommand(command.BaseCommand):
def __init__(self, api, name, may_exist, **columns):
super(AddAddrSetCommand, self).__init__(api)
self.name = name
self.columns = columns
self.may_exist = may_exist

def run_idl(self, txn):
if self.may_exist:
addrset = idlutils.row_by_value(self.api.idl, 'Address_Set',
'name', self.name, None)
if addrset:
return
row = txn.insert(self.api._tables['Address_Set'])
row.name = self.name
for col, val in self.columns.items():
setattr(row, col, val)


class DelAddrSetCommand(command.BaseCommand):
def __init__(self, api, name, if_exists):
super(DelAddrSetCommand, self).__init__(api)
@@ -712,56 +693,6 @@ class DelAddrSetCommand(command.BaseCommand):
self.api._tables['Address_Set'].rows[addrset.uuid].delete()


class UpdateAddrSetCommand(command.BaseCommand):
def __init__(self, api, name, addrs_add, addrs_remove, if_exists):
super(UpdateAddrSetCommand, self).__init__(api)
self.name = name
self.addrs_add = addrs_add
self.addrs_remove = addrs_remove
self.if_exists = if_exists

def run_idl(self, txn):
try:
addrset = idlutils.row_by_value(self.api.idl, 'Address_Set',
'name', self.name)
except idlutils.RowNotFound:
if self.if_exists:
return
msg = _("Address set %s does not exist. "
"Can't update addresses") % self.name
raise RuntimeError(msg)

_updatevalues_in_list(
addrset, 'addresses',
new_values=self.addrs_add,
old_values=self.addrs_remove)


class UpdateAddrSetExtIdsCommand(command.BaseCommand):
def __init__(self, api, name, external_ids, if_exists):
super(UpdateAddrSetExtIdsCommand, self).__init__(api)
self.name = name
self.external_ids = external_ids
self.if_exists = if_exists

def run_idl(self, txn):
try:
addrset = idlutils.row_by_value(self.api.idl, 'Address_Set',
'name', self.name)
except idlutils.RowNotFound:
if self.if_exists:
return
msg = _("Address set %s does not exist. "
"Can't update external IDs") % self.name
raise RuntimeError(msg)

addrset.verify('external_ids')
addrset_external_ids = getattr(addrset, 'external_ids', {})
for ext_id_key, ext_id_value in self.external_ids.items():
addrset_external_ids[ext_id_key] = ext_id_value
addrset.external_ids = addrset_external_ids


class UpdateChassisExtIdsCommand(command.BaseCommand):
def __init__(self, api, name, external_ids, if_exists):
super(UpdateChassisExtIdsCommand, self).__init__(api)


+ 0
- 23
networking_ovn/ovsdb/impl_idl_ovn.py View File

@@ -373,21 +373,9 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
return cmd.DelStaticRouteCommand(self, lrouter, ip_prefix, nexthop,
if_exists)

def create_address_set(self, name, may_exist=True, **columns):
return cmd.AddAddrSetCommand(self, name, may_exist, **columns)

def delete_address_set(self, name, if_exists=True, **columns):
return cmd.DelAddrSetCommand(self, name, if_exists)

def update_address_set(self, name, addrs_add, addrs_remove,
if_exists=True):
return cmd.UpdateAddrSetCommand(self, name, addrs_add, addrs_remove,
if_exists)

def update_address_set_ext_ids(self, name, external_ids, if_exists=True):
return cmd.UpdateAddrSetExtIdsCommand(self, name, external_ids,
if_exists)

def _get_logical_router_port_gateway_chassis(self, lrp):
"""Get the list of chassis hosting this gateway port.

@@ -657,14 +645,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
nat['external_ip'] == external_ip):
return nat

def get_address_set(self, addrset_id, ip_version='ip4'):
addr_name = utils.ovn_addrset_name(addrset_id, ip_version)
try:
return idlutils.row_by_value(self.idl, 'Address_Set',
'name', addr_name)
except idlutils.RowNotFound:
return None

def check_revision_number(self, name, resource, resource_type,
if_exists=True):
return cmd.CheckRevisionNumberCommand(
@@ -691,9 +671,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
def delete_lrouter_ext_gw(self, lrouter_name, if_exists=True):
return cmd.DeleteLRouterExtGwCommand(self, lrouter_name, if_exists)

def is_port_groups_supported(self):
return self.is_table_present('Port_Group')

def get_port_group(self, pg_name):
if uuidutils.is_uuid_like(pg_name):
pg_name = utils.ovn_port_group_name(pg_name)


+ 0
- 54
networking_ovn/ovsdb/ovn_api.py View File

@@ -255,20 +255,6 @@ class API(api.API):
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def create_address_set(self, name, may_exist=True, **columns):
"""Create an address set

:param name: The name of the address set
:type name: string
:param may_exist: Do not fail if address set already exists
:type may_exist: bool
:param columns: Dictionary of address set columns
Supported columns: external_ids, addresses
:type columns: dictionary
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def delete_address_set(self, name, if_exists=True):
"""Delete an address set
@@ -280,35 +266,6 @@ class API(api.API):
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def update_address_set(self, name, addrs_add, addrs_remove,
if_exists=True):
"""Updates addresses in an address set

:param name: The name of the address set
:type name: string
:param addrs_add: The addresses to be added
:type addrs_add: []
:param addrs_remove: The addresses to be removed
:type addrs_remove: []
:param if_exists: Do not fail if the address set does not exist
:type if_exists: bool
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def update_address_set_ext_ids(self, name, external_ids, if_exists=True):
"""Update external IDs for an address set

:param name: The name of the address set
:type name: string
:param external_ids: The external IDs for the address set
:type external_ids: dict
:param if_exists: Do not fail if the address set does not exist
:type if_exists: bool
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def get_all_chassis_gateway_bindings(self,
chassis_candidate_list=None):
@@ -577,17 +534,6 @@ class API(api.API):
:returns: :class:`Command` with no result
"""

@abc.abstractmethod
def get_address_set(self, addrset_id, ip_version='ip4'):
"""Get a Address Set by its ID.

:param addrset_id: The Address Set ID
:type addrset_id: string
:param ip_version: Either "ip4" or "ip6". Defaults to "ip4"
:type addr_name: string
:returns: The Address Set row or None
"""

@abc.abstractmethod
def set_lswitch_port_to_virtual_type(self, lport_name, vip,
virtual_parent, if_exists=True):


+ 3
- 9
networking_ovn/tests/functional/test_maintenance.py View File

@@ -177,15 +177,9 @@ class _TestMaintenanceHelper(base.TestOVNFunctionalBase):
return self.deserialize(self.fmt, res)['security_group']

def _find_security_group_row_by_id(self, sg_id):
if self.nb_api.is_port_groups_supported():
for row in self.nb_api._tables['Port_Group'].rows.values():
if row.name == utils.ovn_port_group_name(sg_id):
return row
else:
for row in self.nb_api._tables['Address_Set'].rows.values():
if (row.external_ids.get(
ovn_const.OVN_SG_EXT_ID_KEY) == sg_id):
return row
for row in self.nb_api._tables['Port_Group'].rows.values():
if row.name == utils.ovn_port_group_name(sg_id):
return row

def _create_security_group_rule(self, sg_id):
data = {'security_group_rule': {'security_group_id': sg_id,


+ 1
- 94
networking_ovn/tests/functional/test_ovn_db_resources.py View File

@@ -825,103 +825,10 @@ class TestPortSecurity(base.TestOVNFunctionalBase):
return port_acls

def _verify_port_acls(self, port_id, expected_acls):
if self.nb_api.is_port_groups_supported():
port_acls = self._get_port_related_acls(port_id)
else:
port_acls = self._get_port_related_acls_port_group_not_supported(
port_id)
port_acls = self._get_port_related_acls(port_id)
self.assertItemsEqual(expected_acls, port_acls)

@mock.patch('networking_ovn.ovsdb.impl_idl_ovn.OvsdbNbOvnIdl.'
'is_port_groups_supported', lambda *args: False)
def test_port_security_port_group_not_supported(self):
n1 = self._make_network(self.fmt, 'n1', True)
res = self._create_subnet(self.fmt, n1['network']['id'], '10.0.0.0/24')
subnet = self.deserialize(self.fmt, res)['subnet']
p = self._make_port(self.fmt, n1['network']['id'],
fixed_ips=[{'subnet_id': subnet['id']}])
port_id = p['port']['id']
sg_id = p['port']['security_groups'][0].replace('-', '_')
expected_acls_with_sg_ps_enabled = [
{'match': 'inport == "' + str(port_id) + '" && ip',
'action': 'drop',
'priority': 1001,
'direction': 'from-lport'},
{'match': 'outport == "' + str(port_id) + '" && ip',
'action': 'drop',
'priority': 1001,
'direction': 'to-lport'},
{'match': 'inport == "' + str(port_id) + '" && ip4 && ip4.dst == '
'{255.255.255.255, 10.0.0.0/24} && udp && udp.src == 68 '
'&& udp.dst == 67',
'action': 'allow',
'priority': 1002,
'direction': 'from-lport'},
{'match': 'inport == "' + str(port_id) + '" && ip6',
'action': 'allow-related',
'priority': 1002,
'direction': 'from-lport'},
{'match': 'inport == "' + str(port_id) + '" && ip4',
'action': 'allow-related',
'priority': 1002,
'direction': 'from-lport'},
{'match': 'outport == "' + str(port_id) + '" && ip4 && '
'ip4.src == $as_ip4_' + str(sg_id),
'action': 'allow-related',
'priority': 1002,
'direction': 'to-lport'},
{'match': 'outport == "' + str(port_id) + '" && ip6 && '
'ip6.src == $as_ip6_' + str(sg_id),
'action': 'allow-related',
'priority': 1002,
'direction': 'to-lport'},
]
self._verify_port_acls(port_id, expected_acls_with_sg_ps_enabled)

# clear the security groups.
data = {'port': {'security_groups': []}}
port_req = self.new_update_request('ports', data, p['port']['id'])
port_req.get_response(self.api)

# No security groups and port security enabled - > ACLs should be
# added to drop the packets.
expected_acls_with_no_sg_ps_enabled = [
{'match': 'inport == "' + str(port_id) + '" && ip',
'action': 'drop',
'priority': 1001,
'direction': 'from-lport'},
{'match': 'outport == "' + str(port_id) + '" && ip',
'action': 'drop',
'priority': 1001,
'direction': 'to-lport'},
]
self._verify_port_acls(port_id, expected_acls_with_no_sg_ps_enabled)

# Disable port security
data = {'port': {'port_security_enabled': False}}
port_req = self.new_update_request('ports', data, p['port']['id'])
port_req.get_response(self.api)
# No security groups and port security disabled - > No ACLs should be
# added (allowing all the traffic).
self._verify_port_acls(port_id, [])

# Enable port security again with no security groups - > ACLs should
# be added back to drop the packets.
data = {'port': {'port_security_enabled': True}}
port_req = self.new_update_request('ports', data, p['port']['id'])
port_req.get_response(self.api)
self._verify_port_acls(port_id, expected_acls_with_no_sg_ps_enabled)

# Set security groups back
data = {'port': {'security_groups': p['port']['security_groups']}}
port_req = self.new_update_request('ports', data, p['port']['id'])
port_req.get_response(self.api)
self._verify_port_acls(port_id, expected_acls_with_sg_ps_enabled)

def test_port_security_port_group(self):
if not self.nb_api.is_port_groups_supported():
self.skipTest('Port groups is not supported')

n1 = self._make_network(self.fmt, 'n1', True)
res = self._create_subnet(self.fmt, n1['network']['id'], '10.0.0.0/24')
subnet = self.deserialize(self.fmt, res)['subnet']


+ 22
- 125
networking_ovn/tests/functional/test_ovn_db_sync.py View File

@@ -12,8 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.

import mock

from neutron.services.segments import db as segments_db
from neutron.tests.unit.api import test_extensions
from neutron.tests.unit.extensions import test_extraroute
@@ -60,9 +58,6 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
self.delete_lrouter_routes = []
self.delete_lrouter_nats = []
self.delete_acls = []
self.create_address_sets = []
self.delete_address_sets = []
self.update_address_sets = []
self.create_port_groups = []
self.delete_port_groups = []
self.expected_dhcp_options_rows = []
@@ -581,17 +576,6 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
'neutron-' + r1['id']))
self.delete_lrouters.append('neutron-' + r2['id'])

address_set_name = n1_prtr['port']['security_groups'][0]
self.create_address_sets.extend([('fake_sg', 'ip4'),
('fake_sg', 'ip6')])
self.delete_address_sets.append((address_set_name, 'ip6'))
address_adds = ['10.0.0.101', '10.0.0.102']
address_dels = []
for address in n1_prtr['port']['fixed_ips']:
address_dels.append(address['ip_address'])
self.update_address_sets.append((address_set_name, 'ip4',
address_adds, address_dels))

self.create_port_groups.extend([{'name': 'pg1', 'acls': []},
{'name': 'pg2', 'acls': []}])
self.delete_port_groups.append(
@@ -762,26 +746,10 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
txn.add(self.nb_api.delete_acl(lswitch_name,
lport_name, True))

for name, ip_version in self.create_address_sets:
ovn_name = utils.ovn_addrset_name(name, ip_version)
external_ids = {ovn_const.OVN_SG_EXT_ID_KEY: name}
txn.add(self.nb_api.create_address_set(
ovn_name, True, external_ids=external_ids))

for name, ip_version in self.delete_address_sets:
ovn_name = utils.ovn_addrset_name(name, ip_version)
txn.add(self.nb_api.delete_address_set(ovn_name, True))

for name, ip_version, ip_adds, ip_dels in self.update_address_sets:
ovn_name = utils.ovn_addrset_name(name, ip_version)
txn.add(self.nb_api.update_address_set(ovn_name,
ip_adds, ip_dels, True))

if self.nb_api.is_port_groups_supported():
for pg in self.create_port_groups:
txn.add(self.nb_api.pg_add(**pg))
for pg in self.delete_port_groups:
txn.add(self.nb_api.pg_del(pg))
for pg in self.create_port_groups:
txn.add(self.nb_api.pg_add(**pg))
for pg in self.delete_port_groups:
txn.add(self.nb_api.pg_del(pg))

for lport_name in self.reset_lport_dhcpv4_options:
txn.add(self.nb_api.set_lswitch_port(lport_name, True,
@@ -1089,54 +1057,39 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
def _validate_acls(self, should_match=True):
# Get the neutron DB ACLs.
db_acls = []
sg_cache = {}
subnet_cache = {}

_plugin_nb_ovn = self.mech_driver._nb_ovn
if not _plugin_nb_ovn.is_port_groups_supported():
for db_port in self._list('ports')['ports']:
acls = acl_utils.add_acls(self.plugin,
context.get_admin_context(),
db_port,
sg_cache,
subnet_cache,
self.mech_driver._nb_ovn)
for acl in acls:
db_acls.append(acl_utils.filter_acl_dict(acl))
else:
# ACLs due to SGs and default drop port group
for sg in self._list('security-groups')['security_groups']:
for sgr in sg['security_group_rules']:
acl = acl_utils._add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(sg['id']), sgr,
self.mech_driver._nb_ovn)
db_acls.append(TestOvnNbSync._build_acl_for_pgs(**acl))

for acl in acl_utils.add_acls_for_drop_port_group(
ovn_const.OVN_DROP_PORT_GROUP_NAME):

# ACLs due to SGs and default drop port group
for sg in self._list('security-groups')['security_groups']:
for sgr in sg['security_group_rules']:
acl = acl_utils._add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(sg['id']), sgr)
db_acls.append(TestOvnNbSync._build_acl_for_pgs(**acl))

for acl in acl_utils.add_acls_for_drop_port_group(
ovn_const.OVN_DROP_PORT_GROUP_NAME):
db_acls.append(TestOvnNbSync._build_acl_for_pgs(**acl))

# Get the list of ACLs stored in the OVN plugin IDL.
plugin_acls = []
for row in _plugin_nb_ovn._tables['Logical_Switch'].rows.values():
for acl in getattr(row, 'acls', []):
plugin_acls.append(self._build_acl_to_compare(acl))
if self.nb_api.is_port_groups_supported():
for row in _plugin_nb_ovn._tables['Port_Group'].rows.values():
for acl in getattr(row, 'acls', []):
plugin_acls.append(
self._build_acl_to_compare(
acl, extra_fields=['external_ids']))
for row in _plugin_nb_ovn._tables['Port_Group'].rows.values():
for acl in getattr(row, 'acls', []):
plugin_acls.append(
self._build_acl_to_compare(
acl, extra_fields=['external_ids']))

# Get the list of ACLs stored in the OVN monitor IDL.
monitor_acls = []
for row in self.nb_api.tables['Logical_Switch'].rows.values():
for acl in getattr(row, 'acls', []):
monitor_acls.append(self._build_acl_to_compare(acl))
if _plugin_nb_ovn.is_port_groups_supported():
for row in self.nb_api.tables['Port_Group'].rows.values():
for acl in getattr(row, 'acls', []):
monitor_acls.append(self._build_acl_to_compare(acl))
for row in self.nb_api.tables['Port_Group'].rows.values():
for acl in getattr(row, 'acls', []):
monitor_acls.append(self._build_acl_to_compare(acl))

if should_match:
self.assertItemsEqual(db_acls, plugin_acls)
@@ -1358,57 +1311,8 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
AssertionError, self.assertItemsEqual, r_nats,
monitor_nats)

def _validate_address_sets(self, should_match=True):
_plugin_nb_ovn = self.mech_driver._nb_ovn
if _plugin_nb_ovn.is_port_groups_supported():
# If Port Groups are supported, no Address Sets are expected.
# This validation is still useful as we expect existing ones to
# be deleted after the sync.
db_sgs = []
else:
db_ports = self._list('ports')['ports']
sgs = self._list('security-groups')['security_groups']
db_sgs = {}
for sg in sgs:
for ip_version in ['ip4', 'ip6']:
name = utils.ovn_addrset_name(sg['id'], ip_version)
db_sgs[name] = []

for port in db_ports:
sg_ids = utils.get_lsp_security_groups(port)
addresses = acl_utils.acl_port_ips(port)
for sg_id in sg_ids:
for ip_version in addresses:
name = utils.ovn_addrset_name(sg_id, ip_version)
db_sgs[name].extend(addresses[ip_version])

nb_address_sets = _plugin_nb_ovn.get_address_sets()
nb_sgs = {}
for nb_sgid, nb_values in nb_address_sets.items():
nb_sgs[nb_sgid] = nb_values['addresses']

mn_sgs = {}
for row in self.nb_api.tables['Address_Set'].rows.values():
mn_sgs[getattr(row, 'name')] = getattr(row, 'addresses')

if should_match:
self.assertItemsEqual(nb_sgs, db_sgs)
self.assertItemsEqual(mn_sgs, db_sgs)
else:
# This condition is to cover the case when we use Port Groups
# and we completely deleted the NB DB. At this point, the expected
# number of Address Sets is 0 and the observed number in NB is
# also 0 so we can't have the asserts below as both will be empty.
if _plugin_nb_ovn.is_port_groups_supported() and nb_sgs:
self.assertRaises(AssertionError, self.assertItemsEqual,
nb_sgs, db_sgs)
self.assertRaises(AssertionError, self.assertItemsEqual,
mn_sgs, db_sgs)

def _validate_port_groups(self, should_match=True):
_plugin_nb_ovn = self.mech_driver._nb_ovn
if not _plugin_nb_ovn.is_port_groups_supported():
return

db_pgs = []
for sg in self._list('security-groups')['security_groups']:
@@ -1481,7 +1385,6 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
self._validate_dhcp_opts(should_match=should_match)
self._validate_acls(should_match=should_match)
self._validate_routers_and_router_ports(should_match=should_match)
self._validate_address_sets(should_match=should_match)
self._validate_port_groups(should_match=should_match)
self._validate_dns_records(should_match=should_match)

@@ -1635,9 +1538,3 @@ class TestOvnNbSyncOverSsl(TestOvnNbSync):
class TestOvnSbSyncOverSsl(TestOvnSbSync):
def get_ovsdb_server_protocol(self):
return 'ssl'


@mock.patch('networking_ovn.ovsdb.impl_idl_ovn.OvsdbNbOvnIdl.'
'is_port_groups_supported', lambda *args: False)
class TestOvnNbSyncNoPgs(TestOvnNbSync):
pass

+ 3
- 102
networking_ovn/tests/unit/common/test_acl.py View File

@@ -548,114 +548,15 @@ class TestACLs(base.TestCase):
ip_version = 'ip4'
sg_id = sg_rule['security_group_id']

addrset_name = ovn_utils.ovn_addrset_name(sg_id, ip_version)
pg_name = ovn_utils.ovn_pg_addrset_name(sg_id, ip_version)

match = ovn_acl.acl_remote_group_id(sg_rule, ip_version)
self.assertEqual('', match)

sg_rule['remote_group_id'] = sg_id
match = ovn_acl.acl_remote_group_id(sg_rule, ip_version)
self.assertEqual(' && ip4.src == $' + addrset_name, match)
self.assertEqual(' && ip4.src == $' + pg_name, match)

sg_rule['direction'] = 'egress'
match = ovn_acl.acl_remote_group_id(sg_rule, ip_version)
self.assertEqual(' && ip4.dst == $' + addrset_name, match)

def _test_update_acls_for_security_group(self, use_cache=True):
sg = fakes.FakeSecurityGroup.create_one_security_group().info()
remote_sg = fakes.FakeSecurityGroup.create_one_security_group().info()
sg_rule = fakes.FakeSecurityGroupRule.create_one_security_group_rule({
'security_group_id': sg['id'],
'remote_group_id': remote_sg['id']
}).info()
port = fakes.FakePort.create_one_port({
'security_groups': [sg['id']]
}).info()
self.plugin.get_ports.return_value = [port]
if use_cache:
sg_ports_cache = {sg['id']: [{'port_id': port['id']}],
remote_sg['id']: []}
else:
sg_ports_cache = None
self.plugin._get_port_security_group_bindings.return_value = \
[{'port_id': port['id']}]

# Build ACL for validation.
expected_acl = ovn_acl._add_sg_rule_acl_for_port(port, sg_rule)
expected_acl.pop('lport')
expected_acl.pop('lswitch')

# Validate ACLs when port has security groups.
ovn_acl.update_acls_for_security_group(self.plugin,
self.admin_context,
self.driver._nb_ovn,
sg['id'],
sg_rule,
sg_ports_cache=sg_ports_cache)
self.driver._nb_ovn.update_acls.assert_called_once_with(
[port['network_id']],
mock.ANY,
{port['id']: expected_acl},
need_compare=False,
is_add_acl=True
)

def test_update_acls_for_security_group_cache(self):
self._test_update_acls_for_security_group(use_cache=True)

def test_update_acls_for_security_group_no_cache(self):
self._test_update_acls_for_security_group(use_cache=False)

def test_acl_port_ips(self):
port4 = fakes.FakePort.create_one_port({
'fixed_ips': [{'subnet_id': 'subnet-ipv4',
'ip_address': '10.0.0.1'}],
}).info()
port46 = fakes.FakePort.create_one_port({
'fixed_ips': [{'subnet_id': 'subnet-ipv4',
'ip_address': '10.0.0.2'},
{'subnet_id': 'subnet-ipv6',
'ip_address': 'fde3:d45:df72::1'}],
}).info()
port6 = fakes.FakePort.create_one_port({
'fixed_ips': [{'subnet_id': 'subnet-ipv6',
'ip_address': '2001:db8::8'}],
}).info()

addresses = ovn_acl.acl_port_ips(port4)
self.assertEqual({'ip4': [port4['fixed_ips'][0]['ip_address']],
'ip6': []},
addresses)

addresses = ovn_acl.acl_port_ips(port46)
self.assertEqual({'ip4': [port46['fixed_ips'][0]['ip_address']],
'ip6': [port46['fixed_ips'][1]['ip_address']]},
addresses)

addresses = ovn_acl.acl_port_ips(port6)
self.assertEqual({'ip4': [],
'ip6': [port6['fixed_ips'][0]['ip_address']]},
addresses)

def test_sg_disabled(self):
sg = fakes.FakeSecurityGroup.create_one_security_group().info()
port = fakes.FakePort.create_one_port({
'security_groups': [sg['id']]
}).info()

with mock.patch('networking_ovn.common.acl.is_sg_enabled',
return_value=False):
acl_list = ovn_acl.add_acls(self.plugin,
self.admin_context,
port, {}, {}, self.driver._ovn)
self.assertEqual([], acl_list)

ovn_acl.update_acls_for_security_group(self.plugin,
self.admin_context,
self.driver._ovn,
sg['id'],
None)
self.driver._ovn.update_acls.assert_not_called()

addresses = ovn_acl.acl_port_ips(port)
self.assertEqual({'ip4': [], 'ip6': []}, addresses)
self.assertEqual(' && ip4.dst == $' + pg_name, match)

+ 7
- 19
networking_ovn/tests/unit/common/test_maintenance.py View File

@@ -101,10 +101,8 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
self.periodic.check_for_inconsistencies()
mock_fix_net.assert_called_once_with(fake_row)

def _test_migrate_to_port_groups_helper(self, pg_supported, a_sets,
migration_expected, never_again):
self.fake_ovn_client._nb_idl.is_port_groups_supported.return_value = (
pg_supported)
def _test_migrate_to_port_groups_helper(self, a_sets, migration_expected,
never_again):
self.fake_ovn_client._nb_idl.get_address_sets.return_value = a_sets
with mock.patch.object(ovn_db_sync.OvnNbSynchronizer,
'migrate_to_port_groups') as mtpg:
@@ -119,24 +117,15 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
else:
mtpg.assert_not_called()

def test_migrate_to_port_groups_port_groups_not_supported(self):
self._test_migrate_to_port_groups_helper(pg_supported=False,
a_sets=None,
migration_expected=False,
never_again=True)

def test_migrate_to_port_groups_not_needed(self):
self._test_migrate_to_port_groups_helper(pg_supported=True,
a_sets=None,
self._test_migrate_to_port_groups_helper(a_sets=None,
migration_expected=False,
never_again=True)

def test_migrate_to_port_groups(self):
# Check normal migration path: if port groups are supported by the
# schema and the migration has to be done, it will take place and
# won't be attempted in the future.
self._test_migrate_to_port_groups_helper(pg_supported=True,
a_sets=['as1', 'as2'],
# Check normal migration path: if the migration has to be done, it will
# take place and won't be attempted in the future.
self._test_migrate_to_port_groups_helper(a_sets=['as1', 'as2'],
migration_expected=True,
never_again=True)

@@ -146,8 +135,7 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
return_value=False)):
# Check that if this worker doesn't have the lock, it won't
# perform the migration and it will try again later.
self._test_migrate_to_port_groups_helper(pg_supported=True,
a_sets=['as1', 'as2'],
self._test_migrate_to_port_groups_helper(a_sets=['as1', 'as2'],
migration_expected=False,
never_again=False)



+ 0
- 10
networking_ovn/tests/unit/fakes.py View File

@@ -71,10 +71,6 @@ class FakeOvsdbNbOvnIdl(object):
self.idl = mock.Mock()
self.add_static_route = mock.Mock()
self.delete_static_route = mock.Mock()
self.create_address_set = mock.Mock()
self.update_address_set_ext_ids = mock.Mock()
self.delete_address_set = mock.Mock()
self.update_address_set = mock.Mock()
self.get_all_chassis_gateway_bindings = mock.Mock()
self.get_chassis_gateways = mock.Mock()
self.get_gateway_chassis_binding = mock.Mock()
@@ -123,12 +119,6 @@ class FakeOvsdbNbOvnIdl(object):
self.get_lrouter.return_value = None
self.delete_lrouter_ext_gw = mock.Mock()
self.delete_lrouter_ext_gw.return_value = None
self.is_port_groups_supported = mock.Mock()
# TODO(lucasagomes): Flip this return value to True at some point,
# port groups should be the default method used by networking-ovn
self.is_port_groups_supported.return_value = False
self.get_address_set = mock.Mock()
self.get_address_set.return_value = None
self.pg_acl_add = mock.Mock()
self.pg_acl_del = mock.Mock()
self.pg_del = mock.Mock()


+ 88
- 233
networking_ovn/tests/unit/ml2/test_mech_driver.py View File

@@ -46,7 +46,6 @@ from oslo_utils import timeutils
from oslo_utils import uuidutils
from webob import exc

from networking_ovn.common import acl as ovn_acl
from networking_ovn.common import config as ovn_config
from networking_ovn.common import constants as ovn_const
from networking_ovn.common import ovn_client
@@ -114,28 +113,25 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
resources.SECURITY_GROUP, events.AFTER_CREATE, {},
security_group=self.fake_sg)
external_ids = {ovn_const.OVN_SG_EXT_ID_KEY: self.fake_sg['id']}
ip4_name = ovn_utils.ovn_addrset_name(self.fake_sg['id'], 'ip4')
ip6_name = ovn_utils.ovn_addrset_name(self.fake_sg['id'], 'ip6')
create_address_set_calls = [mock.call(name=name,
external_ids=external_ids)
for name in [ip4_name, ip6_name]]

self.nb_ovn.create_address_set.assert_has_calls(
create_address_set_calls, any_order=True)
pg_name = ovn_utils.ovn_port_group_name(self.fake_sg['id'])

self.nb_ovn.pg_add.assert_called_once_with(
name=pg_name, acls=[], external_ids=external_ids)

mock_bump.assert_called_once_with(
self.fake_sg, ovn_const.TYPE_SECURITY_GROUPS)

def test__delete_security_group(self):
@mock.patch.object(db_rev, 'delete_revision')
def test__delete_security_group(self, mock_del_rev):
self.mech_driver._delete_security_group(
resources.SECURITY_GROUP, events.AFTER_CREATE, {},
security_group_id=self.fake_sg['id'])
ip4_name = ovn_utils.ovn_addrset_name(self.fake_sg['id'], 'ip4')
ip6_name = ovn_utils.ovn_addrset_name(self.fake_sg['id'], 'ip6')
delete_address_set_calls = [mock.call(name=name)
for name in [ip4_name, ip6_name]]
pg_name = ovn_utils.ovn_port_group_name(self.fake_sg['id'])

self.nb_ovn.delete_address_set.assert_has_calls(
delete_address_set_calls, any_order=True)
self.nb_ovn.pg_del.assert_called_once_with(name=pg_name)

mock_del_rev.assert_called_once_with(
self.fake_sg['id'], ovn_const.TYPE_SECURITY_GROUPS)

@mock.patch.object(db_rev, 'bump_revision')
def test__process_sg_rule_notifications_sgr_create(self, mock_bump):
@@ -172,91 +168,6 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
mock_delrev.assert_called_once_with(
rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES)

def test_add_acls_no_sec_group(self):
fake_port_no_sg = fakes.FakePort.create_one_port().info()
expected_acls = ovn_acl.drop_all_ip_traffic_for_port(fake_port_no_sg)
acls = ovn_acl.add_acls(self.mech_driver._plugin,
mock.Mock(),
fake_port_no_sg,
{}, {}, self.mech_driver._nb_ovn)
self.assertEqual(expected_acls, acls)

def test_add_acls_no_sec_group_no_port_security(self):
fake_port_no_sg_no_ps = fakes.FakePort.create_one_port(
attrs={'port_security_enabled': False}).info()
acls = ovn_acl.add_acls(self.mech_driver._plugin,
mock.Mock(),
fake_port_no_sg_no_ps,
{}, {}, self.mech_driver._nb_ovn)
self.assertEqual([], acls)

def _test_add_acls_with_sec_group_helper(self, native_dhcp=True):
fake_port_sg = fakes.FakePort.create_one_port(
attrs={'security_groups': [self.fake_sg['id']],
'fixed_ips': [{'subnet_id': self.fake_subnet['id'],