[RFE] [QoS] add qos rule type packet per second (pps)

[LIU Yulong]

For cloud providers, to limit the packet per second (pps) of VM NIC is popular and sometimes essential. Transit large set packets for VM in physical compute hosts will consume the CPU/phy-nic performance. And for small packets, even the bandwidth is low, the pps can still be higher, which can be an attact point inside the cloud while some VMs are becoming hacked.

So for neutron, IMO, we should add such type of QoS rule, packet per second (pps). Am I missing something? Thoughts?

[Slawek Kaplonski (slaweq)]

From the API PoV I don't think there is any specific issue with such new rule type.
Can You share some more details about how You want to implement that on specific backends? Does tc supports such limitation?
And do You want to have it for ingress, egress or both directions?

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hello:

I have the same questions as Slawek:
- What backends will support this? As far as I know, packets per seconds can be limited with iptables. That means this could apply to LB or L3 QoS, but not OVS, SR-IOV or OVN because we don't use iptables on those backends (or we should not use it).
- If this is going to be a traffic shaping rule, why don't we add this parameter to the BW limit rules? Instead of a new rule, this could be an optional parameter for BW limit rule.

Regards.

[LIU Yulong (dragon889)]

Packet per second is a very general network performance metric. Like bandwidth, it is usually used to evaluate the packet forwarding performance of a device. But for a cloud, this will become a attack point. Your ovs, ovn or any other L2 drivers may get extremly high usage of CPU when user send small packet (topically 64B small) from the VM to others even the device is in lower bandwidth QoS limitation. Then your host services, other users' VMs will be under a higher failuer point. This is the main reason. For a networking developer, IMO, these background should be clear to you.

And this can be used for network quality assurance. The resource consumption of the system is determined according to the user's VM specifications. With the PPS limitation, the VM will not consume more CPU with smaller bandwidth.

@Rodolfo
For the question about how to implement this in agent side, IMO, the meter [1] can be used to achive the goal for OVN or OVS. Iptables can be used only for ml2/ovs.

[1] https://docs.pica8.com/pages/viewpage.action?pageId=5112993

And finally, for this RFE here, my original idea is to add the new rule type at first. For the real limitation drivers, IMO, we can start a new RFE for it.

[LIU Yulong (dragon889)]

For DPDK, SRIOV ml2/ovs and ml2/ovn, ovs meter is fine to use, but it may need higher kernel and ovs version.

FYI:
[1] http://workshop.netfilter.org/2017/wiki/images/d/db/Nfws-ovs-metering.pdf
[2] http://www.openvswitch.org//support/dist-docs/ovs-ofctl.8.txt
[3] https://github.com/openvswitch/ovs/blob/master/NEWS#L312
[4] https://docs.openvswitch.org/en/latest/topics/dpdk/qos/

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hi Liu:

Thanks for the info. Now I see how this could be implemented in the backend (OVS or OVN, using OF meters). As you said, that will require OpenFlow 1.3+ [1] and OVS 2.10.0, but those versions could be considered "old" enough for this feature. A sanity check should be required.

In any case, I think this could be a good feature to be implemented. But as I commented, I don't think we need a new QoS rule but a new parameter in BW limit rule. Now we have "max-kbps" and "max-burst-kbits". Another non-required parameter could be added, something like "max-packets".

Having said that, not all backends will support this parameter, so the idea of adding a new rule could be better to provide specific backend support for this rule.

I suggest to discuss this in the Neutron drivers meeting.

Regards.

[1]https://manpages.ubuntu.com/manpages/disco/man8/ovs-ofctl.8.html

[LIU Yulong (dragon889)]

I don't think it should be added to bandwidth rule as a new parameter. Here is my reasons:

Firstly, "Packet per second" is not so much related to bandwidth. The "pps" is throughput rate, and while the "bandwidth" is traffic summary. The "pps" is how many packet it can send per second, while the "bandwidth" is how large total data it can send per second. "pps" can get higher, while the "bandwidth" is not so much.

The limitation of "Packet per second" also has the "rate" and "value" for its measurement strategy [1]. And for our cloud, the direction is also a required attribute. So, IMO, because it has such attributes we should add a new type for it.

Finally, we already have some code which is to verify the QoS rule type [2][3][4][5][6], "if it is bandwidth rule, then do some work." If add new input action for it, may cause many refactor works, in case some user just create new parameter in BW limit rule while the code could only handle the old parameters.

[1] http://workshop.netfilter.org/2017/wiki/images/d/db/Nfws-ovs-metering.pdf
[2] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py#L78
[3] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py#L159
[4] https://github.com/openstack/neutron/blob/master/neutron/agent/l2/extensions/qos.py#L122
[5] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py#L70
[6] https://github.com/openstack/neutron/blob/master/neutron/agent/l3/extensions/qos/base.py#L175

[Slawek Kaplonski (slaweq)]

Lets discuss that RFE on the next drivers meeting on Friday 29.01.2021.

[Slawek Kaplonski (slaweq)]

Hi,

On our last drivers meeting we decided to approve this RFE. So please propose quick spec and go for it :)

[LIU Yulong (dragon889)]

Spec work:
https://review.opendev.org/c/openstack/neutron-specs/+/779940

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-specs/+/779940
Committed: https://opendev.org/openstack/neutron-specs/commit/149f143595e372cf5cfa612491b2ada32b46940a
Submitter: "Zuul (22348)"
Branch: master

commit 149f143595e372cf5cfa612491b2ada32b46940a
Author: LIU Yulong <email address hidden>
Date: Thu Mar 11 15:51:50 2021 +0800

    Add spec for QoS rule type packet per second

    Related-bug: #1912460
    Change-Id: Ie9e7054d333dad42a5e874a4fee383361457fde6

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit db45b9226d531ece95178e555cea1b02a9f60df7
Author: LIU Yulong <email address hidden>
Date: Wed May 12 10:57:48 2021 +0800

    Adds API extension for QoS rule type pps

    This patch adds new API extension for QoS rule
    type packet per second(pps).

    Related-bug: #1912460
    Change-Id: I4148fb3c0da6c45f24ddd4d640c2ad0d34778e51

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit 2ea7d66216f96705d7a50d39a30046193bba17f1
Author: LIU Yulong <email address hidden>
Date: Tue Jun 15 16:33:34 2021 +0800

    Bump neutron-lib to 2.12.0

    Related-bug: #1912460
    Change-Id: If86e42c7c56bb0fc571d68818febf54a29a31aa2

[OpenStack Infra (hudson-openstack)]

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

commit 8e30639452312019de17f3e1e8364f579752944e
Author: LIU Yulong <email address hidden>
Date: Wed Jun 2 15:38:02 2021 +0800

    [QoS] Add rule type packet per second (pps)

    This patch adds new API extension to QoS service plugin
    to allow CURD actions for packet rate limit (packet per
    second) rule in Neutron server side.

    NOTE: This patch will NOT implement the real functionality
    in L2/L3 backend to limit the pps.

    Co-Authored-By: NANALI <email address hidden>

    Closes-bug: #1912460
    Change-Id: Icc88accb88d9cec40c960c56f032c3c27317b42e

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit 8ca7087ed978e412c4b31e543f3c31b824fbb779
Author: LIU Yulong <email address hidden>
Date: Thu Aug 12 18:38:37 2021 +0800

    Move packet_rate_limit type to neutron-lib

    These are the TODO works of:
    neutron/services/qos/constants.py
    neutron/extensions/qos_pps_rule.py

    Related-bug: #1912460
    Change-Id: I295d8004efd563a5044631ec504d4eb9ae1be55d

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by "liuyulong <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/804380
Reason: There are continuous developments for these constants[1]. Seems neutron developers are happy to add new constants in neutron. Looks soomthly, not involved the neutron-lib. So left this as it is.

[1] https://review.opendev.org/c/openstack/neutron/+/803045/15/neutron/services/qos/constants.py

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by "liuyulong <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/804380
Reason: Restore if someday we want this.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit 053a9d24eca2c9372bd2b767504dd6b6db30643f
Author: LIU Yulong <email address hidden>
Date: Fri Aug 6 14:11:00 2021 +0800

    Add table for pps limitaion

    Table 59 will be used for pps limitation, the pipeline change is:
    all original flows with ``goto table 60`` will be changed to
    ``goto table 59``, while table 59 has a default rule is goto
    table 60. Then we can add pps flows to table 59 for all ports.

    Basic limit pipeline is:
    Ingress: packets get into br-int table 0, before send to table 60,
    in table 59, check the destanation MAC and local_vlan ID, if the
    dest is resident in this host, do the meter pps action and send
    to table 60.
    Egress: match src MAC and in_port, before send to table 60,
    in table 59, do the meter pps action and send to table 60.

    Why table 59? Because for ovs-agent flow structure, all packets
    will be send to table 60 to do next actions such as security group.
    Between table 0 and table 60, there are tables for ARP poison/spoofing
    prevention rules and MAC spoof filtering. We want similar security
    checks to take effect first, so it can drop packets before filling
    our limit queues (pps limitation based on data forwarding queue).
    And we do not want packets go through the long march of security group
    flows, in case of performance side effect when there are large amount
    of packets try to send, so limit it before goto security group flows.

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: I943f610c3b6bcf05e2e752ca3b57981f523f88a8

[OpenStack Infra (hudson-openstack)]

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

commit b4a192a74c32d70278510b0a768c42220ff03b6f
Author: LIU Yulong <email address hidden>
Date: Tue Feb 15 08:49:36 2022 +0800

    Bump python-neutronclient version to 7.8.0

    Fullstack test cases need this release [1] of python-neutronclient.
    Update the cliff version because:
        python-neutronclient 7.8.0 depends on cliff>=3.4.0
        cliff==3.4.0 requires stevedore>=2.0.1
        python-neutronclient 7.8.0 depends on osc-lib>=1.12.0

    [1] https://review.opendev.org/c/openstack/releases/+/828442

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: Ibc1fbb90bf22128cd783c8d8598fb3c49591eae2

[OpenStack Infra (hudson-openstack)]

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

commit 0232ead2c33eccc2ccd67f96a12381f2ce7fb470
Author: LIU Yulong <email address hidden>
Date: Wed Aug 11 18:44:38 2021 +0800

    Meter flows and ovsdb action for ovs bridge

    Add meter flows actions and ovsdb actions for pps
    limitation. Meter flow actions are:
    * list_meter_features
    * create_meter
    * delete_meter
    * update_meter
    * apply_meter_to_port
    * remove_meter_from_port

    Ovsdb actions are:
    * get_port_tag_by_name
    * get_value_from_other_config
    * set_value_to_other_config
    * remove_value_from_other_config

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: Idc9a2b1f39964fc3b603310ac7f22c1bc58d27f7

[OpenStack Infra (hudson-openstack)]

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

commit 5765186516ae6af3477fc1c1cdd0367e8c878189
Author: LIU Yulong <email address hidden>
Date: Thu Sep 2 16:22:03 2021 +0800

    Support pps limitation for openvswitch agent

    Add packet rate limit rule to the openvswitch QoS
    driver SUPPORTED_RULES list. This patch adds the
    ability to limit neutron port packet I/O rate. We
    will leverage the ovs meter to achieve the limitation.

    The meter action is only supoorted when datapath is
    in user mode (with ovs >= 2.7) or ovs kernel datapath with
    kernel version >= 4.15 (and ovs >= 2.10).

    [1] https://docs.openvswitch.org/en/latest/faq/releases/

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: Ib6341ad539afc9f94f1783a721cf5f793ccdc7d8

[OpenStack Infra (hudson-openstack)]

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

commit f1a082ce5066ff4b9f74a7628e1133f521423b6e
Author: LIU Yulong <email address hidden>
Date: Mon Nov 22 16:08:02 2021 +0800

    Fullstack tests of packet rate limit for ovs qos driver

    Depends-On: https://review.opendev.org/c/openstack/python-neutronclient/+/818717

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: If2d8f4e89987dcb55fcbef126a02bff4433c6aa8

[OpenStack Infra (hudson-openstack)]

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

commit b80f152edfd082de66ab474d7c8c191d8765bd99
Author: LIU Yulong <email address hidden>
Date: Tue Feb 15 14:16:31 2022 +0800

    Add policy for packet rate limit rules

    This is going to add policy rules for packet rate limit
    rules of https://review.opendev.org/c/openstack/neutron/+/796363

    Partially-Implements: bp/packet-rate-limit
    Related-Bug: #1938966
    Related-Bug: #1912460
    Change-Id: I20e45f73869d23f93acf4d7bc4cd378d1fa9a986