QoS policies with minimum-bandwidth rule should be rejected on non-physnet ports/networks

Bug #1819029 reported by Bence Romsics
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Undecided
Slawek Kaplonski

Bug Description

We seem to have forgot to reject some API operations that are actually not supported (and weren't planned to be supported) by the Stein implementation of the Guaranteed Minimum Bandwidth feature.

That is QoS policies with a minimum-bandwidth rule should not be used on ports/networks that are not backed by a physnet. But currently we allow this:

$ openstack network show private | egrep provider
| provider:network_type | vxlan
| provider:physical_network | None
| provider:segmentation_id | 1062

$ openstack network qos policy create policy0
$ openstack network qos rule create policy0 --type minimum-bandwidth --min-kbps 1000 --egress
$ openstack port create port0 --network private --qos-policy policy0

The port-create seems to work today, but on non-physnet networks there's no guarantee at all (as planned in the blueprint). Therefore I think API operations like these should be rejected now, otherwise we may set up false expectations in our users.

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

Changed in neutron:
status: New → In Progress
Akihiro Motoki (amotoki)
Changed in neutron:
milestone: none → stein-rc1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-tempest-plugin (master)

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

Changed in neutron:
assignee: Bence Romsics (bence-romsics) → Lajos Katona (lajos-katona)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron-lib (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-tempest-plugin (master)

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

Changed in neutron:
assignee: Lajos Katona (lajos-katona) → Bence Romsics (bence-romsics)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron-lib (master)

Change abandoned by Lajos Katona (<email address hidden>) on branch: master
Review: https://review.openstack.org/644796
Reason: As this feature will be available from Stein, we don't need this shim extension, see: http://eavesdrop.openstack.org/irclogs/%23openstack-neutron/%23openstack-neutron.2019-03-20.log.html#t2019-03-20T18:25:56

Changed in neutron:
assignee: Bence Romsics (bence-romsics) → Lajos Katona (lajos-katona)
Changed in neutron:
assignee: Lajos Katona (lajos-katona) → Bence Romsics (bence-romsics)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-tempest-plugin (master)

Reviewed: https://review.openstack.org/644847
Committed: https://git.openstack.org/cgit/openstack/neutron-tempest-plugin/commit/?id=c1052e777ded77ff565c78b3e106d4bbf782d7c5
Submitter: Zuul
Branch: master

commit c1052e777ded77ff565c78b3e106d4bbf782d7c5
Author: Lajos Katona <email address hidden>
Date: Wed Mar 20 13:16:01 2019 +0100

    Remove test_port_resource_request_no_provider_net

    PortTestCasesResourceRequest.test_port_resource_request_no_provider_net
    assumed that creating a port with QoS mimimum bandwidth policy rule on a
    network without physnet is allowed, but it can't be as that would mean
    that the user got a bandwidth guarantee, but the current placement based
    bandwidth aware scheduling works only with physnet based ports.
    This patch removes this test assuming that port creation with QoS
    policies tested by legacy test for Stein-n branches.

    Change-Id: Iae7ff22e94029d19895ac46ab9bcc9f7a3c4a250
    Related-Bug: #1819029
    Needed-By: https://review.openstack.org/641712

Changed in neutron:
assignee: Bence Romsics (bence-romsics) → Akihiro Motoki (amotoki)
Changed in neutron:
assignee: Akihiro Motoki (amotoki) → Slawek Kaplonski (slaweq)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/641712
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=d1696619c5636bc74d5238520db80b01cb81921d
Submitter: Zuul
Branch: master

commit d1696619c5636bc74d5238520db80b01cb81921d
Author: Bence Romsics <email address hidden>
Date: Thu Mar 7 10:07:12 2019 +0100

    Reject min-bw rule operations on non-physnet networks/ports

    Change-Id: I54d421d0993bd9515ab5ba32f75f40d1ef46eccb
    Closes-Bug: #1819029
    Depends-On: https://review.openstack.org/644847

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 14.0.0.0rc1

This issue was fixed in the openstack/neutron 14.0.0.0rc1 release candidate.

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

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

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

Reviewed: https://review.openstack.org/644515
Committed: https://git.openstack.org/cgit/openstack/neutron-tempest-plugin/commit/?id=92fbbab347f4ea254f976e8dd68f1c06c1cfee59
Submitter: Zuul
Branch: master

commit 92fbbab347f4ea254f976e8dd68f1c06c1cfee59
Author: Lajos Katona <email address hidden>
Date: Mon Mar 18 13:17:11 2019 +0100

    Min bw rule operations should be rejected on non-physnet ports/networks

    Change-Id: I329ed7697650c55a8b71ba46481c6584db1a1bfb
    Depends-On: https://review.openstack.org/641712
    Related-Bug: #1819029

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

Reviewed: https://review.openstack.org/647379
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=0db0fb71dec900f8cf92c84223a368de80d0ad1d
Submitter: Zuul
Branch: master

commit 0db0fb71dec900f8cf92c84223a368de80d0ad1d
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Mar 25 10:23:50 2019 +0000

    Specify physical network in QoS fullstack tests

    Since [1], those ports and networks without a physical network defined,
    will reject a QoS minimum bandwidth rule.

    [1] https://review.openstack.org/#/c/641712/

    Change-Id: If631ed20db174b988a53d311ec9e4bf9152c17c3
    Related-Bug: #1819029

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (stable/stein)

Related fix proposed to branch: stable/stein
Review: https://review.openstack.org/651458

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (stable/stein)

Reviewed: https://review.openstack.org/651458
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=f2bfb65b2b0292fbbe201dd93f268697ce99b5eb
Submitter: Zuul
Branch: stable/stein

commit f2bfb65b2b0292fbbe201dd93f268697ce99b5eb
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Mon Mar 25 10:23:50 2019 +0000

    Specify physical network in QoS fullstack tests

    Since [1], those ports and networks without a physical network defined,
    will reject a QoS minimum bandwidth rule.

    [1] https://review.openstack.org/#/c/641712/

    Change-Id: If631ed20db174b988a53d311ec9e4bf9152c17c3
    Related-Bug: #1819029
    Closes-Bug: #1824095
    (cherry picked from commit 0db0fb71dec900f8cf92c84223a368de80d0ad1d)

tags: added: in-stable-stein
tags: added: neutron-proactive-backport-potential
tags: removed: neutron-proactive-backport-potential stein-rc-potential
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.opendev.org/705694

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

Related fix proposed to branch: master
Review: https://review.opendev.org/705695

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

Reviewed: https://review.opendev.org/705694
Committed: https://git.openstack.org/cgit/openstack/neutron-lib/commit/?id=6f3fb7fc8199b8c374e6e97a2e5a617a47b30671
Submitter: Zuul
Branch: master

commit 6f3fb7fc8199b8c374e6e97a2e5a617a47b30671
Author: Bence Romsics <email address hidden>
Date: Tue Feb 4 14:02:22 2020 +0100

    Extend qos DriverBase with validate_rule_for_port()

    The openvswitch qos driver supports the min-bw qos rule only on
    networks backed by physnets. Currently the qos driver interface does
    not have a method that takes both the policy/rule and the network/port,
    therefore we don't have enough information in the driver to tell if
    we support the min-bw rule for a port or not. This change introduces
    validate_rule_for_port(..., rule, port) so we have all the information
    to tell.

    The base method validate_rule_for_port() is concrete and returns True
    for backwards compatibility.

    Change-Id: I58a47bb895bede219cfeebbf0665aaf01660ce89
    Needed-By: https://review.opendev.org/705695
    Partial-Bug: #1861442
    Related-Bug: #1819029

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

Reviewed: https://review.opendev.org/705695
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=321afc8f897c8132ddc6edd2b59285111d0d73e0
Submitter: Zuul
Branch: master

commit 321afc8f897c8132ddc6edd2b59285111d0d73e0
Author: Bence Romsics <email address hidden>
Date: Tue Feb 4 14:02:30 2020 +0100

    Move rejection of min-bw rule on non-physnet port to the ovs qos driver

    The ovs qos driver only supports the min-bw rule on physnet ports.
    However other (future) qos drivers may support the min-bw rule on
    non-physnet ports too, therefore implementing the rejection of the
    unsupported combination in the qos plugin was a bug.

    This change moves the rejection from the qos plugin to the ovs qos
    driver.

    Change-Id: I02d77a03c411dc8ab303a7d7b53d7ea93cc9f4c6
    Closes-Bug: #1861442
    Depends-On: https://review.opendev.org/705694
    Related-Bug: #1819029

tags: added: neutron-proactive-backport-potential
tags: removed: neutron-proactive-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/neutron/+/787492

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (stable/train)

Change abandoned by "Slawek Kaplonski <email address hidden>" on branch: stable/train
Review: https://review.opendev.org/c/openstack/neutron/+/787492
Reason: This review is > 4 weeks without comment and currently blocked by a core reviewer with a -2. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and contacting the reviewer with the -2 on this review to ensure you address their concerns.

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.