From 80ecde3c07282f63e848f135d6a0273530b5dd7c Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 18:49:06 -0700 Subject: [PATCH 1/4] userspace: add port mirror snapshot foundation --- pkg/dataplane/userspace/manager_test.go | 107 +++++++++++++++++++++++ pkg/dataplane/userspace/protocol.go | 11 +++ pkg/dataplane/userspace/protocol_test.go | 40 +++++++++ pkg/dataplane/userspace/snapshot.go | 78 +++++++++++++++++ 4 files changed, 236 insertions(+) diff --git a/pkg/dataplane/userspace/manager_test.go b/pkg/dataplane/userspace/manager_test.go index 700562ea1..b71984928 100644 --- a/pkg/dataplane/userspace/manager_test.go +++ b/pkg/dataplane/userspace/manager_test.go @@ -1923,6 +1923,113 @@ func TestBuildSnapshotIncludesThreeColorPolicerSchemaWhileGateClosed(t *testing. } } +func TestDeriveUserspaceCapabilitiesKeepsPortMirroringFailClosed(t *testing.T) { + cfg := &config.Config{} + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span1": { + Name: "span1", + InputRate: 10, + Input: []string{"ge-0/0/0.0"}, + Output: "ge-0/0/1.0", + }, + }, + } + + caps := deriveUserspaceCapabilities(cfg) + if caps.ForwardingSupported { + t.Fatal("ForwardingSupported = true, want false until mirror snapshot and runtime clone/inject are both wired") + } + found := false + for _, r := range caps.UnsupportedReasons { + if r == "port mirroring is not implemented in the userspace dataplane" { + found = true + } + } + if !found { + t.Fatalf("expected port mirroring unsupported reason, got: %+v", caps.UnsupportedReasons) + } +} + +func TestBuildMirrorConfigSnapshots(t *testing.T) { + cfg := &config.Config{} + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span1": { + Name: "span1", + InputRate: 50, + Input: []string{"ge-0/0/0.0"}, + Output: "ge-0/0/1.0", + }, + }, + } + interfaces := []InterfaceSnapshot{ + {Name: "ge-0/0/0.0", LinuxName: "ge-0-0-0.0", Ifindex: 11}, + {Name: "ge-0/0/1.0", LinuxName: "ge-0-0-1.0", Ifindex: 22}, + } + + got, err := buildMirrorConfigSnapshots(cfg, interfaces) + if err != nil { + t.Fatalf("buildMirrorConfigSnapshots: %v", err) + } + want := []MirrorConfigSnapshot{{IngressIfindex: 11, OutputIfindex: 22, Rate: 50}} + if !reflect.DeepEqual(got, want) { + t.Fatalf("mirror snapshots = %+v, want %+v", got, want) + } +} + +func TestBuildMirrorConfigSnapshotsRejectsDuplicateIngressIfindex(t *testing.T) { + cfg := &config.Config{} + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span1": { + Name: "span1", + Input: []string{"ge-0/0/0.0"}, + Output: "ge-0/0/1.0", + }, + "span2": { + Name: "span2", + Input: []string{"ge-0-0-0.0"}, + Output: "ge-0/0/1.0", + }, + }, + } + interfaces := []InterfaceSnapshot{ + {Name: "ge-0/0/0.0", LinuxName: "ge-0-0-0.0", Ifindex: 11}, + {Name: "ge-0/0/1.0", LinuxName: "ge-0-0-1.0", Ifindex: 22}, + } + + _, err := buildMirrorConfigSnapshots(cfg, interfaces) + if err == nil || !strings.Contains(err.Error(), "duplicate port-mirroring ingress ifindex 11") { + t.Fatalf("error = %v, want duplicate ingress ifindex rejection", err) + } +} + +func TestBuildMirrorConfigSnapshotsSkipsMissingOutputIfindex(t *testing.T) { + cfg := &config.Config{} + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span1": { + Name: "span1", + Input: []string{"ge-0/0/0.0"}, + Output: "ge-0/0/9.0", + }, + }, + } + interfaces := []InterfaceSnapshot{ + {Name: "ge-0/0/0.0", LinuxName: "ge-0-0-0.0", Ifindex: 11}, + {Name: "ge-0/0/9.0", LinuxName: "ge-0-0-9.0", Ifindex: 0}, + } + + got, err := buildMirrorConfigSnapshots(cfg, interfaces) + if err != nil { + t.Fatalf("buildMirrorConfigSnapshots: %v", err) + } + if len(got) != 0 { + t.Fatalf("mirror snapshots = %+v, want missing output ifindex skipped", got) + } +} + 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 7d9bb999a..e96691e17 100644 --- a/pkg/dataplane/userspace/protocol.go +++ b/pkg/dataplane/userspace/protocol.go @@ -64,6 +64,7 @@ type ConfigSnapshot struct { ThreeColorPolicers []ThreeColorPolicerSnapshot `json:"three_color_policers,omitempty"` ClassOfService *ClassOfServiceSnapshot `json:"class_of_service,omitempty"` FlowExport *FlowExportSnapshot `json:"flow_export,omitempty"` + MirrorConfigs []MirrorConfigSnapshot `json:"mirror_configs,omitempty"` Config *config.Config `json:"config,omitempty"` Userspace config.UserspaceConfig `json:"userspace"` DeferWorkers bool `json:"defer_workers,omitempty"` @@ -357,6 +358,16 @@ type FlowExportSnapshot struct { InactiveTimeout int `json:"inactive_timeout,omitempty"` // seconds, 0=default 15 } +// MirrorConfigSnapshot captures one ingress SPAN mapping for userspace-dp. +// It is snapshot/admission state only until the userspace runtime clone path is +// wired. Runtime delivery must use full-L2 cross-binding inject; the L3 TUN +// slow-path is not a valid mirror sink because it strips Ethernet framing. +type MirrorConfigSnapshot struct { + IngressIfindex int `json:"ingress_ifindex"` + OutputIfindex int `json:"output_ifindex"` + Rate uint32 `json:"rate"` +} + type PolicyApplicationSnapshot struct { Name string `json:"name"` Protocol string `json:"protocol,omitempty"` diff --git a/pkg/dataplane/userspace/protocol_test.go b/pkg/dataplane/userspace/protocol_test.go index 716467b50..b51b24510 100644 --- a/pkg/dataplane/userspace/protocol_test.go +++ b/pkg/dataplane/userspace/protocol_test.go @@ -60,6 +60,46 @@ func TestBindingStatusTXSharedRecycleUnknownSlotDropsRoundTrip(t *testing.T) { } } +func TestConfigSnapshotMirrorConfigsRoundTrip(t *testing.T) { + in := ConfigSnapshot{ + Version: 1, + MirrorConfigs: []MirrorConfigSnapshot{ + {IngressIfindex: 11, OutputIfindex: 22, Rate: 100}, + {IngressIfindex: 12, OutputIfindex: 22}, + }, + } + 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["mirror_configs"]; !ok { + t.Fatalf("wire key missing from ConfigSnapshot JSON: %s", string(raw)) + } + var mirrorObjects []map[string]json.RawMessage + if err := json.Unmarshal(obj["mirror_configs"], &mirrorObjects); err != nil { + t.Fatalf("unmarshal mirror_configs: %v", err) + } + for i, mirror := range mirrorObjects { + for _, key := range []string{"ingress_ifindex", "output_ifindex", "rate"} { + if _, ok := mirror[key]; !ok { + t.Fatalf("mirror_configs[%d] missing wire key %q: %s", i, key, string(raw)) + } + } + } + + var back ConfigSnapshot + if err := json.Unmarshal(raw, &back); err != nil { + t.Fatalf("unmarshal ConfigSnapshot: %v", err) + } + if !reflect.DeepEqual(back.MirrorConfigs, in.MirrorConfigs) { + t.Fatalf("mirror config round-trip mismatch: got %+v, want %+v", back.MirrorConfigs, in.MirrorConfigs) + } +} + func TestBindingCountersSnapshotTXSharedRecycleUnknownSlotDropsRoundTrip(t *testing.T) { in := BindingCountersSnapshot{ WorkerID: 3, diff --git a/pkg/dataplane/userspace/snapshot.go b/pkg/dataplane/userspace/snapshot.go index 84db97eb2..abcee96ec 100644 --- a/pkg/dataplane/userspace/snapshot.go +++ b/pkg/dataplane/userspace/snapshot.go @@ -66,6 +66,7 @@ func buildSnapshotWithSchedulerState(cfg *config.Config, ucfg config.UserspaceCo ThreeColorPolicers: buildThreeColorPolicerSnapshots(cfg), ClassOfService: buildClassOfServiceSnapshot(cfg), FlowExport: buildFlowExportSnapshot(cfg), + MirrorConfigs: mustBuildMirrorConfigSnapshots(cfg, interfaces), Config: cfg, Summary: SnapshotSummary{ HostName: cfg.System.HostName, @@ -79,6 +80,83 @@ func buildSnapshotWithSchedulerState(cfg *config.Config, ucfg config.UserspaceCo } } +func mustBuildMirrorConfigSnapshots(cfg *config.Config, interfaces []InterfaceSnapshot) []MirrorConfigSnapshot { + mirrors, err := buildMirrorConfigSnapshots(cfg, interfaces) + if err != nil { + // The mirror table contract is one output per ingress ifindex. Keep + // snapshot publication fail-closed by omitting an ambiguous mirror table + // if this helper is called on an invalid config. + slog.Warn("userspace snapshot: invalid port-mirroring config skipped", "err", err) + return nil + } + return mirrors +} + +func buildMirrorConfigSnapshots(cfg *config.Config, interfaces []InterfaceSnapshot) ([]MirrorConfigSnapshot, error) { + if cfg == nil || cfg.ForwardingOptions.PortMirroring == nil || len(cfg.ForwardingOptions.PortMirroring.Instances) == 0 { + return nil, nil + } + ifindexByName := make(map[string]int, len(interfaces)) + for _, iface := range interfaces { + if iface.Ifindex > 0 { + ifindexByName[iface.Name] = iface.Ifindex + if iface.LinuxName != "" { + ifindexByName[iface.LinuxName] = iface.Ifindex + } + } + } + + instanceNames := make([]string, 0, len(cfg.ForwardingOptions.PortMirroring.Instances)) + for name := range cfg.ForwardingOptions.PortMirroring.Instances { + instanceNames = append(instanceNames, name) + } + sort.Strings(instanceNames) + + seenIngress := make(map[int]string) + out := make([]MirrorConfigSnapshot, 0) + for _, name := range instanceNames { + inst := cfg.ForwardingOptions.PortMirroring.Instances[name] + if inst == nil { + continue + } + if inst.Output == "" { + slog.Warn("port-mirroring instance has no output interface", "name", name) + continue + } + outputIfindex := ifindexByName[inst.Output] + if outputIfindex <= 0 { + outputIfindex = ifindexByName[config.LinuxIfName(inst.Output)] + } + if outputIfindex <= 0 { + slog.Warn("port-mirroring output interface not found", + "name", name, "interface", inst.Output) + continue + } + + for _, input := range inst.Input { + ingressIfindex := ifindexByName[input] + if ingressIfindex <= 0 { + ingressIfindex = ifindexByName[config.LinuxIfName(input)] + } + if ingressIfindex <= 0 { + slog.Warn("port-mirroring input interface not found", + "name", name, "interface", input) + continue + } + if previous, ok := seenIngress[ingressIfindex]; ok { + return nil, fmt.Errorf("duplicate port-mirroring ingress ifindex %d in instances %q and %q", ingressIfindex, previous, name) + } + seenIngress[ingressIfindex] = name + out = append(out, MirrorConfigSnapshot{ + IngressIfindex: ingressIfindex, + OutputIfindex: outputIfindex, + Rate: uint32(inst.InputRate), + }) + } + } + return out, nil +} + const ( // Keep logical-only synthetic ifindexes in a high private range so // they do not collide with kernel-assigned ifindexes in practical From 57243839fdfb6c0ecf80ca6bd048d5c202b91797 Mon Sep 17 00:00:00 2001 From: Paul Saab Date: Sat, 16 May 2026 19:27:22 -0700 Subject: [PATCH 2/4] userspace: test port mirror snapshot wire shape --- pkg/dataplane/userspace/manager_test.go | 33 ++++++++++++ userspace-dp/src/main_tests.rs | 71 ++++++++++++++++++++----- userspace-dp/src/protocol.rs | 12 +++++ 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/pkg/dataplane/userspace/manager_test.go b/pkg/dataplane/userspace/manager_test.go index b71984928..194133e2a 100644 --- a/pkg/dataplane/userspace/manager_test.go +++ b/pkg/dataplane/userspace/manager_test.go @@ -1978,6 +1978,39 @@ func TestBuildMirrorConfigSnapshots(t *testing.T) { } } +func TestBuildSnapshotIncludesMirrorConfigsFromRealInterfaceSnapshot(t *testing.T) { + cfg := &config.Config{} + cfg.Interfaces.Interfaces = map[string]*config.InterfaceConfig{ + "lo": {Name: "lo"}, + } + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span-loopback": { + Name: "span-loopback", + InputRate: 7, + Input: []string{"lo"}, + Output: "lo", + }, + }, + } + + snap := buildSnapshot(cfg, config.UserspaceConfig{}, 1, 0) + var loIfindex int + for _, iface := range snap.Interfaces { + if iface.Name == "lo" { + loIfindex = iface.Ifindex + break + } + } + if loIfindex <= 0 { + t.Fatalf("buildSnapshot did not resolve loopback ifindex in interfaces: %+v", snap.Interfaces) + } + want := []MirrorConfigSnapshot{{IngressIfindex: loIfindex, OutputIfindex: loIfindex, Rate: 7}} + if !reflect.DeepEqual(snap.MirrorConfigs, want) { + t.Fatalf("MirrorConfigs = %+v, want %+v", snap.MirrorConfigs, want) + } +} + func TestBuildMirrorConfigSnapshotsRejectsDuplicateIngressIfindex(t *testing.T) { cfg := &config.Config{} cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ diff --git a/userspace-dp/src/main_tests.rs b/userspace-dp/src/main_tests.rs index 964c975c4..b3520b1e5 100644 --- a/userspace-dp/src/main_tests.rs +++ b/userspace-dp/src/main_tests.rs @@ -369,8 +369,8 @@ fn build_synced_session_entry_preserves_fabric_ingress() { ..SessionSyncRequest::default() }; - let entry = build_synced_session_entry(&req, &test_zone_name_to_id()) - .expect("synced session entry"); + let entry = + build_synced_session_entry(&req, &test_zone_name_to_id()).expect("synced session entry"); assert!(entry.metadata.fabric_ingress); assert!(entry.origin.is_peer_synced()); assert_eq!(entry.metadata.owner_rg_id, 1); @@ -392,8 +392,8 @@ fn build_synced_session_entry_preserves_tunnel_endpoint_id() { ..SessionSyncRequest::default() }; - let entry = build_synced_session_entry(&req, &test_zone_name_to_id()) - .expect("synced session entry"); + let entry = + build_synced_session_entry(&req, &test_zone_name_to_id()).expect("synced session entry"); assert_eq!(entry.decision.resolution.tunnel_endpoint_id, 3); assert_eq!(entry.decision.resolution.egress_ifindex, 586); assert_eq!( @@ -425,8 +425,8 @@ fn build_synced_session_entry_prefers_id_over_legacy_zone_name() { tx_ifindex: 5, ..SessionSyncRequest::default() }; - let entry = build_synced_session_entry(&req, &test_zone_name_to_id()) - .expect("synced session entry"); + let entry = + build_synced_session_entry(&req, &test_zone_name_to_id()).expect("synced session entry"); assert_eq!(entry.metadata.ingress_zone, 1); assert_eq!(entry.metadata.egress_zone, 2); } @@ -451,8 +451,8 @@ fn build_synced_session_entry_falls_back_to_zone_name_when_id_zero() { tx_ifindex: 5, ..SessionSyncRequest::default() }; - let entry = build_synced_session_entry(&req, &test_zone_name_to_id()) - .expect("synced session entry"); + let entry = + build_synced_session_entry(&req, &test_zone_name_to_id()).expect("synced session entry"); let m = test_zone_name_to_id(); assert_eq!(entry.metadata.ingress_zone, m["lan"]); assert_eq!(entry.metadata.egress_zone, m["wan"]); @@ -479,8 +479,8 @@ fn build_synced_session_entry_unknown_zone_name_does_not_drop_session() { tx_ifindex: 5, ..SessionSyncRequest::default() }; - let entry = build_synced_session_entry(&req, &test_zone_name_to_id()) - .expect("synced session entry"); + let entry = + build_synced_session_entry(&req, &test_zone_name_to_id()).expect("synced session entry"); assert_eq!(entry.metadata.ingress_zone, 0); assert_eq!(entry.metadata.egress_zone, 0); } @@ -879,8 +879,7 @@ fn binding_counters_snapshot_serializes_with_expected_wire_keys() { // Round-trip: the daemon's JSON → Rust decode path must be // symmetric with the Rust encode path. let json = serde_json::to_string(&snap).expect("serialize snapshot"); - let round: BindingCountersSnapshot = - serde_json::from_str(&json).expect("deserialize snapshot"); + let round: BindingCountersSnapshot = serde_json::from_str(&json).expect("deserialize snapshot"); assert_eq!(round, snap); } @@ -1013,6 +1012,51 @@ fn config_snapshot_three_color_policers_roundtrip() { ); } +#[test] +fn config_snapshot_mirror_configs_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 + }, + "mirror_configs": [ + {"ingress_ifindex": 11, "output_ifindex": 22, "rate": 100}, + {"ingress_ifindex": 12, "output_ifindex": 22, "rate": 0} + ] + }"#; + let snap: ConfigSnapshot = serde_json::from_str(json).expect("mirror snapshot decodes"); + assert_eq!( + snap.mirror_configs, + vec![ + MirrorConfigSnapshot { + ingress_ifindex: 11, + output_ifindex: 22, + rate: 100, + }, + MirrorConfigSnapshot { + ingress_ifindex: 12, + output_ifindex: 22, + rate: 0, + }, + ], + "Rust snapshot DTO must round-trip the Go mirror_configs wire shape" + ); + + let encoded = serde_json::to_value(&snap).expect("mirror snapshot serializes"); + assert!( + encoded.get("mirror_configs").is_some(), + "mirror_configs wire key missing from Rust serialization: {encoded}" + ); +} + #[test] fn tx_latency_hist_serialization_roundtrip() { // #812 plan §6.1 test #4. Construct a BindingCountersSnapshot @@ -1057,8 +1101,7 @@ fn tx_latency_hist_serialization_roundtrip() { v_min_throttles: 19, }; let json = serde_json::to_string(&snap).expect("serialize snapshot"); - let back: BindingCountersSnapshot = - serde_json::from_str(&json).expect("deserialize snapshot"); + let back: BindingCountersSnapshot = serde_json::from_str(&json).expect("deserialize snapshot"); assert_eq!(back, snap); assert_eq!(back.tx_submit_latency_hist.len(), 16); assert_eq!(back.tx_submit_latency_hist[0], 9001); diff --git a/userspace-dp/src/protocol.rs b/userspace-dp/src/protocol.rs index a0bfb0642..43c1b98b6 100644 --- a/userspace-dp/src/protocol.rs +++ b/userspace-dp/src/protocol.rs @@ -323,6 +323,8 @@ pub(crate) struct ConfigSnapshot { pub class_of_service: Option, #[serde(rename = "flow_export", default)] pub flow_export: Option, + #[serde(rename = "mirror_configs", default)] + pub mirror_configs: Vec, #[serde(default)] pub userspace: serde_json::Value, #[serde(default)] @@ -331,6 +333,16 @@ pub(crate) struct ConfigSnapshot { pub defer_workers: bool, } +#[derive(Clone, Debug, Serialize, Deserialize, Default, PartialEq, Eq)] +pub(crate) struct MirrorConfigSnapshot { + #[serde(rename = "ingress_ifindex", default)] + pub ingress_ifindex: i32, + #[serde(rename = "output_ifindex", default)] + pub output_ifindex: i32, + #[serde(default)] + pub rate: u32, +} + #[derive(Clone, Debug, Serialize, Deserialize, Default)] pub(crate) struct ZoneSnapshot { pub name: String, From 319baca027358088ad2e94c7ddf36828d34027b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 06:36:14 +0000 Subject: [PATCH 3/4] userspace: harden mirror snapshot rate validation Agent-Logs-Url: https://github.com/psaab/xpf/sessions/11ea982a-ae2c-47db-b452-b224abc2589b Co-authored-by: psaab <196946+psaab@users.noreply.github.com> --- _Log.md | 4 ++++ pkg/dataplane/userspace/manager_test.go | 23 +++++++++++++++++++++++ pkg/dataplane/userspace/protocol_test.go | 2 +- pkg/dataplane/userspace/snapshot.go | 7 +++++-- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/_Log.md b/_Log.md index abc8acf97..1790b13f3 100644 --- a/_Log.md +++ b/_Log.md @@ -7,6 +7,10 @@ - **File(s)**: `pkg/daemon/daemon_ha_userspace.go`, `pkg/daemon/userspace_sync_test.go`, `_Log.md` - **Validation**: `gofmt -w pkg/daemon/daemon_ha_userspace.go pkg/daemon/userspace_sync_test.go`; `go test ./pkg/daemon ./pkg/dataplane/userspace ./pkg/logging`; `git diff --check` +- **Timestamp**: 2026-05-17T06:45:00Z + - **Action**: PR #1376 re-review follow-up — renamed the mirror snapshot fail-closed wrapper to match behavior, rejected negative port-mirroring input rates before uint32 conversion to prevent wraparound, and updated mirror protocol/build tests accordingly. + - **File(s)**: `pkg/dataplane/userspace/snapshot.go`, `pkg/dataplane/userspace/protocol_test.go`, `pkg/dataplane/userspace/manager_test.go`, `_Log.md` + - **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` diff --git a/pkg/dataplane/userspace/manager_test.go b/pkg/dataplane/userspace/manager_test.go index 194133e2a..e5566c92b 100644 --- a/pkg/dataplane/userspace/manager_test.go +++ b/pkg/dataplane/userspace/manager_test.go @@ -2063,6 +2063,29 @@ func TestBuildMirrorConfigSnapshotsSkipsMissingOutputIfindex(t *testing.T) { } } +func TestBuildMirrorConfigSnapshotsRejectsNegativeInputRate(t *testing.T) { + cfg := &config.Config{} + cfg.ForwardingOptions.PortMirroring = &config.PortMirroringConfig{ + Instances: map[string]*config.PortMirrorInstance{ + "span1": { + Name: "span1", + InputRate: -1, + Input: []string{"ge-0/0/0.0"}, + Output: "ge-0/0/1.0", + }, + }, + } + interfaces := []InterfaceSnapshot{ + {Name: "ge-0/0/0.0", LinuxName: "ge-0-0-0.0", Ifindex: 11}, + {Name: "ge-0/0/1.0", LinuxName: "ge-0-0-1.0", Ifindex: 22}, + } + + _, err := buildMirrorConfigSnapshots(cfg, interfaces) + if err == nil || !strings.Contains(err.Error(), "negative input rate") { + t.Fatalf("error = %v, want negative input rate rejection", err) + } +} + func TestDeriveUserspaceCapabilitiesAllowsFirewallFilters(t *testing.T) { cfg := &config.Config{} cfg.Firewall.FiltersInet = map[string]*config.FirewallFilter{ diff --git a/pkg/dataplane/userspace/protocol_test.go b/pkg/dataplane/userspace/protocol_test.go index b51b24510..7f26e2b58 100644 --- a/pkg/dataplane/userspace/protocol_test.go +++ b/pkg/dataplane/userspace/protocol_test.go @@ -62,7 +62,7 @@ func TestBindingStatusTXSharedRecycleUnknownSlotDropsRoundTrip(t *testing.T) { func TestConfigSnapshotMirrorConfigsRoundTrip(t *testing.T) { in := ConfigSnapshot{ - Version: 1, + Version: ProtocolVersion, MirrorConfigs: []MirrorConfigSnapshot{ {IngressIfindex: 11, OutputIfindex: 22, Rate: 100}, {IngressIfindex: 12, OutputIfindex: 22}, diff --git a/pkg/dataplane/userspace/snapshot.go b/pkg/dataplane/userspace/snapshot.go index abcee96ec..30956f8e6 100644 --- a/pkg/dataplane/userspace/snapshot.go +++ b/pkg/dataplane/userspace/snapshot.go @@ -66,7 +66,7 @@ func buildSnapshotWithSchedulerState(cfg *config.Config, ucfg config.UserspaceCo ThreeColorPolicers: buildThreeColorPolicerSnapshots(cfg), ClassOfService: buildClassOfServiceSnapshot(cfg), FlowExport: buildFlowExportSnapshot(cfg), - MirrorConfigs: mustBuildMirrorConfigSnapshots(cfg, interfaces), + MirrorConfigs: buildMirrorConfigSnapshotsFailClosed(cfg, interfaces), Config: cfg, Summary: SnapshotSummary{ HostName: cfg.System.HostName, @@ -80,7 +80,7 @@ func buildSnapshotWithSchedulerState(cfg *config.Config, ucfg config.UserspaceCo } } -func mustBuildMirrorConfigSnapshots(cfg *config.Config, interfaces []InterfaceSnapshot) []MirrorConfigSnapshot { +func buildMirrorConfigSnapshotsFailClosed(cfg *config.Config, interfaces []InterfaceSnapshot) []MirrorConfigSnapshot { mirrors, err := buildMirrorConfigSnapshots(cfg, interfaces) if err != nil { // The mirror table contract is one output per ingress ifindex. Keep @@ -146,6 +146,9 @@ func buildMirrorConfigSnapshots(cfg *config.Config, interfaces []InterfaceSnapsh if previous, ok := seenIngress[ingressIfindex]; ok { return nil, fmt.Errorf("duplicate port-mirroring ingress ifindex %d in instances %q and %q", ingressIfindex, previous, name) } + if inst.InputRate < 0 { + return nil, fmt.Errorf("port-mirroring instance %q has negative input rate %d", name, inst.InputRate) + } seenIngress[ingressIfindex] = name out = append(out, MirrorConfigSnapshot{ IngressIfindex: ingressIfindex, From 170b882e8b0f6d2e3668175d579591fd874e47d8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 06:38:03 +0000 Subject: [PATCH 4/4] chore: restore go.mod after automation drift Agent-Logs-Url: https://github.com/psaab/xpf/sessions/11ea982a-ae2c-47db-b452-b224abc2589b 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 1790b13f3..c1f0bb497 100644 --- a/_Log.md +++ b/_Log.md @@ -7,6 +7,10 @@ - **File(s)**: `pkg/daemon/daemon_ha_userspace.go`, `pkg/daemon/userspace_sync_test.go`, `_Log.md` - **Validation**: `gofmt -w pkg/daemon/daemon_ha_userspace.go pkg/daemon/userspace_sync_test.go`; `go test ./pkg/daemon ./pkg/dataplane/userspace ./pkg/logging`; `git diff --check` +- **Timestamp**: 2026-05-17T06:39:00Z + - **Action**: PR #1376 housekeeping — restored `go.mod` after an unintended direct/indirect dependency classification flip from automation so this branch remains scoped to mirror snapshot fixes. + - **File(s)**: `go.mod`, `_Log.md` + - **Timestamp**: 2026-05-17T06:45:00Z - **Action**: PR #1376 re-review follow-up — renamed the mirror snapshot fail-closed wrapper to match behavior, rejected negative port-mirroring input rates before uint32 conversion to prevent wraparound, and updated mirror protocol/build tests accordingly. - **File(s)**: `pkg/dataplane/userspace/snapshot.go`, `pkg/dataplane/userspace/protocol_test.go`, `pkg/dataplane/userspace/manager_test.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