Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions _Log.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

## 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`

- **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`
Expand Down
45 changes: 45 additions & 0 deletions pkg/config/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -228,6 +231,48 @@ 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.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)
}
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
Expand Down
32 changes: 25 additions & 7 deletions pkg/config/compiler_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,27 @@ 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 {
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
}
if sr.FindChild("color-aware") != nil {
tcp.ColorAwareConfigured = true
}
for _, child := range sr.Children {
switch child.Name() {
Expand All @@ -94,10 +106,18 @@ func compileFirewall(node *Node, fw *FirewallConfig) error {
}
}

if tr := tcpInst.node.FindChild("two-rate"); tr != nil {
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
}
if tr.FindChild("color-aware") != nil {
tcp.ColorAwareConfigured = true
}
for _, child := range tr.Children {
switch child.Name() {
Expand Down Expand Up @@ -133,8 +153,6 @@ func compileFirewall(node *Node, fw *FirewallConfig) error {
}
}
}

fw.ThreeColorPolicers[tcp.Name] = tcp
}

for _, familyNode := range node.FindChildren("family") {
Expand Down
121 changes: 121 additions & 0 deletions pkg/config/parser_ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4539,6 +4539,127 @@ 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",
},
{
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 {
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 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 {
Expand Down
20 changes: 13 additions & 7 deletions pkg/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading