[3/3] Add `CGROUP_SOCK_ADDR` matcher support by yaakov-stein · Pull Request #465 · facebook/bpfilter

yaakov-stein · 2026-03-08T21:58:18Z

Stacked PRs:

Completes CGROUP_SOCK_ADDR support (#355) by implementing matcher codegen, per-hook matcher validation, CLI counter semantics, E2E tests, and documentation.

Summary

#459 added the CGROUP_SOCK_ADDR flavor with only default-verdict support and no working matchers. This adds matcher codegen so rules can actually match against fields available in the bpf_sock_addr context.

Matcher codegen

The _bf_cgroup_sock_addr_gen_inline_matcher() callback loads fields directly from struct bpf_sock_addr (user_ip4/user_ip6, user_port, family, protocol) and delegates to the shared bf_cmp_*() functions from #464. No packet parsing prologue is needed.

Supported matchers:

Meta: l3_proto, l4_proto, probability, dport
IPv4 (CONNECT4 only): daddr, dnet, proto
IPv6 (CONNECT6 only): daddr, dnet
TCP/UDP: dport

Unsupported matchers are enforced at validation time via unsupported_hooks flags in matcher.c, so invalid rules are rejected before codegen.

CLI counter display

CGROUP_SOCK_ADDR hooks intercept connect() syscalls, not packets. The CLI now prints calls instead of packets and bytes for these hooks.

E2E tests

New tests/e2e/hooks/ directory for per-hook behavioral tests that combine dry-run validation (supported + unsupported matchers) with sandbox testing in an isolated cgroup. Each supported matcher is verified by attaching a chain and confirming that matching traffic is dropped and, where possible, non-matching traffic passes through. Also adds CGROUP_SOCK_ADDR coverage to chain_attach.sh.

Documentation

Updates bfcli.rst and daemon.rst: new hooks in the hooks list, cgpath option, counter semantics, REDIRECT incompatibility, and a connection-level note for meta.probability.

Testing

unit + e2e
e2e.hooks for both CONNECT4 and CONNECT6
e2e.cli.chain_attach with new CGROUP_SOCK_ADDR section
e2e.rulesets with new .bf files for both CONNECT4 and CONNECT6

github-actions · 2026-03-08T22:14:29Z

src/bfcli/print.c

+    bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
+                       chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;


Claude: Pointer dereference before assert() precondition check

is_pkt_hook dereferences chain->hook at line 89 before assert(chain) validates the pointer at line 92. The project convention (confirmed across _bf_matcher_generate_ip4_addr, bf_matcher_generate_packet, etc.) places assert() checks before any use of the validated pointer.

Suggested change

bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&

chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;

bool is_pkt_hook;

Then assign after the asserts:

assert(chain); assert(counters); is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 && chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;

github-actions · 2026-03-08T22:14:31Z

src/bpfilter/cgen/cgroup_sock_addr.c

+    const struct bf_matcher_meta *meta =
+        bf_matcher_get_meta(bf_matcher_get_type(matcher));


Claude: Pointer dereference before assert() precondition check

bf_matcher_get_meta(bf_matcher_get_type(matcher)) dereferences matcher before assert(matcher) at line 230. Compare with bf_matcher_generate_packet in packet.c which correctly assigns meta after the assertions.

Suggested change

const struct bf_matcher_meta *meta =

bf_matcher_get_meta(bf_matcher_get_type(matcher));

const struct bf_matcher_meta *meta;

Then assign after the asserts:

assert(program); assert(matcher); meta = bf_matcher_get_meta(bf_matcher_get_type(matcher));

github-actions · 2026-03-08T22:14:32Z

src/bpfilter/cgen/cgroup_sock_addr.c

+    return 0;
+}
+
+static int _bf_sock_addr_load_ctx(struct bf_program *program)


Claude: Inconsistent static function naming prefix

Eight internal static functions here use a shortened _bf_sock_addr_ prefix (_bf_sock_addr_load_ctx, _bf_sock_addr_generate_ip4_addr, etc.) while the ops-facing functions use the full _bf_cgroup_sock_addr_ prefix.

The project convention is that static functions match the module name: cgroup_skb.c uses _bf_cgroup_skb_*, tc.c uses _bf_tc_*, nf.c uses _bf_nf_*. For consistency, consider using _bf_cgroup_sock_addr_* throughout.

github-actions · 2026-03-08T22:14:56Z

Replace gen_inline_get_mark and gen_inline_get_skb in bf_flavor_ops with a single gen_inline_matcher callback that gives each flavor full control over matcher codegen. Packet-based flavors intercept matcher types they handle specially (e.g., meta.mark on __sk_buff-based flavors) and delegate the rest to bf_matcher_generate_packet(). Extract bf_matcher_generate_packet() from program.c into cgen/ matcher/packet.{h,c} where it belongs alongside the matcher codegen it dispatches to. Export bf_matcher_generate_meta_mark() and bf_matcher_generate_meta_flow_hash() as comparison-only helpers. Callers load the value into BPF_REG_1 before calling. This is a prerequisite for CGROUP_SOCK_ADDR support, where the flavor needs to intercept matchers like ip4.daddr and generate bytecode that reads from bpf_sock_addr fields instead of packet headers.

Separate comparison logic from data loading in matcher codegen. bf_cmp_value, bf_cmp_masked_value, bf_cmp_range, and bf_cmp_bitfield encapsulate the BPF bytecode patterns previously duplicated across ip4.c, ip6.c, tcp.c, udp.c, icmp.c, and meta.c. No behavioral change; emitted bytecode is identical.

Remove ip4.c/h, ip6.c/h, tcp.c/h, udp.c/h, and icmp.c/h by absorbing their loading logic into packet.c. Protocol guards and header loads now go through bf_stub_rule_check_protocol and bf_stub_load_header driven by _bf_matcher_metas, while comparisons use the shared cmp.c functions. This centralises all packet matcher codegen in a single dispatch point, making it straightforward to add new flavor-specific dispatch without touching per-protocol files.

Add BF_FLAVOR_CGROUP_SOCK_ADDR and BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4/CONNECT6 to support BPF_PROG_TYPE_CGROUP_SOCK_ADDR programs. This is the foundational enum/mapping work for sock_addr filtering. All mapping tables (hook strings, flavor, prog_type, attach_type), BPF type constants, link creation, and hookopts are updated. All existing matchers are blocked on the new hooks via unsupported_hooks. Flavor ops are registered as NULL and codegen is added in a follow-up.

Implement bf_flavor_ops for BF_FLAVOR_CGROUP_SOCK_ADDR so chains with a default policy can be loaded and attached to a cgroup.

Replace the -ENOTSUP stub with a dispatcher that loads fields from bpf_sock_addr (user_ip4, user_ip6, user_port, protocol) and reuses the shared bf_cmp_*() comparison functions. Update unsupported_hooks to unblock the supported matchers on CONNECT4/CONNECT6 hooks.

Introduce a tests/e2e/hooks/ directory for tests that combine dry-run validation with behavioral sandbox testing on a per-hook basis. Each test verifies every supported matcher against actual traffic in an isolated cgroup, rather than only checking dry-run acceptance.

github-actions · 2026-03-10T02:04:46Z

tests/e2e/cli/chain_attach.sh

+CGROUP_PATH=/sys/fs/cgroup/bftest_chain_attach
+mkdir -p ${CGROUP_PATH}
+${FROM_NS} bfcli chain load --from-str "chain chain_attach_csa4_0 BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 ACCEPT rule meta.dport eq 9990 DROP"
+${FROM_NS} bfcli chain load --from-str "chain chain_attach_csa4_1 BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 ACCEPT"
+${FROM_NS} bfcli chain attach --name chain_attach_csa4_0 --option cgpath=${CGROUP_PATH}
+${FROM_NS} bfcli chain attach --name chain_attach_csa4_1 --option cgpath=${CGROUP_PATH}
+(! ${FROM_NS} bash -c "echo \$\$ > ${CGROUP_PATH}/cgroup.procs && echo > /dev/udp/${HOST_IP_ADDR}/9990" 2>/dev/null)
+${FROM_NS} bfcli chain flush --name chain_attach_csa4_0
+${FROM_NS} bfcli chain flush --name chain_attach_csa4_1


Claude: Cgroup directory leaks on early test failure

The cgroup at CGROUP_PATH is created with mkdir -p but the EXIT trap is not updated to include rmdir. If any command between mkdir (line 44) and rmdir (line 52) fails, set -e triggers the original EXIT trap which only calls cleanup — the cgroup directory is left behind.

The cgroup_sock_addr_connect4.sh test handles this correctly:

trap 'ret=$?; rmdir ${CGROUP_PATH} 2>/dev/null; cleanup; exit ${ret}' EXIT

Consider applying the same pattern here.

yaakov-stein · 2026-03-11T23:19:03Z

src/bpfilter/cgen/cgroup_sock_addr.c

+    return 0;
+}
+
+static int _bf_sock_addr_load_ctx(struct bf_program *program)


Note: inline this, it's unnecessary to have a function for one instruction

meta-cla bot added the cla signed label Mar 8, 2026

yaakov-stein changed the title ~~Add CGROUP_SOCK_ADDR matcher support~~ [3/3] Add CGROUP_SOCK_ADDR matcher support Mar 8, 2026

github-actions bot reviewed Mar 8, 2026

View reviewed changes

yaakov-stein added 4 commits March 9, 2026 06:50

daemon: cgen: emit missing skb load in _bf_nf_gen_inline_get_skb

7a33678

This was referenced Mar 9, 2026

[2/3] Add CGROUP_SOCK_ADDR initial chain support #459

Open

[1/3] Separate comparison logic from data loading in matchers #464

Open

[0/3] Enable flavor specific matching #456

Open

yaakov-stein added 6 commits March 9, 2026 18:39

daemon: cgen: add cgroup_sock_addr flavor codegen

9167c68

Implement bf_flavor_ops for BF_FLAVOR_CGROUP_SOCK_ADDR so chains with a default policy can be loaded and attached to a cgroup.

cli: display proper naming for cgroup_sock_addr counters

37eae15

doc: document cgroup_sock_addr hooks and counter semantics

d0d91e6

yaakov-stein force-pushed the add_cgroup_sock_addr_matcher_support branch from 08ad95f to d0d91e6 Compare March 10, 2026 01:48

github-actions bot reviewed Mar 10, 2026

View reviewed changes

yaakov-stein commented Mar 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3/3] Add `CGROUP_SOCK_ADDR` matcher support#465

[3/3] Add `CGROUP_SOCK_ADDR` matcher support#465
yaakov-stein wants to merge 10 commits intofacebook:mainfrom
yaakov-stein:add_cgroup_sock_addr_matcher_support

yaakov-stein commented Mar 8, 2026 •

edited

Loading

Uh oh!

github-actions bot Mar 8, 2026

Uh oh!

github-actions bot Mar 8, 2026

Uh oh!

github-actions bot Mar 8, 2026

Uh oh!

github-actions bot commented Mar 8, 2026 •

edited

Loading

Uh oh!

github-actions bot Mar 10, 2026

Uh oh!

yaakov-stein Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
		chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;

	bool is_pkt_hook = chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4 &&
	chain->hook != BF_HOOK_CGROUP_SOCK_ADDR_CONNECT6;
	bool is_pkt_hook;

		const struct bf_matcher_meta *meta =
		bf_matcher_get_meta(bf_matcher_get_type(matcher));

	const struct bf_matcher_meta *meta =
	bf_matcher_get_meta(bf_matcher_get_type(matcher));
	const struct bf_matcher_meta *meta;

Conversation

yaakov-stein commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Matcher codegen

CLI counter display

E2E tests

Documentation

Testing

Uh oh!

github-actions bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Must fix

Suggestions

Nits

Uh oh!

github-actions bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

yaakov-stein Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yaakov-stein commented Mar 8, 2026 •

edited

Loading

github-actions bot commented Mar 8, 2026 •

edited

Loading