Skip to content

lib: add bf_vector and use it in cgen #487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pzmarzly wants to merge 2 commits into
base: main
Choose a base branch
from
Open

lib: add bf_vector and use it in cgen #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
121571f
lib: core: add bf_vector
pzmarzly Feb 13, 2026
dd1fe11
lib: cgen: use bf_vector for img
pzmarzly Feb 13, 2026
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
2 changes: 1 addition & 1 deletion .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ EmptyLineAfterAccessModifier: Never
EmptyLineBeforeAccessModifier: Always
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros: ['bf_list_foreach', 'bf_list_foreach_rev', 'bf_rpack_array_foreach']
ForEachMacros: ['bf_list_foreach', 'bf_list_foreach_rev', 'bf_rpack_array_foreach', 'bf_vector_foreach']
IncludeBlocks: Regroup
IncludeCategories:
# net/if.h needs to be included BEFORE linux/if.h to avoid conflicts
Expand Down
2 changes: 2 additions & 0 deletions src/libbpfilter/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ set(libbpfilter_srcs
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/if.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/io.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/core/list.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/core/vector.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/logger.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/matcher.h
${CMAKE_CURRENT_SOURCE_DIR}/include/bpfilter/pack.h
Expand All @@ -46,6 +47,7 @@ set(libbpfilter_srcs
${CMAKE_CURRENT_SOURCE_DIR}/if.c
${CMAKE_CURRENT_SOURCE_DIR}/io.c
${CMAKE_CURRENT_SOURCE_DIR}/core/list.c
${CMAKE_CURRENT_SOURCE_DIR}/core/vector.c
${CMAKE_CURRENT_SOURCE_DIR}/logger.c
${CMAKE_CURRENT_SOURCE_DIR}/matcher.c
${CMAKE_CURRENT_SOURCE_DIR}/pack.c
Expand Down
14 changes: 9 additions & 5 deletions src/libbpfilter/cgen/dump.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,14 @@ void bf_program_dump_bytecode(const struct bf_program *program)

bf_dump_prefix_push(&prefix);

bf_dbg("Bytecode for program at %p, %lu insn:", program, program->img_size);
bf_dbg("Bytecode for program at %p, %lu insn:", program, program->img.size);

for (size_t i = 0; i < program->img_size; ++i) {
if (i == program->img_size - 1)
for (size_t i = 0; i < program->img.size; ++i) {
struct bpf_insn *insn = bf_vector_get(&program->img, i);
if (!insn)
bf_abort("bytecode dump: invalid instruction index %lu", i);

if (i == program->img.size - 1)
bf_dump_prefix_last(&prefix);

if (double_insn) {
Expand All @@ -116,10 +120,10 @@ void bf_program_dump_bytecode(const struct bf_program *program)
continue;
}

print_bpf_insn(&callbacks, &program->img[i], true);
print_bpf_insn(&callbacks, insn, true);
++bfdd.idx;

double_insn = program->img[i].code == (BPF_LD | BPF_IMM | BPF_DW);
double_insn = insn->code == (BPF_LD | BPF_IMM | BPF_DW);
}

// Force flush, otherwise output on stderr might appear.
Expand Down
10 changes: 8 additions & 2 deletions src/libbpfilter/cgen/jmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@
void bf_jmpctx_cleanup(struct bf_jmpctx *ctx)
{
if (ctx->program) {
struct bpf_insn *insn = &ctx->program->img[ctx->insn_idx];
size_t off = ctx->program->img_size - ctx->insn_idx - 1U;
struct bpf_insn *insn =
bf_vector_get(&ctx->program->img, ctx->insn_idx);
if (!insn) {
bf_abort("jump fixup references invalid instruction index %lu",
ctx->insn_idx);
}

size_t off = ctx->program->img.size - ctx->insn_idx - 1U;
Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable should be defined at the beginning of the scope.


if (off > SHRT_MAX)
bf_warn("jump offset overflow: %ld", off);
Expand Down
2 changes: 1 addition & 1 deletion src/libbpfilter/cgen/jmp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ struct bf_program;
*/
#define bf_jmpctx_get(program, insn) \
({ \
size_t __idx = (program)->img_size; \
size_t __idx = (program)->img.size; \
int __r = bf_program_emit((program), (insn)); \
if (__r < 0) \
return __r; \
Expand Down
102 changes: 42 additions & 60 deletions src/libbpfilter/cgen/program.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@

#define _BF_LOG_BUF_SIZE \
(UINT32_MAX >> 8) /* verifier maximum in kernels <= 5.1 */
#define _BF_PROGRAM_DEFAULT_IMG_SIZE (1 << 6)
#define _BF_LOG_MAP_N_ENTRIES 1000
#define _BF_LOG_MAP_SIZE \
_bf_round_next_power_of_2(sizeof(struct bf_log) * _BF_LOG_MAP_N_ENTRIES)
Expand Down Expand Up @@ -107,6 +106,7 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain,
_program->flavor = bf_hook_to_flavor(chain->hook);
_program->runtime.ops = bf_flavor_ops_get(_program->flavor);
_program->runtime.chain = chain;
_program->img = bf_vector_default(sizeof(struct bpf_insn));
Copy link
Copy Markdown

@github-actions github-actions bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude: suggestion: The old _BF_PROGRAM_DEFAULT_IMG_SIZE was 64 (1 << 6); the new generic _BF_VECTOR_INIT_CAP is 8. Since BPF instruction images routinely contain hundreds of instructions, starting at capacity 8 means ~4 extra reallocations (8 -> 16 -> 32 -> 64 -> 128) per program generation compared to the old code.

Consider adding an initial reserve in bf_program_new after creating the vector:

_program->img = bf_vector_default(sizeof(struct bpf_insn));
bf_vector_reserve(&_program->img, 64);

Alternatively, the caller-side reserve preserves the generic _BF_VECTOR_INIT_CAP for other use cases while optimizing the hot path.

Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with Claude, programs are at least 200 instructions, so we can reserve 512 elements.

_program->fixups = bf_list_default(bf_fixup_free, NULL);
_program->handle = handle;

Expand All @@ -125,7 +125,7 @@ void bf_program_free(struct bf_program **program)
return;

bf_list_clean(&(*program)->fixups);
free((*program)->img);
bf_vector_clean(&(*program)->img);

bf_printer_free(&(*program)->printer);

Expand All @@ -152,9 +152,9 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix)
bf_printer_dump(program->printer, prefix);
bf_dump_prefix_pop(prefix);

DUMP(prefix, "img: %p", program->img);
DUMP(prefix, "img_size: %lu", program->img_size);
DUMP(prefix, "img_cap: %lu", program->img_cap);
DUMP(prefix, "img: %p", program->img.data);
DUMP(prefix, "img.size: %lu", program->img.size);
DUMP(prefix, "img.cap: %lu", program->img.cap);

DUMP(prefix, "fixups: bf_list<struct bf_fixup>[%lu]",
bf_list_size(&program->fixups));
Expand All @@ -177,27 +177,6 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix)
bf_dump_prefix_pop(prefix);
}

int bf_program_grow_img(struct bf_program *program)
{
size_t new_cap = _BF_PROGRAM_DEFAULT_IMG_SIZE;
int r;

assert(program);

if (program->img)
new_cap = _bf_round_next_power_of_2(program->img_cap << 1);

r = bf_realloc((void **)&program->img, new_cap * sizeof(struct bpf_insn));
if (r < 0) {
return bf_err_r(r, "failed to grow program img from %lu to %lu insn",
program->img_cap, new_cap);
}

program->img_cap = new_cap;

return 0;
}

static void _bf_program_fixup_insn(struct bpf_insn *insn,
enum bf_fixup_insn type, int32_t value)
{
Expand Down Expand Up @@ -230,16 +209,23 @@ static int _bf_program_fixup(struct bf_program *program,
int32_t value;
size_t offset;
struct bf_fixup *fixup = bf_list_node_get_data(fixup_node);
struct bpf_insn *insn = &program->img[fixup->insn];
struct bpf_insn *insn;
struct bf_map *map;

if (type != fixup->type)
continue;

insn = bf_vector_get(&program->img, fixup->insn);
if (!insn) {
return bf_err_r(-EINVAL,
"fixup references invalid instruction index %lu",
fixup->insn);
}

switch (type) {
case BF_FIXUP_TYPE_JMP_NEXT_RULE:
insn_type = BF_FIXUP_INSN_OFF;
value = (int)(program->img_size - fixup->insn - 1U);
value = (int)(program->img.size - fixup->insn - 1U);
break;
case BF_FIXUP_TYPE_COUNTERS_MAP_FD:
insn_type = BF_FIXUP_INSN_IMM;
Expand Down Expand Up @@ -413,7 +399,7 @@ static int _bf_program_generate_elfstubs(struct bf_program *program)

bf_list_foreach (&program->fixups, fixup_node) {
struct bf_fixup *fixup = bf_list_node_get_data(fixup_node);
size_t off = program->img_size;
size_t off = program->img.size;

if (fixup->type != BF_FIXUP_ELFSTUB_CALL)
continue;
Expand All @@ -430,7 +416,7 @@ static int _bf_program_generate_elfstubs(struct bf_program *program)
fixup->attr.elfstub_id);
}

start_at = program->img_size;
start_at = program->img.size;

for (size_t i = 0; i < elfstub->ninsns; ++i) {
r = bf_program_emit(program, elfstub->insns[i]);
Expand All @@ -451,8 +437,18 @@ static int _bf_program_generate_elfstubs(struct bf_program *program)
ld_insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
ld_insn[1].imm = (int)bf_printer_msg_offset(msg);

program->img[insn_idx] = ld_insn[0];
program->img[insn_idx + 1] = ld_insn[1];
r = bf_vector_set(&program->img, insn_idx, &ld_insn[0]);
if (r) {
return bf_err_r(
r, "failed to set ELF stub instruction at index %lu",
insn_idx);
}
r = bf_vector_set(&program->img, insn_idx + 1, &ld_insn[1]);
if (r) {
return bf_err_r(
r, "failed to set ELF stub instruction at index %lu",
insn_idx + 1);
}

r = bf_fixup_new(&fixup, BF_FIXUP_TYPE_PRINTER_MAP_FD, insn_idx,
NULL);
Expand All @@ -474,19 +470,9 @@ static int _bf_program_generate_elfstubs(struct bf_program *program)

int bf_program_emit(struct bf_program *program, struct bpf_insn insn)
{
int r;

assert(program);

if (program->img_size == program->img_cap) {
r = bf_program_grow_img(program);
if (r)
return r;
}

program->img[program->img_size++] = insn;

return 0;
return bf_vector_add(&program->img, &insn);
}
Comment on lines 471 to 476
Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static inline in program.h.


int bf_program_emit_kfunc_call(struct bf_program *program, const char *name)
Expand Down Expand Up @@ -517,13 +503,11 @@ int bf_program_emit_fixup(struct bf_program *program, enum bf_fixup_type type,

assert(program);

if (program->img_size == program->img_cap) {
r = bf_program_grow_img(program);
if (r)
return r;
}
r = bf_vector_reserve(&program->img, program->img.size + 1);
if (r)
return r;

r = bf_fixup_new(&fixup, type, program->img_size, attr);
r = bf_fixup_new(&fixup, type, program->img.size, attr);
if (r)
return r;

Expand All @@ -535,8 +519,8 @@ int bf_program_emit_fixup(struct bf_program *program, enum bf_fixup_type type,

/* This call could fail and return an error, in which case it is not
* properly handled. However, this shouldn't be an issue as we previously
* test whether enough room is available in cgen.img, which is currently
* the only reason for EMIT() to fail. */
* reserved enough room in program->img, which is currently the only
* reason for EMIT() to fail. */
EMIT(program, insn);
Comment on lines 506 to 524
Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code is weird in order to ensure the state of bf_program is always valid (i.e. we don't insert a fixup for an instruction that doesn't exist).

We don't need to maintain a valid bf_program state anymore, as it doesn't survive past the generation step. Hence, you can push the fixup to the list, then use bf_program_emit(), and return an error on failure. It will be properly taken care of by bpfilter.


return 0;
Expand All @@ -550,13 +534,11 @@ int bf_program_emit_fixup_elfstub(struct bf_program *program,

assert(program);

if (program->img_size == program->img_cap) {
r = bf_program_grow_img(program);
if (r)
return r;
}
r = bf_vector_reserve(&program->img, program->img.size + 1);
if (r)
return r;

r = bf_fixup_new(&fixup, BF_FIXUP_ELFSTUB_CALL, program->img_size, NULL);
r = bf_fixup_new(&fixup, BF_FIXUP_ELFSTUB_CALL, program->img.size, NULL);
if (r)
return r;

Expand Down Expand Up @@ -808,13 +790,13 @@ int bf_program_load(struct bf_program *prog)

r = bf_bpf_prog_load(prog->handle->prog_name,
bf_hook_to_bpf_prog_type(prog->runtime.chain->hook),
prog->img, prog->img_size,
prog->img.data, prog->img.size,
bf_hook_to_bpf_attach_type(prog->runtime.chain->hook),
log_buf, log_buf ? _BF_LOG_BUF_SIZE : 0,
bf_ctx_token(), &prog->handle->prog_fd);
if (r) {
return bf_err_r(r, "failed to load bf_program (%lu bytes):\n%s\nerrno:",
prog->img_size, log_buf ? log_buf : "<NO LOG BUFFER>");
return bf_err_r(r, "failed to load bf_program (%lu insns):\n%s\nerrno:",
prog->img.size, log_buf ? log_buf : "<NO LOG BUFFER>");
}

return r;
Expand Down
6 changes: 2 additions & 4 deletions src/libbpfilter/cgen/program.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <bpfilter/chain.h>
#include <bpfilter/core/list.h>
#include <bpfilter/core/vector.h>
#include <bpfilter/dump.h>
#include <bpfilter/elfstub.h>
#include <bpfilter/flavor.h>
Expand Down Expand Up @@ -203,9 +204,7 @@ struct bf_program

/* Bytecode */
uint32_t elfstubs_location[_BF_ELFSTUB_MAX];
struct bpf_insn *img;
size_t img_size;
size_t img_cap;
bf_vector img;
bf_list fixups;

/** Runtime data used to interact with the program and cache information.
Expand Down Expand Up @@ -238,7 +237,6 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain,
void bf_program_free(struct bf_program **program);

void bf_program_dump(const struct bf_program *program, prefix_t *prefix);
int bf_program_grow_img(struct bf_program *program);

int bf_program_emit(struct bf_program *program, struct bpf_insn insn);
int bf_program_emit_kfunc_call(struct bf_program *program, const char *name);
Expand Down
Loading
Loading