From d3e288427305a1cd70f073516e256a046ee1f453 Mon Sep 17 00:00:00 2001 From: Herbert Wolverson Date: Mon, 13 Nov 2023 08:18:26 -0600 Subject: [PATCH 1/2] LTS data collection was occasionally overflowing on calculating the mean average. If a large-enough block had to be summed, the total could overflow the storage type. To prevent this from ever happening, we instead calculate the median - which is likely to give a better representation anyway. Also fixes a compile warning. --- .../lqos_bus/src/bus/unix_socket_server.rs | 2 +- .../src/collector/collation/min_max.rs | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/rust/lqos_bus/src/bus/unix_socket_server.rs b/src/rust/lqos_bus/src/bus/unix_socket_server.rs index e209f67b..fcb93ab1 100644 --- a/src/rust/lqos_bus/src/bus/unix_socket_server.rs +++ b/src/rust/lqos_bus/src/bus/unix_socket_server.rs @@ -78,7 +78,7 @@ impl UnixSocketServer { } fn make_socket_public() -> Result<(), UnixSocketServerError> { - lqos_utils::run_success!( + let _ = lqos_utils::run_success!( "/bin/chmod", "-R", "a+rwx", diff --git a/src/rust/lts_client/src/collector/collation/min_max.rs b/src/rust/lts_client/src/collector/collation/min_max.rs index c46d1fc8..f2a5fd58 100644 --- a/src/rust/lts_client/src/collector/collation/min_max.rs +++ b/src/rust/lts_client/src/collector/collation/min_max.rs @@ -7,6 +7,21 @@ pub(crate) struct MinMaxAvg { pub(crate) avg: T, } +fn median(stats: &[T]) -> T +where + T: Bounded + Zero + std::ops::AddAssign + Copy + std::cmp::Ord + CheckedDiv + NumCast, +{ + let mut sorted = stats.to_vec(); + sorted.sort(); + let len = sorted.len(); + let mid = len / 2; + if len % 2 == 0 { + (sorted[mid] + sorted[mid - 1]).checked_div(&T::from(2).unwrap()).unwrap_or(T::zero()) + } else { + sorted[mid] + } +} + impl< T: Bounded + Zero @@ -20,17 +35,13 @@ impl< pub(crate) fn from_slice(stats: &[T]) -> Self { let mut min = T::max_value(); let mut max = T::min_value(); - let mut avg = T::zero(); stats.iter().for_each(|n| { - avg += *n; min = T::min(min, *n); max = T::max(max, *n); }); - let len = T::from(stats.len()).unwrap(); - avg = avg.checked_div(&len).unwrap_or(T::zero()); - Self { max, min, avg } + Self { max, min, avg: median(stats) } } } From 7ed0f6fdacd51f7dc20ee333ff547b7b2fd3ee47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Chac=C3=B3n?= Date: Mon, 13 Nov 2023 21:27:06 -0700 Subject: [PATCH 2/2] Update integrationUISP.py --- src/integrationUISP.py | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/integrationUISP.py b/src/integrationUISP.py index dace05b5..af27988a 100644 --- a/src/integrationUISP.py +++ b/src/integrationUISP.py @@ -328,26 +328,25 @@ def findNodesBranchedOffPtMP(siteList, dataLinks, sites, rootSite, foundAirFiber parent = site['parent'] for link in dataLinks: if (link['to']['site'] is not None) and (link['to']['site']['identification'] is not None): - if ('identification' in link['to']['site']) and (link['to']['site']['identification'] is not None): - if ('identification' in link['from']['site']) and (link['from']['site']['identification'] is not None): - # Respect parent defined by topology and overrides - if link['from']['site']['identification']['id'] == trueParent: - if link['to']['site']['identification']['id'] == id: - if (link['from']['device']['overview']['wirelessMode'] == 'ap-ptmp') or (link['from']['device']['overview']['wirelessMode'] == 'ap'): - if 'overview' in link['to']['device']: - if ('downlinkCapacity' in link['to']['device']['overview']) and ('uplinkCapacity' in link['to']['device']['overview']): - if (link['to']['device']['overview']['downlinkCapacity'] is not None) and (link['to']['device']['overview']['uplinkCapacity'] is not None): - apID = link['from']['device']['identification']['id'] - # Capacity of the PtMP client radio feeding the PoP will be used as the site bandwidth limit - download = int(round(link['to']['device']['overview']['downlinkCapacity']/1000000)) - upload = int(round(link['to']['device']['overview']['uplinkCapacity']/1000000)) - nodeOffPtMP[id] = {'download': download, - 'upload': upload, - parent: apID - } - if usePtMPasParent: - site['parent'] = apID - print('Site ' + name + ' will use PtMP AP as parent.') + if ('identification' in link['to']['site']) and (link['to']['site']['identification'] is not None) and link['from'] is not None and link['from']['site'] is not None and link['from']['site']['identification'] is not None: + # Respect parent defined by topology and overrides + if link['from']['site']['identification']['id'] == trueParent: + if link['to']['site']['identification']['id'] == id: + if (link['from']['device']['overview']['wirelessMode'] == 'ap-ptmp') or (link['from']['device']['overview']['wirelessMode'] == 'ap'): + if 'overview' in link['to']['device']: + if ('downlinkCapacity' in link['to']['device']['overview']) and ('uplinkCapacity' in link['to']['device']['overview']): + if (link['to']['device']['overview']['downlinkCapacity'] is not None) and (link['to']['device']['overview']['uplinkCapacity'] is not None): + apID = link['from']['device']['identification']['id'] + # Capacity of the PtMP client radio feeding the PoP will be used as the site bandwidth limit + download = int(round(link['to']['device']['overview']['downlinkCapacity']/1000000)) + upload = int(round(link['to']['device']['overview']['uplinkCapacity']/1000000)) + nodeOffPtMP[id] = {'download': download, + 'upload': upload, + parent: apID + } + if usePtMPasParent: + site['parent'] = apID + print('Site ' + name + ' will use PtMP AP as parent.') return siteList, nodeOffPtMP def handleMultipleInternetNodes(sites, dataLinks, uispSite):