From c48fb5a7b73f2698f0827c13edff6ec8f6e808f5 Mon Sep 17 00:00:00 2001 From: Doug Goldstein Date: Fri, 9 Oct 2026 13:28:00 -0500 Subject: [PATCH] fix(neutron-understack): select the ACTIVE port binding, not bindings[0] Indexing port.bindings[0] assumes the binding order and picks the wrong binding for a port with an INACTIVE binding alongside the ACTIVE one, as happens during live migration. Add utils.active_port_binding(), which wraps neutron-lib's get_port_binding_by_status_and_host(), and use it in parent_port_is_bound(), the trunk code and the undersync driver. A port with no active binding is now reported as not bound. Document the dict, OVO and SQLAlchemy model port representations and how to select a binding from each in the Neutron networking design guide. --- docs/design-guide/neutron-networking.md | 43 +++++++++++++++++++ .../neutron_understack/trunk.py | 8 ++-- .../neutron_understack/undersync_mech.py | 8 ++-- .../neutron_understack/utils.py | 10 ++++- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/docs/design-guide/neutron-networking.md b/docs/design-guide/neutron-networking.md index 99f10cd3f..49e605103 100644 --- a/docs/design-guide/neutron-networking.md +++ b/docs/design-guide/neutron-networking.md @@ -499,6 +499,49 @@ Neutron based on certain data provided to port creation and update API calls. - the port has a binding host - the port is either unbound or has previously failed to bind +### Port Representations + +Neutron exposes a port in three different shapes and you will see all of them +in the Neutron source and in our drivers: + +| Representation | How you get it | Notes | +| -------------- | -------------- | ----- | +| Dict | `core_plugin.get_port()`, `PortContext.current` | API-shaped. Binding fields appear flattened as `binding:vif_type`, `binding:vnic_type`, `binding:host_id`, etc. | +| OVO | `neutron.objects.ports.Port.get_object()` | Typed versioned object. Bindings are in `port.bindings`, binding levels in `port.binding_levels`. | +| SQLAlchemy model | `payload.metadata["port_db"]`, `ml2.db.get_port()` | ML2 internal. Bindings are in `port.port_bindings`. Don't go looking for these; use them when Neutron hands one to you. | + +Use the representation that matches where the port came from, and don't +re-fetch a port just to change its shape: + +- If Neutron hands you a dict (`PortContext.current`) or a SQLAlchemy model + (for example `payload.metadata["port_db"]` in a registry callback), work with + that directly. +- When we have only a port ID, as with a trunk's parent port, fetch the **OVO** + with `utils.fetch_port_object()`. It is typed, and it is what Neutron's own + newer code uses when it needs binding details. We don't use `get_port()` for + this. + +#### Selecting a binding + +A port can have several bindings (one `ACTIVE`, any number of `INACTIVE` ones +during live migration), so never index with `port.bindings[0]`. Select the +binding with `get_port_binding_by_status_and_host()` from +`neutron_lib.plugins.utils`, then read `vif_type`, `vnic_type`, `profile` and +`host` from the result. For an OVO port, `utils.active_port_binding()` wraps +this and returns the `ACTIVE` binding, or `None` if there isn't one: + +```python +binding = utils.active_port_binding(port) +if binding and binding.vnic_type == portbindings.VNIC_BAREMETAL: + ... +``` + +For a SQLAlchemy model, call the neutron-lib helper directly with +`port.port_bindings` (the model's attribute name) instead of `port.bindings`. +The helper only subscripts each binding (`binding["status"]`, +`binding["host"]`), so it works on either. Pass `raise_if_not_found=True` to get +`PortBindingNotFound` instead of `None`. + ### Router Interface Lifecycle When a subnet is attached to a router, the understack ML2 driver sets up a path diff --git a/python/neutron-understack/neutron_understack/trunk.py b/python/neutron-understack/neutron_understack/trunk.py index 6b7e7fd40..d50b162c5 100644 --- a/python/neutron-understack/neutron_understack/trunk.py +++ b/python/neutron-understack/neutron_understack/trunk.py @@ -160,8 +160,9 @@ def _handle_segment_allocation( def _add_subports_networks_to_parent_port_switchport( self, parent_port: Port, subports: list[SubPort] ) -> None: - binding_profile = parent_port.bindings[0].profile - binding_host = parent_port.bindings[0].host + parent_binding = utils.active_port_binding(parent_port) + binding_profile = parent_binding.profile + binding_host = parent_binding.host physnet = binding_profile.get("physical_network") if not physnet: @@ -182,7 +183,8 @@ def _clean_parent_port_switchport_config( parent_port_obj = utils.fetch_port_object(trunk.port_id) if not utils.parent_port_is_bound(parent_port_obj): return - self._handle_segment_deallocation(subports, parent_port_obj.bindings[0].host) + parent_binding = utils.active_port_binding(parent_port_obj) + self._handle_segment_deallocation(subports, parent_binding.host) def _delete_unused_segment(self, segment_id: str) -> NetworkSegment: network_segment = utils.network_segment_by_id(segment_id) diff --git a/python/neutron-understack/neutron_understack/undersync_mech.py b/python/neutron-understack/neutron_understack/undersync_mech.py index e16de473d..e9b07130b 100644 --- a/python/neutron-understack/neutron_understack/undersync_mech.py +++ b/python/neutron-understack/neutron_understack/undersync_mech.py @@ -144,7 +144,7 @@ def _validate_parent_physnet(self, resource, event, trunk_plugin, payload): parent_port = utils.fetch_port_object(trunk.port_id) if not utils.parent_port_is_bound(parent_port): return - if not parent_port.bindings[0].profile.get("physical_network"): + if not utils.active_port_binding(parent_port).profile.get(api.PHYSICAL_NETWORK): raise exc.BadRequest( resource="port", msg=_missing_physnet_msg(parent_port.id) ) @@ -165,7 +165,9 @@ def _sync_trunk_parent(self, trunk) -> None: if not utils.parent_port_is_bound(parent_port): return - physnet = parent_port.bindings[0].profile.get("physical_network") + physnet = utils.active_port_binding(parent_port).profile.get( + api.PHYSICAL_NETWORK + ) if not physnet: # _validate_parent_physnet rejects this while it is still # abortable, so the binding profile changed underneath us. Raising @@ -192,7 +194,7 @@ def bind_port(self, context: PortContext) -> None: binding_profile = port.get(portbindings.PROFILE) or {} if not binding_profile.get("physical_network"): LOG.debug( - "Skipping port %s without a physical_network in its " "binding_profile", + "Skipping port %s without a physical_network in its binding_profile", port["id"], ) return diff --git a/python/neutron-understack/neutron_understack/utils.py b/python/neutron-understack/neutron_understack/utils.py index e3017158e..83e332bad 100644 --- a/python/neutron-understack/neutron_understack/utils.py +++ b/python/neutron-understack/neutron_understack/utils.py @@ -15,6 +15,7 @@ from neutron_lib import context as n_context from neutron_lib.api.definitions import segment as segment_def from neutron_lib.plugins import directory +from neutron_lib.plugins import utils as plugin_utils from neutron_lib.plugins.ml2 import api from oslo_config import cfg @@ -472,8 +473,15 @@ def release_segment_if_unused(segment: NetworkSegment) -> None: release_dynamic_segment(segment.id) +def active_port_binding(port: port_obj.Port) -> port_obj.PortBinding | None: + """The port's ACTIVE binding, or None if it has none.""" + return plugin_utils.get_port_binding_by_status_and_host( + port.bindings, constants.ACTIVE, port_id=port.id + ) + + def parent_port_is_bound(port: port_obj.Port) -> bool: - port_binding = port.bindings[0] + port_binding = active_port_binding(port) return bool( port_binding and port_binding.vif_type == portbindings.VIF_TYPE_OTHER