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

Bug #1912460 reported by LIU Yulong
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Wishlist
LIU Yulong

Bug Description

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?

Changed in neutron:
importance: Undecided → Wishlist
tags: added: rfe
tags: added: qos
summary: - [QoS] add qos rule type packet per second (pps)
+ [RFE] [QoS] add qos rule type packet per second (pps)
Revision history for this message
Slawek Kaplonski (slaweq) wrote :

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?

Revision history for this message
Rodolfo Alonso (rodolfo-alonso-hernandez) wrote :

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.

Revision history for this message
LIU Yulong (dragon889) wrote :

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.

Revision history for this message
LIU Yulong (dragon889) wrote :
Revision history for this message
Rodolfo Alonso (rodolfo-alonso-hernandez) wrote :

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

Revision history for this message
LIU Yulong (dragon889) wrote :

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

Revision history for this message
Slawek Kaplonski (slaweq) wrote :

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

tags: added: rfe-triaged
removed: rfe
Revision history for this message
Slawek Kaplonski (slaweq) wrote :

Hi,

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

tags: added: rfe-approved
removed: rfe-triaged
Revision history for this message
LIU Yulong (dragon889) wrote :
Changed in neutron:
assignee: nobody → LIU Yulong (dragon889)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-specs (master)

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

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/c/openstack/neutron-lib/+/790905

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

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

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

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

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

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

Changed in neutron:
milestone: none → xena-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

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

Changed in neutron:
status: In Progress → Fix Released
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/c/openstack/neutron/+/804213

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/c/openstack/neutron-lib/+/804378

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/c/openstack/neutron/+/804380

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

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 19.0.0.0rc1

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

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

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

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/c/openstack/neutron/+/819418

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

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.

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/c/openstack/neutron/+/829145

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

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

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

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

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

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

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

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

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

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

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

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

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

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

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers