Open vSwitch agent - does not report to the segment plugin

[Harald Jensås]

The atteched devstack configuration `local.conf` can be used to reproduce this issue.

In networking-baremetal CI we are seeing an issue where the DHCP agent does not create a namespace for subnets on a routed provider network. No DHCP namespace is created because this test[1] `if (any(s for s in network.subnets if s.enable_dhcp)` returns false.

The OVS agent does have the correct configuration, with mappings to the physical_network "mynetwork" on bridge "brbm".

$ openstack network agent show ef7ca33a-de9c-4a2b-9af5-e1c9cb029a25 -f yaml
admin_state_up: true
agent_type: Open vSwitch agent
alive: true
availability_zone: null
binary: neutron-openvswitch-agent
configuration:
  arp_responder_enabled: false
  baremetal_smartnic: false
  bridge_mappings:
    mynetwork: brbm
    public: br-ex

But looking in the database, there are only `segment host mappings` for baremetal nodes.

mysql> select * from segmenthostmappings;
+--------------------------------------+--------------------------------------+
| segment_id | host |
+--------------------------------------+--------------------------------------+
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | 30e70889-d50c-42a8-8776-f5e8cdce3609 |
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | a3930bb2-a742-44f8-954e-49be373477db |
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | cbb3f623-f300-4dd3-ab09-c05b1e9b9a75 |
| ede8201d-0766-4a18-9df0-1b6292f99592 | 30e70889-d50c-42a8-8776-f5e8cdce3609 |
| ede8201d-0766-4a18-9df0-1b6292f99592 | a3930bb2-a742-44f8-954e-49be373477db |
| ede8201d-0766-4a18-9df0-1b6292f99592 | cbb3f623-f300-4dd3-ab09-c05b1e9b9a75 |
+--------------------------------------+--------------------------------------+
6 rows in set (0.00 sec)

If I manually stop the OVS agent service, manually delete the agent and then restart the OVS agent the agent is re-created and mappings are created.

stack@devstack:~/devstack$ sudo systemctl stop <email address hidden>
stack@devstack:~/devstack$ openstack network agent delete 3921f433-b2f1-4c8f-90d2-deaad6bb5814
stack@devstack:~/devstack$ sudo systemctl start <email address hidden>

mysql> select * from segmenthostmappings;
+--------------------------------------+--------------------------------------+
| segment_id | host |
+--------------------------------------+--------------------------------------+
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | 30e70889-d50c-42a8-8776-f5e8cdce3609 |
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | a3930bb2-a742-44f8-954e-49be373477db |
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | cbb3f623-f300-4dd3-ab09-c05b1e9b9a75 |
| 712e8a82-a5f9-4506-9520-fc5b4b01529e | devstack |
| e0673f45-e9b4-4672-874f-e2eb65d74d70 | devstack |
| e2c8c728-9e55-4047-b56f-701375887da9 | devstack |
| ede8201d-0766-4a18-9df0-1b6292f99592 | 30e70889-d50c-42a8-8776-f5e8cdce3609 |
| ede8201d-0766-4a18-9df0-1b6292f99592 | a3930bb2-a742-44f8-954e-49be373477db |
| ede8201d-0766-4a18-9df0-1b6292f99592 | cbb3f623-f300-4dd3-ab09-c05b1e9b9a75 |
| ede8201d-0766-4a18-9df0-1b6292f99592 | devstack |
+--------------------------------------+--------------------------------------+

To get the DHCP namespace up and running the DHCP agent service must be restarted as well.

stack@devstack:~/devstack$ ip netns list
qdhcp-6fd5f856-bcf6-4821-a85c-18139e934061 (id: 2)
qrouter-ea62445b-ea09-4a2d-88e9-87bede21d6c6 (id: 1)
qdhcp-4b9aae0e-482d-402f-a7c4-2acaad2f3620 (id: 0)

stack@devstack:~/devstack$ sudo systemctl restart <email address hidden>

stack@devstack:~/devstack$ ip netns list
qdhcp-b435b564-ce44-4fc1-bec2-2a4ccb27346f (id: 3) <-- The DHCP was created.
qdhcp-6fd5f856-bcf6-4821-a85c-18139e934061 (id: 2)
qrouter-ea62445b-ea09-4a2d-88e9-87bede21d6c6 (id: 1)
qdhcp-4b9aae0e-482d-402f-a7c4-2acaad2f3620 (id: 0)

In this CI job[2] with patch[3] we can see that the host is added with no segments:

May 19 17:47:34.266428 ubuntu-focal-ovh-bhs1-0029706921 neutron-server[84188]: DEBUG neutron.services.segments.db [None req-ac845447-48ed-4c73-85c9-347133259602 None None] Segments set() mapped to the host ubuntu-focal-ovh-bhs1-0029706921 {{(pid=84188) update_segment_host_mapping /opt/stack/neutron/neutron/services/segments/db.py:237}}

[1] https://opendev.org/openstack/neutron/src/branch/master/neutron/agent/dhcp/agent.py#L403
[2] https://review.opendev.org/c/openstack/networking-baremetal/+/842578
[3] https://review.opendev.org/842576

[Harald Jensås (harald-jensas)]

It seems the issue is that new segment mappings are not created when new networks are created after the agent was created. If I stop the OVS agent, delete the agent, and restart it I see segment host mappings for the existing networks. But when I add a new network the segment host mappings are not updated.

stack@devstack:~$ sudo systemctl stop <email address hidden>
stack@devstack:~$ openstack network agent delete 4309835f-389b-4cf2-945c-e16c80cc4356
stack@devstack:~$ sudo systemctl start <email address hidden>
stack@devstack:~$ openstack network create --share --provider-physical-network mynetwork --provider-network-type vlan --provider-segment 140 new-seg-net
+---------------------------+--------------------------------------+
| Field | Value |
+---------------------------+--------------------------------------+
| admin_state_up | UP |
| availability_zone_hints | |
| availability_zones | |
| created_at | 2022-05-24T10:26:48Z |
| description | |
| dns_domain | None |
| id | d290cc37-d716-478f-8ccc-9ac1de871136 |
| ipv4_address_scope | None |
| ipv6_address_scope | None |
| is_default | None |
| is_vlan_transparent | None |
| mtu | 1500 |
| name | new-seg-net |
| port_security_enabled | True |
| project_id | b3f72ae335f7401f91da365388e336eb |
| provider:network_type | vlan |
| provider:physical_network | mynetwork |
| provider:segmentation_id | 140 |
| qos_policy_id | None |
| revision_number | 1 |
| router:external | Internal |
| segments | None |
| shared | True |
| status | ACTIVE |
| subnets | |
| tags | |
| updated_at | 2022-05-24T10:26:48Z |
+---------------------------+--------------------------------------+
stack@devstack:~$ openstack network segment list --long --fit
+--------------------------------------+------+--------------------------------------+--------------+---------+------------------...

[Harald Jensås (harald-jensas)]

If I add debug:

diff --git a/neutron/common/utils.py b/neutron/common/utils.py
index ec6067eb91..e0767cb720 100644
--- a/neutron/common/utils.py
+++ b/neutron/common/utils.py
@@ -1043,8 +1043,10 @@ def is_session_active(session):
     if session.autocommit: # old behaviour, to be removed with sqlalchemy 2.0
         return session.is_active
     if not session.transaction:
+ print('HJENSAS - not session.transaction')
         return False
     if not (session.dirty or session.deleted or session.new):
+ print('HJENSAS - not (session.dirty or session.deleted or session.new)')
         return False
     return True

diff --git a/neutron/services/segments/db.py b/neutron/services/segments/db.py
index 35ea24b6a5..78faa1f263 100644
--- a/neutron/services/segments/db.py
+++ b/neutron/services/segments/db.py
@@ -328,6 +328,7 @@ def _add_segment_host_mapping_for_segment(resource, event, trigger,
     context = payload.context
     segment = payload.latest_state
     if not utils.is_session_active(context.session):
+ print('HJENSAS - not utils.is_session_active(context.session)')
         # The session might be in partial rollback state, due to errors in
         # peer callback. In that case, there is no need to add the mapping.
         # Just return here.

Result:
May 24 19:11:11 devstack neutron-server[144755]: HJENSAS - not utils.is_session_active(context.session)
May 24 19:11:11 devstack neutron-server[144755]: HJENSAS - not (session.dirty or session.deleted or session.new)
May 24 19:11:11 devstack neutron-server[144755]: HJENSAS - not (session.dirty or session.deleted or session.new)

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hi Harald:

Thanks for the analysis. I think the issue you have detected is related to "is_session_active" and how the new, deleted and dirty session transactions are read. The methods the "session" object has should translate what is stored in "session.transaction._new", "session.transaction._dirty" and "session.transaction._deleted" (or at least this is what "is_session_active" is expecting).

I'll push a patch to change that but I would like Mike Bayers to check what I'm stating here. For sure he'll know more than me about this area.

Regards.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/843256

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/843294

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/843294
Committed: https://opendev.org/openstack/neutron/commit/d89d7bd5e6477a1ad24165831dbc5f1e2fc357b5
Submitter: "Zuul (22348)"
Branch: master

commit d89d7bd5e6477a1ad24165831dbc5f1e2fc357b5
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Sat May 14 16:55:33 2022 +0000

    Remove session active check in "_add_segment_host_mapping_for_segment"

    Method ``_add_segment_host_mapping_for_segment`` is called by the event
    (resources.SEGMENT, events.PRECOMMIT_CREATE), from
    ``SegmentDbMixin._create_segment_db``, and is called inside a database
    writer context. That means it is irrelevant to check if the session is
    active (must be always).

    Closes-Bug: #1975542
    Change-Id: Ib19dacf886486876237ed1157fb95ae157ed430e

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/neutron/+/843624

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/neutron/+/843626

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/neutron/+/843627

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/843626
Committed: https://opendev.org/openstack/neutron/commit/874bccaae4a011d52378ae98c8747fe1734e9111
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 874bccaae4a011d52378ae98c8747fe1734e9111
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Sat May 14 16:55:33 2022 +0000

    Remove session active check in "_add_segment_host_mapping_for_segment"

    Method ``_add_segment_host_mapping_for_segment`` is called by the event
    (resources.SEGMENT, events.PRECOMMIT_CREATE), from
    ``SegmentDbMixin._create_segment_db``, and is called inside a database
    writer context. That means it is irrelevant to check if the session is
    active (must be always).

    Closes-Bug: #1975542

    Conflicts:
        neutron/services/segments/db.py

    Change-Id: Ib19dacf886486876237ed1157fb95ae157ed430e
    (cherry picked from commit d89d7bd5e6477a1ad24165831dbc5f1e2fc357b5)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/843627
Committed: https://opendev.org/openstack/neutron/commit/e2546075591f55cc242185a58a8cf9f2875bff58
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit e2546075591f55cc242185a58a8cf9f2875bff58
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Sat May 14 16:55:33 2022 +0000

    Remove session active check in "_add_segment_host_mapping_for_segment"

    Method ``_add_segment_host_mapping_for_segment`` is called by the event
    (resources.SEGMENT, events.PRECOMMIT_CREATE), from
    ``SegmentDbMixin._create_segment_db``, and is called inside a database
    writer context. That means it is irrelevant to check if the session is
    active (must be always).

    Closes-Bug: #1975542

    Conflicts:
        neutron/services/segments/db.py

    Change-Id: Ib19dacf886486876237ed1157fb95ae157ed430e
    (cherry picked from commit d89d7bd5e6477a1ad24165831dbc5f1e2fc357b5)
    (cherry picked from commit 874bccaae4a011d52378ae98c8747fe1734e9111)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/843624
Committed: https://opendev.org/openstack/neutron/commit/150b0d57095fa213d3024f66f6da641bacfa6719
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 150b0d57095fa213d3024f66f6da641bacfa6719
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Sat May 14 16:55:33 2022 +0000

    Remove session active check in "_add_segment_host_mapping_for_segment"

    Method ``_add_segment_host_mapping_for_segment`` is called by the event
    (resources.SEGMENT, events.PRECOMMIT_CREATE), from
    ``SegmentDbMixin._create_segment_db``, and is called inside a database
    writer context. That means it is irrelevant to check if the session is
    active (must be always).

    Closes-Bug: #1975542
    Change-Id: Ib19dacf886486876237ed1157fb95ae157ed430e
    (cherry picked from commit d89d7bd5e6477a1ad24165831dbc5f1e2fc357b5)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/843256
Committed: https://opendev.org/openstack/neutron/commit/6b529ea3c559ed1a016e2aa2c30769429192d528
Submitter: "Zuul (22348)"
Branch: master

commit 6b529ea3c559ed1a016e2aa2c30769429192d528
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Sat May 14 11:16:05 2022 +0000

    [sqlalchemy-20] Use session.transaction information to decide if active

    The "session.transaction" member "_connections" is a dictionary with the
    active database connections of the current transaction. Neutron will use
    it to determine if the session is active or not. This parameter will
    provide more accurate information than "new", "dirty" or "deleted". A
    read (SELECT) operation won't be included on those lists while the
    transaction is currently active.

    Closes-Bug: #1975542
    Change-Id: Ibb75b324950c639d30c77d4e5269a8c53d6a0bb5

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 18.5.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 19.4.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 20.2.0 release.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-lib/+/851193

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-lib/+/851193
Committed: https://opendev.org/openstack/neutron-lib/commit/5b1dab0e8f6d27dabfa5088f950a2d4ad186e0a5
Submitter: "Zuul (22348)"
Branch: master

commit 5b1dab0e8f6d27dabfa5088f950a2d4ad186e0a5
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Fri Jul 22 03:44:21 2022 +0200

    [sqlalchemy-20] Use session.transaction information to decide if active

    The "session.transaction" member "_connections" is a dictionary with the
    active database connections of the current transaction. Neutron will use
    it to determine if the session is active or not. This parameter will
    provide more accurate information than "new", "dirty" or "deleted". A
    read (SELECT) operation won't be included on those lists while the
    transaction is currently active.

    This patch is patch is a direct transcription of [1].

    [1]https://review.opendev.org/c/openstack/neutron/+/843256

    Related-Bug: #1975542

    Change-Id: Ibc04a1940c3833d3e10dbfc011598ac11c58ba13

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 21.0.0.0rc1 release candidate.