Change a QoS policy on a bound port does not handle minimum bandwidth direction change

[Balazs Gibizer]

When changing the QoS policy having a minimum bandwidth rule on a bound port to another policy where the direction of the min bw rule has changed then neutron accepts the request but fails to update the bandwidth resource allocation in placement accordingly.

Reproduction
------------

Here is a tempest test reproducing the issue https://review.opendev.org/c/openstack/tempest/+/809198

Expected behavior
-----------------
Neutron allocates resources in placement according to the new direction

Actual behavior
---------------
Neutron does not move the resource allocation from one resource class to the other (representing the directions)

Affected version
----------------
Reproduced on recent master in devstack.

stack@master0:~/neutron$ git log --oneline
6db2619628 Merge "[DVR] Set arp entries only for single IPs given as allowed addr pair"

Bug probably exists since neutron added support for changing QoS policy on bound ports with minimum bandwidth rules.

[Akihiro Motoki (amotoki)]

Looking at the proposed review https://review.opendev.org/c/openstack/neutron/+/805637/ which mentions "Closed-Bug", it is in the patch chain of min pps rule. I wonder what is the actual dependency. Can we wait for the min pps feature to fix this bug? I am not sure what is the expected thing.

[Balazs Gibizer (balazs-gibizer)]

@Akihiro: the min pps feature changes how the QoS policy update on a bound port is handled and that change fixes the bug on master. However the bug exists on stable branches so a different not feature related fix is needed for stable.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hello Balazs:

Are you sure the QoS policy is updated? According to [1], we reject any update for QoS rules when (1) this rule is min-bw and (2) is bound.

BTW, I don't see in [2] those jobs failing. Can you point me in the right direction?

Regards.

[1]https://review.opendev.org/c/openstack/neutron/+/636970
[2]https://review.opendev.org/c/openstack/tempest/+/809198

[Przemyslaw Szczerbik (pszczerbik)]

Hi Rodolfo,

The issue is about replacing QoS policy of a bound port. [1] introduced support for this. Steps to reproduce:

openstack network qos policy create qosp_min_bw_e
openstack network qos rule create --type minimum-bandwidth --egress --min-kbps 1 qosp_min_bw_e
openstack port create port0 --network net-physnet0 --qos-policy qosp_min_bw_e
openstack --os-compute-api-version 2.72 server create --image cirros-0.5.2-x86_64-disk --flavor cirros256 --nic port-id=port0 vm0 --wait
openstack network qos policy create qosp_min_bw_i
openstack network qos rule create --type minimum-bandwidth --ingress --min-kbps 1 qosp_min_bw_i
openstack port set port0 --qos-policy qosp_min_bw_i

Update of a QoS policy that is attached to a bound port is still not supported.

[1] https://review.opendev.org/c/openstack/neutron/+/747774

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-lib/+/813660

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/805637
Committed: https://opendev.org/openstack/neutron/commit/8db15cb2f3dec37441103df15ff28c7e72de682c
Submitter: "Zuul (22348)"
Branch: master

commit 8db15cb2f3dec37441103df15ff28c7e72de682c
Author: Przemyslaw Szczerbik <email address hidden>
Date: Thu Aug 5 13:48:50 2021 +0200

    Add port-resource-request-groups extension

    port-resource-request-groups extension provides support for the
    new format of resource_request. The new format allows to request
    multiple groups of resources and traits from the same RP subtree.

    Closes-Bug: #1943724
    Partial-Bug: #1922237
    Depends-On: https://review.opendev.org/c/openstack/tempest/+/809168/
    See-Also: https://review.opendev.org/785236
    Change-Id: I99a49b107b1872ddf83d1d8497a26a8d728feb07

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/neutron-lib/+/817528

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/neutron/+/817529

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-lib/+/813660
Committed: https://opendev.org/openstack/neutron-lib/commit/d2bd7760efdef2c47ae67923aec37f914f8400ce
Submitter: "Zuul (22348)"
Branch: master

commit d2bd7760efdef2c47ae67923aec37f914f8400ce
Author: Przemyslaw Szczerbik <email address hidden>
Date: Tue Sep 7 15:37:07 2021 +0200

    Make update_qos_minbw_allocation() more generic

    update_qos_minbw_allocation() function is used to update Placement
    allocation. Initially, only minimum bandwidth rule allocated resources
    in Placement, but with the introduction of minimum packet rate rule
    that's changed. Because of that, we should rename the function to make
    it more generic and to avoid confusion.

    The second problem with this function is that it allows to update
    resources only of a single RP at a time, even if multiple RPs are
    associated with the same consumer UUID. With introduction of a
    minimum packet rate rule it makes sense to allow to update resources of
    multiple RPs in a single API call. To accommodate this, we need to
    slightly modify arguments this function takes, and embed RP UUID
    in alloc_diff, rather than pass it as a separate parameter.

    Addresses TODO comment from [1].

    [1] https://opendev.org/openstack/neutron/src/branch/master/neutron/services/qos/qos_plugin.py#L68

    Partial-Bug: #1943724
    Related-Bug: #1922237
    See-Also: https://review.opendev.org/785236
    Change-Id: Ie28b95e8ed351ab88db1fc75c83a02c474582e0b

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-lib/+/817528
Committed: https://opendev.org/openstack/neutron-lib/commit/997be4d13b183b5f9bac1462da5aeaec0c44714d
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 997be4d13b183b5f9bac1462da5aeaec0c44714d
Author: Przemyslaw Szczerbik <email address hidden>
Date: Tue Nov 2 11:24:44 2021 +0100

    [stable-only] Fix allocation update for min bw rule with different direction

    Neutron API supports QoS policy change on a bound port. The new QoS
    policy can contain a different set of rules, or different values. In
    case of QoS minimum bandwidth rule, it can be min_kbps or direction
    attribute. If the new QoS minimum bandwidth rule is different than the
    one in previous QoS policy, Placement allocation has to be updated.

    An issue arises when QoS minimum bandwidth rule direction is changed.
    Ingress and egress directions are represented with separate resource
    classes: NET_BW_IGR_KILOBIT_PER_SEC and NET_BW_EGR_KILOBIT_PER_SEC
    respectively. During direction change, resource class representing old
    direction is freed and a resource class representing the new direction
    is requested. update_qos_minbw_allocation() implementation always
    expects requested resource class to be present in the allocation that
    is being updated, but that assumption is wrong. If current Placement
    allocation record doesn't contain particular resource class, we need to
    use 0 as a default value.

    Similarly, we should defer deletion of RP from request body until all
    items from minbw_alloc_diff are processed. Only then we can be certain
    that this RR is not going to have any resources.

    This commit is based on 47a08e36bf864806ca71ea77ffcf6602d4e5db04.

    Partial-Bug: #1943724
    Change-Id: I9a0df4d461c0e73ba07ca18648f16a982a2e267b

[yatin (yatinkarel)]

Xena jobs failing since tempest test is added https://zuul.opendev.org/t/openstack/builds?job_name=neutron-ovs-tempest-multinode-full&project=openstack%2Fneutron&branch=stable%2Fxena&skip=0 , seems https://review.opendev.org/c/openstack/neutron/+/817529 will clear it.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/817529
Committed: https://opendev.org/openstack/neutron/commit/fb50dc2a10f3711781e65dd175946456cfe0d1ed
Submitter: "Zuul (22348)"
Branch: stable/xena

commit fb50dc2a10f3711781e65dd175946456cfe0d1ed
Author: Przemyslaw Szczerbik <email address hidden>
Date: Wed Nov 3 11:03:43 2021 +0100

    [stable-only] Update Placement allocation when min bw rule direction is changed

    This patch fixes logic responsible for detecting if Placement
    allocation needs to be updated. This commit is based on
    8db15cb2f3dec37441103df15ff28c7e72de682c.

    Depends-On: https://review.opendev.org/c/openstack/neutron-lib/+/817528
    Closes-Bug: #1943724
    Change-Id: I110772330c0e7298a09b94212f7d56600b5a9bde

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 19.2.0 release.