Hi, thanks for this great proposal
On Wed, Feb 5, 2014 at 3:10 PM, Henry Gessau <ges...@cisco.com> wrote: > Bob, this is fantastic, I really appreciate all the detail. A couple of > questions ... > > On Wed, Feb 05, at 2:16 am, Robert Kukura <rkuk...@redhat.com> wrote: > >> A couple of interrelated issues with the ML2 plugin's port binding have >> been discussed over the past several months in the weekly ML2 meetings. >> These effect drivers being implemented for icehouse, and therefore need >> to be addressed in icehouse: >> >> * MechanismDrivers need detailed information about all binding changes, >> including unbinding on port deletion >> (https://bugs.launchpad.net/neutron/+bug/1276395) >> * MechanismDrivers' bind_port() methods are currently called inside >> transactions, but in some cases need to make remote calls to controllers >> or devices (https://bugs.launchpad.net/neutron/+bug/1276391) >> * Semantics of concurrent port binding need to be defined if binding is >> moved outside the triggering transaction. >> >> I've taken the action of writing up a unified proposal for resolving >> these issues, which follows... >> >> 1) An original_bound_segment property will be added to PortContext. When >> the MechanismDriver update_port_precommit() and update_port_postcommit() >> methods are called and a binding previously existed (whether its being >> torn down or not), this property will provide access to the network >> segment used by the old binding. In these same cases, the portbinding >> extension attributes (such as binding:vif_type) for the old binding will >> be available via the PortContext.original property. It may be helpful to >> also add bound_driver and original_bound_driver properties to >> PortContext that behave similarly to bound_segment and >> original_bound_segment. >> >> 2) The MechanismDriver.bind_port() method will no longer be called from >> within a transaction. This will allow drivers to make remote calls on >> controllers or devices from within this method without holding a DB >> transaction open during those calls. Drivers can manage their own >> transactions within bind_port() if needed, but need to be aware that >> these are independent from the transaction that triggered binding, and >> concurrent changes to the port could be occurring. >> >> 3) Binding will only occur after the transaction that triggers it has >> been completely processed and committed. That initial transaction will >> unbind the port if necessary. Four cases for the initial transaction are >> possible: >> >> 3a) In a port create operation, whether the binding:host_id is supplied >> or not, all drivers' port_create_precommit() methods will be called, the >> initial transaction will be committed, and all drivers' >> port_create_postcommit() methods will be called. The drivers will see >> this as creation of a new unbound port, with PortContext properties as >> shown. If a value for binding:host_id was supplied, binding will occur >> afterwards as described in 4 below. >> >> PortContext.original: None >> PortContext.original_bound_segment: None >> PortContext.original_bound_driver: None >> PortContext.current['binding:host_id']: supplied value or None >> PortContext.current['binding:vif_type']: 'unbound' >> PortContext.bound_segment: None >> PortContext.bound_driver: None >> >> 3b) Similarly, in a port update operation on a previously unbound port, >> all drivers' port_update_precommit() and port_update_postcommit() >> methods will be called, with PortContext properies as shown. If a value >> for binding:host_id was supplied, binding will occur afterwards as >> described in 4 below. >> >> PortContext.original['binding:host_id']: previous value or None >> PortContext.original['binding:vif_type']: 'unbound' or 'binding_failed' >> PortContext.original_bound_segment: None >> PortContext.original_bound_driver: None >> PortContext.current['binding:host_id']: current value or None >> PortContext.current['binding:vif_type']: 'unbound' >> PortContext.bound_segment: None >> PortContext.bound_driver: None >> >> 3c) In a port update operation on a previously bound port that does not >> trigger unbinding or rebinding, all drivers' update_port_precommit() and >> update_port_postcommit() methods will be called with PortContext >> properties reflecting unchanged binding states as shown. >> >> PortContext.original['binding:host_id']: previous value >> PortContext.original['binding:vif_type']: previous value >> PortContext.original_bound_segment: previous value >> PortContext.original_bound_driver: previous value >> PortContext.current['binding:host_id']: previous value >> PortContext.current['binding:vif_type']: previous value >> PortContext.bound_segment: previous value >> PortContext.bound_driver: previous value >> >> 3d) In a the port update operation on a previously bound port that does >> trigger unbinding or rebinding, all drivers' update_port_precommit() and >> update_port_postcommit() methods will be called with PortContext >> properties reflecting the previously bound and currently unbound binding >> states as shown. If a value for binding:host_id was supplied, binding >> will occur afterwards as described in 4 below. >> >> PortContext.original['binding:host_id']: previous value >> PortContext.original['binding:vif_type']: previous value >> PortContext.original_bound_segment: previous value >> PortContext.original_bound_driver: previous value >> PortContext.current['binding:host_id']: new or current value >> PortContext.current['binding:vif_type']: 'unbound' >> PortContext.bound_segment: None >> PortContext.bound_driver: None >> >> 4) If a port create or update operation triggers binding or rebinding, >> it is attempted after the initial transaction is processed and committed >> as described in 3 above. The binding process itself is just as before, >> except it happens after and outside the transaction. Since binding now >> occurs outside the transaction, its possible that multiple threads or >> processes could concurrently attempt to bind the same port, although >> this is should be a rare occurrence. Rather than trying to prevent this >> with some sort of distributed lock or complicated state machine, >> concurrent attempts to bind are allowed to proceed in parallel. When a >> thread completes its attempt to bind (either successfully or >> unsuccessfully) it then performs a second transaction to update the DB >> with the result of its binding attempt. When doing so, it checks to see >> if some other thread has already committed relevant changes to the port >> between the two transactions. There are three possible cases: >> >> 4a) If the thread's binding attempt succeeded, and no other thread has >> committed either a new binding or changes that invalidate this thread's >> new binding between the two transactions, the thread commits its own >> binding results, calling all drivers' update_port_precommit() and >> update_port_postcommit() methods with PortContext properties reflecting >> the new binding as shown. It then returns the updated port dictionary to >> the caller. >> >> PortContext.original['binding:host_id']: previous value >> PortContext.original['binding:vif_type']: 'unbound' >> PortContext.original_bound_segment: None >> PortContext.original_bound_driver: None >> PortContext.current['binding:host_id']: previous value > > Are you not expecting/allowing the host_id to change in this scenario? Why? The change of host_id might occur for live-migration. Nova informs neutron of live-migration with port-update(new host). As I understand, this should trigger two update_port_pre/postcommit(). The first one is 3d, which only change current host_id and remove binding: PortContext.current['binding:host_id']: new or current value PortContext.current['binding:vif_type']: 'unbound' the second one is 4a, which come from the binding process : PortContext.current['binding:host_id']: previous value PortContext.current['binding:vif_type']: new value PortContext.bound_segment: new value PortContext.bound_driver: new value > >> PortContext.current['binding:vif_type']: new value >> PortContext.bound_segment: new value >> PortContext.bound_driver: new value >> >> 4b) If the thread's binding attempt either succeeded or failed, but some >> other thread has committed a new successful binding between the two >> transactions, the thread returns a port dictionary with attributes based >> on the DB state from the new transaction, including the other thread's >> binding and any other port state changes. No further calls to mechanism >> drivers are needed here since they are the responsibility of the other >> thread that bound the port. >> >> 4c) If some other thread committed changes to the port's >> binding-relevant state but has not committed a successful binding, then >> this thread attempts to bind again using that updated state, repeating 4. >> >> 5) Port deletion no longer does anything special to unbind the port. All >> drivers' delete_port_precommit() and delete_port_postcommit() methods >> are called with PortContext properties reflecting the binding state >> before deletion as shown. >> >> PortContext.original: None >> PortContext.original_bound_segment: None >> PortContext.original_bound_driver: None >> PortContext.current['binding:host_id']: previous value or None >> PortContext.current['binding:vif_type']: previous value >> PortContext.bound_segment: previous value >> PortContext.bound_driver: previous value > > Could this part of the port deletion also be done by port update? > >> >> 6) In order to ensure successful bindings are created and returned >> whenever possible, the get port and get ports operations also attempt to >> bind the port as in 4 above when binding:host_id is available but there >> is no existing successful binding in the DB. >> >> 7) We can either eliminate MechanismDriver.unbind_port(), or call it on >> the previously bound driver within the transaction in 3d and 5 above. If >> we do keep it, the old binding state must be consistently reflected in >> the PortContext as either current or original state, TBD. Since all >> drivers see unbinding as a port update where current_bound_segment is >> None and original_bound_segment is not None, calling unbind_port() seems >> redundant. >> >> 8) If bindings shouldn't spontaneously become invalid, maybe we can >> eliminate MechanismDriver.validate_bound_port(). >> >> >> I've provided a lot of details, and the above may seem complicated. But >> I think its actually much more consistent and predictable than the >> current port binding code, and implementation should be straightforward. >> >> -Bob > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev