Skip to content

Commit 674766b

Browse files
daniel-nolandclaude
andcommitted
perf(acl-filter): skip the lookups for a peering without ACLs
For a packet on a flow that no rule matches directly, the filter builds the reply's summary from the reverse flow, looks that up too, and only then falls back to the peering's default action. A peering without an ACL takes that whole path for every packet of every flow: it upgrades the `Weak` to the reverse flow (an atomic on a cold cache line, then another to drop it) and reads that flow's key, to look up rules that cannot exist. Every peering with an ACL registers a default action for both directions, and only such a peering has rules, so a pair without a default action can match nothing either way. The filter now allows it without looking anything up. The config-oracle fuzz test checks the invariant this rests on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
1 parent d9f4b78 commit 674766b

2 files changed

Lines changed: 24 additions & 9 deletions

File tree

‎acl-filter/src/fuzz.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,19 @@ fn reference_lookup_matches_config_oracle() {
236236
probe.src_vni, probe.dst_vni,
237237
);
238238

239+
// The filter skips both lookups for a pair with no default action, which is only
240+
// right if a rule in either direction implies a default action in both.
241+
if got.is_some() {
242+
assert!(
243+
got_default.is_some()
244+
&& tables
245+
.find_default_action(probe.dst_vni, probe.src_vni)
246+
.is_some(),
247+
"a rule matches {probe:?} but its peering has no default action both ways\n\
248+
spec: {overlay_spec:?}",
249+
);
250+
}
251+
239252
assert_eq!(
240253
resolved_action(got, got_default),
241254
resolved_action(want, want_default),

‎acl-filter/src/lib.rs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ impl AclFilter {
122122
let guard = self.tablesr.load();
123123
let tables = &guard.acls;
124124

125+
// Every peering with an ACL has a default action registered for both directions, and only
126+
// such a peering has rules. Without one, nothing can match either way: allow the packet
127+
// without the lookups, and without reaching the reverse flow (a cold cache line, and an
128+
// atomic on it, for every packet of every flow).
129+
let Some(default_action) = tables.find_default_action(summary.src_vni, summary.dst_vni)
130+
else {
131+
return (AclAction::Allow, false);
132+
};
133+
125134
// Look up for an ACL directly matching the packet
126135
let lookup_result = tables.lookup(summary);
127136
if let Some(result) = lookup_result {
@@ -155,15 +164,8 @@ impl AclFilter {
155164
}
156165
}
157166

158-
// Look for a fallback default action for the peering
159-
let verdict = tables
160-
.find_default_action(summary.src_vni, summary.dst_vni)
161-
.unwrap_or(
162-
// No default action was found for this peering, this means no ACL list was
163-
// configured for the peering. Allow packet to go through.
164-
AclAction::Allow,
165-
);
166-
(verdict, false)
167+
// Fall back to the peering's default action
168+
(default_action, false)
167169
}
168170
}
169171

0 commit comments

Comments
 (0)