Skip to content

Commit 3f59be1

Browse files
daniel-nolandclaude
andcommitted
feat(dataplane): a configurable supervisor poll, and telemetry as of the scrape
Worker counters reached telemetry two hops late: the driver's supervisor collected them once a second, and the telemetry publisher copied them into the exporter on a second-long ticker of its own. A scrape could see counts two seconds old, so anything checking them -- a test after a burst of traffic, the flowgen runner against a live gateway -- had to wait that long for them to settle. - `--supervisor-poll-ms` (10 ms to 10 s, default 1000) sets how often the supervisor collects the workers' counters, for either driver. The rate it reports is now over the time actually elapsed since the last poll rather than the nominal period, and the driver status says what the period is. - The `/metrics` handler publishes the pipeline's, the flow table's and the workers' counts before it renders, instead of a ticker doing so between scrapes: a scrape sees them as of the scrape, the workers' as of the supervisor's last poll. The watchdog's pat and check periods are unchanged: they are about liveness, not freshness. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
1 parent cba47b3 commit 3f59be1

9 files changed

Lines changed: 114 additions & 36 deletions

File tree

‎args/src/lib.rs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,9 @@ pub const DEFAULT_DP_UX_PATH: &str = "/var/run/frr/hh/dataplane.sock";
241241
/// runtime inspection and control.
242242
pub const DEFAULT_DP_UX_PATH_CLI: &str = "/var/run/dataplane/cli.sock";
243243

244+
/// How often, in milliseconds, a driver's supervisor collects its workers' counters by default.
245+
pub const DEFAULT_SUPERVISOR_POLL_MS: u32 = 1000;
246+
244247
/// Default path to the FRR agent socket.
245248
///
246249
/// This socket is used to connect to the FRR agent that controls FRR
@@ -619,6 +622,8 @@ pub struct DpdkDriverConfigSection {
619622
pub eal_args: Vec<String>,
620623
/// Packet-processing worker threads to run, each owning one rx/tx queue pair per port
621624
pub num_workers: NonZero<u16>,
625+
/// How often, in milliseconds, the driver's supervisor collects its workers' counters.
626+
pub supervisor_poll_ms: u32,
622627
/// Hugepage placement selected by init, or `None` for EAL defaults.
623628
pub hugepages: Option<HugepagePlan>,
624629
/// Hugepage memory to require on each NIC NUMA node, in GiB, overriding what init computes
@@ -647,6 +652,8 @@ pub struct KernelDriverConfigSection {
647652
pub interfaces: Vec<InterfaceArg>,
648653
/// Packet-processing worker threads to run
649654
pub num_workers: NonZero<u16>,
655+
/// How often, in milliseconds, the driver's supervisor collects its workers' counters.
656+
pub supervisor_poll_ms: u32,
650657
}
651658

652659
/// Configuration for the dataplane's command-line interface (CLI).
@@ -925,6 +932,16 @@ impl DriverConfigSection {
925932
DriverConfigSection::Kernel(kernel) => kernel.num_workers,
926933
}
927934
}
935+
936+
/// How often the driver's supervisor collects its workers' counters and checks on them.
937+
#[must_use]
938+
pub fn supervisor_poll(&self) -> Duration {
939+
let ms = match self {
940+
DriverConfigSection::Dpdk(dpdk) => dpdk.supervisor_poll_ms,
941+
DriverConfigSection::Kernel(kernel) => kernel.supervisor_poll_ms,
942+
};
943+
Duration::from_millis(u64::from(ms))
944+
}
928945
}
929946

930947
/// Probe a raw descriptor without constructing a borrow of a possibly closed file.
@@ -1515,11 +1532,13 @@ impl TryFrom<CmdArgs> for LaunchConfiguration {
15151532
hugepages: None,
15161533
hugepages_gib: value.hugepages_gib,
15171534
num_workers: value.num_workers,
1535+
supervisor_poll_ms: value.supervisor_poll_ms,
15181536
})
15191537
}
15201538
Some("kernel") => DriverConfigSection::Kernel(KernelDriverConfigSection {
15211539
interfaces: value.interfaces().collect(),
15221540
num_workers: value.num_workers,
1541+
supervisor_poll_ms: value.supervisor_poll_ms,
15231542
}),
15241543
Some(other) => return Err(InvalidCmdArguments::InvalidDriver(other.to_string())),
15251544
None => return Err(InvalidCmdArguments::NoDriverSpecified),
@@ -1629,6 +1648,17 @@ Separate interfaces with commas and options with slashes."
16291648
)]
16301649
hugepages_gib: Option<u32>,
16311650

1651+
/// How often the driver's supervisor collects its workers' counters, in milliseconds.
1652+
#[arg(
1653+
long,
1654+
value_name = "MS",
1655+
default_value_t = DEFAULT_SUPERVISOR_POLL_MS,
1656+
value_parser = clap::value_parser!(u32).range(10..=10_000),
1657+
help = "How often, in milliseconds in [10..10000], the driver's supervisor collects its \
1658+
workers' counters: how stale the worker counters in telemetry and the CLI may be"
1659+
)]
1660+
supervisor_poll_ms: u32,
1661+
16321662
#[arg(
16331663
long,
16341664
value_name = "path to a network namespace",
@@ -1883,6 +1913,12 @@ impl CmdArgs {
18831913
self.num_workers
18841914
}
18851915

1916+
/// How often the driver's supervisor collects its workers' counters, in milliseconds.
1917+
#[must_use]
1918+
pub fn supervisor_poll_ms(&self) -> u32 {
1919+
self.supervisor_poll_ms
1920+
}
1921+
18861922
/// Get the list of kernel network interfaces to use.
18871923
///
18881924
/// Returns the interfaces specified via `--interface` arguments.
@@ -2180,6 +2216,22 @@ mod tests {
21802216
assert!(parse("0").is_err());
21812217
assert!(parse("65").is_err());
21822218
}
2219+
2220+
#[test]
2221+
fn a_supervisor_poll_outside_ten_ms_to_ten_s_is_refused_at_parse_time() {
2222+
use crate::{CmdArgs, DEFAULT_SUPERVISOR_POLL_MS, Parser};
2223+
let parse = |n: &str| CmdArgs::try_parse_from(["dataplane", "--supervisor-poll-ms", n]);
2224+
assert_eq!(
2225+
CmdArgs::try_parse_from(["dataplane"])
2226+
.unwrap()
2227+
.supervisor_poll_ms(),
2228+
DEFAULT_SUPERVISOR_POLL_MS
2229+
);
2230+
assert_eq!(parse("10").unwrap().supervisor_poll_ms(), 10);
2231+
assert_eq!(parse("10000").unwrap().supervisor_poll_ms(), 10_000);
2232+
assert!(parse("9").is_err());
2233+
assert!(parse("10001").is_err());
2234+
}
21832235
}
21842236

21852237
#[cfg(test)]

‎dataplane/src/drivers/dpdk/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ mod worker;
2424
use crate::packet_processor::PipelineFactory;
2525
use std::convert::Infallible;
2626
use std::num::NonZero;
27+
use std::time::Duration;
2728

2829
use concurrency::thread;
2930
#[allow(unused_imports)] // used under loom/shuttle backends
@@ -73,6 +74,7 @@ impl DriverDpdk {
7374
workers_subsystem: &Subsystem,
7475
ports: &'p [Port<'_>],
7576
num_workers: NonZero<u16>,
77+
supervisor_poll: Duration,
7678
setup_pipeline: &PipelineFactory<'p, Mbuf<'p>>,
7779
status_writer: DriverStatusWriter,
7880
bridge: Option<&mut DatapathEnds>,
@@ -165,6 +167,7 @@ impl DriverDpdk {
165167
scope,
166168
"dpdk",
167169
MBUF_BURST,
170+
supervisor_poll,
168171
workers_subsystem,
169172
monitors,
170173
status_writer,

‎dataplane/src/drivers/kernel/mod.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ mod sockstats;
2020
mod worker;
2121

2222
use crate::packet_processor::PipelineFactory;
23+
use std::time::Duration;
24+
2325
use concurrency::thread;
2426
use lifecycle::Subsystem;
2527
use lldp::Link;
@@ -123,11 +125,13 @@ impl DriverKernel {
123125
///
124126
/// # Errors
125127
/// Returns [`DriverError`] on interface setup or thread spawn failure.
128+
#[allow(clippy::too_many_arguments)] // each is the driver's to give
126129
pub fn start<'scope>(
127130
scope: &'scope thread::Scope<'scope, '_>,
128131
workers_subsystem: &Subsystem,
129132
args: impl IntoIterator<Item = args::InterfaceArg>,
130133
num_workers: usize,
134+
supervisor_poll: Duration,
131135
setup_pipeline: &PipelineFactory<'static, TestBuffer>,
132136
status_writer: DriverStatusWriter,
133137
mut cp_ends: Option<DatapathEnds>,
@@ -205,6 +209,7 @@ impl DriverKernel {
205209
scope,
206210
"kernel",
207211
Self::MAX_RX_PKT_BATCH,
212+
supervisor_poll,
208213
workers_subsystem,
209214
worker_monitors,
210215
status_writer,

‎dataplane/src/drivers/status.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ impl WorkerStatus {
109109
pub struct DriverStatus {
110110
/// The most frames one receive call can return, which differs between drivers.
111111
pub max_rx_batch: usize,
112+
/// How often the supervisor collects the workers' counters into this status.
113+
pub poll_period: std::time::Duration,
112114
pub workers: Vec<WorkerStatus>,
113115
}
114116

@@ -239,7 +241,7 @@ impl Display for DriverStatus {
239241
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
240242
Heading("Packet driver status").fmt(f)?;
241243
writeln!(f, " max rx batch: {} pkts", self.max_rx_batch)?;
242-
write!(f, " activity poll: {} s", supervisor::TASK_POLL_PERIOD)?;
244+
write!(f, " activity poll: {:?}", self.poll_period)?;
243245
write!(f, " watchdog pat: {} s", supervisor::TASK_PAT_PERIOD)?;
244246
writeln!(f, " watchdog check: {} s", supervisor::TASK_CHECK_PERIOD)?;
245247

‎dataplane/src/drivers/supervisor.rs‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ pub(crate) const TASK_GRACE_PERIOD: u16 = 4;
4747
/// Interval, in seconds, at which the supervisor will check rx task watchdogs
4848
pub(crate) const TASK_CHECK_PERIOD: u16 = TASK_PAT_PERIOD + TASK_GRACE_PERIOD;
4949

50-
/// Interval, in seconds, at which the supervisor checks rx task activity, ignoring watchdogs
51-
pub(crate) const TASK_POLL_PERIOD: u16 = 1;
52-
5350
/// The supervisor's view of one rx task: the interface it serves and the watchdog it reports on.
5451
#[derive(Clone)]
5552
pub(crate) struct RxTaskMonitor {
@@ -93,23 +90,25 @@ impl<'scope, E> WorkerMonitor<'scope, E> {
9390
///
9491
/// `driver` names the driver in the supervisor's thread name and logs. The supervisor
9592
/// just joins-and-logs on worker termination; worker fatal reporting is left to the driver.
96-
/// `publish_counters` runs on each status poll and once after all workers have stopped.
93+
/// The supervisor polls every `poll_period`, collecting the workers' counters into the
94+
/// [`DriverStatus`]; `publish_counters` runs on each poll and once after all workers have stopped.
9795
///
9896
/// # Errors
9997
///
10098
/// Returns an error if the supervisor thread cannot be spawned.
99+
#[allow(clippy::too_many_arguments)] // each is the driver's to give
101100
pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
102101
scope: &'scope thread::Scope<'scope, '_>,
103102
driver: &'static str,
104103
max_rx_batch: usize,
104+
poll_period: Duration,
105105
subsystem: &Subsystem,
106106
mut monitors: Vec<WorkerMonitor<'scope, E>>,
107107
status_writer: DriverStatusWriter,
108108
mut publish_counters: impl FnMut() + Send + 'scope,
109109
) -> std::io::Result<()> {
110110
// the two time intervals that matter for liveness detection
111111
let check_period = Duration::from_secs(u64::from(TASK_CHECK_PERIOD));
112-
let poll_period = Duration::from_secs(u64::from(TASK_POLL_PERIOD));
113112

114113
let subsystem = subsystem.clone();
115114

@@ -135,6 +134,8 @@ pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
135134

136135
// the next instant when the rx tasks watchdogs should be checked.
137136
let mut next_watchdog_check = clock::now().add(check_period);
137+
// when the counters were last collected, for rates over the time actually elapsed
138+
let mut last_poll = clock::now();
138139

139140
loop {
140141
// check if we must run. Otherwise (got cancelled) join all workers
@@ -145,6 +146,8 @@ pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
145146
// check the current time and decide if we should check whether the rx tasks patted the watchdogs.
146147
// If so, compute the next time we should check them again in the future.
147148
let now = clock::now();
149+
let elapsed = now.saturating_duration_since(last_poll);
150+
last_poll = now;
148151
let check_watchdog = now >= next_watchdog_check;
149152
if check_watchdog {
150153
while next_watchdog_check <= now {
@@ -174,7 +177,13 @@ pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
174177
any_running = true;
175178

176179
// check the worker's rx tasks and update the corresponding status
177-
check_worker_rx_tasks(driver, monitor, wk_status, check_watchdog);
180+
check_worker_rx_tasks(
181+
driver,
182+
monitor,
183+
wk_status,
184+
check_watchdog,
185+
elapsed,
186+
);
178187
}
179188
} else {
180189
// A worker monitor without a handle means that the worker was joined already
@@ -192,6 +201,7 @@ pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
192201
// publish the status of the driver
193202
status_writer.publish(DriverStatus {
194203
max_rx_batch,
204+
poll_period,
195205
workers: workers_status.clone(),
196206
});
197207

@@ -214,6 +224,7 @@ pub(crate) fn spawn_supervisor<'scope, E: Display + Send + Sync + 'scope>(
214224
// want to log the last state
215225
let last = DriverStatus {
216226
max_rx_batch,
227+
poll_period,
217228
workers: workers_status.clone(),
218229
};
219230
status_writer.publish(last);
@@ -278,13 +289,15 @@ fn must_run<E: Display>(
278289
}
279290

280291
/// Check the activity of the rx tasks for a worker (watchdog, if `check_watchdog` is true)
281-
/// from its `WorkerMonitor` and update the corresponding `WorkerStatus`
292+
/// from its `WorkerMonitor` and update the corresponding `WorkerStatus`, with rates over `elapsed`,
293+
/// the time since the last check.
282294
#[allow(clippy::cast_precision_loss)]
283295
fn check_worker_rx_tasks<E>(
284296
driver: &str,
285297
monitor: &WorkerMonitor<'_, E>,
286298
wk_status: &mut WorkerStatus,
287299
check_watchdog: bool,
300+
elapsed: Duration,
288301
) {
289302
for (idx, task) in monitor.rx_tasks.iter().enumerate() {
290303
let rx_task_status = &mut wk_status.rx_tasks[idx];
@@ -309,7 +322,11 @@ fn check_worker_rx_tasks<E>(
309322
);
310323
}
311324
Activity::Active => {
312-
rx_task_status.pps = counters.rx as f64 / f64::from(TASK_POLL_PERIOD);
325+
rx_task_status.pps = if elapsed.is_zero() {
326+
0.0
327+
} else {
328+
counters.rx as f64 / elapsed.as_secs_f64()
329+
};
313330
debug!(
314331
"{driver} worker {} on {}: rx {} ({:.0} pps), tx {}, pipeline drops {}, \
315332
tx drops {}, punt drops {}, parse errors {}",
@@ -366,6 +383,7 @@ mod tests {
366383
scope,
367384
"test",
368385
32,
386+
Duration::from_secs(1),
369387
&subsystem,
370388
vec![WorkerMonitor::new(0, worker, vec![task])],
371389
writer,

‎dataplane/src/runtime.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,7 @@ fn run_kernel_driver(
567567
workers,
568568
config.driver.interfaces().cloned().collect::<Vec<_>>(),
569569
config.driver.num_workers().get().into(),
570+
config.driver.supervisor_poll(),
570571
&ingredients.factory(),
571572
status_writer,
572573
bridge,
@@ -624,6 +625,7 @@ fn run_dpdk_driver(
624625
workers,
625626
&ports,
626627
config.driver.num_workers(),
628+
config.driver.supervisor_poll(),
627629
&ingredients.factory(),
628630
status_writer,
629631
bridge.as_mut(),

0 commit comments

Comments
 (0)