From ea7374901a54aea7bc8af7a9f41ed8a5d127ddb7 Mon Sep 17 00:00:00 2001 From: Herbert Wolverson Date: Sat, 3 Oct 2026 11:56:51 -0500 Subject: [PATCH] Fix node minimum bandwidth accumulating across siblings findBandwidthMins() reused one minDownload/minUpload counter for both each node's own minimum and the running sibling total, so every sibling after the first inherited the circuit minimums of the siblings before it. Those inflated minimums are written to queuingStructure.json and become HTB rates, which can guarantee child nodes more bandwidth than their parent supplies. Reset the counters per node and keep a separate running total for the parent. Also fix the no-minimum-key fallback, which used minUpload for the download minimum. The function is hoisted to module level so the behavior can be unit tested, with regression coverage for sibling accumulation and order, nested roll-up, and the download fallback. --- src/LibreQoS.py | 58 +++++----- src/test_libreqos_shaping_inputs.py | 158 ++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 24 deletions(-) diff --git a/src/LibreQoS.py b/src/LibreQoS.py index a0b9e08c..7104d937 100755 --- a/src/LibreQoS.py +++ b/src/LibreQoS.py @@ -1456,6 +1456,39 @@ def apply_effective_runtime_circuit_overrides(subscriberCircuits): return overlay_count + +def findBandwidthMins(data, subscriberCircuits): + """Raise each node's minimum bandwidth to cover its own circuits and children. + + Mutates ``data`` in place. Returns the summed minimums of all circuits and + child nodes in the tree so callers can propagate them to a parent node. + """ + totalDownload = 0 + totalUpload = 0 + for elem in data: + minDownload = 0 + minUpload = 0 + for circuit in subscriberCircuits: + if elem == circuit['ParentNode']: + minDownload += circuit['minDownload'] + minUpload += circuit['minUpload'] + if 'children' in data[elem]: + minDL, minUL = findBandwidthMins(data[elem]['children'], subscriberCircuits) + minDownload += minDL + minUpload += minUL + if 'downloadBandwidthMbpsMin' in data[elem]: + data[elem]['downloadBandwidthMbpsMin'] = max(data[elem]['downloadBandwidthMbpsMin'], minDownload) + else: + data[elem]['downloadBandwidthMbpsMin'] = max(data[elem]['downloadBandwidthMbps'], minDownload) + if 'uploadBandwidthMbpsMin' in data[elem]: + data[elem]['uploadBandwidthMbpsMin'] = max(data[elem]['uploadBandwidthMbpsMin'], minUpload) + else: + data[elem]['uploadBandwidthMbpsMin'] = max(data[elem]['uploadBandwidthMbps'], minUpload) + totalDownload += minDownload + totalUpload += minUpload + return totalDownload, totalUpload + + def refreshShapers(): # Starting @@ -1832,31 +1865,8 @@ def refreshShapers(): genPNcounter = 0 print("Generated parent nodes created") - # Find the bandwidth minimums for each node by combining mimimums of devices lower in that node's hierarchy - def findBandwidthMins(data, depth): - tabs = ' ' * depth - minDownload = 0 - minUpload = 0 - for elem in data: - for circuit in subscriberCircuits: - if elem == circuit['ParentNode']: - minDownload += circuit['minDownload'] - minUpload += circuit['minUpload'] - if 'children' in data[elem]: - minDL, minUL = findBandwidthMins(data[elem]['children'], depth+1) - minDownload += minDL - minUpload += minUL - if 'downloadBandwidthMbpsMin' in data[elem]: - data[elem]['downloadBandwidthMbpsMin'] = max(data[elem]['downloadBandwidthMbpsMin'], minDownload) - else: - data[elem]['downloadBandwidthMbpsMin'] = max(data[elem]['downloadBandwidthMbps'], minUpload) - if 'uploadBandwidthMbpsMin' in data[elem]: - data[elem]['uploadBandwidthMbpsMin'] = max(data[elem]['uploadBandwidthMbpsMin'], minUpload) - else: - data[elem]['uploadBandwidthMbpsMin'] = max(data[elem]['uploadBandwidthMbps'], minUpload) - return minDownload, minUpload logging.info("Finding the bandwidth minimums for each node") - minDownload, minUpload = findBandwidthMins(network, 0) + findBandwidthMins(network, subscriberCircuits) logging.info("Found the bandwidth minimums for each node") # Child nodes inherit bandwidth maximums of parents. We apply this here to avoid bugs when compression is applied with flattenA(). diff --git a/src/test_libreqos_shaping_inputs.py b/src/test_libreqos_shaping_inputs.py index 83f681b6..e1ff471e 100644 --- a/src/test_libreqos_shaping_inputs.py +++ b/src/test_libreqos_shaping_inputs.py @@ -522,6 +522,164 @@ class TestLibreQoSShapingInputs(unittest.TestCase): self.assertEqual(missing_parents, {}) +class TestFindBandwidthMins(unittest.TestCase): + def test_sibling_minimums_do_not_accumulate(self): + def build_network(child_order): + children = { + "Crew": { + "downloadBandwidthMbps": 100, + "uploadBandwidthMbps": 100, + "downloadBandwidthMbpsMin": 20, + "uploadBandwidthMbpsMin": 20, + }, + "Guest": { + "downloadBandwidthMbps": 178, + "uploadBandwidthMbps": 88, + "downloadBandwidthMbpsMin": 60, + "uploadBandwidthMbpsMin": 20, + }, + "Infra": { + "downloadBandwidthMbps": 82, + "uploadBandwidthMbps": 30, + "downloadBandwidthMbpsMin": 1, + "uploadBandwidthMbpsMin": 1, + }, + "MGMT": { + "downloadBandwidthMbps": 30, + "uploadBandwidthMbps": 15, + "downloadBandwidthMbpsMin": 10, + "uploadBandwidthMbpsMin": 4, + }, + } + return { + "Site": { + "downloadBandwidthMbps": 189, + "uploadBandwidthMbps": 69, + "downloadBandwidthMbpsMin": 120, + "uploadBandwidthMbpsMin": 60, + "children": {name: children[name] for name in child_order}, + } + } + + circuits = [ + {"ParentNode": "Crew", "minDownload": 10, "minUpload": 10}, + {"ParentNode": "Crew", "minDownload": 10, "minUpload": 10}, + {"ParentNode": "Guest", "minDownload": 60, "minUpload": 20}, + {"ParentNode": "Infra", "minDownload": 1, "minUpload": 1}, + {"ParentNode": "MGMT", "minDownload": 10, "minUpload": 4}, + ] + + for child_order in ( + ["Crew", "Guest", "Infra", "MGMT"], + ["MGMT", "Infra", "Guest", "Crew"], + ): + network = build_network(child_order) + totals = LibreQoS.findBandwidthMins(network, circuits) + + self.assertEqual(totals, (91, 45)) + self.assertEqual(network["Site"]["downloadBandwidthMbpsMin"], 120) + self.assertEqual(network["Site"]["uploadBandwidthMbpsMin"], 60) + generated = { + name: ( + node["downloadBandwidthMbpsMin"], + node["uploadBandwidthMbpsMin"], + ) + for name, node in network["Site"]["children"].items() + } + self.assertEqual( + generated, + { + "Crew": (20, 20), + "Guest": (60, 20), + "Infra": (1, 1), + "MGMT": (10, 4), + }, + ) + + def test_child_minimums_roll_up_once(self): + network = { + "Site": { + "downloadBandwidthMbps": 100, + "uploadBandwidthMbps": 100, + "downloadBandwidthMbpsMin": 0, + "uploadBandwidthMbpsMin": 0, + "children": { + "Tower": { + "downloadBandwidthMbps": 50, + "uploadBandwidthMbps": 50, + "downloadBandwidthMbpsMin": 0, + "uploadBandwidthMbpsMin": 0, + "children": { + "AP-1": { + "downloadBandwidthMbps": 25, + "uploadBandwidthMbps": 25, + "downloadBandwidthMbpsMin": 0, + "uploadBandwidthMbpsMin": 0, + }, + "AP-2": { + "downloadBandwidthMbps": 25, + "uploadBandwidthMbps": 25, + "downloadBandwidthMbpsMin": 0, + "uploadBandwidthMbpsMin": 0, + }, + }, + }, + "Tower-2": { + "downloadBandwidthMbps": 40, + "uploadBandwidthMbps": 40, + "downloadBandwidthMbpsMin": 0, + "uploadBandwidthMbpsMin": 0, + }, + }, + }, + } + circuits = [ + {"ParentNode": "Tower", "minDownload": 4, "minUpload": 1}, + {"ParentNode": "AP-1", "minDownload": 10, "minUpload": 2}, + {"ParentNode": "AP-2", "minDownload": 5, "minUpload": 1}, + {"ParentNode": "Tower-2", "minDownload": 7, "minUpload": 3}, + ] + + totals = LibreQoS.findBandwidthMins(network, circuits) + + self.assertEqual(totals, (26, 7)) + self.assertEqual(network["Site"]["downloadBandwidthMbpsMin"], 26) + self.assertEqual(network["Site"]["uploadBandwidthMbpsMin"], 7) + self.assertEqual( + network["Site"]["children"]["Tower"]["downloadBandwidthMbpsMin"], 19 + ) + self.assertEqual( + network["Site"]["children"]["Tower-2"]["downloadBandwidthMbpsMin"], 7 + ) + self.assertEqual( + network["Site"]["children"]["Tower"]["children"]["AP-1"][ + "downloadBandwidthMbpsMin" + ], + 10, + ) + self.assertEqual( + network["Site"]["children"]["Tower"]["children"]["AP-2"][ + "downloadBandwidthMbpsMin" + ], + 5, + ) + + def test_missing_minimum_keys_use_download_minimum(self): + network = { + "Leaf": { + "downloadBandwidthMbps": 100, + "uploadBandwidthMbps": 50, + } + } + circuits = [{"ParentNode": "Leaf", "minDownload": 150, "minUpload": 40}] + + totals = LibreQoS.findBandwidthMins(network, circuits) + + self.assertEqual(totals, (150, 40)) + self.assertEqual(network["Leaf"]["downloadBandwidthMbpsMin"], 150) + self.assertEqual(network["Leaf"]["uploadBandwidthMbpsMin"], 50) + + class TestFirstRunSinceBoot(unittest.TestCase): def setUp(self): temp_dir = tempfile.TemporaryDirectory() # nosec B108