OVN transaction could not be completed due to a race condition

[Arnau Verdaguer]

When executing the test "test_connectivity_through_2_routers" it is highly possible to have
a race condition:

networking_ovn.common.exceptions.RevisionConflict: OVN revision number for {PORT_ID} (type: ports) is equal or higher than the given resource. Skipping update.

Bugzilla reference: https://bugzilla.redhat.com/show_bug.cgi?id=1860448

===== Ubuntu SRU Details =====
[Impact]
See bug description.

[Test Case]
Deploy openstack with OVN. Run the test_connectivity_through_2_routers test from https://github.com/openstack/neutron-tempest-plugin. This could also be tested manually based on what that test does. Ensure the router port status is not set to DOWN at any point.

[Where problems could occur]
The existing bug could still occur if the assumpion that specifying the port type is not correct. Presumably this is not the case, but that is a theoritical potential for where problems could occur. All of these patches have already landed in the corresponding upstream branches.

[Arnau Verdaguer (averdagu)]

The race conditions is triggered when the port (test_ap{1|2}_wan_port) is added to the router.

This happens because during the handling of the OSP cli command a LogicalSwitchPortUpdateUpEvent
is received that triggers a new worker who will work with the same port that the worker handling
the OSP CLI cmd is working.

Another problem is that during the course of the request the port status will flap from:
unknown -> UP -> DOWN -> UP
This will cause that both workers will be modifying the same port during more time, increasing
the chances of getting the race condition error.

The flapping error is triggered because on the update_port function, the _nb_idl.set_lswitch_port
will not include the port type on the command. When this command reaches the nbdb since the port
type is not "router" this will cause that the status is set to DOWN (And later on to UP again).

[Bence Romsics (bence-romsics)]

Hi,

What do you think, what exactly is racing? test_connectivity_through_2_routers and an OSP cli command? What was the OSP cli command? Or that test with other tests? Which other tests then?

For the record, the bugzilla ticket says there was an earlier launchpad ticket of which this may be a duplicate:
https://bugs.launchpad.net/neutron/+bug/1885898

However both the bugzilla and the earlier launchpad tickets are quite old and because of that the logs are no longer accessible. Do you have fresh logs?

Also this problem may be present in the upstream gate:
http://logstash.openstack.org/#/dashboard/file/logstash.json?query=message:%5C%22line%20125,%20in%20test_connectivity_through_2_routers%5C%22%20AND%20voting:1&from=864000s

But there we don't have onv-controller logs, so it's hard to say if those are the same issues. What do you think?

Just a tip: Usually when working with race conditions (if you already have an idea of what is racing) a carefully placed sleep() can help a lot by making the race window arbitrarily bigger and therefore giving a chance to turn the bug into a deterministic one.

Cheers,
Bence

[Arnau Verdaguer (averdagu)]

Hi Bence!

test_connectivity_through_2_routers is triggering the error. While debugging it I converted the
test into a bash script:

'''
#!/bin/bash

. overcloudrc admin

openstack network create test_ap1_net
openstack network create test_ap2_net
openstack network create test_wan_net

openstack subnet create test_ap1_subnet --subnet-range 192.168.210.0/24 --gateway 192.168.210.254 --network test_ap1_net
openstack subnet create test_ap2_subnet --subnet-range 192.168.220.0/24 --gateway 192.168.220.254 --network test_ap2_net

openstack router create test_ap1_rt

openstack router create test_ap2_rt

# Don't know how to wait for router HA being active so... sleep 5 :)

sleep 5

openstack port create test_ap1_internal_port --network test_ap1_net
openstack port create test_ap2_internal_port --network test_ap2_net
openstack port create test_ap1_wan_port --network nova
openstack port create test_ap2_wan_port --network nova

# Here the error can happen
openstack router add port test_ap1_rt test_ap1_wan_port
# To undo the last command:
# openstack router remove port test_ap1_rt test_ap1_wan_port # (the port will be deleted)
# openstack port create test_ap1_wan_port --network nova
'''

And I found that the OSP CLI cmd "openstack router add port test_ap1_rt test_ap1_wan_port" by itself triggers the error.

You can find a newer log of the error here:
https://86679a46b4c1d4c5128d-a5c518380d1db6458f4f9d5f7d87c4d0.ssl.cf5.rackcdn.com/817525/8/check/neutron-tempest-plugin-scenario-ovn/74c7787/controller/logs/screen-q-svc.txt

(Timestamp: Nov 28 18:54:06.040340)

I've been working with the race condition for a while and I come up with a patch that
will decrease the possibility of a race condition:
https://review.opendev.org/c/openstack/neutron/+/821401

Cheers,
Arnau

[Slawek Kaplonski (slaweq)]

This bug has had a related patch abandoned and has been automatically un-assigned due to inactivity. Please re-assign yourself if you are continuing work or adjust the state as appropriate if it is no longer valid.

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Slawek Kaplonski <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/821401
Reason: This review is > 4 weeks without comment, and failed Zuul jobs the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

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

commit 8c482b83f2cf6f5495f4df2e5698595db704798d
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/855335
Committed: https://opendev.org/openstack/neutron/commit/3d036d59fe53c1654fa7749b3ed7a17203f9f903
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 3d036d59fe53c1654fa7749b3ed7a17203f9f903
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Conflicts:
      neutron/common/ovn/constants.py

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
    (cherry picked from commit 8c482b83f2cf6f5495f4df2e5698595db704798d)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/855063
Committed: https://opendev.org/openstack/neutron/commit/423c92814bcd7ae5e66d05a861be14e4d84f6e25
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 423c92814bcd7ae5e66d05a861be14e4d84f6e25
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Conflicts:
            neutron/common/ovn/constants.py

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
    (cherry picked from commit 8c482b83f2cf6f5495f4df2e5698595db704798d)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit 62862df103456b9bc2674a2fd9049f6d77421e1b
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Conflicts:
            neutron/common/ovn/constants.py

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
    (cherry picked from commit 8c482b83f2cf6f5495f4df2e5698595db704798d)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/855065
Committed: https://opendev.org/openstack/neutron/commit/6e1b7d15d7d68f2d409ce34e9a518c6aa0289022
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 6e1b7d15d7d68f2d409ce34e9a518c6aa0289022
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Conflicts:
            neutron/common/ovn/constants.py

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
    (cherry picked from commit 8c482b83f2cf6f5495f4df2e5698595db704798d)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/855064
Committed: https://opendev.org/openstack/neutron/commit/1eaf41c96b230e28ee9154b2713111917d88249e
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 1eaf41c96b230e28ee9154b2713111917d88249e
Author: Arnau Verdaguer <email address hidden>
Date: Thu Dec 16 21:53:27 2021 +0100

    [ovn] Specify port type if it's a router port when updating

    In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
    LogicalSwitchPortUpdateDownEvent is mandatory to specify the
    router port type when udpating the port.

    If the port is not specified when updating the port, the
    transactions will trigger a modification on the ovnnb db
    that will set the port status to down[0]. Triggering an unnecessary
    DownEvent followed by another UpEvent. Those unnecessary event
    most likely will trigger a revision conflict.

    [0] - https://github.com/ovn-org/ovn/blob/
    4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

    Conflicts:
            neutron/common/ovn/constants.py

    Closes-Bug: #1955578
    Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
    (cherry picked from commit 8c482b83f2cf6f5495f4df2e5698595db704798d)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/networking-ovn train-eol release.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Corey Bryant (corey.bryant)]

A new package version with this fix has been uploaded to the focal unapproved queue and victoria staging PPA.

[Andreas Hasenack (ahasenack)]

Hello Arnau, or anyone else affected,

Accepted neutron into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/neutron/2:16.4.2-0ubuntu6.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron ussuri-eol release.

[Brian Haley (brian-haley)]

Verified this was fixed on a juju-deployed focal/ussuri cloud, no revision conflicts seen with a port configured to reproduce where with previous 6.3 version there was.

[Chris Halse Rogers (raof)]

The verification of the Stable Release Update for neutron has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package neutron - 2:16.4.2-0ubuntu6.4

---------------
neutron (2:16.4.2-0ubuntu6.4) focal; urgency=medium

  [ Corey Bryant ]
  * d/p/ovn-db-sync-continue-on-duplicate-normalise.patch: Cherry-picked
    from upstream to allow ovn_db_sync to continue on duplicate normalised
    CIDR (LP: #1961112).
  * d/p/ovn-db-sync-check-for-router-port-differences.patch:
    Cherry-picked from upstream to ensure router ports are marked
    for needing updates only if they have changed (LP: #2030773).
  * d/p/ovn-specify-port-type-if-router-port-when-updating.patch:
    Specify port type if it's a router port when updating to avoid
    port flapping (LP: #1955578).
  * d/p/fix-acl-sync-when-default-sg-group-created.patch:
    Cherry-picked form upstream to fix ACL sync when default security
    group is created (LP: #2008943).

  [ Mustafa Kemal GILOR ]
  * d/p/add_uplink_status_propagation.patch: Add the
    'uplink-status-propagation' extension to ML2/OVN (LP: #2032770).

 -- Corey Bryant <email address hidden> Wed, 08 Nov 2023 11:41:21 -0500

[James Page (james-page)]

Hello Arnau, or anyone else affected,

Accepted neutron into ussuri-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:ussuri-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-ussuri-needed to verification-ussuri-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ussuri-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron victoria-eom release.

[Brian Haley (brian-haley)]

I have tested neutron version 2:16.4.2-0ubuntu6.4~cloud0 from the cloud-archive:ussuri-proposed repository and can verify the code has this change, and the failure does not occur. I followed the steps from Comment #3:

openstack network create test_ap1_net
openstack network create test_ap2_net
openstack network create test_wan_net

openstack subnet create test_ap1_subnet --subnet-range 192.168.210.0/24 --gateway 192.168.210.254 --network test_ap1_net
openstack subnet create test_ap2_subnet --subnet-range 192.168.220.0/24 --gateway 192.168.220.254 --network test_ap2_net

openstack router create test_ap1_rt

openstack router create test_ap2_rt

# Don't know how to wait for router HA being active so... sleep 5 :)

sleep 5

openstack port create test_ap1_internal_port --network test_ap1_net
openstack port create test_ap2_internal_port --network test_ap2_net
openstack port create test_ap1_wan_port --network nova
openstack port create test_ap2_wan_port --network nova

# Here the error can happen
openstack router add port test_ap1_rt test_ap1_wan_port

I did not see any tracebacks in neutron-server.log regarding the OVN revision number for the port.

[James Page (james-page)]

The verification of the Stable Release Update for neutron has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[James Page (james-page)]

This bug was fixed in the package neutron - 2:16.4.2-0ubuntu6.4~cloud0
---------------

 neutron (2:16.4.2-0ubuntu6.4~cloud0) bionic-ussuri; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 neutron (2:16.4.2-0ubuntu6.4) focal; urgency=medium
 .
   [ Corey Bryant ]
   * d/p/ovn-db-sync-continue-on-duplicate-normalise.patch: Cherry-picked
     from upstream to allow ovn_db_sync to continue on duplicate normalised
     CIDR (LP: #1961112).
   * d/p/ovn-db-sync-check-for-router-port-differences.patch:
     Cherry-picked from upstream to ensure router ports are marked
     for needing updates only if they have changed (LP: #2030773).
   * d/p/ovn-specify-port-type-if-router-port-when-updating.patch:
     Specify port type if it's a router port when updating to avoid
     port flapping (LP: #1955578).
   * d/p/fix-acl-sync-when-default-sg-group-created.patch:
     Cherry-picked form upstream to fix ACL sync when default security
     group is created (LP: #2008943).
 .
   [ Mustafa Kemal GILOR ]
   * d/p/add_uplink_status_propagation.patch: Add the
     'uplink-status-propagation' extension to ML2/OVN (LP: #2032770).