From d4fc3d255dd534e4d6f3e2b04b03416bd6b852f4 Mon Sep 17 00:00:00 2001 From: Herbert Wolverson Date: Tue, 21 Jul 2026 15:12:10 -0500 Subject: [PATCH] Stabilize flat-network binpacking assignments --- docs-es/v2.0/configuration-advanced-es.md | 2 + docs/v2.0/configuration-advanced.md | 2 + src/LibreQoS.py | 197 ++++++- src/rust/lqos_config/src/planner.rs | 560 +++++++++++++++++-- src/rust/lqos_topology/src/flat_mode.rs | 75 ++- src/rust/lqos_topology/src/lib.rs | 2 +- src/rust/lqos_topology/src/tests/group_06.rs | 236 ++++++++ src/rust/lqos_topology/src/tests/helpers.rs | 5 +- src/rust/lqos_topology/src/tests/mod.rs | 1 + src/test_libreqos_shaping_inputs.py | 108 ++++ 10 files changed, 1108 insertions(+), 80 deletions(-) create mode 100644 src/rust/lqos_topology/src/tests/group_06.rs diff --git a/docs-es/v2.0/configuration-advanced-es.md b/docs-es/v2.0/configuration-advanced-es.md index e06312e6..5015d131 100644 --- a/docs-es/v2.0/configuration-advanced-es.md +++ b/docs-es/v2.0/configuration-advanced-es.md @@ -135,6 +135,8 @@ Para redes sin nodos padre (sin puntos de acceso o sitios estrictamente definido echo "{}" > network.json ``` +Con el binpacking de CPU habilitado, el modo plano conserva las asignaciones anteriores que siguen siendo válidas y limita cada actualización a un solo cambio de circuito existente. Esto reduce las interrupciones del shaping al agregar o eliminar circuitos. Un cambio en la cantidad de colas disponibles puede invalidar los buckets anteriores y requerir una reasignación más amplia. + #### Nodos virtuales (solo lógicos) LibreQoS soporta **nodos virtuales** en `network.json` para agrupar y para monitoreo/agregación en la WebUI/Insight. Los nodos virtuales **no** se incluyen en el árbol físico de shaping HTB (no crean clases HTB y no aplican límites de ancho de banda). diff --git a/docs/v2.0/configuration-advanced.md b/docs/v2.0/configuration-advanced.md index 293bc17e..3a51c78d 100644 --- a/docs/v2.0/configuration-advanced.md +++ b/docs/v2.0/configuration-advanced.md @@ -99,6 +99,8 @@ compile_mode = "full" Use `compile_mode = "flat"` only when hierarchy is not part of the shaping plan. In flat mode, LibreQoS assigns circuits to generated CPU bucket queues such as `Generated_PN_1`; the original `Parent Node` remains a logical reference, but the effective shaping parent in `shaping_inputs.json` will be a generated bucket with `resolution_source: "flat_bucket"`. +With CPU binpacking enabled, flat mode preserves valid prior bucket assignments and limits each refresh to at most one existing-circuit rebalance. This reduces shaping interruption when circuits are added or removed. A change to the available queue count can invalidate prior buckets and require broader reassignment. + #### Static queue visibility policy Current builds separate logical topology from queue-visible topology. diff --git a/src/LibreQoS.py b/src/LibreQoS.py index 1687b12c..885e7bbc 100755 --- a/src/LibreQoS.py +++ b/src/LibreQoS.py @@ -736,6 +736,133 @@ def planner_circuit_identity_key(circuit): return circuit_id +def flat_binpacking_enabled(network, binpacking_enabled): + if not binpacking_enabled: + return False + if loaded_network_is_flat(network): + return True + if not isinstance(network, dict) or not network: + return False + expected_bucket_names = { + "Generated_PN_" + str(index + 1) for index in range(len(network)) + } + if set(network) != expected_bucket_names: + return False + for index in range(len(network)): + bucket_name = "Generated_PN_" + str(index + 1) + bucket = network.get(bucket_name) + expected_bucket_id = "libreqos:generated:flat:bucket:" + str(index) + if ( + not isinstance(bucket, dict) + or bucket.get("id") != expected_bucket_id + or bucket.get("children", {}) != {} + ): + return False + return True + + +def planner_top_level_item_key(circuit, flat_network): + if flat_network: + return "flat_circuit:" + planner_circuit_identity_key(circuit) + if 'idForCircuitsWithoutParentNodes' not in circuit: + raise ValueError("Missing transient planner ID for unparented circuit") + return str(circuit['idForCircuitsWithoutParentNodes']) + + +def reusable_identity_state(state, reuse_saved_identities): + if not reuse_saved_identities or not isinstance(state, dict): + return {}, {} + sites = state.get('sites', {}) + circuits = state.get('circuits', {}) + return ( + sites if isinstance(sites, dict) else {}, + circuits if isinstance(circuits, dict) else {}, + ) + + +def generated_parent_node_from_queue_key(queue_key, queues_available): + if not isinstance(queue_key, str) or queues_available <= 0: + return None + queue_index = _parse_int_token(queue_key.removeprefix("CpueQueue")) + if not queue_key.startswith("CpueQueue") or queue_index is None: + return None + if queue_index < 0 or queue_index >= queues_available: + return None + return "Generated_PN_" + str(queue_index + 1) + + +def plan_flat_generated_parent_assignments( + items, + generated_parent_nodes, + previous_assignments, + last_change_ts, + now_ts, + move_budget_per_run, +): + queues_available = len(generated_parent_nodes) + item_ids = {str(item["id"]) for item in items} + rust_previous_assignments = { + item_id: generated_parent_node_queue_key(parent_name, queues_available) + for item_id, parent_name in previous_assignments.items() + } + rust_previous_assignments = { + item_id: queue_key + for item_id, queue_key in rust_previous_assignments.items() + if queue_key is not None + } + try: + plan_result = plan_top_level_cpu_bins( + items, + queues_available, + prev_assign=rust_previous_assignments, + last_change_ts=last_change_ts, + now_ts=now_ts, + mode="stable_greedy", + move_budget_per_run=move_budget_per_run, + cooldown_seconds=3600.0, + hysteresis_threshold=0.03, + ) + changed = list(plan_result.get("changed", []) or []) + assignments = { + item_id: parent_name + for item_id, queue_key in (plan_result.get("assignment", {}) or {}).items() + if ( + parent_name := generated_parent_node_from_queue_key( + queue_key, + queues_available, + ) + ) is not None + } + if set(assignments) != item_ids: + raise ValueError("Shared Rust flat planner returned incomplete assignments") + return assignments, changed + except Exception as e: + warnings.warn( + f"Shared Rust flat planner failed ({e}); preserving prior assignments.", + stacklevel=2, + ) + + assignments = dict(previous_assignments) + weights_by_item = {str(item["id"]): float(item["weight"]) for item in items} + bin_loads = {parent_name: 0.0 for parent_name in generated_parent_nodes} + for item_id, parent_name in assignments.items(): + bin_loads[parent_name] += weights_by_item.get(item_id, 1.0) + unassigned = [ + (item_id, weight) + for item_id, weight in weights_by_item.items() + if item_id not in assignments + ] + unassigned.sort(key=lambda item: (-item[1], item[0])) + for item_id, weight in unassigned: + target_parent = min( + bin_loads.items(), + key=lambda entry: (entry[1], entry[0]), + )[0] + assignments[item_id] = target_parent + bin_loads[target_parent] += weight + return assignments, [] + + def calculateR2q(maxRateInMbps): # So we've learned that r2q defaults to 10, and is used to calculate quantum. Quantum is rateInBytes/r2q by # default. This default gives errors at high rates, and tc clamps the quantum to 200000. Setting a high quantum @@ -1404,6 +1531,7 @@ def refreshShapers(): # Flat networks ({}) don't require ParentNode entries. Treat every circuit as # unparented so they can be distributed across generated parent nodes / CPUs. flat_network = loaded_network_is_flat(network) + binpacking_enabled = use_bin_packing_to_balance_cpu() # Virtual Nodes (logical-only): build a physical shaping topology that skips them, # while leaving ShapedDevices.csv (and monitoring) unchanged. @@ -1426,6 +1554,11 @@ def refreshShapers(): # Avoid bloating queuingStructure.json when there are no virtual nodes. logical_to_physical_node = {} + reuse_flat_binpacking_identities = flat_binpacking_enabled( + network, + binpacking_enabled, + ) + # Re-map circuits that are directly parented to a virtual node to the nearest real ancestor (milestone c). if not flat_network and len(virtual_nodes) > 0 and isinstance(logical_to_physical_node, dict): next_id = max(dictForCircuitsWithoutParentNodes.keys(), default=-1) + 1 @@ -1531,7 +1664,7 @@ def refreshShapers(): # Planner/device weights (fetched only when planner/binpacking is enabled). # When disabled, we keep this empty and fall back to rate-based weights later. weight_by_circuit_id = {} - if use_bin_packing_to_balance_cpu(): + if binpacking_enabled: print("Using internal planner to sort circuits by CPU core") # Build item list with weights for circuits lacking a ParentNode items = [] @@ -1549,9 +1682,10 @@ def refreshShapers(): pass for circuit in subscriberCircuits: if circuit.get('ParentNode') == 'none' and 'idForCircuitsWithoutParentNodes' in circuit: - item_id = circuit['idForCircuitsWithoutParentNodes'] + transient_item_id = circuit['idForCircuitsWithoutParentNodes'] + item_id = planner_top_level_item_key(circuit, flat_network) # Prefer provided weights; default to 1.0 - w = dictForCircuitsWithoutParentNodes.get(item_id, 1.0) + w = dictForCircuitsWithoutParentNodes.get(transient_item_id, 1.0) # If a specific circuit weight exists, prefer it if 'circuitID' in circuit and str(circuit['circuitID']) in weight_by_circuit_id: w = weight_by_circuit_id[str(circuit['circuitID'])] @@ -1570,14 +1704,18 @@ def refreshShapers(): capacities = {pn: 1.0 for pn in generatedPNs} # Load planner state - try: - import bin_planner - except ImportError: - bin_planner = None + bin_planner = None + if not flat_network: + try: + import bin_planner + except ImportError: + pass # Store planner state directly in lqos_directory (no hidden subdirs) state_path = get_planner_state_path() state = {} - if bin_planner is not None: + if flat_network: + state = load_planner_state(state_path, None) + elif bin_planner is not None: state = load_planner_state(state_path, bin_planner) now_ts = time.time() prev_assign = {} @@ -1598,14 +1736,27 @@ def refreshShapers(): "alpha": 0.1, "hysteresis_threshold": 0.03, "cooldown_seconds": 3600, - "move_budget_per_run": max(1, min(32, int(0.01 * max(1, len(items))))), + "move_budget_per_run": ( + 1 + if flat_network + else max(1, min(32, int(0.01 * max(1, len(items))))) + ), "salt": state.get("salt", "default_salt") if isinstance(state, dict) else "default_salt", "last_change_ts_by_item": last_change_ts, } if observe_mode: params["move_budget_per_run"] = 0 - if bin_planner is not None: + if flat_network: + assignments, changed = plan_flat_generated_parent_assignments( + items, + generatedPNs, + prev_assign, + last_change_ts, + now_ts, + params["move_budget_per_run"], + ) + elif bin_planner is not None: assignments, changed = bin_planner.plan_assignments( items, bins_list, capacities, prev_assign, now_ts, params ) @@ -1624,21 +1775,24 @@ def refreshShapers(): # Apply assignments to circuits for circuit in subscriberCircuits: if circuit.get('ParentNode') == 'none' and 'idForCircuitsWithoutParentNodes' in circuit: - item_id = circuit['idForCircuitsWithoutParentNodes'] - item_key = str(item_id) + item_key = planner_top_level_item_key(circuit, flat_network) if item_key in assignments: circuit['ParentNode'] = assignments[item_key] circuit['ParentNodeID'] = '' circuit['effectiveParentNodeID'] = '' # Update and save state - if bin_planner is not None and isinstance(state, dict): + if (flat_network or bin_planner is not None) and isinstance(state, dict): if state.get("salt") is None: state["salt"] = "default_salt" if "assignments" not in state or not isinstance(state["assignments"], dict): state["assignments"] = {} if "last_change_ts" not in state or not isinstance(state["last_change_ts"], dict): state["last_change_ts"] = {} + if flat_network: + for stale_item_id in set(state["assignments"]) - item_ids: + state["assignments"].pop(stale_item_id, None) + state["last_change_ts"].pop(stale_item_id, None) for iid, b in assignments.items(): # record last change time if changed if iid in changed: @@ -1646,7 +1800,11 @@ def refreshShapers(): state["assignments"][iid] = b try: print(f"Saving planner state to {state_path} (generated PNs)") - save_planner_state(state, state_path, bin_planner) + save_planner_state( + state, + state_path, + None if flat_network else bin_planner, + ) except Exception as e: warnings.warn(f"Failed to save planner state at {state_path}: {e}", stacklevel=2) @@ -1875,7 +2033,7 @@ def refreshShapers(): return keys # If we're in binpacking mode, we need to sort the network structure a bit - if use_bin_packing_to_balance_cpu() and not flat_network: + if binpacking_enabled and not flat_network: # Binpacking is an Insight feature; if Insight is not enabled/licensed, fall back to # deterministic round-robin placement so "virtual node promotion" can still spread # the physical tree across CPUs. @@ -2114,11 +2272,16 @@ def refreshShapers(): collect_identity_planner_inputs(network, 0, 1) + previous_site_state, previous_circuit_state = reusable_identity_state( + state, + reuse_flat_binpacking_identities, + ) + identity_plan = plan_class_identities( planner_site_inputs, planner_circuit_groups, - site_state={}, - circuit_state={}, + site_state=previous_site_state, + circuit_state=previous_circuit_state, stick_offset=stickOffset, circuit_padding=CIRCUIT_PADDING, ) diff --git a/src/rust/lqos_config/src/planner.rs b/src/rust/lqos_config/src/planner.rs index 44edbee1..242e4a5d 100644 --- a/src/rust/lqos_config/src/planner.rs +++ b/src/rust/lqos_config/src/planner.rs @@ -201,6 +201,103 @@ fn next_free_minor(start_minor: u32, reserved: &BTreeSet) -> u32 { candidate } +fn valid_class_minor(class_minor: u16) -> Option { + let class_minor = u32::from(class_minor); + (3..u32::from(u16::MAX)) + .contains(&class_minor) + .then_some(class_minor) +} + +fn expected_class_majors(queue: u32, stick_offset: u16) -> Option<(u16, u16)> { + let class_major = u16::try_from(queue).ok()?; + (class_major > 0).then_some((class_major, class_major.saturating_add(stick_offset))) +} + +fn reserve_reusable_site_minors( + sites: &[SiteIdentityInput], + previous_sites: &BTreeMap, + stick_offset: u16, + reserved: &mut PlannerMinorReservations, +) -> BTreeMap { + let current_sites = sites + .iter() + .map(|site| (site.site_key.as_str(), site)) + .collect::>(); + let mut reusable = BTreeMap::new(); + + for (site_key, site) in current_sites { + let Some(stored) = previous_sites.get(site_key) else { + continue; + }; + let Some(class_minor) = valid_class_minor(stored.class_minor) else { + continue; + }; + let Some((class_major, up_class_major)) = expected_class_majors(site.queue, stick_offset) + else { + continue; + }; + if stored.queue != site.queue + || stored.parent_path != site.parent_path + || stored.class_major != class_major + || stored.up_class_major != up_class_major + { + continue; + } + + let occupied = reserved.entry(site.queue).or_default(); + if occupied.insert(class_minor) { + reusable.insert(site_key.to_string(), class_minor); + } + } + + reusable +} + +fn reserve_reusable_circuit_minors( + circuit_groups: &[CircuitIdentityGroupInput], + previous_circuits: &BTreeMap, + stick_offset: u16, + reserved: &mut PlannerMinorReservations, +) -> BTreeMap { + let current_circuits = circuit_groups + .iter() + .flat_map(|group| { + group + .circuit_ids + .iter() + .map(move |circuit_id| (circuit_id.as_str(), group)) + }) + .collect::>(); + let mut reusable = BTreeMap::new(); + + for (circuit_id, group) in current_circuits { + let Some(stored) = previous_circuits.get(circuit_id) else { + continue; + }; + let Some(class_minor) = valid_class_minor(stored.class_minor) else { + continue; + }; + let Some((class_major, up_class_major)) = expected_class_majors(group.queue, stick_offset) + else { + continue; + }; + if stored.queue != group.queue + || stored.parent_node != group.parent_node + || stored.class_major != class_major + || stored.up_class_major != up_class_major + { + continue; + } + + let occupied = reserved.entry(group.queue).or_default(); + if occupied.insert(class_minor) { + reusable.insert(circuit_id.to_string(), class_minor); + } + } + + reusable +} + /// Builds reservation maps for identities that are not part of the current planning scope. pub fn build_class_identity_reservations( sites: &[SiteIdentityInput], @@ -220,10 +317,12 @@ pub fn build_class_identity_reservations( if planned_site_keys.contains(site_key.as_str()) { continue; } - reserved_site_minors - .entry(stored.queue) - .or_default() - .insert(stored.class_minor as u32); + if let Some(class_minor) = valid_class_minor(stored.class_minor) { + reserved_site_minors + .entry(stored.queue) + .or_default() + .insert(class_minor); + } } let mut reserved_circuit_minors: PlannerMinorReservations = BTreeMap::new(); @@ -231,10 +330,12 @@ pub fn build_class_identity_reservations( if planned_circuit_ids.contains(circuit_id.as_str()) { continue; } - reserved_circuit_minors - .entry(stored.queue) - .or_default() - .insert(stored.class_minor as u32); + if let Some(class_minor) = valid_class_minor(stored.class_minor) { + reserved_circuit_minors + .entry(stored.queue) + .or_default() + .insert(class_minor); + } } (reserved_site_minors, reserved_circuit_minors) @@ -334,7 +435,7 @@ pub fn plan_top_level_assignments( let rr_assignment = round_robin_assign(items, bins); let greedy_assignment = greedy_assign(items, bins); - let (mut assignment, planner_used) = match params.mode { + let (assignment, planner_used) = match params.mode { TopLevelPlannerMode::RoundRobin => (rr_assignment.clone(), false), TopLevelPlannerMode::Greedy => (greedy_assignment.clone(), false), TopLevelPlannerMode::StableGreedy => { @@ -361,11 +462,11 @@ pub fn plan_top_level_assignments( }); let mut moves_used = 0usize; - for id in weighted_ids { - let Some(current_bin) = assignment.get(&id).cloned() else { + for id in &weighted_ids { + let Some(current_bin) = assignment.get(id).cloned() else { continue; }; - let Some(target_bin) = greedy_assignment.get(&id).cloned() else { + let Some(target_bin) = greedy_assignment.get(id).cloned() else { continue; }; if current_bin == target_bin { @@ -374,36 +475,53 @@ pub fn plan_top_level_assignments( if moves_used >= params.move_budget_per_run { continue; } - let last_changed = last_change_ts.get(&id).copied().unwrap_or(0.0); + let last_changed = last_change_ts.get(id).copied().unwrap_or(0.0); if now_ts - last_changed < params.cooldown_seconds { continue; } let current_load = loads.get(¤t_bin).copied().unwrap_or(0.0); let target_load = loads.get(&target_bin).copied().unwrap_or(0.0); - if current_load - target_load <= min_improvement { + let weight = item_weights.get(id).copied().unwrap_or(1.0); + let imbalance_before = current_load - target_load; + let imbalance_after = ((current_load - weight) - (target_load + weight)).abs(); + if imbalance_before - imbalance_after <= min_improvement { continue; } - let weight = item_weights.get(&id).copied().unwrap_or(1.0); if let Some(load) = loads.get_mut(¤t_bin) { *load -= weight; } if let Some(load) = loads.get_mut(&target_bin) { *load += weight; } - assignment.insert(id, target_bin); + assignment.insert(id.clone(), target_bin); moves_used += 1; } + let used_bins = assignment.values().collect::>(); + if bins.len() > 1 + && assignment.len() > 1 + && used_bins.len() <= 1 + && moves_used < params.move_budget_per_run + { + for id in weighted_ids { + let Some(current_bin) = assignment.get(&id) else { + continue; + }; + let Some(target_bin) = greedy_assignment.get(&id) else { + continue; + }; + if current_bin != target_bin { + assignment.insert(id, target_bin.clone()); + break; + } + } + } + (assignment, true) } }; - let used_bins: std::collections::BTreeSet<&String> = assignment.values().collect(); - if bins.len() > 1 && assignment.len() > 1 && used_bins.len() <= 1 { - assignment = greedy_assignment.clone(); - } - let mut changed: Vec = assignment .iter() .filter_map(|(id, bin)| { @@ -433,39 +551,49 @@ pub fn plan_class_identities_with_constraints( constraints: &ClassIdentityPlannerConstraints, ) -> ClassIdentityPlannerOutput { let mut reserved_site_minors = constraints.reserved_site_minors.clone(); + for (queue, minors) in &constraints.reserved_circuit_minors { + reserved_site_minors + .entry(*queue) + .or_default() + .extend(minors.iter().copied()); + } + let reusable_site_minors = reserve_reusable_site_minors( + sites, + previous_sites, + constraints.stick_offset, + &mut reserved_site_minors, + ); + let reusable_circuit_minors = reserve_reusable_circuit_minors( + circuit_groups, + previous_circuits, + constraints.stick_offset, + &mut reserved_site_minors, + ); let mut next_site_minor_by_queue: BTreeMap = BTreeMap::new(); let mut site_assignments = Vec::with_capacity(sites.len()); let mut site_state = BTreeMap::new(); - for (queue, reserved) in &reserved_site_minors { - let next_minor = next_site_minor_by_queue - .entry(*queue) - .or_insert(constraints.site_minor_start.max(3)); - if let Some(last_reserved) = reserved.iter().next_back().copied() { - *next_minor = (*next_minor).max(last_reserved.saturating_add(1)); - } - } for site in sites { let reserved = reserved_site_minors.entry(site.queue).or_default(); let next_minor = next_site_minor_by_queue .entry(site.queue) .or_insert(constraints.site_minor_start.max(3)); - let reuse_minor = previous_sites.get(&site.site_key).and_then(|stored| { - (stored.queue == site.queue - && stored.parent_path == site.parent_path - && !reserved.contains(&(stored.class_minor as u32))) - .then_some(stored.class_minor as u32) - }); + let reuse_minor = reusable_site_minors.get(&site.site_key).copied(); - let class_minor_u32 = reuse_minor.unwrap_or_else(|| next_free_minor(*next_minor, reserved)); + let class_minor_u32 = if let Some(reused_minor) = reuse_minor { + reused_minor + } else { + let allocated_minor = next_free_minor(*next_minor, reserved); + *next_minor = allocated_minor.saturating_add(1); + if site.has_children { + *next_minor = (*next_minor).saturating_add(1); + } + allocated_minor + }; let class_minor = u16::try_from(class_minor_u32).unwrap_or(u16::MAX); let class_major = u16::try_from(site.queue).unwrap_or(u16::MAX); let up_class_major = class_major.saturating_add(constraints.stick_offset); reserved.insert(class_minor_u32); - *next_minor = (*next_minor).max(class_minor_u32).saturating_add(1); - if site.has_children { - *next_minor = (*next_minor).saturating_add(1); - } let assigned = SiteIdentityAssignment { site_key: site.site_key.clone(), @@ -489,12 +617,6 @@ pub fn plan_class_identities_with_constraints( } let mut reserved_circuit_minors = reserved_site_minors.clone(); - for (queue, extras) in &constraints.reserved_circuit_minors { - reserved_circuit_minors - .entry(*queue) - .or_default() - .extend(extras.iter().copied()); - } let mut next_circuit_minor_by_queue = BTreeMap::new(); for (queue, reserved) in &reserved_circuit_minors { let start = next_site_minor_by_queue @@ -515,19 +637,18 @@ pub fn plan_class_identities_with_constraints( .entry(group.queue) .or_insert_with(|| next_free_minor(constraints.circuit_minor_start.max(3), reserved)); for circuit_id in &group.circuit_ids { - let reuse_minor = previous_circuits.get(circuit_id).and_then(|stored| { - (stored.queue == group.queue - && stored.parent_node == group.parent_node - && !reserved.contains(&(stored.class_minor as u32))) - .then_some(stored.class_minor as u32) - }); - let class_minor_u32 = - reuse_minor.unwrap_or_else(|| next_free_minor(*next_minor, reserved)); + let reuse_minor = reusable_circuit_minors.get(circuit_id).copied(); + let class_minor_u32 = if let Some(reused_minor) = reuse_minor { + reused_minor + } else { + let allocated_minor = next_free_minor(*next_minor, reserved); + *next_minor = allocated_minor.saturating_add(1); + allocated_minor + }; let class_minor = u16::try_from(class_minor_u32).unwrap_or(u16::MAX); let class_major = u16::try_from(group.queue).unwrap_or(u16::MAX); let up_class_major = class_major.saturating_add(constraints.stick_offset); reserved.insert(class_minor_u32); - *next_minor = (*next_minor).max(class_minor_u32).saturating_add(1); circuit_assignments.push(CircuitIdentityAssignment { circuit_id: circuit_id.clone(), @@ -563,7 +684,15 @@ pub fn plan_class_identities_with_constraints( .get(&queue) .copied() .unwrap_or(3); - last_used_minor_by_queue.insert(queue, site_next.max(circuit_next)); + let highest_reserved_next = reserved_circuit_minors + .get(&queue) + .and_then(|reserved| reserved.iter().next_back().copied()) + .unwrap_or(2) + .saturating_add(1); + last_used_minor_by_queue.insert( + queue, + site_next.max(circuit_next).max(highest_reserved_next), + ); } ClassIdentityPlannerOutput { @@ -726,6 +855,99 @@ mod tests { assert!(result.planner_used); } + #[test] + fn stable_greedy_does_not_make_balance_worse() { + let items = vec![ + TopLevelPlannerItem { + id: "a".to_string(), + weight: 2.0, + }, + TopLevelPlannerItem { + id: "b".to_string(), + weight: 2.0, + }, + TopLevelPlannerItem { + id: "c".to_string(), + weight: 3.0, + }, + TopLevelPlannerItem { + id: "d".to_string(), + weight: 5.0, + }, + TopLevelPlannerItem { + id: "e".to_string(), + weight: 3.0, + }, + ]; + let prev_assign = BTreeMap::from([ + ("a".to_string(), "CpueQueue1".to_string()), + ("b".to_string(), "CpueQueue0".to_string()), + ("c".to_string(), "CpueQueue1".to_string()), + ("d".to_string(), "CpueQueue0".to_string()), + ]); + + let result = plan_top_level_assignments( + &items, + &bins(), + &prev_assign, + &BTreeMap::new(), + 10_000.0, + &TopLevelPlannerParams { + mode: TopLevelPlannerMode::StableGreedy, + cooldown_seconds: 0.0, + move_budget_per_run: 1, + hysteresis_threshold: 0.03, + }, + ); + + assert!(result.changed.is_empty()); + for (circuit_id, previous_bin) in prev_assign { + assert_eq!(result.assignment[&circuit_id], previous_bin); + } + } + + #[test] + fn stable_greedy_single_bin_recovery_respects_move_budget() { + let items = (0..6) + .map(|index| TopLevelPlannerItem { + id: format!("circuit-{index}"), + weight: 1.0, + }) + .collect::>(); + let prev_assign = items + .iter() + .map(|item| (item.id.clone(), "CpueQueue0".to_string())) + .collect::>(); + let last_change_ts = items + .iter() + .map(|item| (item.id.clone(), 999.0)) + .collect::>(); + + let result = plan_top_level_assignments( + &items, + &bins(), + &prev_assign, + &last_change_ts, + 1_000.0, + &TopLevelPlannerParams { + mode: TopLevelPlannerMode::StableGreedy, + cooldown_seconds: 3_600.0, + move_budget_per_run: 1, + hysteresis_threshold: 0.0, + }, + ); + + assert_eq!(result.changed.len(), 1); + assert_eq!( + result + .assignment + .values() + .collect::>() + .len(), + 2 + ); + } + #[test] fn class_identity_planner_reuses_site_and_circuit_minors() { let site_inputs = vec![ @@ -965,4 +1187,230 @@ mod tests { assert_eq!(result.sites.len(), 1); assert!(result.sites[0].class_minor >= 0x1000); } + + #[test] + fn class_identity_planner_protects_existing_circuits_from_sorted_insertions() { + let circuit_groups = vec![CircuitIdentityGroupInput { + parent_node: "flat-bucket".to_string(), + queue: 1, + circuit_ids: vec![ + "a-new".to_string(), + "m-existing".to_string(), + "z-existing".to_string(), + ], + }]; + let previous_circuits = BTreeMap::from([ + ( + "m-existing".to_string(), + PlannerCircuitIdentityState { + class_minor: 10, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ( + "z-existing".to_string(), + PlannerCircuitIdentityState { + class_minor: 11, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ]); + + let result = plan_class_identities( + &[], + &circuit_groups, + &BTreeMap::new(), + &previous_circuits, + 0, + 0, + ); + + assert_eq!(result.circuit_state["m-existing"].class_minor, 10); + assert_eq!(result.circuit_state["z-existing"].class_minor, 11); + assert_eq!(result.circuit_state["a-new"].class_minor, 3); + + let repeated = plan_class_identities( + &[], + &circuit_groups, + &BTreeMap::new(), + &result.circuit_state, + 0, + 0, + ); + assert_eq!(repeated.circuit_state, result.circuit_state); + assert_eq!( + repeated.last_used_minor_by_queue, + result.last_used_minor_by_queue + ); + } + + #[test] + fn class_identity_planner_reassigns_only_the_circuit_that_moved() { + let circuit_groups = vec![ + CircuitIdentityGroupInput { + parent_node: "bucket-a".to_string(), + queue: 1, + circuit_ids: vec!["stable".to_string()], + }, + CircuitIdentityGroupInput { + parent_node: "bucket-b".to_string(), + queue: 2, + circuit_ids: vec!["moved".to_string()], + }, + ]; + let previous_circuits = BTreeMap::from([ + ( + "stable".to_string(), + PlannerCircuitIdentityState { + class_minor: 20, + queue: 1, + parent_node: "bucket-a".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ( + "moved".to_string(), + PlannerCircuitIdentityState { + class_minor: 21, + queue: 1, + parent_node: "bucket-a".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ]); + + let result = plan_class_identities( + &[], + &circuit_groups, + &BTreeMap::new(), + &previous_circuits, + 0, + 0, + ); + + assert_eq!(result.circuit_state["stable"], previous_circuits["stable"]); + assert_ne!(result.circuit_state["moved"], previous_circuits["moved"]); + assert_eq!(result.circuit_state["moved"].queue, 2); + assert_eq!(result.circuit_state["moved"].parent_node, "bucket-b"); + } + + #[test] + fn class_identity_planner_rejects_duplicate_stale_and_invalid_saved_identities() { + let circuit_groups = vec![CircuitIdentityGroupInput { + parent_node: "flat-bucket".to_string(), + queue: 1, + circuit_ids: vec![ + "duplicate-a".to_string(), + "duplicate-b".to_string(), + "stale-parent".to_string(), + "reserved-minor".to_string(), + ], + }]; + let previous_circuits = BTreeMap::from([ + ( + "duplicate-a".to_string(), + PlannerCircuitIdentityState { + class_minor: 10, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ( + "duplicate-b".to_string(), + PlannerCircuitIdentityState { + class_minor: 10, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ( + "stale-parent".to_string(), + PlannerCircuitIdentityState { + class_minor: 11, + queue: 1, + parent_node: "old-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ( + "reserved-minor".to_string(), + PlannerCircuitIdentityState { + class_minor: u16::MAX, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + ), + ]); + + let result = plan_class_identities( + &[], + &circuit_groups, + &BTreeMap::new(), + &previous_circuits, + 0, + 0, + ); + let assigned_minors = result + .circuits + .iter() + .map(|circuit| circuit.class_minor) + .collect::>(); + + assert_eq!(result.circuits.len(), assigned_minors.len()); + assert_eq!(result.circuit_state["duplicate-a"].class_minor, 10); + assert_ne!(result.circuit_state["duplicate-b"].class_minor, 10); + assert_ne!(result.circuit_state["stale-parent"].class_minor, 11); + assert_ne!(result.circuit_state["reserved-minor"].class_minor, u16::MAX); + assert!( + assigned_minors + .iter() + .all(|minor| (3..u16::MAX).contains(minor)) + ); + } + + #[test] + fn class_identity_planner_fills_holes_below_high_saved_minors() { + let circuit_groups = vec![CircuitIdentityGroupInput { + parent_node: "flat-bucket".to_string(), + queue: 1, + circuit_ids: vec!["existing".to_string(), "new".to_string()], + }]; + let previous_circuits = BTreeMap::from([( + "existing".to_string(), + PlannerCircuitIdentityState { + class_minor: u16::MAX - 1, + queue: 1, + parent_node: "flat-bucket".to_string(), + class_major: 1, + up_class_major: 1, + }, + )]); + + let result = plan_class_identities( + &[], + &circuit_groups, + &BTreeMap::new(), + &previous_circuits, + 0, + 0, + ); + + assert_eq!(result.circuit_state["existing"].class_minor, u16::MAX - 1); + assert_eq!(result.circuit_state["new"].class_minor, 3); + } } diff --git a/src/rust/lqos_topology/src/flat_mode.rs b/src/rust/lqos_topology/src/flat_mode.rs index c8389dbf..bea5974e 100644 --- a/src/rust/lqos_topology/src/flat_mode.rs +++ b/src/rust/lqos_topology/src/flat_mode.rs @@ -74,6 +74,68 @@ fn runtime_flat_bucket_id(index: usize) -> String { format!("libreqos:generated:flat:bucket:{index}") } +fn current_flat_bucket_names(config: &Config) -> Option> { + let Value::Object(current_network) = read_json_value(&topology_effective_network_path(config))? + else { + return None; + }; + if current_network.is_empty() { + return None; + } + + let mut bucket_names = BTreeSet::new(); + for index in 0..current_network.len() { + let bucket_name = runtime_flat_bucket_name(index); + let expected_id = runtime_flat_bucket_id(index); + let node = current_network.get(&bucket_name)?.as_object()?; + if node.get("id").and_then(Value::as_str) != Some(expected_id.as_str()) { + return None; + } + if !node + .get("children") + .and_then(Value::as_object) + .is_some_and(|children| children.is_empty()) + { + return None; + } + bucket_names.insert(bucket_name); + } + (bucket_names.len() == current_network.len()).then_some(bucket_names) +} + +fn previous_flat_bucket_assignments( + config: &Config, + bucket_names: &[String], +) -> BTreeMap { + let expected_bucket_names = bucket_names.iter().cloned().collect::>(); + if current_flat_bucket_names(config).as_ref() != Some(&expected_bucket_names) { + return BTreeMap::new(); + } + + let bucket_ids_by_name = bucket_names + .iter() + .enumerate() + .map(|(index, name)| (name.as_str(), runtime_flat_bucket_id(index))) + .collect::>(); + let Ok(previous_inputs) = TopologyShapingInputsFile::load(config) else { + return BTreeMap::new(); + }; + + previous_inputs + .circuits + .into_iter() + .filter_map(|circuit| { + if circuit.resolution_source != TopologyShapingResolutionSource::FlatBucket { + return None; + } + let expected_bucket_id = + bucket_ids_by_name.get(circuit.effective_parent_node_name.as_str())?; + (circuit.effective_parent_node_id == *expected_bucket_id) + .then_some((circuit.circuit_id, circuit.effective_parent_node_name)) + }) + .collect() +} + fn runtime_flat_bucket_network(config: &Config) -> Value { let mut root = Map::new(); for index in 0..runtime_flat_bucket_count(config) { @@ -141,24 +203,29 @@ fn build_flat_bucket_assignments( .into_iter() .map(|(id, weight)| TopLevelPlannerItem { id, weight }) .collect::>(); + let previous_assignments = if config.queues.use_binpacking { + previous_flat_bucket_assignments(config, &bucket_names) + } else { + BTreeMap::new() + }; let planner_mode = if config.queues.use_binpacking { - TopLevelPlannerMode::Greedy + TopLevelPlannerMode::StableGreedy } else { TopLevelPlannerMode::RoundRobin }; let planner = plan_top_level_assignments( &items, &bucket_names, - &BTreeMap::new(), + &previous_assignments, &BTreeMap::new(), lqos_utils::unix_time::unix_now() .map(|timestamp| timestamp as f64) .unwrap_or(0.0), &TopLevelPlannerParams { mode: planner_mode, - hysteresis_threshold: 0.0, + hysteresis_threshold: 0.03, cooldown_seconds: 0.0, - move_budget_per_run: usize::MAX, + move_budget_per_run: 1, }, ); diff --git a/src/rust/lqos_topology/src/lib.rs b/src/rust/lqos_topology/src/lib.rs index 558eaa69..2fff949a 100644 --- a/src/rust/lqos_topology/src/lib.rs +++ b/src/rust/lqos_topology/src/lib.rs @@ -26,7 +26,7 @@ use lqos_overrides::{ use lqos_topology_compile::{TopologyCompiledShapingFile, TopologyImportFile}; use lqos_utils::process_lock::{ProcessFileLock, ProcessLockConfig}; use serde_json::{Map, Value}; -use std::collections::{BTreeMap, HashMap, HashSet, hash_map::Entry}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, hash_map::Entry}; use std::fs::File; use std::io::Write; use std::net::IpAddr; diff --git a/src/rust/lqos_topology/src/tests/group_06.rs b/src/rust/lqos_topology/src/tests/group_06.rs new file mode 100644 index 00000000..e57ded5d --- /dev/null +++ b/src/rust/lqos_topology/src/tests/group_06.rs @@ -0,0 +1,236 @@ +fn flat_test_config(prefix: &str, queue_count: u32) -> Config { + let lqos_directory = unique_temp_dir(prefix); + let mut config = Config { + lqos_directory: lqos_directory.to_string_lossy().to_string(), + state_directory: None, + ..Config::default() + }; + config.topology.compile_mode = "flat".to_string(); + config.queues.override_available_queues = Some(queue_count); + config.queues.use_binpacking = true; + config +} + +fn write_flat_test_devices(config: &Config, circuits: &[(&str, f32)]) { + let mut csv = String::from( + "Circuit ID,Circuit Name,Device ID,Device Name,Parent Node,Parent Node ID,Anchor Node ID,MAC,IPv4,IPv6,Download Min Mbps,Upload Min Mbps,Download Max Mbps,Upload Max Mbps,Comment\n", + ); + for (index, (circuit_id, rate)) in circuits.iter().enumerate() { + csv.push_str(&format!( + "\"{circuit_id}\",\"{circuit_id}\",\"device-{index}\",\"Device {index}\",\"\",\"\",\"\",\"02:00:00:00:00:{index:02x}\",\"192.0.2.{}/32\",\"\",\"10\",\"10\",\"{rate}\",\"{rate}\",\"\"\n", + index + 1, + )); + } + fs::write( + PathBuf::from(&config.lqos_directory).join("ShapedDevices.csv"), + csv, + ) + .expect("flat ShapedDevices.csv should write"); +} + +fn flat_test_artifacts(config: &Config) -> EffectiveTopologyArtifacts { + let canonical = TopologyCanonicalStateFile::from_legacy_network_json(&json!({})); + build_effective_topology_artifacts_from_canonical( + config, + &canonical, + &TopologyOverridesFile::default(), + &TopologyAttachmentHealthStateFile::default(), + ) + .expect("flat mode effective artifacts should build") +} + +fn flat_assignment(inputs: &TopologyShapingInputsFile) -> BTreeMap { + inputs + .circuits + .iter() + .map(|circuit| { + ( + circuit.circuit_id.clone(), + circuit.effective_parent_node_name.clone(), + ) + }) + .collect() +} + +fn changed_existing_assignments( + before: &BTreeMap, + after: &BTreeMap, +) -> usize { + before + .iter() + .filter(|(circuit_id, bucket)| { + after + .get(*circuit_id) + .is_some_and(|next_bucket| next_bucket != *bucket) + }) + .count() +} + +#[test] +fn flat_binpacking_reuses_assignments_for_identical_input() { + let config = flat_test_config("lqos-topology-flat-stable-identical", 2); + write_flat_test_devices( + &config, + &[ + ("circuit-a", 100.0), + ("circuit-b", 80.0), + ("circuit-c", 60.0), + ("circuit-d", 40.0), + ], + ); + let artifacts = flat_test_artifacts(&config); + publish_effective_topology_artifacts(&config, &artifacts, "flat-identical") + .expect("initial flat topology should publish"); + let before = + TopologyShapingInputsFile::load(&config).expect("published shaping inputs should load"); + + let after = build_shaping_inputs(&config, &artifacts) + .expect("repeated flat shaping inputs should build") + .expect("repeated flat shaping inputs should be present"); + + assert_eq!(flat_assignment(&before), flat_assignment(&after)); +} + +#[test] +fn flat_binpacking_bounds_existing_moves_when_circuits_change() { + let config = flat_test_config("lqos-topology-flat-stable-membership", 2); + let initial_circuits = [ + ("circuit-b", 100.0), + ("circuit-c", 80.0), + ("circuit-d", 60.0), + ("circuit-e", 40.0), + ]; + write_flat_test_devices(&config, &initial_circuits); + let artifacts = flat_test_artifacts(&config); + publish_effective_topology_artifacts(&config, &artifacts, "flat-before-add") + .expect("initial flat topology should publish"); + let before_add = flat_assignment( + &TopologyShapingInputsFile::load(&config).expect("initial shaping inputs should load"), + ); + + let with_added_circuit = [ + ("circuit-a", 120.0), + ("circuit-b", 100.0), + ("circuit-c", 80.0), + ("circuit-d", 60.0), + ("circuit-e", 40.0), + ]; + write_flat_test_devices(&config, &with_added_circuit); + let after_add_inputs = build_shaping_inputs(&config, &artifacts) + .expect("flat shaping inputs with an added circuit should build") + .expect("flat shaping inputs with an added circuit should be present"); + let after_add = flat_assignment(&after_add_inputs); + assert!(changed_existing_assignments(&before_add, &after_add) <= 1); + + publish_effective_topology_artifacts(&config, &artifacts, "flat-before-remove") + .expect("flat topology with the added circuit should publish"); + let after_add_published = flat_assignment( + &TopologyShapingInputsFile::load(&config) + .expect("added-circuit shaping inputs should load"), + ); + write_flat_test_devices( + &config, + &[ + ("circuit-a", 120.0), + ("circuit-b", 100.0), + ("circuit-d", 60.0), + ("circuit-e", 40.0), + ], + ); + let after_remove = flat_assignment( + &build_shaping_inputs(&config, &artifacts) + .expect("flat shaping inputs with a removed circuit should build") + .expect("flat shaping inputs with a removed circuit should be present"), + ); + + assert!(changed_existing_assignments(&after_add_published, &after_remove) <= 1); +} + +#[test] +fn flat_binpacking_discards_invalid_or_wrong_queue_count_history() { + let mut config = flat_test_config("lqos-topology-flat-invalid-history", 2); + let circuits = [ + ("circuit-a", 100.0), + ("circuit-b", 80.0), + ("circuit-c", 60.0), + ("circuit-d", 40.0), + ]; + write_flat_test_devices(&config, &circuits); + let artifacts = flat_test_artifacts(&config); + let cold_assignment = flat_assignment( + &build_shaping_inputs(&config, &artifacts) + .expect("cold flat shaping inputs should build") + .expect("cold flat shaping inputs should be present"), + ); + publish_effective_topology_artifacts(&config, &artifacts, "flat-invalid-history") + .expect("initial flat topology should publish"); + + let mut invalid_history = + TopologyShapingInputsFile::load(&config).expect("published shaping inputs should load"); + for circuit in &mut invalid_history.circuits { + let (bucket_name, bucket_id) = if circuit.effective_parent_node_name == "Generated_PN_1" { + ("Generated_PN_2", "libreqos:generated:flat:bucket:1") + } else { + ("Generated_PN_1", "libreqos:generated:flat:bucket:0") + }; + circuit.effective_parent_node_name = bucket_name.to_string(); + circuit.effective_parent_node_id = bucket_id.to_string(); + } + invalid_history + .circuits + .iter_mut() + .find(|circuit| circuit.circuit_id == "circuit-a") + .expect("circuit-a should exist") + .effective_parent_node_id = "wrong-bucket-id".to_string(); + invalid_history + .circuits + .iter_mut() + .find(|circuit| circuit.circuit_id == "circuit-b") + .expect("circuit-b should exist") + .resolution_source = TopologyShapingResolutionSource::RuntimeFallback; + let deliberately_retained_assignment = flat_assignment(&invalid_history); + invalid_history + .save(&config) + .expect("invalid shaping history should save"); + + let rebuilt_assignment = flat_assignment( + &build_shaping_inputs(&config, &artifacts) + .expect("flat shaping inputs should rebuild from invalid history") + .expect("rebuilt flat shaping inputs should be present"), + ); + assert_eq!( + rebuilt_assignment["circuit-a"], + cold_assignment["circuit-a"] + ); + assert_eq!( + rebuilt_assignment["circuit-b"], + cold_assignment["circuit-b"] + ); + assert_eq!( + rebuilt_assignment["circuit-c"], + deliberately_retained_assignment["circuit-c"] + ); + assert_eq!( + rebuilt_assignment["circuit-d"], + deliberately_retained_assignment["circuit-d"] + ); + + config.queues.override_available_queues = Some(3); + let three_bucket_artifacts = flat_test_artifacts(&config); + let after_queue_change = flat_assignment( + &build_shaping_inputs(&config, &three_bucket_artifacts) + .expect("flat shaping inputs should rebuild after queue-count change") + .expect("queue-count rebuild should produce shaping inputs"), + ); + + let cold_three_bucket_config = flat_test_config("lqos-topology-flat-cold-three-buckets", 3); + write_flat_test_devices(&cold_three_bucket_config, &circuits); + let cold_three_bucket_artifacts = flat_test_artifacts(&cold_three_bucket_config); + let cold_three_bucket_assignment = flat_assignment( + &build_shaping_inputs(&cold_three_bucket_config, &cold_three_bucket_artifacts) + .expect("cold three-bucket shaping inputs should build") + .expect("cold three-bucket shaping inputs should be present"), + ); + + assert_eq!(after_queue_change, cold_three_bucket_assignment); +} diff --git a/src/rust/lqos_topology/src/tests/helpers.rs b/src/rust/lqos_topology/src/tests/helpers.rs index 7ffe6b75..826a9a4b 100644 --- a/src/rust/lqos_topology/src/tests/helpers.rs +++ b/src/rust/lqos_topology/src/tests/helpers.rs @@ -18,13 +18,14 @@ TopologyCanonicalRateInputSource, TopologyCanonicalStateFile, TopologyEditorNode, TopologyEditorStateFile, TopologyEffectiveAttachmentState, TopologyEffectiveNodeState, TopologyEffectiveStateFile, TopologyQueueVisibilityPolicy, TopologyRuntimeStatusFile, - TopologyShapingResolutionSource, compute_effective_network_file_generation, + TopologyShapingInputsFile, TopologyShapingResolutionSource, + compute_effective_network_file_generation, topology_auto_attachment_option as auto_attachment_option, topology_effective_network_path, topology_effective_state_path, topology_runtime_status_path, topology_shaping_inputs_path, }; use lqos_overrides::{TopologyAttachmentMode, TopologyOverridesFile}; use serde_json::{Value, json}; - use std::collections::{HashMap, HashSet}; + use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/src/rust/lqos_topology/src/tests/mod.rs b/src/rust/lqos_topology/src/tests/mod.rs index 04e53225..d88cddb3 100644 --- a/src/rust/lqos_topology/src/tests/mod.rs +++ b/src/rust/lqos_topology/src/tests/mod.rs @@ -5,3 +5,4 @@ include!("group_02.rs"); include!("group_03.rs"); include!("group_04.rs"); include!("group_05.rs"); +include!("group_06.rs"); diff --git a/src/test_libreqos_shaping_inputs.py b/src/test_libreqos_shaping_inputs.py index 5a0f2b37..2bc8d99d 100644 --- a/src/test_libreqos_shaping_inputs.py +++ b/src/test_libreqos_shaping_inputs.py @@ -98,6 +98,114 @@ def tearDownModule(): class TestLibreQoSShapingInputs(unittest.TestCase): + def test_flat_planner_item_key_uses_stable_circuit_id(self): + circuit = { + "circuitID": "circuit-stable", + "idForCircuitsWithoutParentNodes": 17, + } + + self.assertEqual( + LibreQoS.planner_top_level_item_key(circuit, flat_network=True), + "flat_circuit:circuit-stable", + ) + self.assertEqual( + LibreQoS.planner_top_level_item_key(circuit, flat_network=False), + "17", + ) + self.assertEqual( + LibreQoS.planner_top_level_item_key( + {"circuitID": "0", "idForCircuitsWithoutParentNodes": 17}, + flat_network=True, + ), + "flat_circuit:0", + ) + + def test_flat_binpacking_detection_covers_legacy_and_runtime_topologies(self): + runtime_flat_network = { + "Generated_PN_1": { + "id": "libreqos:generated:flat:bucket:0", + "children": {}, + }, + "Generated_PN_2": { + "id": "libreqos:generated:flat:bucket:1", + "children": {}, + }, + } + + self.assertTrue(LibreQoS.flat_binpacking_enabled({}, True)) + self.assertTrue( + LibreQoS.flat_binpacking_enabled(runtime_flat_network, True) + ) + self.assertFalse( + LibreQoS.flat_binpacking_enabled({"Tower": {}}, True) + ) + self.assertFalse( + LibreQoS.flat_binpacking_enabled({"Generated_PN_bad": {}}, True) + ) + self.assertFalse( + LibreQoS.flat_binpacking_enabled({"Generated_PN_1": {}}, True) + ) + self.assertFalse( + LibreQoS.flat_binpacking_enabled( + {"Generated_PN_1": {}, "Generated_PN_3": {}}, + True, + ) + ) + self.assertFalse( + LibreQoS.flat_binpacking_enabled(runtime_flat_network, False) + ) + malformed_runtime_network = dict(runtime_flat_network) + malformed_runtime_network["Generated_PN_1"] = { + "id": "libreqos:generated:flat:bucket:0", + "children": {"real-child": {}}, + } + self.assertFalse( + LibreQoS.flat_binpacking_enabled(malformed_runtime_network, True) + ) + + def test_flat_planner_failure_preserves_existing_assignments(self): + items = [ + {"id": "flat_circuit:existing", "weight": 10.0}, + {"id": "flat_circuit:new", "weight": 5.0}, + ] + previous_assignments = { + "flat_circuit:existing": "Generated_PN_2", + } + + with patch.object( + LibreQoS, + "plan_top_level_cpu_bins", + side_effect=RuntimeError("planner unavailable"), + ): + with self.assertWarnsRegex(UserWarning, "preserving prior assignments"): + assignments, changed = LibreQoS.plan_flat_generated_parent_assignments( + items, + ["Generated_PN_1", "Generated_PN_2"], + previous_assignments, + {}, + 1000.0, + 1, + ) + + self.assertEqual(assignments["flat_circuit:existing"], "Generated_PN_2") + self.assertEqual(assignments["flat_circuit:new"], "Generated_PN_1") + self.assertEqual(changed, []) + + def test_saved_identity_state_is_reused_only_for_flat_binpacking(self): + state = { + "sites": {"Generated_PN_1": {"class_minor": 3}}, + "circuits": {"circuit-1": {"class_minor": 4}}, + } + + self.assertEqual( + LibreQoS.reusable_identity_state(state, True), + (state["sites"], state["circuits"]), + ) + self.assertEqual( + LibreQoS.reusable_identity_state(state, False), + ({}, {}), + ) + def test_runtime_resolved_circuit_attachment_candidates_ignore_logical_parent(self): candidates = LibreQoS._circuit_attachment_name_candidates( {