Merge pull request #194 from JonathanLennox/fix-cburst-size

Set htb burst and cburst size to the minimum useful size.

Author: thombashi

tcconfig/_netem_param.py

@@ -40,6 +40,14 @@ class NetemParameter:
     def bandwidth_rate(self):
         return self.__bandwidth_rate
 
+    @property
+    def mtu(self):
+        return self.__mtu
+
+    @property
+    def burst(self):
+        return self.__burst
+
     def __init__(
         self,
         device,
@@ -52,6 +60,8 @@ def __init__(
         corruption_rate=None,
         reordering_rate=None,
         packet_limit_count=None,
+        mtu=None,
+        burst=None,
     ):
         self.__device = device
 
@@ -61,6 +71,8 @@ def __init__(
         self.__corruption_rate = convert_rate_to_f(corruption_rate)  # [%]
         self.__reordering_rate = convert_rate_to_f(reordering_rate)  # [%]
         self.__packet_limit_count = convert_rate_to_f(packet_limit_count)  # [COUNT]
+        self.__mtu = mtu  # [bytes]
+        self.__burst = burst  # [bytes]
 
         self.__latency_time = None
         if latency_time:
@@ -116,6 +128,8 @@ def validate_netem_parameter(self):
             self.__corruption_rate,
             self.__reordering_rate,
             self.__packet_limit_count,
+            self.__mtu,
+            self.__burst,
         ]
         if self.__bandwidth_rate:
             netem_param_values.append(self.__bandwidth_rate.kilo_bps)

tcconfig/shaper/htb.py

@@ -4,9 +4,13 @@
 
 import errno
 import re
+import sys
+from bisect import bisect_left
 from typing import List
 
+import pyroute2
 import typepy
+from pyroute2.netlink.rtnl.tcmsg.common import TIME_UNITS_PER_SEC, get_hz
 
 from .._common import is_execute_tc_command, logging_context, run_command_helper
 from .._const import ShapingAlgorithm, TcSubCommand
@@ -17,6 +21,64 @@
 from ._interface import AbstractShaper
 
 
+# Make this a function, so we can mock it for testing
+def tick_in_usec() -> float:
+    return pyroute2.netlink.rtnl.tcmsg.common.tick_in_usec
+
+
+# Emulation of tc's buggy time2tick implementation, which
+# rounds to int twice
+def time2tick_bug(time: int) -> int:
+    return int(int(time) * tick_in_usec())
+
+
+# An accurate implementation of tick2time (unlike tc's), not rounding.
+def tick2time_true(tick: float) -> float:
+    return tick / tick_in_usec()
+
+
+def calc_xmittime_bug(rate: int, size: int) -> int:
+    return int(time2tick_bug(TIME_UNITS_PER_SEC * (float(size) / rate)))
+
+
+def calc_xmitsize_true(rate: int, ticks: int) -> float:
+    return (float(rate) * tick2time_true(ticks)) / TIME_UNITS_PER_SEC
+
+
+# tc tries to set the default burst and cburst size to (int)(rate / get_hz() + mtu),
+# with the comment
+#     compute minimal allowed burst from rate; mtu is added here to make
+#     sure that buffer is larger than mtu and to have some safeguard space
+#
+# Unfortunately, in tc from iproute2 version 6.14 and earlier, there is a rounding
+# bug such that the actual burst size set will often be too small, which will cause
+# bandwidth limitations to be too aggressive.
+#
+# Calculate the minimum necessary size to set burst and cburst to, to ensure that they
+# are at least the size that tc was trying to set them to.
+#
+# TODO: check whether tc is from iproute2 version 6.15 or later, and if so bypass the
+# adjustment and don't set the default.
+def default_burst_size(rate: int, mtu: int) -> int:
+    return int(rate / get_hz() + mtu)
+
+
+def adjusted_burst_size(desired_burst: int, rate: int) -> int:
+    if sys.version_info >= (3, 10):
+        return bisect_left(
+            range(1 << 32),
+            True,
+            key=lambda b: calc_xmitsize_true(rate, calc_xmittime_bug(rate, b)) >= desired_burst,
+        )
+    else:
+        adjusted_burst = desired_burst
+        while adjusted_burst < (1 << 32):
+            if calc_xmitsize_true(rate, calc_xmittime_bug(rate, adjusted_burst)) >= desired_burst:
+                return adjusted_burst
+            adjusted_burst += 1
+        return adjusted_burst
+
+
 class HtbShaper(AbstractShaper):
     __DEFAULT_CLASS_MINOR_ID = 1
 
@@ -111,11 +173,25 @@ def _add_rate(self):
             f"ceil {bandwidth.kilo_bps}Kbit",
         ]
 
+        mtu = self._tc_obj.netem_param.mtu
+        if mtu:
+            command_item_list.extend([f"mtu {mtu:d}"])
+
         if bandwidth != upper_limit_rate:
+            rate = bandwidth.byte_per_sec
+            desired_burst = self._tc_obj.netem_param.burst
+            # TODO: check whether tc is from iproute2 version 6.15 or later, and if so bypass the default
+            # and adjustment.
+            if not desired_burst:
+                if not mtu:
+                    mtu = 1600
+                desired_burst = default_burst_size(rate, mtu)
+            burst = adjusted_burst_size(desired_burst, rate)
+
             command_item_list.extend(
                 [
-                    f"burst {bandwidth.kilo_byte_per_sec}KB",
-                    f"cburst {bandwidth.kilo_byte_per_sec}KB",
+                    f"burst {burst}b",
+                    f"cburst {burst}b",
                 ]
             )
 

tcconfig/shaper/tbf.py

@@ -81,20 +81,22 @@ def _add_rate(self):
         if bandwidth is None:
             bandwidth = upper_limit_rate
 
-        command = " ".join(
-            [
-                base_command,
-                self._dev,
-                f"parent {parent:s}",
-                f"handle {handle:s}",
-                self.algorithm_name,
-                f"rate {bandwidth.kilo_bps}kbit",
-                "buffer {:d}".format(
-                    max(int(bandwidth.kilo_bps), self.__MIN_BUFFER_BYTE)
-                ),  # [byte]
-                "limit 10000",
-            ]
-        )
+        command_item_list = [
+            base_command,
+            self._dev,
+            f"parent {parent:s}",
+            f"handle {handle:s}",
+            self.algorithm_name,
+            f"rate {bandwidth.kilo_bps}kbit",
+            "buffer {:d}".format(max(int(bandwidth.kilo_bps), self.__MIN_BUFFER_BYTE)),  # [byte]
+            "limit 10000",
+        ]
+
+        mtu = self._tc_obj.netem_param.mtu
+        if mtu:
+            command_item_list.extend([f"mtu {mtu:d}"])
+
+        command = " ".join(command_item_list)
 
         run_command_helper(
             command,

tcconfig/tcset.py

@@ -193,6 +193,18 @@ def get_arg_parser():
         default=False,
         help="use iptables for traffic control.",
     )
+    group.add_argument(
+        "--mtu",
+        dest="mtu",
+        type=int,
+        help="MTU size to assume for interface, if not tc's default.  Only used for certain calculations, not enforced.",
+    )
+    group.add_argument(
+        "--burst",
+        dest="burst",
+        type=int,
+        help="burst size to use for traffic shaping (htb only).",
+    )
 
     group = parser.add_routing_group()
     group.add_argument(
@@ -317,6 +329,8 @@ def __create_tc(self, device):
                 corruption_rate=options.corruption_rate,
                 reordering_rate=options.reordering_rate,
                 packet_limit_count=options.packet_limit_count,
+                mtu=options.mtu,
+                burst=options.burst,
             ),
             dst_network=self._extract_dst_network(),
             exclude_dst_network=options.exclude_dst_network,

test/test_burstsize.py

@@ -0,0 +1,41 @@
+"""
+.. codeauthor:: Jonathan Lennox <jonathan.lennox@8x8.com>
+"""
+
+import humanreadable as hr
+import pytest
+
+from tcconfig.shaper.htb import adjusted_burst_size, default_burst_size
+
+
+@pytest.mark.parametrize(
+    ["rate_hr", "mtu", "expected"],
+    [
+        ("1Mbps", 1600, 1600),
+        ("1Gbps", 1600, 1600),
+        ("8Gbps", 1600, 1601),
+        ("80Gbps", 1600, 1610),
+        ("80Gbps", 9000, 9010),
+    ],
+)
+def test_default_burst(rate_hr, mtu, expected):
+    rate = hr.BitsPerSecond(rate_hr).byte_per_sec
+    default_burst = default_burst_size(rate, mtu)
+    assert default_burst == expected
+
+
+@pytest.mark.parametrize(
+    ["desired_burst", "rate_hr", "expected"],
+    [
+        (1600, "1Mbps", 1600),
+        (1600, "1Gbps", 1625),
+        (1600, "8Gbps", 2000),
+        (1610, "80Gbps", 10000),
+        (9010, "80Gbps", 10000),
+    ],
+)
+def test_adjusted_burst(monkeypatch, desired_burst, rate_hr, expected):
+    monkeypatch.setattr("tcconfig.shaper.htb.tick_in_usec", lambda: 15.625)
+    rate = hr.BitsPerSecond(rate_hr).byte_per_sec
+    adjusted_burst = adjusted_burst_size(desired_burst, rate)
+    assert adjusted_burst == expected