"tc_lib.add_tc_policy_class" must always assign a default value to "min_kbps"

Bug #1826565 reported by Rodolfo Alonso
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Medium
Rodolfo Alonso

Bug Description

This method calls Pyroute2 library. This library needs always a defined value for "rate" (minimum bandwidth). The default value assigned should be 1 (kbps).

Changed in neutron:
assignee: nobody → Rodolfo Alonso (rodolfo-alonso-hernandez)
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/655922

Changed in neutron:
status: New → In Progress
Boden R (boden)
Changed in neutron:
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/655922
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=3a4db946fdb0114b5c02f283b55c8679c159848e
Submitter: Zuul
Branch: master

commit 3a4db946fdb0114b5c02f283b55c8679c159848e
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Fri Apr 26 15:10:56 2019 +0000

    "add_tc_policy_class" must always assign a default value to "min_kbps"

    This method calls Pyroute2 library. This library needs always a defined
    value for "rate" (minimum bandwidth). The default value assigned should
    be 1 (kbps).

    Change-Id: I07a8e70248892b7fb4590fdec232fa24d8a937e6
    Closes-Bug: #1826565

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 15.0.0.0b1

This issue was fixed in the openstack/neutron 15.0.0.0b1 development milestone.

Revision history for this message
lixiaoxiao (ohhahali) wrote :

Hi:
  I used tc_lib.add_tc_policy_class fuction in neutron(stein) recently. I find that min_kbps should be not None. I fix the bugs by set min_kbps=1, but it still wrong.
The trace is "error: 'I' format requires 0 <= number <= 4294967295
ERROR pyroute2.netlink [-] error pack: I 37500000000 <type 'int'> ".

My tc class conmand is: tc_lib.add_tc_policy_class(dev_name, '1:', '1:1', 'htb', min_kbps=1, max_kbps=rule.max_kbps, burst_kb=burst_kb).

When I change min_kbps=50, it goes well. It seems that min_kbps=1 is not work. Do you met this problem?

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

Hello lixiaoxiao:

Indeed this is an error. The burst value depends on the rate (min_kbps) value. If rate is too small, then the burst value will exceed 2^32 and won't fit type.

In order to introduce a safe value, we need to check, based on the burst value, the minimum possible rate accepted.

Because burst is mandatory now, if no value is passed, we need to use the default burst rate defined in neutron-lib.services.qos.constants.DEFAULT_BURST_RATE.

Again, because of this, now we need to make max_kbps mandatory.

I'll push a patch to fix this issue.

Regards.

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/711017

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

Reviewed: https://review.opendev.org/711017
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=1e69279817b4fe978e905987db90dfca9b636b0b
Submitter: Zuul
Branch: master

commit 1e69279817b4fe978e905987db90dfca9b636b0b
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Mar 3 13:40:32 2020 +0000

    Check tc_lib.add_tc_policy_class input parameters

    All input rate parameters (ceil, burst and rate) are mandatory in
    Pyroute "tc" class [1]. "tc_lib.add_tc_policy" should provide them.

    max_kbps: Now "max_kbps" is mandatory.

    burst_kb: In case this parameter does not exist, the library uses the
    method previously defined in Neutron, consisting of asigning the 80%
    of "max_kbps".

    min_kbps: In Pyroute2 [2], the value of "burst" depends on the "rate"
    value. If the "rate" value is too small, the "burst" value will exceed
    the 2^32 limit defined in the iproute2 structure. In order to avoid
    the error described in c#4 of the related bug, the method checks if
    the provided "rate" value is greater than the minimum possible value.
    If not, the minimum value is set and a warning message is logged.

    [1]https://github.com/openstack/neutron/blob/fb2453c999f1757cc9085f4533bb2fd56d6b2245/neutron/privileged/agent/linux/tc_lib.py#L107-L108
    [2]https://github.com/svinota/pyroute2/blob/943502a95cbbbac48218d2bde9d92af462682f96/pyroute2/netlink/rtnl/tcmsg/sched_htb.py#L88-L91

    Change-Id: If60f4ea9793b0872e348a6319f2c24127e93f540
    Related-Bug: #1826565

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (stable/train)

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/720044

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

Reviewed: https://review.opendev.org/720044
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=62cbdfbaa2830a526c3390e1d389965133f76cd5
Submitter: Zuul
Branch: stable/train

commit 62cbdfbaa2830a526c3390e1d389965133f76cd5
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Tue Mar 3 13:40:32 2020 +0000

    Check tc_lib.add_tc_policy_class input parameters

    All input rate parameters (ceil, burst and rate) are mandatory in
    Pyroute "tc" class [1]. "tc_lib.add_tc_policy" should provide them.

    max_kbps: Now "max_kbps" is mandatory.

    burst_kb: In case this parameter does not exist, the library uses the
    method previously defined in Neutron, consisting of asigning the 80%
    of "max_kbps".

    min_kbps: In Pyroute2 [2], the value of "burst" depends on the "rate"
    value. If the "rate" value is too small, the "burst" value will exceed
    the 2^32 limit defined in the iproute2 structure. In order to avoid
    the error described in c#4 of the related bug, the method checks if
    the provided "rate" value is greater than the minimum possible value.
    If not, the minimum value is set and a warning message is logged.

    [1]https://github.com/openstack/neutron/blob/fb2453c999f1757cc9085f4533bb2fd56d6b2245/neutron/privileged/agent/linux/tc_lib.py#L107-L108
    [2]https://github.com/svinota/pyroute2/blob/943502a95cbbbac48218d2bde9d92af462682f96/pyroute2/netlink/rtnl/tcmsg/sched_htb.py#L88-L91

    Change-Id: If60f4ea9793b0872e348a6319f2c24127e93f540
    Related-Bug: #1826565
    (cherry picked from commit 1e69279817b4fe978e905987db90dfca9b636b0b)

tags: added: in-stable-train
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.