diff --git a/src/libbpfilter/cgen/fixup.c b/src/libbpfilter/cgen/fixup.c index 47c0af87..715c8b21 100644 --- a/src/libbpfilter/cgen/fixup.c +++ b/src/libbpfilter/cgen/fixup.c @@ -10,6 +10,7 @@ #include #include +#include int bf_fixup_new(struct bf_fixup **fixup, enum bf_fixup_type type, size_t insn_offset, const union bf_fixup_attr *attr) @@ -74,7 +75,8 @@ void bf_fixup_dump(const struct bf_fixup *fixup, prefix_t *prefix) // No specific value to dump break; case BF_FIXUP_TYPE_SET_MAP_FD: - DUMP(prefix, "set_index: %lu", fixup->attr.set_index); + DUMP(prefix, "set: %s", + fixup->attr.set_ptr ? fixup->attr.set_ptr->name : "(null)"); break; default: DUMP(prefix, "unsupported bf_fixup_type: %d", fixup->type); diff --git a/src/libbpfilter/cgen/fixup.h b/src/libbpfilter/cgen/fixup.h index 8b5f832b..2cda6346 100644 --- a/src/libbpfilter/cgen/fixup.h +++ b/src/libbpfilter/cgen/fixup.h @@ -10,6 +10,8 @@ #include #include +struct bf_set; + /** * Field to fixup in a @c bpf_insn structure. */ @@ -44,7 +46,9 @@ enum bf_fixup_type union bf_fixup_attr { - size_t set_index; + /** Set referenced by a `BF_FIXUP_TYPE_SET_MAP_FD` fixup. The fixup + * holds a non-owning pointer to a set in the generator's chain. */ + const struct bf_set *set_ptr; enum bf_elfstub_id elfstub_id; }; diff --git a/src/libbpfilter/cgen/handle.h b/src/libbpfilter/cgen/handle.h index a65a26b9..5bddeb5a 100644 --- a/src/libbpfilter/cgen/handle.h +++ b/src/libbpfilter/cgen/handle.h @@ -51,8 +51,8 @@ struct bf_handle /** Log map. NULL if not created. */ struct bf_map *lmap; - /** List of set maps. If a set is empty in the chain, NULL is inserted in - * this list instead of a `bf_map` to preserve sets indexes. */ + /** List of set maps. Contains at most one map for each unique key + * format. */ bf_list sets; }; diff --git a/src/libbpfilter/cgen/matcher/set.c b/src/libbpfilter/cgen/matcher/set.c index 2dde26f2..5a0593a9 100644 --- a/src/libbpfilter/cgen/matcher/set.c +++ b/src/libbpfilter/cgen/matcher/set.c @@ -5,6 +5,9 @@ #include "cgen/matcher/set.h" +#include +#include + #include #include #include @@ -12,24 +15,60 @@ #include "cgen/program.h" #include "cgen/stub.h" +/** + * @brief Emit bitmask check instructions after a map lookup. + * + * Set map values are bitmasks, sized to fit one bit per set in the + * group. After map_lookup_elem returns a non-NULL pointer in BPF_REG_0, + * this function emits instructions to: + * 1. Load the byte containing this set's bit from the pointer. + * 2. AND it with the set's bit. + * 3. Jump to the next rule if the bit is not set. + */ +static int _bf_emit_bitmask_check(struct bf_program *program, size_t bit_index) +{ + assert(program); + + /* r0 points to the map value. Load the byte containing our bit. */ + EMIT(program, BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_0, + (int16_t)(bit_index / CHAR_BIT))); + + /* AND with the set's bit within that byte. */ + EMIT(program, + BPF_ALU32_IMM(BPF_AND, BPF_REG_0, 1U << (bit_index % CHAR_BIT))); + + /* Jump to next rule if the result is 0 (bit not set). */ + EMIT_FIXUP_JMP_NEXT_RULE(program, BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 0)); + + return 0; +} + static int _bf_matcher_generate_set_trie(struct bf_program *program, const struct bf_matcher *matcher) { assert(program); assert(matcher); + uint32_t set_index = *(uint32_t *)bf_matcher_payload(matcher); const struct bf_set *set = bf_chain_get_set_for_matcher(program->runtime.chain, matcher); - enum bf_matcher_type type = set->key[0]; - const struct bf_matcher_meta *meta = bf_matcher_get_meta(type); + enum bf_matcher_type type; + const struct bf_matcher_meta *meta; + size_t bit_index; int r; if (!set) { - return bf_err_r(-ENOENT, "set #%u not found in %s", - *(uint32_t *)bf_matcher_payload(matcher), + return bf_err_r(-ENOENT, "set #%u not found in %s", set_index, program->runtime.chain->name); } + type = set->key[0]; + meta = bf_matcher_get_meta(type); + + r = bf_program_set_bit_index(program, set, &bit_index); + if (r) + return bf_err_r(r, "set '%s' not assigned to any group", set->name); + r = bf_stub_rule_check_protocol(program, meta); if (r) return bf_err_r(r, "failed to check for protocol"); @@ -67,15 +106,15 @@ static int _bf_matcher_generate_set_trie(struct bf_program *program, bf_matcher_type_to_str(type), type); } - EMIT_LOAD_SET_FD_FIXUP(program, BPF_REG_1, - *(uint32_t *)bf_matcher_payload(matcher)); + EMIT_LOAD_SET_FD_FIXUP(program, BPF_REG_1, set); EMIT(program, BPF_MOV64_REG(BPF_REG_2, BPF_REG_10)); EMIT(program, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, BF_PROG_SCR_OFF(4))); EMIT(program, BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem)); - // Jump to the next rule if map_lookup_elem returned 0 + // Jump to the next rule if map_lookup_elem returned NULL EMIT_FIXUP_JMP_NEXT_RULE(program, BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 0)); - return 0; + + return _bf_emit_bitmask_check(program, bit_index); } int bf_matcher_generate_set(struct bf_program *program, @@ -86,18 +125,23 @@ int bf_matcher_generate_set(struct bf_program *program, const struct bf_set *set = bf_chain_get_set_for_matcher(program->runtime.chain, matcher); + uint32_t set_index = *(uint32_t *)bf_matcher_payload(matcher); + size_t bit_index; size_t offset = 0; int r; if (!set) { - return bf_err_r(-ENOENT, "set #%u not found in %s", - *(uint32_t *)bf_matcher_payload(matcher), + return bf_err_r(-ENOENT, "set #%u not found in %s", set_index, program->runtime.chain->name); } if (set->use_trie) return _bf_matcher_generate_set_trie(program, matcher); + r = bf_program_set_bit_index(program, set, &bit_index); + if (r) + return bf_err_r(r, "set '%s' not assigned to any group", set->name); + // Ensure the packet uses the required protocols for (size_t i = 0; i < set->n_comps; ++i) { enum bf_matcher_type type = set->key[i]; @@ -136,14 +180,13 @@ int bf_matcher_generate_set(struct bf_program *program, offset += meta->hdr_payload_size; } - EMIT_LOAD_SET_FD_FIXUP(program, BPF_REG_1, - *(uint32_t *)bf_matcher_payload(matcher)); + EMIT_LOAD_SET_FD_FIXUP(program, BPF_REG_1, set); EMIT(program, BPF_MOV64_REG(BPF_REG_2, BPF_REG_10)); EMIT(program, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, BF_PROG_SCR_OFF(0))); EMIT(program, BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem)); - // Jump to the next rule if map_lookup_elem returned 0 + // Jump to the next rule if map_lookup_elem returned NULL EMIT_FIXUP_JMP_NEXT_RULE(program, BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 0)); - return 0; + return _bf_emit_bitmask_check(program, bit_index); } diff --git a/src/libbpfilter/cgen/prog/map.c b/src/libbpfilter/cgen/prog/map.c index 42e9d238..f1588ec0 100644 --- a/src/libbpfilter/cgen/prog/map.c +++ b/src/libbpfilter/cgen/prog/map.c @@ -211,7 +211,8 @@ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, } int bf_map_new_from_set(struct bf_map **map, const char *name, - const struct bf_set *set) + const struct bf_set *set, size_t set_size, + size_t value_size) { assert(map); assert(name); @@ -220,7 +221,7 @@ int bf_map_new_from_set(struct bf_map **map, const char *name, return _bf_map_new(map, name, BF_MAP_TYPE_SET, set->use_trie ? BF_BPF_MAP_TYPE_LPM_TRIE : BF_BPF_MAP_TYPE_HASH, - set->elem_size, 1, bf_list_size(&set->elems)); + set->elem_size, value_size, set_size); } int bf_map_new_from_pack(struct bf_map **map, int dir_fd, bf_rpack_node_t node) diff --git a/src/libbpfilter/cgen/prog/map.h b/src/libbpfilter/cgen/prog/map.h index de256123..0debf931 100644 --- a/src/libbpfilter/cgen/prog/map.h +++ b/src/libbpfilter/cgen/prog/map.h @@ -71,10 +71,13 @@ int bf_map_new(struct bf_map **map, const char *name, enum bf_map_type type, * also as filename when pinning the map to the system. Can't be NULL or * empty. * @param set Set to create the map from. Can't be NULL. + * @param set_size Number of elements to reserve room for in the map. + * @param value_size Size (in bytes) of the value field. * @return 0 on success, or a negative error value on error. */ int bf_map_new_from_set(struct bf_map **map, const char *name, - const struct bf_set *set); + const struct bf_set *set, size_t set_size, + size_t value_size); /** * @brief Allocate and initialize a new map from serialized data. diff --git a/src/libbpfilter/cgen/program.c b/src/libbpfilter/cgen/program.c index 20b399ae..6af72365 100644 --- a/src/libbpfilter/cgen/program.c +++ b/src/libbpfilter/cgen/program.c @@ -80,6 +80,149 @@ static inline size_t _bf_round_next_power_of_2(size_t value) return ++value; } +/** + * @brief Sets sharing the same key format, collapsed to a single BPF map. + * + * One `bf_set_group` exists for every unique key format among a chain's + * non-empty sets. The sets list preserves insertion order; a set's position + * is its bit index within the group's bitmask value. + */ +struct bf_set_group +{ + /** Non-empty sets sharing the group's key format. Non-owning pointers + * into the chain's `bf_set` list. Never empty. */ + bf_list sets; + + /** Backing BPF map. Populated during `bf_program_load()`; the map is + * owned by `bf_program->handle->sets`. */ + struct bf_map *map; +}; + +#define _free_bf_set_group_ __attribute__((__cleanup__(_bf_set_group_free))) + +static void _bf_set_group_free(struct bf_set_group **group) +{ + if (!*group) + return; + + bf_list_clean(&(*group)->sets); + freep((void *)group); +} + +static int _bf_set_group_new(struct bf_set_group **group) +{ + _free_bf_set_group_ struct bf_set_group *_group = NULL; + + assert(group); + + _group = calloc(1, sizeof(*_group)); + if (!_group) + return -ENOMEM; + + /* The list holds non-owning const struct bf_set * pointers; no free + * callback. Groups are temporary (not serialized), so no pack callback + * either. */ + _group->sets = bf_list_default(NULL, NULL); + + *group = TAKE_PTR(_group); + + return 0; +} + +static struct bf_set_group * +_bf_program_find_set_group(const struct bf_program *program, + const struct bf_set *set) +{ + assert(program); + + if (!set) + return NULL; + + bf_list_foreach (&program->set_groups, group_node) { + struct bf_set_group *group = bf_list_node_get_data(group_node); + bf_list_foreach (&group->sets, set_node) { + if (bf_list_node_get_data(set_node) == set) + return group; + } + } + + return NULL; +} + +static int _bf_program_build_set_groups(struct bf_program *program) +{ + assert(program); + + bf_list_clean(&program->set_groups); + program->set_groups = + bf_list_default((bf_list_ops_free)_bf_set_group_free, NULL); + + bf_list_foreach (&program->runtime.chain->sets, set_node) { + struct bf_set *set = bf_list_node_get_data(set_node); + struct bf_set_group *match = NULL; + int r; + + if (bf_list_is_empty(&set->elems)) + continue; + + bf_list_foreach (&program->set_groups, group_node) { + struct bf_set_group *group = bf_list_node_get_data(group_node); + const struct bf_set *head = + bf_list_node_get_data(bf_list_get_head(&group->sets)); + + if (bf_set_same_key(set, head)) { + match = group; + break; + } + } + + if (match) { + r = bf_list_add_tail(&match->sets, set); + if (r) + return bf_err_r(r, "failed to add set to existing group"); + } else { + _free_bf_set_group_ struct bf_set_group *new_group = NULL; + + r = _bf_set_group_new(&new_group); + if (r) + return bf_err_r(r, "failed to allocate set group"); + + r = bf_list_add_tail(&new_group->sets, set); + if (r) + return bf_err_r(r, "failed to seed set group"); + + r = bf_list_push(&program->set_groups, (void **)&new_group); + if (r) + return bf_err_r(r, "failed to register set group"); + } + } + + return 0; +} + +int bf_program_set_bit_index(const struct bf_program *program, + const struct bf_set *set, size_t *bit_index) +{ + assert(program); + assert(set); + assert(bit_index); + + bf_list_foreach (&program->set_groups, group_node) { + struct bf_set_group *group = bf_list_node_get_data(group_node); + size_t i = 0; + + bf_list_foreach (&group->sets, set_node) { + if (bf_list_node_get_data(set_node) == set) { + *bit_index = i; + return 0; + } + ++i; + } + } + + return -ENOENT; +} + static const struct bf_flavor_ops *bf_flavor_ops_get(enum bf_flavor flavor) { static const struct bf_flavor_ops *flavor_ops[] = { @@ -114,6 +257,8 @@ int bf_program_new(struct bf_program **program, const struct bf_chain *chain, _program->runtime.chain = chain; _program->img = bf_vector_default(sizeof(struct bpf_insn)); _program->fixups = bf_list_default(bf_fixup_free, NULL); + _program->set_groups = + bf_list_default((bf_list_ops_free)_bf_set_group_free, NULL); _program->handle = handle; r = bf_vector_reserve(&_program->img, 512); @@ -135,6 +280,7 @@ void bf_program_free(struct bf_program **program) return; bf_list_clean(&(*program)->fixups); + bf_list_clean(&(*program)->set_groups); bf_vector_clean(&(*program)->img); bf_printer_free(&(*program)->printer); @@ -179,6 +325,32 @@ void bf_program_dump(const struct bf_program *program, prefix_t *prefix) } bf_dump_prefix_pop(prefix); + DUMP(prefix, "groups: bf_list[%lu]", + bf_list_size(&program->set_groups)); + bf_dump_prefix_push(prefix); + bf_list_foreach (&program->set_groups, group_node) { + const struct bf_set_group *group = bf_list_node_get_data(group_node); + size_t i = 0; + + if (bf_list_is_tail(&program->set_groups, group_node)) + bf_dump_prefix_last(prefix); + + DUMP(prefix, "struct bf_set_group at %p (%lu set(s), map=%p)", group, + bf_list_size(&group->sets), (void *)group->map); + bf_dump_prefix_push(prefix); + bf_list_foreach (&group->sets, set_node) { + const struct bf_set *set = bf_list_node_get_data(set_node); + + if (bf_list_is_tail(&group->sets, set_node)) + bf_dump_prefix_last(prefix); + DUMP(prefix, "bit %lu: bf_set '%s' (%lu element(s))", i, set->name, + bf_list_size(&set->elems)); + ++i; + } + bf_dump_prefix_pop(prefix); + } + bf_dump_prefix_pop(prefix); + DUMP(bf_dump_prefix_last(prefix), "runtime: "); bf_dump_prefix_push(prefix); DUMP(bf_dump_prefix_last(prefix), "ops: %p", program->runtime.ops); @@ -220,7 +392,6 @@ static int _bf_program_fixup(struct bf_program *program, size_t offset; struct bf_fixup *fixup = bf_list_node_get_data(fixup_node); struct bpf_insn *insn; - struct bf_map *map; if (type != fixup->type) continue; @@ -249,15 +420,18 @@ static int _bf_program_fixup(struct bf_program *program, insn_type = BF_FIXUP_INSN_IMM; value = program->handle->lmap->fd; break; - case BF_FIXUP_TYPE_SET_MAP_FD: - map = bf_list_get_at(&program->handle->sets, fixup->attr.set_index); - if (!map) { - return bf_err_r(-ENOENT, "can't find set map at index %lu", - fixup->attr.set_index); + case BF_FIXUP_TYPE_SET_MAP_FD: { + const struct bf_set_group *group = + _bf_program_find_set_group(program, fixup->attr.set_ptr); + if (!group || !group->map) { + return bf_err_r( + -ENOENT, "set map fixup: set '%s' not in any loaded group", + fixup->attr.set_ptr ? fixup->attr.set_ptr->name : "(null)"); } insn_type = BF_FIXUP_INSN_IMM; - value = map->fd; + value = group->map->fd; break; + } case BF_FIXUP_ELFSTUB_CALL: insn_type = BF_FIXUP_INSN_IMM; offset = program->elfstubs_location[fixup->attr.elfstub_id] - @@ -547,6 +721,10 @@ int bf_program_generate(struct bf_program *program) int ret_code; int r; + r = _bf_program_build_set_groups(program); + if (r) + return bf_err_r(r, "failed to build set groups"); + // Save the program's argument into the context. EMIT(program, BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, BF_PROG_CTX_OFF(arg))); @@ -680,65 +858,79 @@ static int _bf_program_load_log_map(struct bf_program *program) return 0; } +/** + * @brief Load set maps, one BPF map per `bf_set_group`. + * + * Sets that share the same key format have already been grouped by + * `_bf_program_build_set_groups()`. Each group collapses to a single BPF map + * whose value is a bitmask: bit `i` of byte `i / CHAR_BIT` identifies the + * `i`-th set in the group. + * + * Group ownership of the created maps is transferred to `handle->sets`; + * the `bf_set_group::map` pointer is a non-owning back-reference used by + * `_bf_program_fixup()` when resolving `BF_FIXUP_TYPE_SET_MAP_FD` fixups. + */ static int _bf_program_load_sets_maps(struct bf_program *new_prog) { char name[BPF_OBJ_NAME_LEN]; - size_t set_idx = 0; + size_t map_idx = 0; int r; assert(new_prog); - bf_list_foreach (&new_prog->runtime.chain->sets, set_node) { - struct bf_set *set = bf_list_node_get_data(set_node); - _free_bf_map_ struct bf_map *map = NULL; - _cleanup_free_ uint8_t *values = NULL; - _cleanup_free_ uint8_t *keys = NULL; - size_t nelems = bf_list_size(&set->elems); - size_t idx = 0; - - if (!nelems) { - r = bf_list_add_tail(&new_prog->handle->sets, NULL); - if (r) - return r; - continue; + bf_list_foreach (&new_prog->set_groups, group_node) { + struct bf_set_group *group = bf_list_node_get_data(group_node); + const struct bf_set *key_set = + bf_list_node_get_data(bf_list_get_head(&group->sets)); + size_t n_sets = bf_list_size(&group->sets); + size_t value_size = (n_sets + CHAR_BIT - 1) / CHAR_BIT; + size_t total_elems = 0; + size_t bit_idx = 0; + _free_bf_map_ struct bf_map *new_map = NULL; + _cleanup_free_ uint8_t *value = NULL; + + bf_list_foreach (&group->sets, set_node) { + const struct bf_set *set = bf_list_node_get_data(set_node); + total_elems += bf_list_size(&set->elems); } (void)snprintf(name, BPF_OBJ_NAME_LEN, _BF_SET_MAP_PREFIX "%04x", - (uint8_t)set_idx++); - r = bf_map_new_from_set(&map, name, set); + (uint16_t)map_idx++); + + r = bf_map_new_from_set(&new_map, name, key_set, total_elems, + value_size); if (r) return r; - values = malloc(nelems); - if (!values) - return bf_err_r(-errno, "failed to allocate map values"); + group->map = new_map; - keys = malloc(set->elem_size * nelems); - if (!keys) - return bf_err_r(errno, "failed to allocate map keys"); + value = malloc(value_size); + if (!value) + return -ENOMEM; - bf_list_foreach (&set->elems, elem_node) { - void *elem = bf_list_node_get_data(elem_node); + bf_list_foreach (&group->sets, set_node) { + const struct bf_set *set = bf_list_node_get_data(set_node); - memcpy(keys + (idx * set->elem_size), elem, set->elem_size); - values[idx] = 1; - ++idx; - } + bf_list_foreach (&set->elems, elem_node) { + void *elem = bf_list_node_get_data(elem_node); - r = bf_bpf_map_update_batch(map->fd, keys, values, nelems, BPF_ANY); - if (r) - return bf_err_r(r, "failed to add set elements to the map"); + memset(value, 0, value_size); + (void)bf_bpf_map_lookup_elem(new_map->fd, elem, value); + value[bit_idx / CHAR_BIT] |= + (uint8_t)(1U << (bit_idx % CHAR_BIT)); + r = bf_map_set_elem(new_map, elem, value); + if (r) + return bf_err_r(r, "failed to add set element to the map"); + } + ++bit_idx; + } - r = bf_list_push(&new_prog->handle->sets, (void **)&map); + r = bf_list_push(&new_prog->handle->sets, (void **)&new_map); if (r) return r; - }; - - r = _bf_program_fixup(new_prog, BF_FIXUP_TYPE_SET_MAP_FD); - if (r) - return r; + } - return 0; + return _bf_program_fixup(new_prog, BF_FIXUP_TYPE_SET_MAP_FD); } int bf_program_load(struct bf_program *prog) diff --git a/src/libbpfilter/cgen/program.h b/src/libbpfilter/cgen/program.h index 9f068e74..b25fea95 100644 --- a/src/libbpfilter/cgen/program.h +++ b/src/libbpfilter/cgen/program.h @@ -168,13 +168,14 @@ * * @param program Program to generate the bytecode for. Can't be NULL. * @param reg Register to store the set file descriptor in. - * @param index Index of the set in the program. + * @param set Non-owning pointer to the referenced `bf_set`. The set must + * live at least until `bf_program_load()` returns. */ -#define EMIT_LOAD_SET_FD_FIXUP(program, reg, index) \ +#define EMIT_LOAD_SET_FD_FIXUP(program, reg, set) \ ({ \ union bf_fixup_attr __attr; \ memset(&__attr, 0, sizeof(__attr)); \ - __attr.set_index = (index); \ + __attr.set_ptr = (set); \ const struct bpf_insn ld_insn[2] = {BPF_LD_MAP_FD(reg, 0)}; \ int __r = bf_program_emit_fixup((program), BF_FIXUP_TYPE_SET_MAP_FD, \ ld_insn[0], &__attr); \ @@ -189,6 +190,8 @@ struct bf_chain; struct bf_counter; struct bf_hookopts; struct bf_handle; +struct bf_set; +struct bf_set_group; struct bf_program { @@ -207,6 +210,11 @@ struct bf_program bf_vector img; bf_list fixups; + /** Non-empty sets from `runtime.chain->sets`, grouped by key format. + * Each group shares a single BPF map using bitmask values; a set's + * position within a group is its bit index. Not serialized. */ + bf_list set_groups; + /** Runtime data used to interact with the program and cache information. * This data is not serialized. */ struct @@ -254,6 +262,24 @@ int bf_program_emit_fixup_elfstub(struct bf_program *program, enum bf_elfstub_id id); int bf_program_generate(struct bf_program *program); +/** + * @brief Get the bit position of a set within its group. + * + * Sets sharing the same key format are grouped together and share a single + * BPF map. Each set's elements are stored in the map with a bitmask value + * where one bit identifies which set owns the element. This function returns + * that bit's position for `set` within its group. + * + * @param program Program whose groups have been built. Can't be NULL. + * @param set Set to locate in the program's groups. Can't be NULL. + * @param bit_index Output: position of `set` within its group's bit layout. + * Can't be NULL. + * @return 0 on success, `-ENOENT` if `set` is not part of any group (for + * example, because it is empty). + */ +int bf_program_set_bit_index(const struct bf_program *program, + const struct bf_set *set, size_t *bit_index); + /** * Load the BPF program into the kernel. * diff --git a/src/libbpfilter/include/bpfilter/set.h b/src/libbpfilter/include/bpfilter/set.h index d0d35d0b..df4c222f 100644 --- a/src/libbpfilter/include/bpfilter/set.h +++ b/src/libbpfilter/include/bpfilter/set.h @@ -126,6 +126,15 @@ void bf_set_dump(const struct bf_set *set, prefix_t *prefix); */ bool bf_set_is_empty(const struct bf_set *set); +/** + * @brief Check whether two sets have the same key format. + * + * @param lhs First set. Can't be NULL. + * @param rhs Second set. Can't be NULL. + * @return true if both sets have the same key components, false otherwise. + */ +bool bf_set_same_key(const struct bf_set *lhs, const struct bf_set *rhs); + int bf_set_add_elem(struct bf_set *set, const void *elem); /** diff --git a/src/libbpfilter/set.c b/src/libbpfilter/set.c index 212c4aa9..b642ca41 100644 --- a/src/libbpfilter/set.c +++ b/src/libbpfilter/set.c @@ -421,30 +421,16 @@ bool bf_set_is_empty(const struct bf_set *set) return bf_list_is_empty(&set->elems); } -/** - * @brief Check if two sets have the same key format. - * - * @param first First set. Can't be NULL. - * @param second Second set. Can't be NULL. - * @return 0 if sets have matching format, or -EINVAL on mismatch. - */ -static int _bf_set_cmp_key_format(const struct bf_set *first, - const struct bf_set *second) +bool bf_set_same_key(const struct bf_set *lhs, const struct bf_set *rhs) { - assert(first); - assert(second); + assert(lhs); + assert(rhs); - if (first->n_comps != second->n_comps) - return bf_err_r( - -EINVAL, - "set key format mismatch: first set has %lu components, second has %lu", - first->n_comps, second->n_comps); - - if (memcmp(first->key, second->key, - first->n_comps * sizeof(enum bf_matcher_type)) != 0) - return bf_err_r(-EINVAL, "set key component type mismatch"); + if (lhs->n_comps != rhs->n_comps) + return false; - return 0; + return memcmp(lhs->key, rhs->key, + lhs->n_comps * sizeof(enum bf_matcher_type)) == 0; } int bf_set_add_many(struct bf_set *dest, struct bf_set **to_add) @@ -455,9 +441,8 @@ int bf_set_add_many(struct bf_set *dest, struct bf_set **to_add) assert(to_add); assert(*to_add); - r = _bf_set_cmp_key_format(dest, *to_add); - if (r) - return r; + if (!bf_set_same_key(dest, *to_add)) + return bf_err_r(-EINVAL, "set key format mismatch"); // @todo This has O(n * m) complexity. We could get to O(n log n + m) by // turning the linked list into an array and sorting it, but we should @@ -492,15 +477,12 @@ int bf_set_add_many(struct bf_set *dest, struct bf_set **to_add) int bf_set_remove_many(struct bf_set *dest, struct bf_set **to_remove) { - int r; - assert(dest); assert(to_remove); assert(*to_remove); - r = _bf_set_cmp_key_format(dest, *to_remove); - if (r) - return r; + if (!bf_set_same_key(dest, *to_remove)) + return bf_err_r(-EINVAL, "set key format mismatch"); // @todo This has O(n * m) complexity. Could be O(m) if we used hashsets. bf_list_foreach (&(*to_remove)->elems, elem_node) { diff --git a/tests/e2e/CMakeLists.txt b/tests/e2e/CMakeLists.txt index c6bf4f26..a8ffdab5 100644 --- a/tests/e2e/CMakeLists.txt +++ b/tests/e2e/CMakeLists.txt @@ -126,6 +126,7 @@ bf_add_e2e_shell_test(e2e parsing/meta_probability.sh) bf_add_e2e_shell_test(e2e parsing/meta_sport.sh) bf_add_e2e_shell_test(e2e parsing/set.sh ROOT) bf_add_e2e_shell_test(e2e parsing/set_empty_index.sh ROOT) +bf_add_e2e_shell_test(e2e parsing/set_group.sh ROOT) bf_add_e2e_shell_test(e2e parsing/named_set.sh) bf_add_e2e_shell_test(e2e parsing/tcp_flags.sh) bf_add_e2e_shell_test(e2e parsing/tcp_dport.sh) diff --git a/tests/e2e/parsing/set_group.sh b/tests/e2e/parsing/set_group.sh new file mode 100755 index 00000000..2cc7e5b4 --- /dev/null +++ b/tests/e2e/parsing/set_group.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +. "$(dirname "$0")"/../e2e_test_util.sh + +make_sandbox + +# Regression test for same-key set map grouping with bitmask values. +# Sets sharing a key format should be grouped into a single BPF map. + +# Two non-empty same-key sets collapse to a single map. +${FROM_NS} ${BFCLI} chain set --from-str "chain merged BF_HOOK_XDP ACCEPT + set a (ip4.saddr) in { 192.0.2.1 } + set b (ip4.saddr) in { 192.0.2.2 } + rule (ip4.saddr) in a counter DROP + rule (ip4.saddr) in b counter DROP" +count=$(${FROM_NS} find ${WORKDIR}/bpf/bpfilter/merged/ -name 'bf_set_*' | wc -l) +[ "${count}" -eq 1 ] || { echo "ERROR: expected 1 map for merged group, got ${count}"; exit 1; } + +# Distinct key formats remain in separate maps. +${FROM_NS} ${BFCLI} chain set --from-str "chain split BF_HOOK_XDP ACCEPT + set a (ip4.saddr) in { 192.0.2.1 } + set b (ip4.daddr) in { 192.0.2.2 } + rule (ip4.saddr) in a counter DROP + rule (ip4.daddr) in b counter DROP" +count=$(${FROM_NS} find ${WORKDIR}/bpf/bpfilter/split/ -name 'bf_set_*' | wc -l) +[ "${count}" -eq 2 ] || { echo "ERROR: expected 2 maps for split groups, got ${count}"; exit 1; } + +# Isolation test: the host's address lives in set b only. +# Chain references set a only. We must not match on set b elements. +${FROM_NS} ${BFCLI} chain set --from-str "chain isolation BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT + set a (ip4.saddr) in { 192.0.2.40 } + set b (ip4.saddr) in { ${HOST_IP_ADDR} } + rule (ip4.saddr) in a counter DROP" +ping -c 1 -W 1 ${NS_IP_ADDR} +${FROM_NS} ${BFCLI} chain flush --name isolation