Restore counter values on chain update

[yaakov-stein]

As mentioned in #389, when bf_cgen_update() replaces a chain, the new BPF program starts with a zeroed counter map. This restores counter values by content-matching old rules to new rules before freeing the old program.

Design

The implementation assumes:

• Chains may have large numbers of rules, sets, and set elements, but rules won't have exceedingly large numbers of matchers
• Update overhead should remain negligible relative to program generation and loading
• Simplicity is preferred over hyper-optimizations where possible

Set mapping. Because set matchers reference sets by position and positions can change between chains, a set index mapping is built first. To efficiently compare large sets, old sets are hashed (FNV-1a with commutative element sum for order-independence), sorted by hash, and binary-searched for each new set. Hash matches are verified deterministically with bf_set_equal(), which copies each set's elements into a flat array, sorts them, and compares with memcmp.

Rule matching. Rules are hashed the same way - FNV-1a of scalar fields plus a commutative sum of per-matcher hashes. A new rules set matchers hash their mapped old index so equivalent rules produce identical hashes regardless of set position (this abstracts away the whole idea of the set so we don't need to worry about it). Old rules are sorted by hash and binary-searched for each new rule, with full order-independent matcher comparison to verify matches. Once again, hash matches are verified deterministically using _bf_rule_equal.

Counter transfer. For each matched pair, the old counter is read and written to the new program's counter map. Policy and error counters are always preserved. This happens after bf_program_load but before bf_link_update, so there is no race.

Performance

Tested with up to 10k rules and 100 sets of 50k elements each. The mapping and transfer overhead was negligible relative to program generation and loading.

Test plan

• Unit tests for bf_fnv1a and bf_set_equal
• E2E test covering rule reordering and set index shuffling
• Manual testing

[pzmarzly]

I know it's a bit late to ask, but what if instead of identifying objects by value (rule contents, set elements, etc.), we gave every element a stable ID (e.g. random 32, 64, or 128 bits)? Then the caller could preserve IDs as its edits the chain, and the counters from old objects would be attributed to new objects if IDs are the same.

With your PR as it is, if you do bfcli chain update-set --add 1.2.3.4 to edit a set, IIUC counters for all rules that reference this set would be reset, since the new set would not be identical to old set, so hashes won't match. But maybe we do want to treat the old set and new sets as logically equivalent, and keep the counters?

That would be a much bigger change though. So another, simpler, idea I had was: what if we compared unnamed sets by-value, but named sets by their name?

@qdeslandes do you have any opinion on this?

[yaakov-stein]

I know it's a bit late to ask, but what if instead of identifying objects by value (rule contents, set elements, etc.), we gave every element a stable ID (e.g. random 32, 64, or 128 bits)? Then the caller could preserve IDs as its edits the chain, and the counters from old objects would be attributed to new objects if IDs are the same.

One of my goals for this was to be transparent to the user and for this to happen automatically behind the scenes - i.e. they shouldn't need to be aware of element id's and need to match them up (however we'd implement it, it seems like it'd add some overhead for the user).

With your PR as it is, if you do bfcli chain update-set --add 1.2.3.4 to edit a set, IIUC counters for all rules that reference this set would be reset, since the new set would not be identical to old set, so hashes won't match. But maybe we do want to treat the old set and new sets as logically equivalent, and keep the counters?

I think that behavior would be lead to confusing results. A counter would report a number that doesn't accurately reflect what has actually been matched by that rule (given that the rules behavior has changed over time).

@qdeslandes do you have any opinion on this?

I think the current approach is cleaner/more correct, but I'm happy to change things up if we'd rather go in that direction.