ML2 MechanismDriver.bind_port() called inside transaction

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

Bug Description

The ML2 plugin calls the bind_port() operation on the registered mechanism drivers in order to establish a binding for a neutron port (i.e. determining the binding:vif_type and what network segment is being used). It currently makes these calls from within a DB transaction. This is fine for mechanism drivers such as those for the L2 agents that base their binding decision solely on information from the neutron DB. But if mechanism drivers for controllers or devices need to make remote calls to determine if binding is possible, they should not do this from within a DB transaction. Therefore, the bind_port() call must be made by the plugin outside of any enclosing DB transaction, and the drivers should manage any needed DB transactions themselves.

Once [re]binding is moved outside the DB transaction that triggers it, the possibility that multiple threads or processes will concurrently try to bind the same port must also be addressed.

Robert Kukura (rkukura)
Changed in neutron:
status: New → In Progress
Thierry Carrez (ttx)
Changed in neutron:
milestone: icehouse-3 → icehouse-rc1
Revision history for this message
Robert Kukura (rkukura) wrote :
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/79511

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

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

commit a57dc2c30ab78ba74cfc51b8fdb457d3374cc87d
Author: Robert Kukura <email address hidden>
Date: Mon Mar 10 15:06:09 2014 -0400

    ML2: Remove validate_port_binding() and unbind_port()

    The API implemented by ML2 mechanism drivers included three methods
    related to port binding that were called within DB transactions, but
    that could potentially involve communication with controllers or
    devices that should not be done within transactions. A subsequent
    patch will move the calls to bind_port() outside of tranactions. This
    patch eliminates the other two methods from the MechanismDriver API.

    The validate_port_binding() method was previously called on the bound
    mechanism driver to check whether an existing binding was still valid,
    so that the port could be rebound if something changed. But since nova
    has no way to handle changes to binding:vif_type or
    binding:vif_details after a port is initially plugged, this turned out
    not to be useful, so the method has been removed from the
    MechanismDriver API. Now, once a port is successfully bound, the
    binding remains until the port is deleted or any of it's
    binding:host_id, binding:vnic_type, or binding:profile attribute
    values are changed.

    The unbind_port() method was previously called on the bound mechanism
    driver as an existing binding was removed. This method was not used by
    any existing mechanism drivers, and was redundant with the
    update_port_precommit() and update_port_postcommit() methods that are
    called on all mechanism drivers when an existing binding is removed,
    so this method has also been removed from the driver API.

    Eliminating the unbind_port() call allows the binding details to be
    made available via the PortContext in delete_port_postcommit() calls,
    completing the resolution of bug 1276395.

    Closes-bug: 1276395
    Partial-bug: 1276391
    Change-Id: I70fb65b478373c4f07f5273baa097fc50e5ba2ef

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/82945

Changed in neutron:
milestone: icehouse-rc1 → none
tags: added: icehouse-rc-potential
tags: added: icehouse-backport-potential
removed: icehouse-rc-potential
Robert Kukura (rkukura)
Changed in neutron:
milestone: none → juno-1
Kyle Mestery (mestery)
Changed in neutron:
milestone: juno-1 → juno-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

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

commit b1677dcb80ce8b83aadb2180efad3527a96bd3bc
Author: Robert Kukura <email address hidden>
Date: Tue Mar 11 21:54:35 2014 -0400

    ML2: Bind ports outside transactions

    The ML2 plugin now calls the bind_port() operation on the registered
    mechanism drivers outside of any enclosing DB transaction. Ports are
    created or updated in one transaction, then a binding is established
    if possible, and finally a second transaction commits the binding
    result.

    With [re]binding moved outside the DB transaction that triggered it,
    it is now possible that multiple threads or processes will
    concurrently try to bind the same port, or that the port will be
    updated between transactions. Concurrent attempts to bind the same
    port are allowed to proceed, which results are used is resolved in the
    second transaction, and binding is retried if necessary.

    Improvements to the Cisco Nexus driver and unit tests from Rich Curran
    needed due to the binding changes are also included.

    Closes-Bug: 1276391
    Closes-Bug: 1335226
    Change-Id: I65dafc330d6e812dad0667d2383858504d0ba299

Changed in neutron:
status: In Progress → Fix Committed
Changed in neutron:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in neutron:
milestone: juno-2 → 2014.2
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.