[OVN] min-BW rule, defined in the LSP.options field, must be accompanied with a max-BW value

[Rodolfo Alonso]

The QoS values defined in the LSP.options field are directly translated to a TC command and set in the interface. There are 3 values [1]:
* qos_min_rate
* qos_max_rate
* qos_burst

Neutron is currently using "qos_min_rate" only. The max-BW and DSCP QoS rules are defined with QoS registers that match an input/output port traffic.

As specified in [2], the "rate" parameter ("qos_max_rate") is mandatory. The "ceil" parameter ("qos_min_rate") is not. Neutron needs to provide a default "qos_max_rate" for any port where "qos_min_rate" is configured (as commented, we are only using "qos_min_rate").

Jira ticket: https://issues.redhat.com/browse/OSPRH-11035

[1]https://github.com/ovn-org/ovn/blob/1bec9e3ddd8500793b52e11c3dc1f41ef1f48591/ovn-nb.xml#L1155-L1168
[2]https://man7.org/linux/man-pages/man8/tc-htb.8.html

[Bence Romsics (bence-romsics)]

Don't we have the relationships the other way around?

guaranteed bw - qos_min_rate in ovn - rate in tc
shaped bw - qos_max_rate in ovn - ceil in tc

But anyway I see your point. This must break setting ceil without rate. But while broken I guess this can be worked around by providing both in the API.

In a fix we can provide a minimal (0 if allowed) rate when there's no user input coming from the API.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hi Bence:

The "tc" [1] defines only "rate" as mandatory (disregard the "burst" parameter, check the parameter description down in this page). That means:
* If we define max-bw, only "qos_max_rate" is needed (and maybe burst)
* If we define min-bw, we need "qos_min_rate" and a value of "qos_max_rate" ("qos_max_rate" >= "qos_min_rate" always).

In any case, in Neutron we only use "LSP.options.qos_xxx_rate" for min-bw rules. For max-bw and DSCP we use QoS registers that filter the traffic depending on the ingress/egress port and apply the needed traffic shaping (actually policing). This is why we can't fix that providing max-bw in the API, this rule will generate a OVN QoS register but won't populate the "LSP.options.qos_max_rate".

In this case we should provide a default "LSP.options.qos_max_rate" value, big enough not to interfere with the traffic egressing the interface.

Regards.

[1]https://man7.org/linux/man-pages/man8/tc-htb.8.html

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/938120

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/934418
Committed: https://opendev.org/openstack/neutron/commit/526f238fa47c8e66b4b3dd300b1630e3fc848619
Submitter: "Zuul (22348)"
Branch: master

commit 526f238fa47c8e66b4b3dd300b1630e3fc848619
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Nov 5 15:42:32 2024 +0000

    [OVN] QoS max and min rules should be defined in LSP for phynet ports

    The ML2/OVN QoS enforcement for physical network ports (flat, VLAN) is
    done in the localnet port of the physical bridge via TC commands.

    A physical network will have a physical bridge in each compute node.
    This physical bridge will have a NIC interface that will provide
    connectivity. On this interface (localnet port), OVN enforces the QoS
    mininum and maximum rate rules (**only egress**), using TC commands
    on the interface. It creates a HTB qdisc and a class per port with
    QoS rules. Each class, that will match the traffic of the port, will
    have a "ceil" and "rate" values [1].

    OVN uses the information stored in the "Logical_Switch_Port.options"
    field, stored as a dictionary, with the keys [2]:
    * qos_min_rate
    * qos_max_rate
    * qos_burst

    This patch is:
    * Moving the max and min rate configuration from the QoS registers
      to the "Logical_Switch_Port.options" dictionary.
    * Logging a warning when a min rate option is defined in a port hosted
      in a tunnelled network (not supported).

    This change was documented in [3], in OVN version 23.06.0

    [1]https://man7.org/linux/man-pages/man8/tc-htb.8.html
    [2]https://github.com/ovn-org/ovn/blob/1bec9e3ddd8500793b52e11c3dc1f41ef1f48591/ovn-nb.xml#L1155-L1168
    [3]https://github.com/ovn-org/ovn/commit/87514ac04271e87c3ad011599378d22ab6bed43c

    Closes-Bug: #2015376
    Change-Id: I4581d31ba04c0d2f3f45ee6d07bcaba82fb038d4

[OpenStack Infra (hudson-openstack)]

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

commit e6c789b196f7e069bbd4224d15edda6f40ae81f8
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Fri Dec 13 12:54:58 2024 +0000

    [OVN] Use the Neutron port ID in ``UpdateLSwitchPortQosOptionsCommand``

    Instead of using the Logical_Switch_Port UUID, that requires to
    retrieve it first from the OVN database, the command
    ``UpdateLSwitchPortQosOptionsCommand`` accepts the Neutron port ID
    (that is the Logical_Switch_Port.name); the command uses ``api.lookup``
    for the ``Logical_Switch_Port`` that accepts both the UUID or the
    name.

    Related-Bug: #2015376
    Change-Id: I433dfaa0a97d1f0aec202151caf13272b79e0a4a

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/938120
Committed: https://opendev.org/openstack/neutron/commit/47d3859c52ff6426520c6771d6a7afd5db3bb809
Submitter: "Zuul (22348)"
Branch: master

commit 47d3859c52ff6426520c6771d6a7afd5db3bb809
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Fri Dec 20 11:15:14 2024 +0100

    [OVN] Improve ``_ovn_qos_rule`` method, removing unnecesary argument

    Related-Bug: #2015376
    Change-Id: I75b1064303078941faee1d8ad2b5e6a385dbaf6b

[OpenStack Infra (hudson-openstack)]

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