os-vif should provide a way to set a default qos qdisc on a ovs port to avoid regressions across kernel versions and configs.

Bug #2017868 reported by sean mooney
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
os-vif
Status tracked in 2023.2
2023.1
In Progress
High
Unassigned
2023.2
Fix Released
High
sean mooney
Wallaby
In Progress
High
Unassigned
Yoga
In Progress
High
Unassigned
Zed
In Progress
High
Unassigned

Bug Description

in the past when hybrid_plug=true was set on a port libvirt would generate the XML using the bridge option with virtual <virtualport type='openvswitch'>

<interface type='bridge'>
  <source bridge='ovsbr'/>
  <virtualport type='openvswitch'>
    <parameters profileid='menial' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
  </virtualport>
</interface>

if hybrid_plug=false nova woudl generate
the XML without the <virtualport type='openvswitch'> element

  <interface type='bridge'>
    <source bridge='br1'/>
    <target dev='vnet7'/>
    <mac address="00:11:22:33:44:55"/>
  </interface>

a undocumented side effect of <virtualport type='openvswitch'> is that libvirt set the qdisc on the created tapp to linux-noop https://libvirt.org/formatdomain.html#bridge-to-lan

when we fixed https://bugs.launchpad.net/nova/+bug/1734320 by delegating port plugging to os vif we did this by changing the interface type generated by nova to ethernet.

<interface type='ethernet'>
    <target dev='mytap1' managed='no'/>
    <model type='virtio'/>
</interface>

https://libvirt.org/formatdomain.html#generic-ethernet-connection

as a result the undocumented side effect of the <virtualport type='openvswitch'> element now no longer takes effect for any ovs ports.

in more recent kernels or if you set a custom default qdisc vai sysctl this can lead to significant throughput reductions for kernel ovs. for small to medium packets this can reduce the throughput to as low as 10% of the previous behaviour and still have a 50% reduction for jumbo frames.

this is is a significant regression and should be corrected.

in the past this has likely accounted for some of the performance delta between hybrid_plug=true and hybrid_plug=false.

ovs provides a way to set the qdic as an option on the port which it will implement on the underlying netdev.

given the previously inconsistent nature we should ensure the same qdics is applied in both the hybrid_plug True or False case but we need to be careful to not set this for ovs ports on windows or vhost-user ports.

With the above context we should make the default qdisc configurable via a new os-vif config option as a pragmatic back portable solution. Eventually, we can work with Neutron to enable a more granular per port qdisc feature if desirable but that is out of scope of this bug.

Tags: ovs
tags: added: ovs
summary: - os-vif should provide a way to set a defult qos profiel on port to avoid
- regresions across kernel versions and configs.
+ os-vif should provide a way to set a default qos qdisc on a ovs port to
+ avoid regressions across kernel versions and configs.
Revision history for this message
sean mooney (sean-k-mooney) wrote :

just cross-reference our downstream tracker fo this since that is also public
https://issues.redhat.com/browse/OSPRH-183

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

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/os-vif/+/881751

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

Reviewed: https://review.opendev.org/c/openstack/os-vif/+/881751
Committed: https://opendev.org/openstack/os-vif/commit/c0d101aa81cff200e1db2a0746598b72e26748e4
Submitter: "Zuul (22348)"
Branch: master

commit c0d101aa81cff200e1db2a0746598b72e26748e4
Author: Sean Mooney <email address hidden>
Date: Tue May 9 19:52:48 2023 +0100

    set default qos policy

    This change modifies the os-vif ovs plugin to set a default
    tc qdisc on ovs interface when the host os is not windows
    and the system datapath is used.

    This change fixes a "silent" bug in the functional test code due
    to a change in an ovsdbapp function signiture to accpet a new paramater.

    Closes-Bug: #2017868
    Change-Id: Id9ef7074634a0f23d67a4401fa8fca363b51bb43

Changed in os-vif:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to os-vif (stable/2023.1)

Fix proposed to branch: stable/2023.1
Review: https://review.opendev.org/c/openstack/os-vif/+/883015

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

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/os-vif/+/883016

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

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/os-vif/+/883071

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/os-vif/+/886710

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

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/os-vif/+/886716

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

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/os-vif/+/886778

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

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-vif (stable/yoga)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/yoga
Review: https://review.opendev.org/c/openstack/os-vif/+/883071
Reason: Abandoning as this is a duplicate with a mismatching change ID. The proper cherry picks can be found with change ID: Id9ef7074634a0f23d67a4401fa8fca363b51bb43

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/yoga
Review: https://review.opendev.org/c/openstack/os-vif/+/886710
Reason: stable/yoga branch of openstack/os-vif is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/yoga if you want to further work on this patch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-vif (stable/wallaby)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/os-vif/+/886778
Reason: stable/wallaby branch of openstack/os-vif is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/wallaby if you want to further work on this patch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-vif (stable/xena)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/xena
Review: https://review.opendev.org/c/openstack/os-vif/+/886716
Reason: stable/xena branch of openstack/os-vif is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/xena if you want to further work on this patch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on os-vif (stable/zed)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/zed
Review: https://review.opendev.org/c/openstack/os-vif/+/883016
Reason: stable/zed branch of openstack/os-vif is about to be deleted. To be able to do that, all open patches need to be abandoned. Please cherry pick the patch to unmaintained/zed if you want to further work on this patch.

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.