[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
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.

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

Other bug subscribers