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

[Rodolfo Alonso]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/655922

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[lixiaoxiao (ohhahali)]

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?

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

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.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/711017

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)