[prashantpcm94]

Summary

bfcli chain update-set --remove of an IPv6 prefix returns success but doesn't remove it:

$ bfcli chain set --from-str "chain c BF_HOOK_TC_INGRESS ACCEPT set n6 (ip6.snet) in { 2001:db8::/64; 2001:db9::/64 } rule (ip6.snet) in n6 counter DROP"
$ bfcli chain update-set --name c --set-name n6 --remove 2001:db8::/64
info   : updated set 'n6' in chain 'c'
$ bfcli chain get --name c
chain c BF_HOOK_TC_INGRESS ACCEPT
    counters policy 0 packets 0 bytes; error 0 packets 0 bytes
    set n6 (ip6.snet) in {
        2001:db8::/64
        2001:db9::/64
    }
    ...

The prefix keeps matching, and { 2001:db8::/64; 2001:db8::/64 } is stored as 2 elements. Matcher payloads and set elements are parsed into buffers allocated with malloc(), and _bf_parse_ipv6_net() leaves the 4 bytes of padding at the end of struct bf_ip6_lpm_key unset. Sets compare elements byte for byte, so each parsed ip6.snet/ip6.dnet element carries whatever was in that memory, and the element to remove usually doesn't equal the stored one. This allocates matchers and set elements with calloc(), so the bytes a parser doesn't write are zero.

Related issue

None.

Testing

CMake release build (-DNO_DOCS=1) on CentOS Stream 9, kernel 6.16.1, gcc 11.5, clang 22.1.3, libbpf 1.6.3, then make -C $BUILD test_bin and ctest (without fuzzing.parser.infinite, which runs until stopped):

99% tests passed, 1 tests failed out of 128
The following tests FAILED:
	127 - check.iwyu (Not Run)

check.iwyu didn't run because iwyu_tool.py isn't installed here (it only checks cgen/program.c). The check.astgrep tests aren't registered because ast-grep isn't installed either.

No test is added: whether the bug shows depends on what malloc() returns, so a test could pass without the fix.

Checked by hand with release builds, the chain above on BF_HOOK_TC_INGRESS, packets run with bpftool prog run, 3 runs each:

• main: --remove 2001:db8::/64 returns 0, chain get still lists both prefixes, and a packet from 2001:db8::1 is still dropped. The duplicate literal keeps 3 copies.
• with this PR: only 2001:db9::/64 is left, the 2001:db8::1 packet passes, the 2001:db9::1 packet is still dropped, and the duplicate literal keeps 1 copy.

Notes for the reviewer

• Debug builds hide the bug: ASan fills new allocations with 0xbe, so every parsed element gets the same padding and they compare equal.
• Chains created before this fix keep their stored padding (bf_set_pack() writes element bytes as they are), so removing their IPv6 networks keeps missing until the chain is set again. No release is affected: main can't restore chains with sets pinned by any build before 5239340 (v0.7.0 included), so only chains pinned by main builds between 5239340 and this fix are concerned.
• The kernel's LPM trie only compares the first prefixlen bits and the generated program never reads the padding, so this doesn't change how an element matches. It changes which elements userspace sees as equal: what --remove finds, and duplicates.

AI disclosure

Claude Code found the issue, drafted the change, the commit message and this description under my direction, and ran the tests listed under Testing. I reviewed the change and the results.

Checklist

• I understand every line in this PR and can explain why it is correct
• I built the project and ran the tests covering this change
• make -C $BUILD test_bin test passes (check.iwyu and check.astgrep not run, see Testing) and the code follows the style guide
• Commits are formatted as component: subcomponent: short description
• I have disclosed any AI usage above

[prashantpcm94]

@claude review

[github-actions]

Claude review of PR #612 (6c17977)

The fix looks correct for newly parsed elements. Reviewers checked every BF_MATCHER_IN parse callback a set key can use, and this is the only one that leaves bytes uninitialized.

Suggestions

• Existing chains keep the non-zero padding — src/libbpfilter/matcher.c:633 — bf_set_pack() serializes elements raw, padding included, and bf_set_new_from_pack() restores them as-is. On chains created before this fix, --remove still misses the stored element and still reports success. Either note that these chains have to be recreated, or also zero the padding on unpack.

Nits

• Explain why the padding is zeroed — src/libbpfilter/matcher.c:633 — nothing reads _padding, so a later cleanup could remove this as a dead store.
• Use calloc() for set elements — src/libbpfilter/matcher.c:633 — bf_set_add_elem_raw() and _bf_set_expand_range() use malloc(set->elem_size). Using calloc() would also cover future parsers or key structs that have padding. This would come on top of the parser fix, not replace it.

Workflow run

[qdeslandes]

I would calloc the matchers instead, so we apply this across the board.

[prashantpcm94]

Fixed in latest rev

[qdeslandes]

LGTM, thanks!