Commit Graph

470 Commits

Author SHA1 Message Date
Brian Haley bcf33e202f Fix keyword-arg-before-vararg warnings
The correct usage when using keyword and varargs is

def func(arg1, *args, arg2=None, **kwargs):

Otherwise you can end up in having multiple values passed
for the parameters in case the method is called with
keyword arguments.

Fixed two calls to _get_ports_query() that were not
passing the 'filters' argument correctly.

Start enforcing this as well.

TrivialFix

Change-Id: Id9d6d841133241bbc87a589117468c4e699c310a
2024-02-19 16:19:57 -05:00
Zuul 2d74a93d68 Merge "Disallow subnet cidr of :: without PD" 2024-02-13 21:48:00 +00:00
Rodolfo Alonso Hernandez f9e40971e9 Forbid the subnet gateway IP deletion if a router interface is attached
When a router interface is created, the corresponding subnet gateway IP
is tested first [1]. If the subnet has no gateway IP, the router
interface cannot be created. This IP will be assigned to this port.

The Neutron API also prevents from modifying the subnet gateway IP
if assigned to a router interface [2]. However the API is not
preventing the subnet gateway IP deletion. This patch is adding
this check.

This patch is being tested in the neutron-tempest-plugin [3].

[1]de58c1b995/neutron/db/l3_db.py (L902-L904)
[2]de58c1b995/neutron/db/db_base_plugin_v2.py (L715)
[3]https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/904710

Closes-Bug: #2036423
Change-Id: I4c7b399a3a052749abdb88fb50be628ee91b63a0
2024-01-17 13:33:41 +00:00
Brian Haley 2f00111940 Disallow subnet cidr of :: without PD
Do not allow the subnet cidr of :: to be used when
creating a subnet, except in the case IPv6 prefix
delegation has been specified in the request.

Closes-bug: #2028159
Change-Id: I480e9a117513996f3c070acd4ba39c2b9fe9c0f1
2024-01-08 17:06:49 -05:00
Brian Haley 2f91d330da Correctly validate subnet arguments when using a subnetpool
When creating a subnet using a subnetpool, we were
failing to validate all the passed API arguments in
the dictionary, leading to a case where you could
specify an invalid DNS nameserver. For example,
using an IPv4 nameserver on an IPv6 subnet. This
could cause daemons the l3-agent starts, like radvd,
to fail to start correctly, leading to a loss of
connectivity.

Specifying a subnet by cidr without a subnetpool
did already correctly fail with an IP version
mismatch error, this is just an edge case that
was never tested.

Since _validate_subnet() was called in so many places
it was moved to a common location and is only not
called for IPv6 prefix-delegation subnets.

Closes-bug: #2036877
Change-Id: I6302e9a373cf93e706cec10f87c3beaf632a0391
2023-11-15 17:01:04 -05:00
Zuul a26e957d34 Merge "Use explicit inner join for networks in port query" 2023-05-17 19:23:02 +00:00
Zuul 272a315109 Merge "Mark "ipv6_pd_enabled" as deprecated and experimental." 2023-05-10 16:10:11 +00:00
Rodolfo Alonso Hernandez 3e65ef863c Mark "ipv6_pd_enabled" as deprecated and experimental.
This functionality will be kept in the code as experimental as long
as no bugs are reported againts this feature.

This patch also marks the config option "ipv6_pd_enabled" as
experimental. In order to enable this flag, it is needed to configure
the "experimental.ipv6_pd_enabled" flag too.

Related-Bug: #1916428
Change-Id: I27aeed74f308d5bdf0210e76d9557f95b66c71bf
2023-05-09 11:06:24 +00:00
Zuul 9319ba00a9 Merge "rbacs: clean-up to use defined constants ACCESS_*" 2023-05-08 23:59:01 +00:00
Zuul d893368356 Merge "Reduce lock contention on subnets" 2023-05-05 20:12:21 +00:00
Felix Huettner c0af5b3b5e Reduce lock contention on subnets
in [1] a lock was introduced with the goal of preventing subnets from
being deleted while ports are being created in them in parallel.
This was acheived by aquiring an exclusive lock on the row of the
subnet in the Subnet table when adding/modifying a port or deleting
the subnet.

However as this was a exclusive lock it also prevented concurrent port
modifications on the same subnet from happening. This can cause
performance issues on environment with large shared subnets (e.g. a
large external subnet).

To reduce the lock contention for this case we split the lock in two
parts:

* For normal port operations we will aquire a shared lock on the
  row of the subnet. This allows multiple such operations to happen in
  parallel.
* For deleting a subnet we will aquire an exclusive lock on the row of
  the subnet. This lock can not be aquired when there is any shared
  lock currently on the row.

With this we maintain the same locking level as before, but reduce the
amount of lock contention happening and thereby improve throughput.

The performance improvement can be measured using rally test [2].
(improving from 21 to 18 seconds).
Alternatively it can be tested using 250 parallel curl calls to create a
port in the same network. This improves from 113s to 42s.

[1]: https://review.opendev.org/c/openstack/neutron/+/713045
[2]: https://github.com/openstack/rally-openstack/blob/master/samples/tasks/scenarios/neutron/create-and-delete-ports.json

Closes-Bug: #2009055
Change-Id: I31b1a9c2f986f59fee0da265acebbd88d2f8e4f8
2023-05-05 17:01:43 +02:00
Sahid Orentino Ferdjaoui 256297fc7f rbacs: clean-up to use defined constants ACCESS_*
Some files are using strings access_as_shared or access_as_external
instead of using defined constants ACCESS_SHARED and ACCESS_EXTERNAL.

This commit is doing the cleaning it does not bring any functional
change.

Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com>
Change-Id: Ib75326c762776c5259740cb2f0abc1163842f95d
2023-05-05 16:08:20 +02:00
Brian Haley 88ce859b56 Change API to validate network MTU minimums
A network's MTU is now only valid if it is the minimum value
allowed based on the IP version of the associated subnets,
68 for IPv4 and 1280 for IPv6.

This minimum is now enforced in the following ways:

1) When a subnet is associated with a network, validate
   the MTU is large enough for the IP version. Not only
   would the subnet be unusable if it was allowed, but the
   Linux kernel can fail adding addresses and configuring
   network settings like the MTU.

2) When a network MTU is changed, validate the MTU is large
   enough for any currently associated subnets. Allowing a
   smaller MTU would render any existing subnets unusable.

Closes-bug: #1988069
Change-Id: Ia4017a8737f9a7c63945df546c8a7243b2673ceb
2023-04-26 12:22:30 -04:00
Ihtisham ul Haq f23d7af8d7 Use explicit inner join for networks in port query
This improves the performance of the database when fetching a list of ports
for a project user. This change creates an inner join with the networks
belonging to the ports.

Previous SQL query:
SELECT ports ...
FROM network, ports ...
WHERE ports.project_id = <project>
OR ports.network_id = networks.id
AND networks.project_id = <project>

Current SQL query:
SELECT ports ...
FROM ports
INNER JOIN networks ON networks.id = ports.network_id
WHERE ports.project_id = <project>
OR networks.project_id = <project>

Closes-Bug: #2016704
Change-Id: I9c49a307956ecfbf8bd2e866cefb21a212c38bd6
2023-04-20 12:47:31 +02:00
Thomas Bachman f759915ab0 Fix default value for MTUs, when not provided
When networks are created using REST APIs, if the MTU isn't specified
in the request, then a default value of 0 is used. Some use cases, such
as the auto-allocated-topology workflow, call the plugin directly to
create networks, bypassing the layer that inserts this default value.
Commit 68625686a4 introduced a different
default value at the DB layer, defined by a constant in neutron-lib.
If the maximum MTU size has been configured lower than this constant,
then the user receives an exception, even though they didn't provide
a value for MTU.

This patch changes the default value used in the DB layer, so that
it's consistent with the workflow seen via REST APIs.

Change-Id: Ica21e891cd2559942abb0ab2b12132e7f6cdd835
Closes-Bug: #1896933
2023-01-05 10:54:04 +00:00
Brian Haley 55b16d7b7c Fix some pylint indentation warnings
Running with a stricter .pylintrc generates a lot of
C0330 warnings (hanging/continued indentation). Fix
the ones in neutron/db.

Trivialfix

Change-Id: I9311cfe5efc51552008072d84aa238e5d0c9de60
2022-11-03 19:50:54 -04:00
Nurmatov Mamatisa 655001594b Use neutron-lib method is_session_active
In patch [1] temporary was added is_session_active
method before n-lib patch [2] release. Now modified to
n-lib method

1) https://review.opendev.org/c/openstack/neutron/+/828739
2) https://review.opendev.org/c/openstack/neutron-lib/+/828738

Change-Id: I1144215b72f7c435e1949b2d66f8bbb268b08c98
2022-08-11 05:58:44 +02:00
Zuul 430abde13e Merge "Add the corresponding DB context to all SQL transactions" 2022-04-08 13:08:32 +00:00
Rodolfo Alonso Hernandez eeb918e1b9 Add the corresponding DB context to all SQL transactions
The goal of this patch is to make the Neutron code compliant
with SQLAlchemy 2.0.

All SQL transactions must be executed inside an explicit
writer/reader context. SQLAlchemy no longer will create an
implicit transaction if the session has no active transaction.

A warning message, only available in debug mode, is added. When
an ORM session calls "do_orm_execute", if there is no active
transaction, a warning message with a traceback will be logged
to help to debug the regression introduced.

Related-Bug: #1964575

Change-Id: I3da37fee205b8d67d10673075b9130147d9eab5f
2022-04-08 09:09:54 +00:00
Rodolfo Alonso Hernandez 83b6ce9e9e Remove exception ``IpAddressAllocationNotFound``
This patch removes the ``IpAddressAllocationNotFound`` exception. This
exception was raised when a IPAM register was called to be deleted
but not found.

As reported in the LP bug, this IPAM register deletion can be called
several times if a port fails during the creation. The IPAM register
deletion calls the DB deletion but doesn't raise any exception if the
register does not exist. The code ensures the IPAM register is
deleted and there is no need to fail if it is not present anymore.

This patch also removes the exception catch and try in "update_port",
that was added in [0] as a fix for [1]. That was added because the
subnet deletion code involved a port update call [2] during the
IP allocation deletion, if any port was still present in the subnet.
Since [3], this code is not needed because the subnet deletion does
not call a port update anymore.

[0]https://review.opendev.org/c/openstack/neutron/+/373536
[1]https://bugs.launchpad.net/neutron/+bug/1622616
[2]https://github.com/openstack/neutron/blob/pike-em/neutron/db/db_base_plugin_v2.py#L1017-L1018
[3]https://review.opendev.org/c/openstack/neutron/+/713045

Closes-Bug: #1965807
Related-Bug: #1954763
Related-Bug: #1622616

Change-Id: I5b96b3a91aacffe118ddbb91a75c4892818ba97a
2022-03-16 16:48:06 +00:00
Rodolfo Alonso Hernandez 9829865073 Refactor session "is_active" handling for sqlalchemy-20
Since sqlalchemy 1.4, "session.autocommit" is False by default; in
sqlalchemy 2.0 this will be the only value accepted.

The ``_orm.Session`` is considered active when [1]:
- there is a transaction and this transaction is active
- there is no transaction [2], the class ``_orm.Session`` will
   autobegin when it is first used.

The second one breaks the way Neutron considers a session is active:
only when a transaction is in place, Neutron considers a session is
active.

[1]https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4/lib/sqlalchemy/orm/session.py#L3918-L3950
[2]https://github.com/sqlalchemy/sqlalchemy/blob/rel_1_4/lib/sqlalchemy/orm/session.py#L3930-L3932

Partial-Bug: #1962153
Topic: sqlalchemy-20

Change-Id: Iabaee4e556afb3dc75a82d99dc4a597fe4d7dd21
2022-02-10 09:03:36 +00:00
Zuul e7b70521d0 Merge "Use elevated context to update router's external gateway" 2022-01-31 20:43:19 +00:00
Slawek Kaplonski 0fd168ae93 Use elevated context to update router's external gateway
When new defaults and scope enforcement are used in Neutron, elevated
context needs to be used to update router's external gateway when new
subnet is added to the external network. Otherwise, router can belongs
to the other project than external network and it will not be updated at
all.

Closes-Bug: #1959331
Change-Id: I888ddf66a15cd20039ff26baccd170da128e1eb7
2022-01-28 09:36:22 +01:00
Rodolfo Alonso Hernandez 08bdc4ded1 Add "bound_drivers" information to port "vif_details"
This new parameter "bound_drivers" is a dictionary with the binding
levels and the driver name. E.g.:
  port['vif_details']['bound_drivers'] = {'0': 'openvswitch'}
  port['vif_details']['bound_drivers'] = {'0': 'ovn'}

If the port is not bound, this key won't be present in "vif_details".
This information is important for Nova, along with the VIF type, to
adequate the port plugin strategy or to know what kind of plugin
events are expected; currently, depending on the driver and the
connection type, Neutron sends a different set of vif-plugged events.
This is specially critical during live migration process, where the
network communication should be halted as little as possible.

Related-Bug: #1821058

Change-Id: I1c42fa4f44cc2311e874b2b9bf2bd40ffd142e91
2022-01-27 04:16:45 +00:00
Zuul 3bd68be547 Merge "Don't fail subnet validation if gw_ip is actually not changed" 2021-12-21 13:32:36 +00:00
Slawek Kaplonski 6809bed632 Don't fail subnet validation if gw_ip is actually not changed
In subnet update API call Neutron checks if gateway_ip was send to be
updated and if so, it checkes if old gateway_ip isn't already allocated
to some router port. If it's already used, Neutron returns 409 response.
This is valid behaviour but sometimes, some automation tools may do
subnet update request and pass the same gateway ip as already used by
the subnet. In such case, as gateway_ip is actually not changed Neutron
should not raise exception in that validation.

Closes-Bug: #1955121
Change-Id: Iba90b44331fdc63273fd3d19c583a24b5295c0ac
2021-12-20 10:08:37 +01:00
yatinkarel 7b61adbb4a List ports when attempt to delete network with ports
When there is attempt to delete network with ports,
a general error message is displayed that one or more
ports are in use on the network. This patch proposes
to also return the ports which are in use as part of
the message.

Also modify test_delete_network_if_port_exists unit
test to check for port id and network id in Error
message.

Also bump required version of neutron-lib to 2.18.0
as that's needed for custom message in NetworkInUse
Exception.

Depends-On: https://review.opendev.org/c/openstack/neutron-lib/+/821806

Closes-Bug: #1953716
Change-Id: Ib0b40402746c6a487a226b238907142384608d3c
2021-12-16 16:31:44 +05:30
Rodolfo Alonso Hernandez 8813b0ed2d Replace "target_tenant" with "target_project" in RBAC OVOs and models
This is part of the remaining technical debt of the specs
https://specs.openstack.org/openstack/neutron-specs/specs/newton/moving-to-keystone-v3.html

Blueprint: https://blueprints.launchpad.net/neutron/+spec/keystone-v3

Change-Id: I2d2fd4d1802c9dfe0778ac8fdddc7b9a8afe7d25
2021-12-03 10:48:57 +00:00
Rodolfo Alonso Hernandez d4d90fb6d7 Improve "get_collection_count" calls
Reduce the object retrieval to one single field to improve the
collection count.

Bumped neutron-lib to 2.16.0. This version contains [1], needed for
this patch.

[1]https://review.opendev.org/c/openstack/neutron-lib/+/807686

Related-Bug: #1942863

Change-Id: I160e8084e97b23a2bacb49ceb40efbac2d0715be
2021-10-18 07:56:13 +00:00
Rodolfo Alonso Hernandez 7dcddeb0bd Replace "tenant_id" with "project_id" in Quota engine
This is part of the remaining technical debt of the specs
https://specs.openstack.org/openstack/neutron-specs/specs/newton/moving-to-keystone-v3.html

Blueprint: https://blueprints.launchpad.net/neutron/+spec/keystone-v3

Change-Id: I1faf520d3cdafe2de873525c8ebe1fa2114bdcd7
2021-09-22 08:27:10 +00:00
Zuul 0411743bb9 Merge "Sanitize MAC addresses" 2021-07-28 14:27:03 +00:00
Zuul 4481b9b8db Merge "Improve Port list and show" 2021-07-27 21:10:39 +00:00
Rodolfo Alonso Hernandez 923284fc37 Use explicit select condition in SQL query in "_port_filter_hook"
Instead of executing a subquery inside a select, use a proper filter
condition instead.

Closes-Bug: #1936675
Change-Id: I97e9ca244c0716510fcd4ec81d54046be9c5f8f8
2021-07-19 13:27:41 +00:00
Rodolfo Alonso Hernandez 827cca2ed7 Sanitize MAC addresses
This patch sanitizes the MAC address coming from a user input:
- The "base_mac" address configuration parameter.
- The "port.mac_address" stored in the database, if the script
  provided is not executed.

This patch relays on [1], that will sanitize any input coming from
the server API.

This patch adds a new script to sanitize all "port.mac_address"
registers stored in the dabatabase.

[1]https://review.opendev.org/c/openstack/neutron-lib/+/788300

Related-Bug: #1926273

Change-Id: I8572906cc435feda1f82263fd94dda47fc1526e1
2021-07-08 16:46:55 +00:00
Oleg Bondarev 0af8497076 Improve Port list and show
Don't eagerly load several fields which are not used in
port list and show.

Port list and show speed-up is ~10-25%.

Depends on neutron-lib change:
https://review.opendev.org/c/openstack/neutron-lib/+/788729

Change-Id: I4bfcac16a54b766e0dc97f58ba4048eb709fa88f
2021-06-24 06:19:07 +00:00
Nurmatov Mamatisa cd8c4f7e30 use callback payloads for SUBNET
This patch switches over to callback payloads for
SUBNET events.

Change-Id: Ic4c3490aed4f899293be993d4663bb537c34ab8b
2021-06-24 00:14:52 +03:00
Slawek Kaplonski d7371e13e4 Revert "Set system_scope='all' in elevated context"
This reverts commit 062336e59b.

Now, we have proper fix for the system_scope='all' in elevated context
in the neutron-lib so we can revert temporary fix made at the end of the
Wallaby cycle.

Related-Bug: #1920001

Conflicts:
    neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py
    neutron/common/utils.py
    neutron/db/address_group_db.py
    neutron/services/segments/db.py

Change-Id: Ife9b647b403bdd76a8a99984ea8858bf95c96bc3
2021-06-15 10:29:20 +02:00
Zuul 0fb97c909e Merge "Improve Subnet create performance" 2021-06-05 04:06:07 +00:00
Oleg Bondarev f52280287f Improve Subnet create performance
- pass network dict from ml2 plugin to _create_subnet_postcommit
- skip ipam subnet fetch for non ipv6 auto-address subnets
- don't count subnets (DB request) if subnet has no segment

Change-Id: Iaecfda2700c5316cb25a93496d24ece366e40a4a
2021-05-21 15:03:52 +00:00
Oleg Bondarev bdd50ffcde Improve Subnet delete performance
- don't re-fetch subnet object from DB
- network is not used in SubnetContext when deleting subnet

Above gives ~35% improvement

Change-Id: I34f850782092f771482a297ae1e68a63ffb027c1
2021-05-21 15:03:38 +00:00
Nurmatov Mamatisa 4aa5de254d use payloads for NETWORK callback events
This patch switches over to the payload style of callbacks for
NETWORK based events. As part of this change a few shims are needed
to handle cases where some callbacks don't yet use payloads and others
do. Once we move over to payloads for all callbacks the shims can be
removed.

NeutronLibImpact

Change-Id: I889364b5d184d47a79fe6ed604ce13a4b334acfa
2021-05-08 20:50:46 +03:00
Zuul 6293a7325c Merge "Improve Network delete performance" 2021-05-04 22:58:53 +00:00
Zuul c170d86ff9 Merge "Improve Subnet update performance" 2021-04-30 16:02:26 +00:00
Zuul d2bb0ec0a5 Merge "Improve port delete performance" 2021-04-22 21:01:22 +00:00
Oleg Bondarev e8909a65d4 Improve Subnet update performance
DB plugin gets subnet object and this obj could be passed to IPAM
to save 2 subnet DB requests.

Change-Id: I3b3fe0ae8d567a8bd15c11f5f4bfa9651d302bfd
2021-04-20 14:21:51 +03:00
Oleg Bondarev 80eddc4039 Improve Network delete performance
- pass existing network dict to PRECOMMIT_DELETE ml2 handler;
- skip MTU and segment update handling in case network is
  about to be deleted.

Above gives up to 50% net delete speed-up

Change-Id: I07c70db027f2ae03ffb5a95072e019e8a5fdc411
2021-04-15 13:06:00 +03:00
Oleg Bondarev 02f4eca1ae Pass existing DB obj to save DB requests
On port and network update ML2 plugin gets a db object
and can pass it to db_base_plugin_v2 when doing super call.

Change-Id: I1213f59fe643807f303e3ad7f24925fa333a5512
2021-04-14 12:50:28 +03:00
Oleg Bondarev b221a4af33 Improve port delete performance
Pass existing port dict to IPAM delete and l3 callback handler
to save 2 port DB requests.
This gives ~10-15% improvement.

Change-Id: Iab5ed8a57aaf68f626e5b386f72d941ef7b22227
2021-04-12 19:05:55 +03:00
Zuul f3c78b8a12 Merge "Set system_scope='all' in elevated context" 2021-03-23 14:25:18 +00:00
Slawek Kaplonski 062336e59b Set system_scope='all' in elevated context
In case when enforce_new_defaults is set to True and new policy rules
are used, context.is_admin flag isn't really working as it was with old
rules.
But in case when elevated context is needed, it means that we need
context which has full rights to the system. So we should also set
"system_scope" parameter to "all" to be sure that system scope queries
can be done with such elevated context always.

It is needed e.g. when elevated context is used to get some data from
db. In such case we need to have db query which will not be scoped to
the single project_id and with new defaults to achieve that system_scope
has to be set to "all".

Proper fix for that should be done in neutron-lib and it is proposed
in [1] already but as we are have frozen neutron-lib version for
stable/wallaby already this patch for neutron is temporary fix for that
issue.
We can revert that patch as soon as we will be in Xena development cycle
and [1] will be merged and released.

[1] https://review.opendev.org/c/openstack/neutron-lib/+/781625

Related-Bug: #1920001
Change-Id: I0068c1de09f5c6fae5bb5cd0d6f26f451e701939
2021-03-19 12:05:56 +01:00