ML2 DVR port binding does not handle concurrency

Bug #1415526 reported by Robert Kukura
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
Robert Kukura

Bug Description

In the non-DVR case, the Ml2Plugin._bind_port() function creates a models.PortBinding instance with copies of fields from the port's actual PortBinding instance from the DB, and passes a PortContext referencing this copy to MechanismManager.bind_port() to attempt to bind the port. This allows the results from the binding attempt to be discarded if Ml2Plugin._commit_port_binding determines the port has been concurrently bound or otherwise modified during the binding attempt, which occurs outside any transaction.

In the DVR case, Ml2Plugin.update_dvr_port_binding() calls MechanismManager.bind_port() using a PortContext referencing the actual models.DVRPortBinding instance associated with the DB. Changes made to this DVRPortBinding instance during the binding attempt get committed to the DB in the following transaction, and Ml2Plugin._commit_dvr_port_binding() is not actually comparing the pre-binding and post-binding state as intended to determine whether to commit the changes.

This does not currently cause any obvious issues in normal operation (unless there are concurrent binding attempts), but the changes in https://review.openstack.org/#/c/116122/16/neutron/plugins/ml2/plugin.py expose this issue because db.set_binding_levels() does not get called on line 366, resulting in DVR tempest failures.

This issue would likely be resolved by the work planned for https://bugs.launchpad.net/neutron/+bug/1367391, but may need to be fixed sooner to allow the hierarchical port binding implementation to be merged.

Tags: ml2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.openstack.org/151913

Changed in neutron:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/151913
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=f9cdb351c991adca4b0ed5fdcec1dc1d765adbda
Submitter: Jenkins
Branch: master

commit f9cdb351c991adca4b0ed5fdcec1dc1d765adbda
Author: Robert Kukura <email address hidden>
Date: Thu Jan 29 17:13:00 2015 -0500

    ML2: Use same port binding logic for DVR ports as non-DVR ports

    DVR ports are now bound using the same function,
    Ml2Plugin._bind_port_if_needed(), that is used to bind non-DVR ports,
    so that concurrent binding attempts are properly handled and mechanism
    driver update_port_precommit() and update_port_postcommit() methods
    are called. The Ml2Plugin._commit_dvr_port_binding() function is
    eliminated, and the DvrPortContext class has been folded into the
    PortContext class, reducing duplicated logic.

    A followup patch will address the duplication of ML2 DB schema for DVR
    and further reduce the duplicated and special-case port binding logic
    supporting DVR.

    Closes-Bug: 1415526
    Closes-Bug: 1416783
    Partial-Bug: 1367391

    Change-Id: Ic32241297c5f8c67dc77d0af836b1cc0a5df988a

Changed in neutron:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in neutron:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in neutron:
milestone: kilo-2 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.