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

@@ -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().

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