From ccbda62b7d75f4bee87357cff78a845246a882b8 Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 18:52:49 -0700 Subject: [PATCH 1/7] userspace: add three-color policer core --- userspace-dp/src/filter/README.md | 39 ++- userspace-dp/src/filter/policer.rs | 477 ++++++++++++++++++++++++++++- 2 files changed, 498 insertions(+), 18 deletions(-) diff --git a/userspace-dp/src/filter/README.md b/userspace-dp/src/filter/README.md index f0ef5f8c7..fd7822077 100644 --- a/userspace-dp/src/filter/README.md +++ b/userspace-dp/src/filter/README.md @@ -15,10 +15,14 @@ Mirrors the BPF firewall-filter pipeline in userspace. - `compiler.rs` — parses the typed config's filter terms and lowers them to `FilterTerm`s (prefix vectors, protocol bitmap, port matcher, DSCP bitmap). -- `engine.rs` — per-term evaluation, first-match-wins. Hands off to - the policer for `then policer ...` actions. -- `policer.rs` — token-bucket implementation; the per-policer field - is `rate_bytes_per_ns` (rate, not interval). +- `engine.rs` — per-term evaluation, first-match-wins. It carries the + matched `then policer ...` name in the filter result; forwarding-path + enforcement is a separate wiring step. +- `policer.rs` — token-bucket implementation plus the #1375 RFC + 2697/2698 three-color meter core. Token math is integer-only: + the legacy token bucket keeps its bits/sec constructor contract, and + the three-color core uses byte/sec rates; both refill scaled `u128` + token buckets from monotonic nanosecond timestamps. - `tests.rs` — co-located unit tests covering matching ports, prefix vectors, TCP flags. @@ -33,3 +37,30 @@ Mirrors the BPF firewall-filter pipeline in userspace. and are surfaced through the status JSON. - `from-interface` is matched at the binding level (caller sets the ingress interface; the term doesn't re-derive it). + +## #1375 Three-Color Policer Foundation + +Implemented here: + +- srTCM (RFC 2697): committed tokens fill at CIR; overflow fills the + excess bucket only after the committed bucket is full. +- trTCM (RFC 2698): committed and peak buckets refill independently at + CIR and PIR. +- Color-aware classification never promotes incoming yellow or red + packets. Color-blind classification ignores inherited color. +- Per-color treatments can carry DSCP rewrite and drop decisions in the + meter decision. + +Still gated before removing the userspace capability rejection: + +- The Go userspace snapshot currently publishes only single-rate + two-color `PolicerSnapshot` fields; three-color snapshot fields and + validation are not wired to Rust yet. +- Filter terms still carry a policer name in the evaluation result. + The hot forwarding path must move to stable policer IDs with + ID-indexed or sharded state before three-color policers are enabled. +- Flow-cache hits do not yet execute policer decisions, because the + forwarding path does not consume this meter core. +- Forwarding-path application of per-color counters, red drops, DSCP + rewrites, Rust status, Go status, CLI, and Prometheus export remain + follow-on wiring. diff --git a/userspace-dp/src/filter/policer.rs b/userspace-dp/src/filter/policer.rs index 015edb072..b43c4c4d2 100644 --- a/userspace-dp/src/filter/policer.rs +++ b/userspace-dp/src/filter/policer.rs @@ -1,18 +1,38 @@ -// Token-bucket policer state extracted from filter.rs (#1049 P2 structural split). -// Pure relocation — bodies are byte-for-byte identical; only the -// enclosing module and visibility paths change. +// Token-bucket and three-color policer state for the userspace filter path. + +const TOKEN_SCALE: u128 = 1_000_000_000; + +#[inline] +fn scaled_bytes(bytes: u64) -> u128 { + u128::from(bytes) * TOKEN_SCALE +} + +#[inline] +fn refill_scaled(rate_bytes_per_sec: u64, elapsed_ns: u64) -> u128 { + u128::from(rate_bytes_per_sec) * u128::from(elapsed_ns) +} + +#[inline] +fn refill_scaled_bits(rate_bits_per_sec: u64, elapsed_ns: u64) -> u128 { + u128::from(rate_bits_per_sec) * u128::from(elapsed_ns) / 8 +} + +#[inline] +fn capped_add(tokens: u128, add: u128, cap: u128) -> u128 { + tokens.saturating_add(add).min(cap) +} /// Token-bucket policer state. #[allow(dead_code)] #[derive(Clone, Debug)] pub(crate) struct PolicerState { pub(crate) name: String, - /// Refill rate in bytes per nanosecond (bandwidth_bps / 8 / 1e9). - pub(crate) rate_bytes_per_ns: f64, + /// Refill rate in bits per second. + pub(crate) rate_bits_per_sec: u64, /// Maximum bucket size in bytes. pub(crate) burst_bytes: u64, - /// Current token count (bytes). - pub(crate) tokens: f64, + /// Current token count in bytes scaled by `TOKEN_SCALE`. + pub(crate) tokens: u128, /// Last refill timestamp (monotonic nanoseconds). pub(crate) last_refill_ns: u64, /// Whether to discard excess traffic (vs. mark). @@ -28,12 +48,11 @@ impl PolicerState { burst_bytes: u64, discard_excess: bool, ) -> Self { - let rate_bytes_per_ns = (bandwidth_bps as f64) / 8.0 / 1_000_000_000.0; Self { name, - rate_bytes_per_ns, + rate_bits_per_sec: bandwidth_bps, burst_bytes, - tokens: burst_bytes as f64, + tokens: scaled_bytes(burst_bytes), last_refill_ns: 0, discard_excess, initialized: false, @@ -47,17 +66,17 @@ impl PolicerState { if !self.initialized { self.initialized = true; self.last_refill_ns = now_ns; - self.tokens = self.burst_bytes as f64; + self.tokens = scaled_bytes(self.burst_bytes); } // Refill tokens if now_ns > self.last_refill_ns { let elapsed_ns = now_ns - self.last_refill_ns; - let refill = elapsed_ns as f64 * self.rate_bytes_per_ns; - self.tokens = (self.tokens + refill).min(self.burst_bytes as f64); + let refill = refill_scaled_bits(self.rate_bits_per_sec, elapsed_ns); + self.tokens = capped_add(self.tokens, refill, scaled_bytes(self.burst_bytes)); self.last_refill_ns = now_ns; } // Try to consume - let cost = packet_bytes as f64; + let cost = scaled_bytes(packet_bytes); if self.tokens >= cost { self.tokens -= cost; true @@ -67,3 +86,433 @@ impl PolicerState { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PacketColor { + Green, + Yellow, + Red, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum ThreeColorMode { + SingleRate, + TwoRate, +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct ColorTreatment { + pub(crate) dscp_rewrite: Option, + pub(crate) drop: bool, +} + +impl ColorTreatment { + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn rewrite(dscp: u8) -> Self { + Self { + dscp_rewrite: Some(dscp & 0x3f), + drop: false, + } + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn drop() -> Self { + Self { + dscp_rewrite: None, + drop: true, + } + } +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct ThreeColorTreatments { + pub(crate) green: ColorTreatment, + pub(crate) yellow: ColorTreatment, + pub(crate) red: ColorTreatment, +} + +impl ThreeColorTreatments { + fn treatment_for(self, color: PacketColor) -> ColorTreatment { + match color { + PacketColor::Green => self.green, + PacketColor::Yellow => self.yellow, + PacketColor::Red => self.red, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct ThreeColorDecision { + pub(crate) color: PacketColor, + pub(crate) dscp_rewrite: Option, + pub(crate) drop: bool, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PolicerConfigError { + ZeroRate, + ZeroBurst, + PeakRateBelowCommittedRate, + PeakBurstBelowCommittedBurst, +} + +/// Compact RFC 2697/2698 policer state. All hot fields are numeric so the +/// state can move to ID-indexed shards without retaining name-keyed lookup. +#[derive(Clone, Debug)] +pub(crate) struct ThreeColorPolicerState { + mode: ThreeColorMode, + color_blind: bool, + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + peak_or_excess_rate_bytes_per_sec: u64, + peak_or_excess_burst_bytes: u64, + committed_tokens: u128, + peak_or_excess_tokens: u128, + last_refill_ns: u64, + initialized: bool, + treatments: ThreeColorTreatments, +} + +impl ThreeColorPolicerState { + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn sr_tcm( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + excess_burst_bytes: u64, + color_blind: bool, + ) -> Result { + Self::sr_tcm_with_treatments( + committed_rate_bytes_per_sec, + committed_burst_bytes, + excess_burst_bytes, + color_blind, + ThreeColorTreatments::default(), + ) + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn sr_tcm_with_treatments( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + excess_burst_bytes: u64, + color_blind: bool, + treatments: ThreeColorTreatments, + ) -> Result { + if committed_rate_bytes_per_sec == 0 { + return Err(PolicerConfigError::ZeroRate); + } + if committed_burst_bytes == 0 || excess_burst_bytes == 0 { + return Err(PolicerConfigError::ZeroBurst); + } + Ok(Self { + mode: ThreeColorMode::SingleRate, + color_blind, + committed_rate_bytes_per_sec, + committed_burst_bytes, + peak_or_excess_rate_bytes_per_sec: 0, + peak_or_excess_burst_bytes: excess_burst_bytes, + committed_tokens: scaled_bytes(committed_burst_bytes), + peak_or_excess_tokens: scaled_bytes(excess_burst_bytes), + last_refill_ns: 0, + initialized: false, + treatments, + }) + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn tr_tcm( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + peak_rate_bytes_per_sec: u64, + peak_burst_bytes: u64, + color_blind: bool, + ) -> Result { + Self::tr_tcm_with_treatments( + committed_rate_bytes_per_sec, + committed_burst_bytes, + peak_rate_bytes_per_sec, + peak_burst_bytes, + color_blind, + ThreeColorTreatments::default(), + ) + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn tr_tcm_with_treatments( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + peak_rate_bytes_per_sec: u64, + peak_burst_bytes: u64, + color_blind: bool, + treatments: ThreeColorTreatments, + ) -> Result { + if committed_rate_bytes_per_sec == 0 || peak_rate_bytes_per_sec == 0 { + return Err(PolicerConfigError::ZeroRate); + } + if committed_burst_bytes == 0 || peak_burst_bytes == 0 { + return Err(PolicerConfigError::ZeroBurst); + } + if peak_rate_bytes_per_sec < committed_rate_bytes_per_sec { + return Err(PolicerConfigError::PeakRateBelowCommittedRate); + } + if peak_burst_bytes < committed_burst_bytes { + return Err(PolicerConfigError::PeakBurstBelowCommittedBurst); + } + Ok(Self { + mode: ThreeColorMode::TwoRate, + color_blind, + committed_rate_bytes_per_sec, + committed_burst_bytes, + peak_or_excess_rate_bytes_per_sec: peak_rate_bytes_per_sec, + peak_or_excess_burst_bytes: peak_burst_bytes, + committed_tokens: scaled_bytes(committed_burst_bytes), + peak_or_excess_tokens: scaled_bytes(peak_burst_bytes), + last_refill_ns: 0, + initialized: false, + treatments, + }) + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn meter( + &mut self, + now_ns: u64, + packet_bytes: u64, + incoming_color: PacketColor, + ) -> ThreeColorDecision { + self.refill(now_ns); + let effective_incoming_color = if self.color_blind { + PacketColor::Green + } else { + incoming_color + }; + let color = match self.mode { + ThreeColorMode::SingleRate => self.meter_sr_tcm(packet_bytes, effective_incoming_color), + ThreeColorMode::TwoRate => self.meter_tr_tcm(packet_bytes, effective_incoming_color), + }; + let treatment = self.treatments.treatment_for(color); + ThreeColorDecision { + color, + dscp_rewrite: treatment.dscp_rewrite, + drop: treatment.drop, + } + } + + fn refill(&mut self, now_ns: u64) { + if !self.initialized { + self.initialized = true; + self.last_refill_ns = now_ns; + self.committed_tokens = scaled_bytes(self.committed_burst_bytes); + self.peak_or_excess_tokens = scaled_bytes(self.peak_or_excess_burst_bytes); + return; + } + if now_ns <= self.last_refill_ns { + return; + } + + let elapsed_ns = now_ns - self.last_refill_ns; + match self.mode { + ThreeColorMode::SingleRate => self.refill_sr_tcm(elapsed_ns), + ThreeColorMode::TwoRate => self.refill_tr_tcm(elapsed_ns), + } + self.last_refill_ns = now_ns; + } + + fn refill_sr_tcm(&mut self, elapsed_ns: u64) { + let refill = refill_scaled(self.committed_rate_bytes_per_sec, elapsed_ns); + let committed_cap = scaled_bytes(self.committed_burst_bytes); + let excess_cap = scaled_bytes(self.peak_or_excess_burst_bytes); + let committed_space = committed_cap.saturating_sub(self.committed_tokens); + let committed_add = refill.min(committed_space); + self.committed_tokens += committed_add; + let excess_add = refill - committed_add; + self.peak_or_excess_tokens = capped_add(self.peak_or_excess_tokens, excess_add, excess_cap); + } + + fn refill_tr_tcm(&mut self, elapsed_ns: u64) { + self.committed_tokens = capped_add( + self.committed_tokens, + refill_scaled(self.committed_rate_bytes_per_sec, elapsed_ns), + scaled_bytes(self.committed_burst_bytes), + ); + self.peak_or_excess_tokens = capped_add( + self.peak_or_excess_tokens, + refill_scaled(self.peak_or_excess_rate_bytes_per_sec, elapsed_ns), + scaled_bytes(self.peak_or_excess_burst_bytes), + ); + } + + fn meter_sr_tcm(&mut self, packet_bytes: u64, incoming_color: PacketColor) -> PacketColor { + let cost = scaled_bytes(packet_bytes); + if incoming_color == PacketColor::Green && self.committed_tokens >= cost { + self.committed_tokens -= cost; + return PacketColor::Green; + } + if incoming_color != PacketColor::Red && self.peak_or_excess_tokens >= cost { + self.peak_or_excess_tokens -= cost; + return PacketColor::Yellow; + } + PacketColor::Red + } + + fn meter_tr_tcm(&mut self, packet_bytes: u64, incoming_color: PacketColor) -> PacketColor { + let cost = scaled_bytes(packet_bytes); + if incoming_color == PacketColor::Green + && self.peak_or_excess_tokens >= cost + && self.committed_tokens >= cost + { + self.peak_or_excess_tokens -= cost; + self.committed_tokens -= cost; + return PacketColor::Green; + } + if incoming_color != PacketColor::Red && self.peak_or_excess_tokens >= cost { + self.peak_or_excess_tokens -= cost; + return PacketColor::Yellow; + } + PacketColor::Red + } +} + +#[cfg(test)] +#[allow(non_snake_case)] +mod tests { + use super::*; + + fn sr_tcm( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + excess_burst_bytes: u64, + color_blind: bool, + ) -> ThreeColorPolicerState { + ThreeColorPolicerState::sr_tcm( + committed_rate_bytes_per_sec, + committed_burst_bytes, + excess_burst_bytes, + color_blind, + ) + .expect("valid srTCM config") + } + + fn tr_tcm( + committed_rate_bytes_per_sec: u64, + committed_burst_bytes: u64, + peak_rate_bytes_per_sec: u64, + peak_burst_bytes: u64, + color_blind: bool, + ) -> ThreeColorPolicerState { + ThreeColorPolicerState::tr_tcm( + committed_rate_bytes_per_sec, + committed_burst_bytes, + peak_rate_bytes_per_sec, + peak_burst_bytes, + color_blind, + ) + .expect("valid trTCM config") + } + + #[test] + fn srTCM_green_yellow_red_at_thresholds() { + let mut policer = sr_tcm(100, 100, 50, true); + + let green = policer.meter(0, 100, PacketColor::Green); + let yellow = policer.meter(0, 50, PacketColor::Green); + let red = policer.meter(0, 1, PacketColor::Green); + + assert_eq!(green.color, PacketColor::Green); + assert_eq!(yellow.color, PacketColor::Yellow); + assert_eq!(red.color, PacketColor::Red); + } + + #[test] + fn srTCM_c_overflow_refills_e_bucket() { + let mut policer = sr_tcm(100, 100, 200, true); + + let first = policer.meter(0, 150, PacketColor::Green); + assert_eq!(first.color, PacketColor::Yellow); + + let green = policer.meter(1_000_000_000, 100, PacketColor::Green); + assert_eq!(green.color, PacketColor::Green); + + let yellow = policer.meter(1_000_000_000, 150, PacketColor::Green); + assert_eq!(yellow.color, PacketColor::Yellow); + } + + #[test] + fn trTCM_independent_CIR_PIR() { + let mut policer = tr_tcm(100, 100, 200, 200, true); + + let initial = policer.meter(0, 100, PacketColor::Green); + assert_eq!(initial.color, PacketColor::Green); + + let yellow = policer.meter(500_000_000, 100, PacketColor::Green); + assert_eq!(yellow.color, PacketColor::Yellow); + + let green = policer.meter(1_000_000_000, 100, PacketColor::Green); + assert_eq!(green.color, PacketColor::Green); + } + + #[test] + fn color_aware_never_promotes_incoming_yellow_or_red() { + let mut sr_policer = sr_tcm(100, 100, 100, false); + + let yellow = sr_policer.meter(0, 50, PacketColor::Yellow); + let red = sr_policer.meter(0, 50, PacketColor::Red); + + assert_eq!(yellow.color, PacketColor::Yellow); + assert_eq!(red.color, PacketColor::Red); + + let mut tr_policer = tr_tcm(100, 100, 100, 100, false); + + let yellow = tr_policer.meter(0, 50, PacketColor::Yellow); + let red = tr_policer.meter(0, 50, PacketColor::Red); + + assert_eq!(yellow.color, PacketColor::Yellow); + assert_eq!(red.color, PacketColor::Red); + } + + #[test] + fn color_blind_ignores_incoming_color() { + let mut policer = sr_tcm(100, 100, 100, true); + + let green = policer.meter(0, 50, PacketColor::Red); + + assert_eq!(green.color, PacketColor::Green); + } + + #[test] + fn u128_bucket_math_boundary_inputs() { + let mut policer = sr_tcm(u64::MAX, u64::MAX, u64::MAX, true); + + let first = policer.meter(0, u64::MAX, PacketColor::Green); + let refilled = policer.meter(u64::MAX, u64::MAX, PacketColor::Green); + + assert_eq!(first.color, PacketColor::Green); + assert_eq!(refilled.color, PacketColor::Green); + } + + #[test] + fn three_color_dscp_rewrite() { + let treatments = ThreeColorTreatments { + green: ColorTreatment::rewrite(10), + yellow: ColorTreatment::rewrite(20), + red: { + let mut treatment = ColorTreatment::drop(); + treatment.dscp_rewrite = Some(30); + treatment + }, + }; + let mut policer = + ThreeColorPolicerState::sr_tcm_with_treatments(100, 100, 50, true, treatments) + .expect("valid srTCM config"); + + let green = policer.meter(0, 100, PacketColor::Green); + let yellow = policer.meter(0, 50, PacketColor::Green); + let red = policer.meter(0, 1, PacketColor::Green); + + assert_eq!(green.dscp_rewrite, Some(10)); + assert!(!green.drop); + assert_eq!(yellow.dscp_rewrite, Some(20)); + assert!(!yellow.drop); + assert_eq!(red.dscp_rewrite, Some(30)); + assert!(red.drop); + } +} From 61ccc1b7a9cfe0a18a3a2eea9ea22faa57c57500 Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 19:38:36 -0700 Subject: [PATCH 2/7] userspace: wire three-color policer snapshot schema --- pkg/config/compiler.go | 39 ++++++++++ pkg/config/parser_ast_test.go | 68 ++++++++++++++++++ pkg/dataplane/userspace/manager_test.go | 51 ++++++++++++++ pkg/dataplane/userspace/protocol.go | 70 ++++++++++-------- pkg/dataplane/userspace/protocol_test.go | 36 ++++++++++ pkg/dataplane/userspace/snapshot.go | 90 +++++++++++++++++------- userspace-dp/src/filter/README.md | 9 ++- userspace-dp/src/main_tests.rs | 46 ++++++++++++ userspace-dp/src/protocol.rs | 21 ++++++ 9 files changed, 371 insertions(+), 59 deletions(-) diff --git a/pkg/config/compiler.go b/pkg/config/compiler.go index c94ecfe42..2712b838e 100644 --- a/pkg/config/compiler.go +++ b/pkg/config/compiler.go @@ -218,6 +218,9 @@ func compileExpanded(tree *ConfigTree) (*Config, error) { if err := validateClassOfServiceStrict(cfg.ClassOfService); err != nil { return nil, err } + if err := validateThreeColorPolicersStrict(cfg.Firewall.ThreeColorPolicers); err != nil { + return nil, err + } if warnings := ValidateConfig(cfg); len(warnings) > 0 { for _, w := range warnings { @@ -228,6 +231,42 @@ func compileExpanded(tree *ConfigTree) (*Config, error) { return cfg, nil } +func validateThreeColorPolicersStrict(policers map[string]*ThreeColorPolicerConfig) error { + for name, pol := range policers { + if pol == nil { + continue + } + displayName := pol.Name + if displayName == "" { + displayName = name + } + if pol.CIR == 0 { + return fmt.Errorf("firewall three-color-policer %q requires positive committed-information-rate", displayName) + } + if pol.CBS == 0 { + return fmt.Errorf("firewall three-color-policer %q requires positive committed-burst-size", displayName) + } + if pol.PBS == 0 { + if pol.TwoRate { + return fmt.Errorf("firewall three-color-policer %q requires positive peak-burst-size", displayName) + } + return fmt.Errorf("firewall three-color-policer %q requires positive excess-burst-size", displayName) + } + if pol.TwoRate { + if pol.PIR == 0 { + return fmt.Errorf("firewall three-color-policer %q requires positive peak-information-rate", displayName) + } + if pol.PIR < pol.CIR { + return fmt.Errorf("firewall three-color-policer %q peak-information-rate must be >= committed-information-rate", displayName) + } + if pol.PBS < pol.CBS { + return fmt.Errorf("firewall three-color-policer %q peak-burst-size must be >= committed-burst-size", displayName) + } + } + } + return nil +} + func validateClassOfServiceStrict(cos *ClassOfServiceConfig) error { if cos == nil { return nil diff --git a/pkg/config/parser_ast_test.go b/pkg/config/parser_ast_test.go index a5e23554e..85e10972a 100644 --- a/pkg/config/parser_ast_test.go +++ b/pkg/config/parser_ast_test.go @@ -4539,6 +4539,74 @@ func TestThreeColorPolicerSetSyntax(t *testing.T) { } } +func TestThreeColorPolicerStrictValidation(t *testing.T) { + tests := []struct { + name string + lines []string + wantErr string + }{ + { + name: "missing committed rate", + lines: []string{ + "set firewall three-color-policer bad single-rate committed-burst-size 100k", + "set firewall three-color-policer bad single-rate excess-burst-size 200k", + }, + wantErr: "committed-information-rate", + }, + { + name: "two-rate missing peak rate", + lines: []string{ + "set firewall three-color-policer bad two-rate committed-information-rate 10m", + "set firewall three-color-policer bad two-rate committed-burst-size 100k", + "set firewall three-color-policer bad two-rate peak-burst-size 200k", + }, + wantErr: "peak-information-rate", + }, + { + name: "peak below committed", + lines: []string{ + "set firewall three-color-policer bad two-rate committed-information-rate 10m", + "set firewall three-color-policer bad two-rate committed-burst-size 100k", + "set firewall three-color-policer bad two-rate peak-information-rate 1m", + "set firewall three-color-policer bad two-rate peak-burst-size 200k", + }, + wantErr: "peak-information-rate must be >= committed-information-rate", + }, + { + name: "peak burst below committed burst", + lines: []string{ + "set firewall three-color-policer bad two-rate committed-information-rate 10m", + "set firewall three-color-policer bad two-rate committed-burst-size 200k", + "set firewall three-color-policer bad two-rate peak-information-rate 20m", + "set firewall three-color-policer bad two-rate peak-burst-size 100k", + }, + wantErr: "peak-burst-size must be >= committed-burst-size", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree := &ConfigTree{} + for _, line := range tt.lines { + cmd, err := ParseSetCommand(line) + if err != nil { + t.Fatalf("ParseSetCommand(%q): %v", line, err) + } + if err := tree.SetPath(cmd); err != nil { + t.Fatalf("SetPath(%q): %v", line, err) + } + } + _, err := CompileConfig(tree) + if err == nil { + t.Fatalf("CompileConfig succeeded, want error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("CompileConfig error = %v, want substring %q", err, tt.wantErr) + } + }) + } +} + func TestLogicalInterfacePolicer(t *testing.T) { input := `firewall { policer shared-rate { diff --git a/pkg/dataplane/userspace/manager_test.go b/pkg/dataplane/userspace/manager_test.go index 0dca40b3c..d6ac05ecd 100644 --- a/pkg/dataplane/userspace/manager_test.go +++ b/pkg/dataplane/userspace/manager_test.go @@ -1872,6 +1872,57 @@ func TestDeriveUserspaceCapabilitiesGatesThreeColorPolicers(t *testing.T) { } } +func TestBuildSnapshotIncludesThreeColorPolicerSchemaWhileGateClosed(t *testing.T) { + cfg := &config.Config{} + cfg.Firewall.ThreeColorPolicers = map[string]*config.ThreeColorPolicerConfig{ + "sr": { + Name: "sr", + ColorBlind: true, + CIR: 125000, + CBS: 50000, + PBS: 100000, + ThenAction: "discard", + }, + "tr": { + Name: "tr", + TwoRate: true, + CIR: 125000, + CBS: 50000, + PIR: 250000, + PBS: 100000, + ThenAction: "loss-priority high", + }, + } + + snap := buildSnapshot(cfg, config.UserspaceConfig{}, 1, 0) + if snap.Capabilities.ForwardingSupported { + t.Fatal("ForwardingSupported = true, want three-color policers to remain fail-closed") + } + want := []ThreeColorPolicerSnapshot{ + { + Name: "sr", + Mode: "single-rate", + ColorBlind: true, + CommittedRateBytes: 125000, + CommittedBurstBytes: 50000, + PeakOrExcessBurstBytes: 100000, + ThenAction: "discard", + }, + { + Name: "tr", + Mode: "two-rate", + CommittedRateBytes: 125000, + CommittedBurstBytes: 50000, + PeakOrExcessRateBytes: 250000, + PeakOrExcessBurstBytes: 100000, + ThenAction: "loss-priority high", + }, + } + if !reflect.DeepEqual(snap.ThreeColorPolicers, want) { + t.Fatalf("ThreeColorPolicers = %+v, want %+v", snap.ThreeColorPolicers, want) + } +} + func TestDeriveUserspaceCapabilitiesAllowsFirewallFilters(t *testing.T) { cfg := &config.Config{} cfg.Firewall.FiltersInet = map[string]*config.FirewallFilter{ diff --git a/pkg/dataplane/userspace/protocol.go b/pkg/dataplane/userspace/protocol.go index ddbc0f763..43e37a192 100644 --- a/pkg/dataplane/userspace/protocol.go +++ b/pkg/dataplane/userspace/protocol.go @@ -37,35 +37,36 @@ type ControlResponse struct { } type ConfigSnapshot struct { - Version int `json:"version"` - Generation uint64 `json:"generation"` - FIBGeneration uint32 `json:"fib_generation,omitempty"` - GeneratedAt time.Time `json:"generated_at"` - Summary SnapshotSummary `json:"summary"` - Capabilities UserspaceCapabilities `json:"capabilities"` - MapPins UserspaceMapPins `json:"map_pins"` - Zones []ZoneSnapshot `json:"zones,omitempty"` - Interfaces []InterfaceSnapshot `json:"interfaces,omitempty"` - Fabrics []FabricSnapshot `json:"fabrics,omitempty"` - TunnelEndpoints []TunnelEndpointSnapshot `json:"tunnel_endpoints,omitempty"` - Neighbors []NeighborSnapshot `json:"neighbors,omitempty"` - Routes []RouteSnapshot `json:"routes,omitempty"` - Flow FlowSnapshot `json:"flow,omitempty"` - DefaultPolicy string `json:"default_policy,omitempty"` - Policies []PolicyRuleSnapshot `json:"policies,omitempty"` - SourceNAT []SourceNATRuleSnapshot `json:"source_nat_rules,omitempty"` - StaticNAT []StaticNATRuleSnapshot `json:"static_nat_rules,omitempty"` - DestinationNAT []DestinationNATRuleSnapshot `json:"destination_nat_rules,omitempty"` - NAT64 []NAT64RuleSnapshot `json:"nat64_rules,omitempty"` - Nptv6 []Nptv6RuleSnapshot `json:"nptv6_rules,omitempty"` - Screens []ScreenProfileSnapshot `json:"screens,omitempty"` - Filters []FirewallFilterSnapshot `json:"filters,omitempty"` - Policers []PolicerSnapshot `json:"policers,omitempty"` - ClassOfService *ClassOfServiceSnapshot `json:"class_of_service,omitempty"` - FlowExport *FlowExportSnapshot `json:"flow_export,omitempty"` - Config *config.Config `json:"config,omitempty"` - Userspace config.UserspaceConfig `json:"userspace"` - DeferWorkers bool `json:"defer_workers,omitempty"` + Version int `json:"version"` + Generation uint64 `json:"generation"` + FIBGeneration uint32 `json:"fib_generation,omitempty"` + GeneratedAt time.Time `json:"generated_at"` + Summary SnapshotSummary `json:"summary"` + Capabilities UserspaceCapabilities `json:"capabilities"` + MapPins UserspaceMapPins `json:"map_pins"` + Zones []ZoneSnapshot `json:"zones,omitempty"` + Interfaces []InterfaceSnapshot `json:"interfaces,omitempty"` + Fabrics []FabricSnapshot `json:"fabrics,omitempty"` + TunnelEndpoints []TunnelEndpointSnapshot `json:"tunnel_endpoints,omitempty"` + Neighbors []NeighborSnapshot `json:"neighbors,omitempty"` + Routes []RouteSnapshot `json:"routes,omitempty"` + Flow FlowSnapshot `json:"flow,omitempty"` + DefaultPolicy string `json:"default_policy,omitempty"` + Policies []PolicyRuleSnapshot `json:"policies,omitempty"` + SourceNAT []SourceNATRuleSnapshot `json:"source_nat_rules,omitempty"` + StaticNAT []StaticNATRuleSnapshot `json:"static_nat_rules,omitempty"` + DestinationNAT []DestinationNATRuleSnapshot `json:"destination_nat_rules,omitempty"` + NAT64 []NAT64RuleSnapshot `json:"nat64_rules,omitempty"` + Nptv6 []Nptv6RuleSnapshot `json:"nptv6_rules,omitempty"` + Screens []ScreenProfileSnapshot `json:"screens,omitempty"` + Filters []FirewallFilterSnapshot `json:"filters,omitempty"` + Policers []PolicerSnapshot `json:"policers,omitempty"` + ThreeColorPolicers []ThreeColorPolicerSnapshot `json:"three_color_policers,omitempty"` + ClassOfService *ClassOfServiceSnapshot `json:"class_of_service,omitempty"` + FlowExport *FlowExportSnapshot `json:"flow_export,omitempty"` + Config *config.Config `json:"config,omitempty"` + Userspace config.UserspaceConfig `json:"userspace"` + DeferWorkers bool `json:"defer_workers,omitempty"` } type FlowSnapshot struct { @@ -335,6 +336,17 @@ type PolicerSnapshot struct { DiscardExcess bool `json:"discard_excess"` } +type ThreeColorPolicerSnapshot struct { + Name string `json:"name"` + Mode string `json:"mode"` // "single-rate" (srTCM) or "two-rate" (trTCM) + ColorBlind bool `json:"color_blind,omitempty"` + CommittedRateBytes uint64 `json:"committed_rate_bytes_per_sec"` + CommittedBurstBytes uint64 `json:"committed_burst_bytes"` + PeakOrExcessRateBytes uint64 `json:"peak_or_excess_rate_bytes_per_sec,omitempty"` + PeakOrExcessBurstBytes uint64 `json:"peak_or_excess_burst_bytes"` + ThenAction string `json:"then_action,omitempty"` +} + // FlowExportSnapshot captures flow monitoring/export configuration for the // userspace dataplane. type FlowExportSnapshot struct { diff --git a/pkg/dataplane/userspace/protocol_test.go b/pkg/dataplane/userspace/protocol_test.go index 12e3d07d7..716467b50 100644 --- a/pkg/dataplane/userspace/protocol_test.go +++ b/pkg/dataplane/userspace/protocol_test.go @@ -89,6 +89,42 @@ func TestBindingCountersSnapshotTXSharedRecycleUnknownSlotDropsRoundTrip(t *test } } +func TestConfigSnapshotThreeColorPolicersRoundTrip(t *testing.T) { + in := ConfigSnapshot{ + Version: 1, + ThreeColorPolicers: []ThreeColorPolicerSnapshot{ + { + Name: "tr", + Mode: "two-rate", + ColorBlind: true, + CommittedRateBytes: 125000, + CommittedBurstBytes: 50000, + PeakOrExcessRateBytes: 250000, + PeakOrExcessBurstBytes: 100000, + ThenAction: "discard", + }, + }, + } + raw, err := json.Marshal(&in) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var obj map[string]json.RawMessage + if err := json.Unmarshal(raw, &obj); err != nil { + t.Fatalf("unmarshal obj: %v", err) + } + if _, ok := obj["three_color_policers"]; !ok { + t.Fatalf("wire key missing from ConfigSnapshot JSON: %s", string(raw)) + } + var back ConfigSnapshot + if err := json.Unmarshal(raw, &back); err != nil { + t.Fatalf("unmarshal ConfigSnapshot: %v", err) + } + if !reflect.DeepEqual(back.ThreeColorPolicers, in.ThreeColorPolicers) { + t.Fatalf("ThreeColorPolicers = %+v, want %+v", back.ThreeColorPolicers, in.ThreeColorPolicers) + } +} + func TestCoSQueueStatusDrainPhaseCountersRoundTrip(t *testing.T) { in := CoSQueueStatus{ QueueID: 0, diff --git a/pkg/dataplane/userspace/snapshot.go b/pkg/dataplane/userspace/snapshot.go index 8f1f4ef7c..635ed2799 100644 --- a/pkg/dataplane/userspace/snapshot.go +++ b/pkg/dataplane/userspace/snapshot.go @@ -35,33 +35,34 @@ func buildSnapshot(cfg *config.Config, ucfg config.UserspaceConfig, generation u policyCount := len(cfg.Security.Policies) interfaces := buildInterfaceSnapshots(cfg) return &ConfigSnapshot{ - Version: ProtocolVersion, - Generation: generation, - FIBGeneration: fibGeneration, - GeneratedAt: time.Now().UTC(), - Capabilities: deriveUserspaceCapabilities(cfg), - MapPins: userspaceMapPins(), - Userspace: ucfg, - Zones: buildZoneSnapshots(cfg), - Interfaces: interfaces, - Fabrics: buildFabricSnapshots(cfg), - TunnelEndpoints: buildTunnelEndpointSnapshots(cfg, interfaces), - Neighbors: buildNeighborSnapshots(cfg), - Routes: buildRouteSnapshots(cfg, interfaces), - Flow: buildFlowSnapshot(cfg), - DefaultPolicy: policyActionString(cfg.Security.DefaultPolicy), - Policies: buildPolicySnapshots(cfg), - SourceNAT: buildSourceNATSnapshots(cfg), - StaticNAT: buildStaticNATSnapshots(cfg), - DestinationNAT: buildDestinationNATSnapshots(cfg), - NAT64: buildNAT64Snapshots(cfg), - Nptv6: buildNptv6Snapshots(cfg), - Screens: buildScreenSnapshots(cfg), - Filters: buildFirewallFilterSnapshots(cfg), - Policers: buildPolicerSnapshots(cfg), - ClassOfService: buildClassOfServiceSnapshot(cfg), - FlowExport: buildFlowExportSnapshot(cfg), - Config: cfg, + Version: ProtocolVersion, + Generation: generation, + FIBGeneration: fibGeneration, + GeneratedAt: time.Now().UTC(), + Capabilities: deriveUserspaceCapabilities(cfg), + MapPins: userspaceMapPins(), + Userspace: ucfg, + Zones: buildZoneSnapshots(cfg), + Interfaces: interfaces, + Fabrics: buildFabricSnapshots(cfg), + TunnelEndpoints: buildTunnelEndpointSnapshots(cfg, interfaces), + Neighbors: buildNeighborSnapshots(cfg), + Routes: buildRouteSnapshots(cfg, interfaces), + Flow: buildFlowSnapshot(cfg), + DefaultPolicy: policyActionString(cfg.Security.DefaultPolicy), + Policies: buildPolicySnapshots(cfg), + SourceNAT: buildSourceNATSnapshots(cfg), + StaticNAT: buildStaticNATSnapshots(cfg), + DestinationNAT: buildDestinationNATSnapshots(cfg), + NAT64: buildNAT64Snapshots(cfg), + Nptv6: buildNptv6Snapshots(cfg), + Screens: buildScreenSnapshots(cfg), + Filters: buildFirewallFilterSnapshots(cfg), + Policers: buildPolicerSnapshots(cfg), + ThreeColorPolicers: buildThreeColorPolicerSnapshots(cfg), + ClassOfService: buildClassOfServiceSnapshot(cfg), + FlowExport: buildFlowExportSnapshot(cfg), + Config: cfg, Summary: SnapshotSummary{ HostName: cfg.System.HostName, DataplaneType: cfg.System.DataplaneType, @@ -1762,6 +1763,41 @@ func buildPolicerSnapshots(cfg *config.Config) []PolicerSnapshot { return out } +func buildThreeColorPolicerSnapshots(cfg *config.Config) []ThreeColorPolicerSnapshot { + if cfg == nil || len(cfg.Firewall.ThreeColorPolicers) == 0 { + return nil + } + names := make([]string, 0, len(cfg.Firewall.ThreeColorPolicers)) + for name := range cfg.Firewall.ThreeColorPolicers { + names = append(names, name) + } + sort.Strings(names) + out := make([]ThreeColorPolicerSnapshot, 0, len(names)) + for _, name := range names { + pol := cfg.Firewall.ThreeColorPolicers[name] + if pol == nil { + continue + } + mode := "single-rate" + peakOrExcessRate := uint64(0) + if pol.TwoRate { + mode = "two-rate" + peakOrExcessRate = pol.PIR + } + out = append(out, ThreeColorPolicerSnapshot{ + Name: name, + Mode: mode, + ColorBlind: pol.ColorBlind, + CommittedRateBytes: pol.CIR, + CommittedBurstBytes: pol.CBS, + PeakOrExcessRateBytes: peakOrExcessRate, + PeakOrExcessBurstBytes: pol.PBS, + ThenAction: pol.ThenAction, + }) + } + return out +} + func buildClassOfServiceSnapshot(cfg *config.Config) *ClassOfServiceSnapshot { if cfg == nil || cfg.ClassOfService == nil { return nil diff --git a/userspace-dp/src/filter/README.md b/userspace-dp/src/filter/README.md index fd7822077..084c19b6e 100644 --- a/userspace-dp/src/filter/README.md +++ b/userspace-dp/src/filter/README.md @@ -53,9 +53,9 @@ Implemented here: Still gated before removing the userspace capability rejection: -- The Go userspace snapshot currently publishes only single-rate - two-color `PolicerSnapshot` fields; three-color snapshot fields and - validation are not wired to Rust yet. +- The Go snapshot schema, Rust wire DTO, and commit-time structural + validation are wired for three-color policers. They are published only + so the control plane and dataplane agree on the future wire shape. - Filter terms still carry a policer name in the evaluation result. The hot forwarding path must move to stable policer IDs with ID-indexed or sharded state before three-color policers are enabled. @@ -64,3 +64,6 @@ Still gated before removing the userspace capability rejection: - Forwarding-path application of per-color counters, red drops, DSCP rewrites, Rust status, Go status, CLI, and Prometheus export remain follow-on wiring. +- Until those runtime pieces land, any config containing + `firewall three-color-policer` stays fail-closed for userspace + forwarding through the capability gate. diff --git a/userspace-dp/src/main_tests.rs b/userspace-dp/src/main_tests.rs index a1d1f436b..108b8406c 100644 --- a/userspace-dp/src/main_tests.rs +++ b/userspace-dp/src/main_tests.rs @@ -918,6 +918,52 @@ fn binding_counters_snapshot_tolerates_pre_split_wire() { assert_eq!(snap.rx_fill_ring_empty_descs, 7); } +#[test] +fn config_snapshot_three_color_policers_roundtrip() { + let json = r#"{ + "version": 1, + "generation": 42, + "generated_at": "2026-05-17T00:00:00Z", + "summary": { + "host_name": "fw", + "dataplane_type": "userspace", + "interface_count": 0, + "zone_count": 0, + "policy_count": 0, + "scheduler_count": 0, + "ha_enabled": false + }, + "three_color_policers": [ + { + "name": "tr", + "mode": "two-rate", + "color_blind": true, + "committed_rate_bytes_per_sec": 125000, + "committed_burst_bytes": 50000, + "peak_or_excess_rate_bytes_per_sec": 250000, + "peak_or_excess_burst_bytes": 100000, + "then_action": "discard" + } + ] + }"#; + let snap: ConfigSnapshot = serde_json::from_str(json).expect("three-color snapshot decodes"); + assert_eq!(snap.three_color_policers.len(), 1); + let policer = &snap.three_color_policers[0]; + assert_eq!(policer.name, "tr"); + assert_eq!(policer.mode, "two-rate"); + assert!(policer.color_blind); + assert_eq!(policer.committed_rate_bytes_per_sec, 125000); + assert_eq!(policer.committed_burst_bytes, 50000); + assert_eq!(policer.peak_or_excess_rate_bytes_per_sec, 250000); + assert_eq!(policer.peak_or_excess_burst_bytes, 100000); + + let encoded = serde_json::to_value(&snap).expect("three-color snapshot serializes"); + assert!( + encoded.get("three_color_policers").is_some(), + "three_color_policers wire key missing from Rust serialization: {encoded}" + ); +} + #[test] fn tx_latency_hist_serialization_roundtrip() { // #812 plan §6.1 test #4. Construct a BindingCountersSnapshot diff --git a/userspace-dp/src/protocol.rs b/userspace-dp/src/protocol.rs index ef9a1782e..0e70fdbc2 100644 --- a/userspace-dp/src/protocol.rs +++ b/userspace-dp/src/protocol.rs @@ -315,6 +315,8 @@ pub(crate) struct ConfigSnapshot { pub filters: Vec, #[serde(default)] pub policers: Vec, + #[serde(rename = "three_color_policers", default)] + pub three_color_policers: Vec, #[serde(rename = "class_of_service", default)] pub class_of_service: Option, #[serde(rename = "flow_export", default)] @@ -556,6 +558,25 @@ pub(crate) struct PolicerSnapshot { pub discard_excess: bool, } +#[derive(Clone, Debug, Serialize, Deserialize, Default)] +pub(crate) struct ThreeColorPolicerSnapshot { + pub name: String, + #[serde(default)] + pub mode: String, + #[serde(rename = "color_blind", default)] + pub color_blind: bool, + #[serde(rename = "committed_rate_bytes_per_sec", default)] + pub committed_rate_bytes_per_sec: u64, + #[serde(rename = "committed_burst_bytes", default)] + pub committed_burst_bytes: u64, + #[serde(rename = "peak_or_excess_rate_bytes_per_sec", default)] + pub peak_or_excess_rate_bytes_per_sec: u64, + #[serde(rename = "peak_or_excess_burst_bytes", default)] + pub peak_or_excess_burst_bytes: u64, + #[serde(rename = "then_action", default)] + pub then_action: String, +} + #[derive(Clone, Debug, Serialize, Deserialize, Default)] pub(crate) struct FlowExportSnapshot { #[serde(rename = "collector_address", default)] From d36b0e7d14c9c1282bfe5959e9a3be23eb50500f Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 20:19:09 -0700 Subject: [PATCH 3/7] config: reject ambiguous three-color policers --- pkg/config/compiler.go | 6 ++++++ pkg/config/compiler_firewall.go | 10 ++++++++++ pkg/config/parser_ast_test.go | 24 ++++++++++++++++++++++++ pkg/config/types.go | 20 +++++++++++++------- pkg/configstore/store_test.go | 25 +++++++++++++++++++++++++ userspace-dp/src/filter/README.md | 3 +++ 6 files changed, 81 insertions(+), 7 deletions(-) diff --git a/pkg/config/compiler.go b/pkg/config/compiler.go index 2712b838e..41611ff78 100644 --- a/pkg/config/compiler.go +++ b/pkg/config/compiler.go @@ -240,6 +240,12 @@ func validateThreeColorPolicersStrict(policers map[string]*ThreeColorPolicerConf if displayName == "" { displayName = name } + if pol.SingleRateConfigured && pol.TwoRateConfigured { + return fmt.Errorf("firewall three-color-policer %q cannot configure both single-rate and two-rate", displayName) + } + if pol.ColorBlindConfigured && pol.ColorAwareConfigured { + return fmt.Errorf("firewall three-color-policer %q cannot configure both color-blind and color-aware", displayName) + } if pol.CIR == 0 { return fmt.Errorf("firewall three-color-policer %q requires positive committed-information-rate", displayName) } diff --git a/pkg/config/compiler_firewall.go b/pkg/config/compiler_firewall.go index 5392ca5dd..ca65919f0 100644 --- a/pkg/config/compiler_firewall.go +++ b/pkg/config/compiler_firewall.go @@ -72,9 +72,14 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { } if sr := tcpInst.node.FindChild("single-rate"); sr != nil { + tcp.SingleRateConfigured = true tcp.TwoRate = false if sr.FindChild("color-blind") != nil { tcp.ColorBlind = true + tcp.ColorBlindConfigured = true + } + if sr.FindChild("color-aware") != nil { + tcp.ColorAwareConfigured = true } for _, child := range sr.Children { switch child.Name() { @@ -95,9 +100,14 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { } if tr := tcpInst.node.FindChild("two-rate"); tr != nil { + tcp.TwoRateConfigured = true tcp.TwoRate = true if tr.FindChild("color-blind") != nil { tcp.ColorBlind = true + tcp.ColorBlindConfigured = true + } + if tr.FindChild("color-aware") != nil { + tcp.ColorAwareConfigured = true } for _, child := range tr.Children { switch child.Name() { diff --git a/pkg/config/parser_ast_test.go b/pkg/config/parser_ast_test.go index 85e10972a..f45312739 100644 --- a/pkg/config/parser_ast_test.go +++ b/pkg/config/parser_ast_test.go @@ -4582,6 +4582,30 @@ func TestThreeColorPolicerStrictValidation(t *testing.T) { }, wantErr: "peak-burst-size must be >= committed-burst-size", }, + { + name: "ambiguous single and two rate", + lines: []string{ + "set firewall three-color-policer bad single-rate committed-information-rate 10m", + "set firewall three-color-policer bad single-rate committed-burst-size 100k", + "set firewall three-color-policer bad single-rate excess-burst-size 200k", + "set firewall three-color-policer bad two-rate committed-information-rate 10m", + "set firewall three-color-policer bad two-rate committed-burst-size 100k", + "set firewall three-color-policer bad two-rate peak-information-rate 20m", + "set firewall three-color-policer bad two-rate peak-burst-size 200k", + }, + wantErr: "cannot configure both single-rate and two-rate", + }, + { + name: "ambiguous color mode", + lines: []string{ + "set firewall three-color-policer bad single-rate color-blind", + "set firewall three-color-policer bad single-rate color-aware", + "set firewall three-color-policer bad single-rate committed-information-rate 10m", + "set firewall three-color-policer bad single-rate committed-burst-size 100k", + "set firewall three-color-policer bad single-rate excess-burst-size 200k", + }, + wantErr: "cannot configure both color-blind and color-aware", + }, } for _, tt := range tests { diff --git a/pkg/config/types.go b/pkg/config/types.go index 75aaef554..17ae858d2 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -927,13 +927,19 @@ type PolicerConfig struct { // ThreeColorPolicerConfig defines a three-color policer (RFC 2697/2698). type ThreeColorPolicerConfig struct { Name string - TwoRate bool // true=two-rate (RFC 2698), false=single-rate (RFC 2697) - ColorBlind bool // color-blind mode (default: color-aware) - CIR uint64 // committed information rate (bytes/sec) - CBS uint64 // committed burst size (bytes) - PIR uint64 // peak information rate (bytes/sec, two-rate only) - PBS uint64 // peak/excess burst size (bytes) - ThenAction string // action on exceed/violate: "discard" or "loss-priority" + TwoRate bool // true=two-rate (RFC 2698), false=single-rate (RFC 2697) + ColorBlind bool // color-blind mode (default: color-aware) + // Explicit mode/color markers are retained for commit-time ambiguity + // checks. The dataplane snapshot still carries only the canonical mode. + SingleRateConfigured bool + TwoRateConfigured bool + ColorAwareConfigured bool + ColorBlindConfigured bool + CIR uint64 // committed information rate (bytes/sec) + CBS uint64 // committed burst size (bytes) + PIR uint64 // peak information rate (bytes/sec, two-rate only) + PBS uint64 // peak/excess burst size (bytes) + ThenAction string // action on exceed/violate: "discard" or "loss-priority" } // FirewallFilter defines a named firewall filter with ordered terms. diff --git a/pkg/configstore/store_test.go b/pkg/configstore/store_test.go index 4f685b7e5..72794bf5b 100644 --- a/pkg/configstore/store_test.go +++ b/pkg/configstore/store_test.go @@ -86,6 +86,31 @@ func TestCommitCheck_AcceptsValidScheduler(t *testing.T) { } } +func TestCommitCheck_RejectsAmbiguousThreeColorPolicer(t *testing.T) { + s := newTestStore(t) + if err := s.EnterConfigure(); err != nil { + t.Fatalf("EnterConfigure: %v", err) + } + for _, cmd := range []string{ + "firewall three-color-policer bad single-rate color-blind", + "firewall three-color-policer bad single-rate color-aware", + "firewall three-color-policer bad single-rate committed-information-rate 10m", + "firewall three-color-policer bad single-rate committed-burst-size 100k", + "firewall three-color-policer bad single-rate excess-burst-size 200k", + } { + if err := s.SetFromInput(cmd); err != nil { + t.Fatalf("SetFromInput(%q): %v", cmd, err) + } + } + _, err := s.CommitCheck() + if err == nil { + t.Fatal("expected CommitCheck to reject ambiguous three-color policer, got nil") + } + if !strings.Contains(err.Error(), "cannot configure both color-blind and color-aware") { + t.Fatalf("CommitCheck error = %v", err) + } +} + func TestEnterExitConfigure(t *testing.T) { s := newTestStore(t) diff --git a/userspace-dp/src/filter/README.md b/userspace-dp/src/filter/README.md index 084c19b6e..e7c558562 100644 --- a/userspace-dp/src/filter/README.md +++ b/userspace-dp/src/filter/README.md @@ -56,6 +56,9 @@ Still gated before removing the userspace capability rejection: - The Go snapshot schema, Rust wire DTO, and commit-time structural validation are wired for three-color policers. They are published only so the control plane and dataplane agree on the future wire shape. + Commit validation rejects ambiguous mode declarations (`single-rate` + with `two-rate`) and ambiguous color declarations (`color-blind` with + `color-aware`) before they can reach the helper. - Filter terms still carry a policer name in the evaluation result. The hot forwarding path must move to stable policer IDs with ID-indexed or sharded state before three-color policers are enabled. From 591f4fba1237db86da9ede80c2026fed21cca0de Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 21:31:13 -0700 Subject: [PATCH 4/7] config: merge split three-color policer blocks --- pkg/config/compiler_firewall.go | 12 +++--- pkg/configstore/store_test.go | 61 +++++++++++++++++++++++++++++++ userspace-dp/src/filter/README.md | 6 ++- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/pkg/config/compiler_firewall.go b/pkg/config/compiler_firewall.go index ca65919f0..af6291467 100644 --- a/pkg/config/compiler_firewall.go +++ b/pkg/config/compiler_firewall.go @@ -66,9 +66,13 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { fw.ThreeColorPolicers = make(map[string]*ThreeColorPolicerConfig) } for _, tcpInst := range namedInstances(node.FindChildren("three-color-policer")) { - tcp := &ThreeColorPolicerConfig{ - Name: tcpInst.name, - ThenAction: "discard", + tcp := fw.ThreeColorPolicers[tcpInst.name] + if tcp == nil { + tcp = &ThreeColorPolicerConfig{ + Name: tcpInst.name, + ThenAction: "discard", + } + fw.ThreeColorPolicers[tcpInst.name] = tcp } if sr := tcpInst.node.FindChild("single-rate"); sr != nil { @@ -143,8 +147,6 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { } } } - - fw.ThreeColorPolicers[tcp.Name] = tcp } for _, familyNode := range node.FindChildren("family") { diff --git a/pkg/configstore/store_test.go b/pkg/configstore/store_test.go index 72794bf5b..2b800c685 100644 --- a/pkg/configstore/store_test.go +++ b/pkg/configstore/store_test.go @@ -111,6 +111,67 @@ func TestCommitCheck_RejectsAmbiguousThreeColorPolicer(t *testing.T) { } } +func TestCommitCheck_RejectsAmbiguousThreeColorPolicerAcrossLoadOverrideBlocks(t *testing.T) { + s := newTestStore(t) + if err := s.EnterConfigure(); err != nil { + t.Fatalf("EnterConfigure: %v", err) + } + hier := `firewall { + three-color-policer bad { + single-rate { + color-blind; + committed-information-rate 10m; + committed-burst-size 100k; + excess-burst-size 200k; + } + } + three-color-policer bad { + two-rate { + color-blind; + committed-information-rate 10m; + peak-information-rate 20m; + committed-burst-size 100k; + peak-burst-size 200k; + } + } +}` + if err := s.LoadOverride(hier); err != nil { + t.Fatalf("LoadOverride: %v", err) + } + _, err := s.CommitCheck() + if err == nil { + t.Fatal("expected CommitCheck to reject duplicate hierarchical single-rate/two-rate blocks") + } + if !strings.Contains(err.Error(), "cannot configure both single-rate and two-rate") { + t.Fatalf("CommitCheck error = %v", err) + } +} + +func TestCommitCheck_RejectsThreeColorPolicerPeakBelowCommitted(t *testing.T) { + s := newTestStore(t) + if err := s.EnterConfigure(); err != nil { + t.Fatalf("EnterConfigure: %v", err) + } + for _, cmd := range []string{ + "firewall three-color-policer bad two-rate color-blind", + "firewall three-color-policer bad two-rate committed-information-rate 20m", + "firewall three-color-policer bad two-rate peak-information-rate 10m", + "firewall three-color-policer bad two-rate committed-burst-size 100k", + "firewall three-color-policer bad two-rate peak-burst-size 200k", + } { + if err := s.SetFromInput(cmd); err != nil { + t.Fatalf("SetFromInput(%q): %v", cmd, err) + } + } + _, err := s.CommitCheck() + if err == nil { + t.Fatal("expected CommitCheck to reject peak-information-rate below committed-information-rate") + } + if !strings.Contains(err.Error(), "peak-information-rate must be >= committed-information-rate") { + t.Fatalf("CommitCheck error = %v", err) + } +} + func TestEnterExitConfigure(t *testing.T) { s := newTestStore(t) diff --git a/userspace-dp/src/filter/README.md b/userspace-dp/src/filter/README.md index e7c558562..75f82da50 100644 --- a/userspace-dp/src/filter/README.md +++ b/userspace-dp/src/filter/README.md @@ -58,7 +58,11 @@ Still gated before removing the userspace capability rejection: so the control plane and dataplane agree on the future wire shape. Commit validation rejects ambiguous mode declarations (`single-rate` with `two-rate`) and ambiguous color declarations (`color-blind` with - `color-aware`) before they can reach the helper. + `color-aware`) before they can reach the helper. Duplicate + hierarchical `firewall three-color-policer ` blocks are compiled + as one logical policer before that validation, so load + merge/override cannot hide an ambiguity behind last-write-wins map + assignment. - Filter terms still carry a policer name in the evaluation result. The hot forwarding path must move to stable policer IDs with ID-indexed or sharded state before three-color policers are enabled. From ac55aeeb7c8ae9378108b98d24f3da320f8a925c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 04:42:37 +0000 Subject: [PATCH 5/7] config: compile all three-color same-mode siblings Agent-Logs-Url: https://github.com/psaab/xpf/sessions/f42ea8d5-2bfb-4a18-95e8-1b5dbdb6bd80 Co-authored-by: psaab <196946+psaab@users.noreply.github.com> --- _Log.md | 4 ++++ pkg/config/compiler_firewall.go | 4 ++-- pkg/config/parser_ast_test.go | 29 +++++++++++++++++++++++++++++ pkg/configstore/store_test.go | 30 ++++++++++++++++++++++++++++++ userspace-dp/src/filter/README.md | 4 +++- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/_Log.md b/_Log.md index 2df9fab26..94a98d7e3 100644 --- a/_Log.md +++ b/_Log.md @@ -2,6 +2,10 @@ ## 2026-05-17 +- **Timestamp**: 2026-05-17T04:58:00Z + - **Action**: PR #1395 round-4 follow-up — fixed three-color policer compiler handling for duplicate same-mode sibling blocks by iterating all `single-rate`/`two-rate` children, added hierarchical ambiguity regression coverage in parser/configstore tests, and updated filter module docs to reflect same-mode sibling merge semantics. + - **File(s)**: `pkg/config/compiler_firewall.go`, `pkg/config/parser_ast_test.go`, `pkg/configstore/store_test.go`, `userspace-dp/src/filter/README.md`, `_Log.md` + - **Timestamp**: 2026-05-17T01:12:00Z - **Action**: PR #1391 post-smoke follow-up — live q10(24G)+q0(best-effort) contention on `7e7eb07e` showed serviceable-only exact suppression still let q0 drain ~15.6 GB of surplus while exact was backlogged. Reworked the gate from binary serviceability to residual-rate budgeting: non-exact surplus can consume only `root_rate - backlogged_exact_guarantee_rates`, shared exact queues publish queue masks so one queue's reservation is counted once across workers, and shared interfaces use an interface-global residual token bucket rather than per-worker residual buckets. - **File(s)**: `userspace-dp/src/afxdp/cos/queue_service/mod.rs`, `userspace-dp/src/afxdp/cos/queue_service/tests.rs`, `userspace-dp/src/afxdp/cos/tx_completion.rs`, `userspace-dp/src/afxdp/types/shared_cos_lease.rs`, `userspace-dp/src/afxdp/types/cos.rs`, `userspace-dp/src/afxdp/cos/builders.rs`, `userspace-dp/src/afxdp/worker/cos_tests.rs`, `userspace-dp/src/afxdp/cos/README.md`, `userspace-dp/src/afxdp/types/README.md`, `_Log.md` diff --git a/pkg/config/compiler_firewall.go b/pkg/config/compiler_firewall.go index af6291467..d6224a14b 100644 --- a/pkg/config/compiler_firewall.go +++ b/pkg/config/compiler_firewall.go @@ -75,7 +75,7 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { fw.ThreeColorPolicers[tcpInst.name] = tcp } - if sr := tcpInst.node.FindChild("single-rate"); sr != nil { + for _, sr := range tcpInst.node.FindChildren("single-rate") { tcp.SingleRateConfigured = true tcp.TwoRate = false if sr.FindChild("color-blind") != nil { @@ -103,7 +103,7 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { } } - if tr := tcpInst.node.FindChild("two-rate"); tr != nil { + for _, tr := range tcpInst.node.FindChildren("two-rate") { tcp.TwoRateConfigured = true tcp.TwoRate = true if tr.FindChild("color-blind") != nil { diff --git a/pkg/config/parser_ast_test.go b/pkg/config/parser_ast_test.go index f45312739..f94d39a66 100644 --- a/pkg/config/parser_ast_test.go +++ b/pkg/config/parser_ast_test.go @@ -4631,6 +4631,35 @@ func TestThreeColorPolicerStrictValidation(t *testing.T) { } } +func TestThreeColorPolicerStrictValidation_HierarchicalDuplicateSameModeSiblings(t *testing.T) { + input := `firewall { + three-color-policer bad { + single-rate { + color-blind; + committed-information-rate 10m; + committed-burst-size 100k; + excess-burst-size 200k; + } + single-rate { + color-aware; + } + } +} +` + p := NewParser(input) + tree, err := p.Parse() + if err != nil { + t.Fatalf("parse error: %v", err) + } + _, compileErr := CompileConfig(tree) + if compileErr == nil { + t.Fatal("CompileConfig succeeded, want ambiguous color mode error") + } + if !strings.Contains(compileErr.Error(), "cannot configure both color-blind and color-aware") { + t.Fatalf("CompileConfig error = %v", compileErr) + } +} + func TestLogicalInterfacePolicer(t *testing.T) { input := `firewall { policer shared-rate { diff --git a/pkg/configstore/store_test.go b/pkg/configstore/store_test.go index 2b800c685..0ffa40c59 100644 --- a/pkg/configstore/store_test.go +++ b/pkg/configstore/store_test.go @@ -147,6 +147,36 @@ func TestCommitCheck_RejectsAmbiguousThreeColorPolicerAcrossLoadOverrideBlocks(t } } +func TestCommitCheck_RejectsAmbiguousThreeColorPolicerAcrossLoadOverrideSameModeSiblings(t *testing.T) { + s := newTestStore(t) + if err := s.EnterConfigure(); err != nil { + t.Fatalf("EnterConfigure: %v", err) + } + hier := `firewall { + three-color-policer bad { + single-rate { + color-blind; + committed-information-rate 10m; + committed-burst-size 100k; + excess-burst-size 200k; + } + single-rate { + color-aware; + } + } +}` + if err := s.LoadOverride(hier); err != nil { + t.Fatalf("LoadOverride: %v", err) + } + _, err := s.CommitCheck() + if err == nil { + t.Fatal("expected CommitCheck to reject duplicate hierarchical single-rate color mode ambiguity") + } + if !strings.Contains(err.Error(), "cannot configure both color-blind and color-aware") { + t.Fatalf("CommitCheck error = %v", err) + } +} + func TestCommitCheck_RejectsThreeColorPolicerPeakBelowCommitted(t *testing.T) { s := newTestStore(t) if err := s.EnterConfigure(); err != nil { diff --git a/userspace-dp/src/filter/README.md b/userspace-dp/src/filter/README.md index 75f82da50..0a439a24b 100644 --- a/userspace-dp/src/filter/README.md +++ b/userspace-dp/src/filter/README.md @@ -62,7 +62,9 @@ Still gated before removing the userspace capability rejection: hierarchical `firewall three-color-policer ` blocks are compiled as one logical policer before that validation, so load merge/override cannot hide an ambiguity behind last-write-wins map - assignment. + assignment. Repeated same-mode sub-blocks (`single-rate` or + `two-rate`) are merged before strict validation as well, so split + color-mode declarations in sibling blocks are surfaced and rejected. - Filter terms still carry a policer name in the evaluation result. The hot forwarding path must move to stable policer IDs with ID-indexed or sharded state before three-color policers are enabled. From 361f4310b31a7b24fa7759c2d820f9d4ef8ad005 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 04:44:10 +0000 Subject: [PATCH 6/7] config: avoid redundant three-color mode loop writes Agent-Logs-Url: https://github.com/psaab/xpf/sessions/f42ea8d5-2bfb-4a18-95e8-1b5dbdb6bd80 Co-authored-by: psaab <196946+psaab@users.noreply.github.com> --- _Log.md | 4 ++++ go.mod | 2 +- pkg/config/compiler_firewall.go | 10 ++++++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/_Log.md b/_Log.md index 94a98d7e3..8b487ea49 100644 --- a/_Log.md +++ b/_Log.md @@ -2,6 +2,10 @@ ## 2026-05-17 +- **Timestamp**: 2026-05-17T05:03:00Z + - **Action**: PR #1395 round-4 follow-up cleanup — moved three-color mode marker assignment outside repeated same-mode child loops to avoid redundant writes while preserving duplicate-sibling merge semantics. + - **File(s)**: `pkg/config/compiler_firewall.go`, `_Log.md` + - **Timestamp**: 2026-05-17T04:58:00Z - **Action**: PR #1395 round-4 follow-up — fixed three-color policer compiler handling for duplicate same-mode sibling blocks by iterating all `single-rate`/`two-rate` children, added hierarchical ambiguity regression coverage in parser/configstore tests, and updated filter module docs to reflect same-mode sibling merge semantics. - **File(s)**: `pkg/config/compiler_firewall.go`, `pkg/config/parser_ast_test.go`, `pkg/configstore/store_test.go`, `userspace-dp/src/filter/README.md`, `_Log.md` diff --git a/go.mod b/go.mod index e3175fa5f..9af98c17b 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/insomniacslk/dhcp v0.0.0-20251020182700-175e84fbb167 github.com/mdlayher/ndp v1.1.0 github.com/prometheus/client_golang v1.23.2 + github.com/prometheus/client_model v0.6.2 github.com/vishvananda/netlink v1.3.1 golang.org/x/net v0.47.0 golang.org/x/sync v0.18.0 @@ -27,7 +28,6 @@ require ( github.com/mdlayher/socket v0.5.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pierrec/lz4/v4 v4.1.14 // indirect - github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.66.1 // indirect github.com/prometheus/procfs v0.16.1 // indirect github.com/u-root/uio v0.0.0-20230220225925-ffce2a382923 // indirect diff --git a/pkg/config/compiler_firewall.go b/pkg/config/compiler_firewall.go index d6224a14b..5dd4b7e2b 100644 --- a/pkg/config/compiler_firewall.go +++ b/pkg/config/compiler_firewall.go @@ -75,9 +75,12 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { fw.ThreeColorPolicers[tcpInst.name] = tcp } - for _, sr := range tcpInst.node.FindChildren("single-rate") { + singleRates := tcpInst.node.FindChildren("single-rate") + if len(singleRates) > 0 { tcp.SingleRateConfigured = true tcp.TwoRate = false + } + for _, sr := range singleRates { if sr.FindChild("color-blind") != nil { tcp.ColorBlind = true tcp.ColorBlindConfigured = true @@ -103,9 +106,12 @@ func compileFirewall(node *Node, fw *FirewallConfig) error { } } - for _, tr := range tcpInst.node.FindChildren("two-rate") { + twoRates := tcpInst.node.FindChildren("two-rate") + if len(twoRates) > 0 { tcp.TwoRateConfigured = true tcp.TwoRate = true + } + for _, tr := range twoRates { if tr.FindChild("color-blind") != nil { tcp.ColorBlind = true tcp.ColorBlindConfigured = true From 3f79822aacc69e12cb60ec868b5440bb3ff36033 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 04:44:34 +0000 Subject: [PATCH 7/7] chore: revert unintended go.mod classification drift Agent-Logs-Url: https://github.com/psaab/xpf/sessions/f42ea8d5-2bfb-4a18-95e8-1b5dbdb6bd80 Co-authored-by: psaab <196946+psaab@users.noreply.github.com> --- _Log.md | 4 ++++ go.mod | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/_Log.md b/_Log.md index 8b487ea49..649c0645c 100644 --- a/_Log.md +++ b/_Log.md @@ -2,6 +2,10 @@ ## 2026-05-17 +- **Timestamp**: 2026-05-17T05:06:00Z + - **Action**: PR #1395 cleanup — reverted an unintended `go.mod` direct/indirect dependency classification change introduced by automated tooling so the round-4 fix stays scoped to three-color policer compiler logic/tests/docs. + - **File(s)**: `go.mod`, `_Log.md` + - **Timestamp**: 2026-05-17T05:03:00Z - **Action**: PR #1395 round-4 follow-up cleanup — moved three-color mode marker assignment outside repeated same-mode child loops to avoid redundant writes while preserving duplicate-sibling merge semantics. - **File(s)**: `pkg/config/compiler_firewall.go`, `_Log.md` diff --git a/go.mod b/go.mod index 9af98c17b..e3175fa5f 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ require ( github.com/insomniacslk/dhcp v0.0.0-20251020182700-175e84fbb167 github.com/mdlayher/ndp v1.1.0 github.com/prometheus/client_golang v1.23.2 - github.com/prometheus/client_model v0.6.2 github.com/vishvananda/netlink v1.3.1 golang.org/x/net v0.47.0 golang.org/x/sync v0.18.0 @@ -28,6 +27,7 @@ require ( github.com/mdlayher/socket v0.5.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pierrec/lz4/v4 v4.1.14 // indirect + github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.66.1 // indirect github.com/prometheus/procfs v0.16.1 // indirect github.com/u-root/uio v0.0.0-20230220225925-ffce2a382923 // indirect