Prevent `bf_map` and `bf_link` serialization by qdeslandes · Pull Request #420 · facebook/bpfilter

qdeslandes · 2026-02-14T16:23:16Z

bf_map represent BPF map, and bf_link BPF links. Both those objects are part of the runtime context of the daemon required for every chain, as such they are serialized on disk.

Eventually, this creates 2 sources of truth for the chain: the BPF object and its metadata running on the system, the serialized version of the bpfilter runtime object.

This change disables the serialization of the bf_map and bf_link objects, and rely on the BPF objects metadata instead, reducing the size of the serialized context, as well as the potential discrepancies between the sources of truth.

This change requires #419

bf_program creates the bf_link object in bf_program_new(), for every new program. If the program is not attached, then program->link->hookopts is NULL, and that's how we know. This approach is not ideal as we might create a bf_link object we don't need. Additionally, it leaves bf_link in a temporary state as long as we don't attach the program to the kernel: the bf_link is in the bf_program object and serialized, but not materialized. This commit only create bf_link when the BPF link object is needed. Calling bf_link_new() will create the BPF link, and fail on error. Doing so, we know that any bf_link object refers to a program attached to the kernel.

bf_program creates the necessary maps objects by default (in bf_program_new()), then update them with the proper size and number of elements. This commit improves this behaviour by following bf_link's example: bf_map are not created until they are required by the program. Calling bf_map_new() will create the BPF map object, which makes the behaviour consistent with bf_link.

Create bf_handle to store references to all the BPF objects defined for a chain, instead of centralizing them in bf_program. The benefit is that bf_cgen can take the ownership of bf_handle and discard the bf_program object that becomes irrelevant once the bytecode is loaded.

Replace bf_program in bf_cgen with bf_handle. The bytecode generation context (bf_program) don't need to be stored anymore, as we can keep a reference to the BPF objects.

Refactor BTF data handling to always create BTF data for BPF maps. The main purpose is to define a decl tag defining the map's bpfilter type.

Use the BPF map FD to restore the handle's maps instead of the serialized context. Most of the information from bf_map can be collected using BPF_OBJ_GET_INFO_BY_FD, the internal map type is stored as a decl tag in the associated BTF data.

Similarly to bf_map, restore bf_handle.link from the link's FD (up to two links). The hook options can't be restored fully from the link's data for now, so we serialize handle->link->hookopts directly from bf_handle for now.

Copilot

Pull request overview

This pull request prevents serialization of bf_map and bf_link objects, instead relying on BPF object metadata from the kernel. This is a significant refactoring that introduces bf_handle to manage BPF object references (prog_fd, maps, link) separately from the bytecode generation context (bf_program).

Changes:

Introduces bf_handle abstraction to hold BPF object references that persist after program generation
Maps and links are now restored from pinned BPF objects using BTF metadata and kernel info queries rather than from serialized data
BTF data is created for all maps (except ring buffers) to enable map type identification during restoration

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/bpfilter/cgen/handle.h	New header defining bf_handle structure and API
src/bpfilter/cgen/handle.c	Implementation of bf_handle with restoration from pinned objects
src/bpfilter/cgen/program.h	Refactored to remove map/link storage, add handle reference
src/bpfilter/cgen/program.c	Maps now created during load; program takes/releases handle
src/bpfilter/cgen/prog/map.h	Removed map lifecycle functions, added bf_map_new_from_fd
src/bpfilter/cgen/prog/map.c	BTF creation for maps, type detection from BTF, restoration from FD
src/bpfilter/cgen/prog/link.h	Link creation now includes attach, added bf_link_new_from_fd
src/bpfilter/cgen/prog/link.c	Link creation merged with attach, restoration from FD
src/bpfilter/cgen/cgen.h	Changed from bf_program to bf_handle reference
src/bpfilter/cgen/cgen.c	Updated to use bf_handle operations
src/bpfilter/xlate/cli.c	Updated to access link/maps through handle
src/libbpfilter/bpf.h	Added bf_bpf_obj_get_info, map/BTF fd_by_id functions
src/libbpfilter/bpf.c	Implemented new BPF syscall wrappers
src/libbpfilter/include/bpfilter/btf.h	Changed BTF type IDs from uint32_t to int
src/libbpfilter/include/bpfilter/bpf_types.h	Added BPF command constants for new operations
tests/e2e/daemon/restore_with_sets.sh	New test for set restoration
tests/e2e/daemon/restore_attached.sh	Enhanced test verification
tests/e2e/matchers/set.sh	Updated to query maps via program instead of pinned paths
tests/e2e/e2e_test_util.sh	Added --skip-cleanup flag for stop_bpfilter
tests/unit/libbpfilter/bpf.c	Updated bf_bpf_btf_load signature with length parameter

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-14T16:31:15Z

src/bpfilter/cgen/prog/map.c

+    if (type != BF_MAP_TYPE_LOG && key_size == 0)
+        return bf_err_r(-EINVAL, "bf_map %s: key size can't be 0", name);
+    if (type == BF_MAP_TYPE_LOG && key_size != 0)
+        return bf_err_r(-EINVAL, "bf_map %s: key size must be", name);


The error message is incomplete. It should read "bf_map %s: key size must be 0" but it's missing "0" at the end.

Suggested change

return bf_err_r(-EINVAL, "bf_map %s: key size must be", name);

return bf_err_r(-EINVAL, "bf_map %s: key size must be 0", name);

Copilot · 2026-02-14T16:31:15Z

src/bpfilter/cgen/handle.c

+    if (handle->lmap) {
+        DUMP(prefix, "lmap: struct bf_map *");
+        bf_dump_prefix_push(prefix);
+        bf_map_dump(handle->lmap, bf_dump_prefix_last(prefix));
+        bf_dump_prefix_pop(prefix);
+    } else {
+        DUMP(prefix, "lmap: struct bf_map * (NULL)");
+    }
+


The lmap is dumped twice (lines 232-239 and 241-248). This is duplicated code and should be removed. Only one of these blocks should remain.

Suggested change

if (handle->lmap) {

DUMP(prefix, "lmap: struct bf_map *");

bf_dump_prefix_push(prefix);

bf_map_dump(handle->lmap, bf_dump_prefix_last(prefix));

bf_dump_prefix_pop(prefix);

} else {

DUMP(prefix, "lmap: struct bf_map * (NULL)");

}

Copilot · 2026-02-14T16:31:15Z

src/bpfilter/cgen/prog/link.c

+    case BPF_LINK_TYPE_TCX:
+        if (info.tcx.attach_type == BF_BPF_TCX_INGRESS)
+            _link->hook = BF_HOOK_TC_INGRESS;
+        else if (info.tcx.attach_type == BF_BPF_TCX_ENGRESS)


Typo in the constant name: "ENGRESS" should be "EGRESS". This appears to be comparing against a constant defined elsewhere in the codebase.

Suggested change

else if (info.tcx.attach_type == BF_BPF_TCX_ENGRESS)

else if (info.tcx.attach_type == BF_BPF_TCX_EGRESS)

Copilot · 2026-02-14T16:31:16Z

src/bpfilter/cgen/prog/link.c

+            struct cgid_file_handle handle = {.handle_bytes = 128};
+            int mount_id;
+
+            if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_PRIORITIES)))


The condition check is incorrect. The code checks for BF_HOOKOPTS_PRIORITIES flag but then tries to access cgpath. It should check for BF_HOOKOPTS_CGPATH flag instead. The error message says "missing cgpath in hookopts" but the flag being checked is PRIORITIES.

Suggested change

if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_PRIORITIES)))

if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_CGPATH)))

qdeslandes added 12 commits February 14, 2026 08:48

cli,lib: fix style

fc16f93

tests: e2e: add test to validate sets are restored on daemon restart

0671059

daemon: cgen: remove bf_program in favor of bf_handle

0a6b768

Replace bf_program in bf_cgen with bf_handle. The bytecode generation context (bf_program) don't need to be stored anymore, as we can keep a reference to the BPF objects.

lib: bpf: require BTF data len on bf_bpf_btf_load()

86884e5

daemon: cgen: tag maps with BTF decl tags for identification

bb2bf93

Refactor BTF data handling to always create BTF data for BPF maps. The main purpose is to define a decl tag defining the map's bpfilter type.

lib: bpf: add bpf syscall for get info, link by id, and map by id

4f0a6c0

tests: e2e: restore --skip-cleanup for stop_bpfilter()

a9498db

Copilot AI review requested due to automatic review settings February 14, 2026 16:23

meta-cla bot added the cla signed label Feb 14, 2026

Copilot started reviewing on behalf of qdeslandes February 14, 2026 16:23 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

qdeslandes marked this pull request as draft February 16, 2026 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent `bf_map` and `bf_link` serialization#420

Prevent `bf_map` and `bf_link` serialization#420
qdeslandes wants to merge 12 commits intofacebook:mainfrom
qdeslandes:restore_handle_from_pinned_prog

qdeslandes commented Feb 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	return bf_err_r(-EINVAL, "bf_map %s: key size must be", name);
	return bf_err_r(-EINVAL, "bf_map %s: key size must be 0", name);

	else if (info.tcx.attach_type == BF_BPF_TCX_ENGRESS)
	else if (info.tcx.attach_type == BF_BPF_TCX_EGRESS)

	if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_PRIORITIES)))
	if (!((*hookopts)->used_opts & BF_FLAG(BF_HOOKOPTS_CGPATH)))

Conversation

qdeslandes commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qdeslandes commented Feb 14, 2026 •

edited

Loading