ovs plugin incorrectly assumes datapath is system for vif_ovs

Bug #1632372 reported by sean mooney
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Undecided
Unassigned
neutron
Fix Released
Undecided
Rodolfo Alonso
os-vif
Fix Released
Medium
Rodolfo Alonso

Bug Description

the ovs plugin currently uses hardcoded datapath types when plugin interfaces.

https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L100-L101

https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L127-L128

https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L135-L136

https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L149-L150

while this works in the general case it dose not work if ovs is configured to use the netdev datapath without dpdk. This can be done by setting the datapath type in the neutron agent config
https://github.com/openstack/neutron/blob/f0ca030595cb7484be338c5678738b2e536b2369/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py#L75-L80 and is required to support ovs on bsd where the
kernel datapath is not available.

this bug was introduced as part of https://github.com/openstack/os-vif/commit/3d62d8e23c99bdf4fcfaab15c6dd2e341d5c9bfa

the observed behavior is as follows.

the ovs neutron agent is configured to managed ovs with datapath set to netdev.
the user boots a vm on this host.
os-vif receives a request to plug a openvswich interface.

as part of the plug_bridge method
ensure_ovs_bridge is called with br-int as the bridge name and system as the datapath.
https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L127-L128

the bridge name could be different if we are plunging an interface for the vlan aware vms spec.

the call to ensure_ovs_bridge result in the following command being executed.
'ovs-vsctl --may-exist add-br br-int -- set Bridge br-int datapath_type=system'

this has the effect of changing the datapath type on the bridge from netdev to system.

if the kernel module is loaded the plugging of the vif will succeed but network connectivity will be broken between hosts. This is because the br-int or trunk bridge will not be able to comunicate via patch ports to the other bridges as the datapaths do not match.

if the kernel module is not loaded the system datapath will not be avaiable to activate. in this the neutron ovs neutron agent will lose the ablity to program ovs as all ovs-ofctl commands will fail or the native driver equivalent. in this case the vm will fail to spawn and any existing vms will lose external connectivy.

as a result on a system without the kernel module such as bsd or windows/linux if the module is just not loaded the use of the netev datapath is not possible with vif_ovs port type. This is a regression from mitaka functionality which was introduced in os-vif 1.2.0 by https://github.com/openstack/os-vif/commit/3d62d8e23c99bdf4fcfaab15c6dd2e341d5c9bfa.

Revision history for this message
sean mooney (sean-k-mooney) wrote :

possible solutions.

1.) introduce a config variable to set the datapath type for the VIF_OVS type. this would mirror the ovs agent datapath type variable in neutron
https://github.com/openstack/neutron/blob/f0ca030595cb7484be338c5678738b2e536b2369/neutron/plugins/ml2/drivers/openvswitch/agent/common/config.py#L75-L80
this would require no changes to nova or neutron but would adress the regression in fuctionality but require operators to set the config value if they happened to use the netdev datapath without dpdk e.g. not use vhost-user interfaces.

2.) modify the neutron ml2 dirver here to pass the datapath type to nova via the vif_details.
https://github.com/openstack/neutron/blob/f0ca030595cb7484be338c5678738b2e536b2369/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#L108-L128
then modify the os-vif network object
https://github.com/openstack/os-vif/blob/master/os_vif/objects/network.py to store the datapath and modify the os-vif ovs plugin to read it just as we do with the bridge name here
https://github.com/openstack/os-vif/blob/9fb7fe512902a37432e38d884b8be59ce91582db/vif_plug_ovs/ovs.py#L100-L101
also need to modify nova to retrieve the datapath type form the vif_details and populated the value in the os-vif network object.

3.) since neutron requires all bridges to be of the same datapath type
so that patch ports will work we can list the active datapath types using ovs-appctl.
if only one datapath type is active then that is the type that should be used for the bridge.
if both datapaths are active we could raise an exception and refuse to plug the interface or make a best effort guess.

Revision history for this message
sean mooney (sean-k-mooney) wrote :

solution considerations

option one has the advantage of confining the change change to os-vif.
option two is the correct approach long term as it dose not require any config change on upgrade.
option three is a compromise as while we do not need to make any change on upgrade we cannot hanel
the case where both datapath have been active on the system as we do not have the authroritve infomation
from neutron to select the datapath.

option one could be implemented in a day or two.
option two would not take much longer to pull together once i figure out exactly where in nova to extend the network object.
option three would be similar to option one in timescale.

option two would require back porting in stable newton in all three project where as option one just requires a requirements change in nova/neutron which would have been needed to use the windows support anyway. option three would be the same as option one in this regard.

Revision history for this message
Claudiu Belu (cbelu) wrote :

Hello,

Any updates on this bug?

tags: added: ovs
Changed in neutron:
status: New → Invalid
Revision history for this message
Anindita Das (anindita-das) wrote :

This bug is marked as invalid as nothing can be done in neutron for this bug.
Here is the conversation [1] confirming this.

[1] http://eavesdrop.openstack.org/irclogs/%23openstack-neutron/%23openstack-neutron.2016-10-20.log.html#t2016-10-20T22:53:15

Sean Dague (sdague)
Changed in os-vif:
status: New → Confirmed
Changed in nova:
status: New → Incomplete
Revision history for this message
sean mooney (sean-k-mooney) wrote :

i will try to address this in the pike cycle.
preferable before pike 1 if i find time.

alternatively patches are welcome :)

Changed in os-vif:
importance: Undecided → Medium
Changed in os-vif:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-lib (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/474248

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to os-vif (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/474581

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

Changed in neutron:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
status: Invalid → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/474892

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-vif (master)

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

Changed in os-vif:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-vif (master)

Change abandoned by Rodolfo Alonso Hernandez (<email address hidden>) on branch: master
Review: https://review.openstack.org/474581
Reason: Superseded by https://review.openstack.org/#/c/474914

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

Reviewed: https://review.openstack.org/474248
Committed: https://git.openstack.org/cgit/openstack/neutron-lib/commit/?id=70524bb62f26c109fc7bac18be2b27f333063643
Submitter: Jenkins
Branch: master

commit 70524bb62f26c109fc7bac18be2b27f333063643
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Wed Jun 14 16:06:56 2017 +0100

    Add OVS_DATAPATH_TYPE in portbindings constants

    Added OVS_DATAPATH_TYPE constant in portbindings. This parameter will
    be added to OVS backend VIF_DETAILS. This string parameter is used to
    define if the bridge uses kernel or userspace datapath.

    Change-Id: Ie523c821995c046c7f77783a34e75053fc0abb3d
    Related-Bug: #1632372

Revision history for this message
Sean Dague (sdague) wrote :

Automatically discovered version mitaka in description. If this is incorrect, please update the description to include 'nova version: ...'

tags: added: openstack-version.mitaka
Sean Dague (sdague)
Changed in nova:
status: Incomplete → In Progress
Changed in neutron:
assignee: Rodolfo Alonso (rodolfo-alonso-hernandez) → Kevin Benton (kevinbenton)
Changed in neutron:
assignee: Kevin Benton (kevinbenton) → Rodolfo Alonso (rodolfo-alonso-hernandez)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

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

commit 25af4bba7c04bb184704978f9f207dd292400563
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Wed Jun 14 12:27:15 2017 +0100

    Add datapath_type to vif_details in OVS driver

    Added datapath_type to vif_details returned by OVS
    mech driver.

    Depends-On: Ie523c821995c046c7f77783a34e75053fc0abb3d
    Partial-Bug: #1632372

    Change-Id: Ief83150caf1a32a2c043b0245b36e5ebc3a16379

tags: added: neutron-proactive-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to os-vif (master)

Reviewed: https://review.openstack.org/474914
Committed: https://git.openstack.org/cgit/openstack/os-vif/commit/?id=3b606b06e138787c1b1ffd1dd0fb43e971ee7e29
Submitter: Jenkins
Branch: master

commit 3b606b06e138787c1b1ffd1dd0fb43e971ee7e29
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 15 16:00:33 2017 +0100

    Read datapath_type from VIF object

    When vif_plug_ovs.linux_net.ensure_ovs_bridge function is called, the
    datapath_type of the bridge should be read from the VIF object, instead
    of statically coercing a value.

    Closes-Bug: #1632372

    Change-Id: Ia813c39e2917ff373f8e1f85c75fc22d109c94d3

Changed in os-vif:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.openstack.org/474892
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=ce8bf6734e554b116e82e924cbe81a5968441926
Submitter: Jenkins
Branch: master

commit ce8bf6734e554b116e82e924cbe81a5968441926
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu Jun 15 14:54:05 2017 +0100

    Add datapath type information to OVS vif objects

    Added a new parameter to vif objects for modelling the following
    OVS port profiles:
      - VIFPortProfileOpenVSwitch
      - VIFPortProfileFPOpenVSwitch

    This new parameter named "datapath_type" is a string
    coerced variable. The posible values stored in this
    field are defined in [1].

    This information is required by ``os-vif`` to define or ensure
    correctly the datapath type of the OVS bridge when plugging a
    port to this bridge. Currently the datapath type is hardcoded
    in ``os-vif`` and doesn't work with anything other than the kernel
    OVS versions.

    [1] https://github.com/openstack/neutron/blob/11.0.0/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py#L129

    Depends-On: Ia813c39e2917ff373f8e1f85c75fc22d109c94d3
    Related-Bug: #1632372

    Change-Id: I6194606498bff177d9856ed9f0c86ca0a4eef32f

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/os-vif 1.8.0

This issue was fixed in the openstack/os-vif 1.8.0 release.

Changed in nova:
status: In Progress → Fix Released
Changed in neutron:
status: In Progress → Fix Released
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.