Restore counter values on chain update

src/libbpfilter/helper.c

@@ -157,3 +157,17 @@ char *bf_trim(char *str)
 
     return bf_rtrim(bf_ltrim(str));
 }
+
+uint64_t bf_fnv1a(const void *data, size_t len, uint64_t hash)
+{
+    assert(data);
+
+    const uint8_t *bytes = data;
+
+    for (size_t i = 0; i < len; ++i) {
+        hash ^= bytes[i];
+        hash *= BF_FNV1A_PRIME;
+    }
+
+    return hash;
+}

src/libbpfilter/include/bpfilter/helper.h

@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <strings.h>
@@ -423,3 +424,21 @@ int bf_read_file(const char *path, void **buf, size_t *len);
  * @return 0 on success, negative errno value on error.
  */
 int bf_write_file(const char *path, const void *buf, size_t len);
+
+// FNV-1a constants from Fowler-Noll-Vo hash specification:
+// https://en.wikipedia.org/wiki/Fowler-Noll-Vo_hash_function
+#define BF_FNV1A_INIT 0xcbf29ce484222325ULL
+#define BF_FNV1A_PRIME 0x100000001b3ULL
+
+/**
+ * @brief Compute a FNV-1a 64-bit hash.
+ *
+ * Pass @ref BF_FNV1A_INIT as @p hash for the initial call. To hash
+ * multiple fields, chain calls by passing the previous return value.
+ *
+ * @param data Data to hash. Can't be NULL.
+ * @param len Number of bytes to hash.
+ * @param hash Initial or chained hash value.
+ * @return Updated hash value.
+ */
+uint64_t bf_fnv1a(const void *data, size_t len, uint64_t hash);

src/libbpfilter/include/bpfilter/set.h

@@ -167,3 +167,18 @@ int bf_set_add_many(struct bf_set *dest, struct bf_set **to_add);
  * - `-EINVAL`: set key format doesn't match between dest and to_remove.
  */
 int bf_set_remove_many(struct bf_set *dest, struct bf_set **to_remove);
+
+/**
+ * @brief Check if two sets have identical content.
+ *
+ * Compares key structure and elements (order-independent). Uses internal
+ * static state for sorting, so this function is not reentrant.
+ *
+ * @todo Use a hash set instead of sorting to make this function reentrant.
+ *
+ * @param lhs First set. Can't be NULL.
+ * @param rhs Second set. Can't be NULL.
+ * @return 1 if the sets are equal, 0 if not, or a negative errno value
+ *         on failure.
+ */
+int bf_set_equal(const struct bf_set *lhs, const struct bf_set *rhs);

src/libbpfilter/set.c

@@ -524,3 +524,80 @@ int bf_set_remove_many(struct bf_set *dest, struct bf_set **to_remove)
 
     return 0;
 }
+
+// qsort comparator for set elements. Uses a static size since qsort
+// doesn't support context and the daemon is single-threaded.
+static size_t _bf_set_elem_cmp_size;
+
+static int _bf_set_elem_cmp(const void *lhs, const void *rhs)
+{
+    return memcmp(lhs, rhs, _bf_set_elem_cmp_size);
+}
+
+static int _bf_set_get_sorted_elems(const struct bf_set *set, void **out)
+{
+    size_t n_elems;
+    void *arr;
+    size_t offset = 0;
+
+    assert(set);
+    assert(out);
+
+    n_elems = bf_list_size(&set->elems);
+
+    if (n_elems == 0) {
+        *out = NULL;
+        return 0;
+    }
+
+    arr = malloc(n_elems * set->elem_size);
+    if (!arr)
+        return -ENOMEM;
+
+    bf_list_foreach (&set->elems, elem_node) {
+        memcpy((uint8_t *)arr + offset, bf_list_node_get_data(elem_node),
+               set->elem_size);
+        offset += set->elem_size;
+    }
+
+    _bf_set_elem_cmp_size = set->elem_size;
+    qsort(arr, n_elems, set->elem_size, _bf_set_elem_cmp);
+
+    *out = arr;
+
+    return 0;
+}
+
+int bf_set_equal(const struct bf_set *lhs, const struct bf_set *rhs)
+{
+    _cleanup_free_ void *lhs_sorted = NULL;
+    _cleanup_free_ void *rhs_sorted = NULL;
+    size_t n_elems;
+    int r;
+
+    assert(lhs);
+    assert(rhs);
+
+    if (lhs->n_comps != rhs->n_comps)
+        return 0;
+    if (memcmp(lhs->key, rhs->key,
+               lhs->n_comps * sizeof(enum bf_matcher_type)) != 0)
+        return 0;
+    if (lhs->elem_size != rhs->elem_size)
+        return 0;
+
+    n_elems = bf_list_size(&lhs->elems);
+    if (bf_list_size(&rhs->elems) != n_elems)
+        return 0;
+    if (n_elems == 0)
+        return 1;
+
+    r = _bf_set_get_sorted_elems(lhs, &lhs_sorted);
+    if (r)
+        return r;
+    r = _bf_set_get_sorted_elems(rhs, &rhs_sorted);
+    if (r)
+        return r;
+
+    return memcmp(lhs_sorted, rhs_sorted, n_elems * lhs->elem_size) == 0;
+}

tests/e2e/CMakeLists.txt

@@ -57,6 +57,7 @@ bf_add_e2e_test(e2e cli/chain_load.sh ROOT)
 bf_add_e2e_test(e2e cli/chain_set.sh ROOT)
 bf_add_e2e_test(e2e cli/chain_update.sh ROOT)
 bf_add_e2e_test(e2e cli/chain_update_set.sh ROOT)
+bf_add_e2e_test(e2e cli/counter_restore.sh ROOT)
 bf_add_e2e_test(e2e cli/hookopts.sh)
 bf_add_e2e_test(e2e cli/nf_inet_dual_stack.sh ROOT)
 bf_add_e2e_test(e2e cli/options_error.sh)

tests/e2e/cli/counter_restore.sh

@@ -0,0 +1,88 @@
+#!/usr/bin/env bash
+
+set -eux
+set -o pipefail
+
+. "$(dirname "$0")"/../e2e_test_util.sh
+
+make_sandbox
+start_bpfilter
+
+get_rule_counter() {
+    local chain_name=$1
+    local rule_idx=$2
+    ${FROM_NS} bfcli chain get --name "${chain_name}" \
+        | grep "counters" \
+        | grep -v "policy\|error" \
+        | sed -n "${rule_idx}p" \
+        | awk '{print $2}'
+}
+
+get_policy_counter() {
+    local chain_name=$1
+    ${FROM_NS} bfcli chain get --name "${chain_name}" \
+        | grep "counters policy" \
+        | awk '{print $3}'
+}
+
+CHAIN="counter_restore"
+
+# Test 1: counter preservation with rule reordering
+
+${FROM_NS} bfcli chain set --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT rule ip4.proto icmp counter DROP"
+(! ping -c 1 -W 0.1 ${NS_IP_ADDR})
+COUNTER=$(get_rule_counter ${CHAIN} 1)
+POLICY=$(get_policy_counter ${CHAIN})
+test "${COUNTER}" -eq 1
+
+${FROM_NS} bfcli chain update --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP ACCEPT rule ip4.saddr eq ${HOST_IP_ADDR} counter ACCEPT rule ip4.proto icmp counter DROP"
+test "$(get_rule_counter ${CHAIN} 1)" -eq 0
+test "$(get_rule_counter ${CHAIN} 2)" -eq "${COUNTER}"
+test "$(get_policy_counter ${CHAIN})" -ge "${POLICY}"
+
+POLICY=$(get_policy_counter ${CHAIN})
+${FROM_NS} bfcli chain update --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP ACCEPT rule ip4.proto icmp counter ACCEPT"
+test "$(get_rule_counter ${CHAIN} 1)" -eq 0
+test "$(get_policy_counter ${CHAIN})" -ge "${POLICY}"
+
+${FROM_NS} bfcli chain flush --name ${CHAIN}
+
+# Test 2: set index reordering
+# Rules reference sets by content, so counters should be preserved
+# despite the set index shuffle.
+
+${FROM_NS} bfcli chain set --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT \
+    set s1 (ip4.saddr) in { 10.0.0.1 } \
+    set s2 (ip4.saddr) in { 10.0.0.2 } \
+    rule (ip4.saddr) in s1 counter ACCEPT \
+    rule (ip4.saddr) in s2 counter DROP"
+
+ping -c 1 -W 0.1 ${NS_IP_ADDR}
+COUNTER_S1=$(get_rule_counter ${CHAIN} 1)
+test "${COUNTER_S1}" -eq 1
+
+${FROM_NS} bfcli chain update --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP ACCEPT \
+    set s2 (ip4.saddr) in { 10.0.0.2 } \
+    set s1 (ip4.saddr) in { 10.0.0.1 } \
+    set s3 (ip4.saddr) in { 10.0.0.3 } \
+    rule (ip4.saddr) in s1 counter ACCEPT \
+    rule (ip4.saddr) in s2 counter DROP"
+test "$(get_rule_counter ${CHAIN} 1)" -eq "${COUNTER_S1}"
+test "$(get_rule_counter ${CHAIN} 2)" -eq 0
+
+COUNTER_S2=$(get_rule_counter ${CHAIN} 2)
+${FROM_NS} bfcli chain update --from-str \
+    "chain ${CHAIN} BF_HOOK_XDP ACCEPT \
+    set s1 (ip4.saddr) in { 10.0.0.99 } \
+    set s2 (ip4.saddr) in { 10.0.0.2 } \
+    rule (ip4.saddr) in s1 counter ACCEPT \
+    rule (ip4.saddr) in s2 counter DROP"
+test "$(get_rule_counter ${CHAIN} 1)" -eq 0
+test "$(get_rule_counter ${CHAIN} 2)" -eq "${COUNTER_S2}"
+
+${FROM_NS} bfcli chain flush --name ${CHAIN}

tests/unit/libbpfilter/helper.c

@@ -356,12 +356,37 @@ static void overwrite_existing_file(void **state)
     assert_memory_equal(read_buf, second_data, strlen(second_data));
 }
 
+static void fnv1a_hash(void **state)
+{
+    uint32_t val_a = 42;
+    uint32_t val_b = 99;
+    uint64_t hash_a;
+    uint64_t hash_b;
+    uint64_t hash_ab;
+    uint64_t hash_ba;
+
+    (void)state;
+
+    hash_a = bf_fnv1a(&val_a, sizeof(val_a), BF_FNV1A_INIT);
+    hash_b = bf_fnv1a(&val_b, sizeof(val_b), BF_FNV1A_INIT);
+
+    assert_int_equal(hash_a,
+                     bf_fnv1a(&val_a, sizeof(val_a), BF_FNV1A_INIT));
+    assert_int_not_equal(hash_a, hash_b);
+
+    // Chaining: order matters for sequential hashing
+    hash_ab = bf_fnv1a(&val_b, sizeof(val_b), hash_a);
+    hash_ba = bf_fnv1a(&val_a, sizeof(val_a), hash_b);
+    assert_int_not_equal(hash_ab, hash_ba);
+}
+
 int main(void)
 {
     const struct CMUnitTest tests[] = {
         cmocka_unit_test(close_fd),
         cmocka_unit_test(string_copy),
         cmocka_unit_test(realloc_mem),
+        cmocka_unit_test(fnv1a_hash),
         cmocka_unit_test(trim_left),
         cmocka_unit_test(trim_right),
         cmocka_unit_test(trim_both),

tests/unit/libbpfilter/set.c

@@ -396,6 +396,71 @@ static void remove_many_mismatched_key_type(void **state)
     assert_non_null(to_remove);
 }
 
+static void equal_with_reordered_elements(void **state)
+{
+    _free_bf_set_ struct bf_set *lhs = NULL;
+    _free_bf_set_ struct bf_set *rhs = NULL;
+    enum bf_matcher_type key[] = {BF_MATCHER_IP4_DADDR, BF_MATCHER_TCP_SPORT};
+    uint8_t elem_a[8] = {10, 0, 0, 1, 0, 80, 0, 0};
+    uint8_t elem_b[8] = {10, 0, 0, 2, 0, 90, 0, 0};
+    uint8_t elem_c[8] = {10, 0, 0, 3, 1, 0, 0, 0};
+
+    (void)state;
+
+    assert_ok(bf_set_new(&lhs, "set_a", key, ARRAY_SIZE(key)));
+    assert_ok(bf_set_add_elem(lhs, elem_a));
+    assert_ok(bf_set_add_elem(lhs, elem_b));
+    assert_ok(bf_set_add_elem(lhs, elem_c));
+
+    assert_ok(bf_set_new(&rhs, "set_b", key, ARRAY_SIZE(key)));
+    assert_ok(bf_set_add_elem(rhs, elem_c));
+    assert_ok(bf_set_add_elem(rhs, elem_a));
+    assert_ok(bf_set_add_elem(rhs, elem_b));
+
+    assert_int_equal(bf_set_equal(lhs, rhs), 1);
+    assert_int_equal(bf_set_equal(lhs, lhs), 1);
+}
+
+static void not_equal(void **state)
+{
+    _free_bf_set_ struct bf_set *set_a = NULL;
+    _free_bf_set_ struct bf_set *set_b = NULL;
+    _free_bf_set_ struct bf_set *set_diff_key = NULL;
+    _free_bf_set_ struct bf_set *set_empty = NULL;
+    _free_bf_set_ struct bf_set *set_empty2 = NULL;
+    enum bf_matcher_type key[] = {BF_MATCHER_IP4_DADDR, BF_MATCHER_TCP_SPORT};
+    enum bf_matcher_type key2[] = {BF_MATCHER_IP4_SADDR, BF_MATCHER_TCP_SPORT};
+    uint8_t elem_a[8] = {10, 0, 0, 1, 0, 80, 0, 0};
+    uint8_t elem_b[8] = {10, 0, 0, 2, 0, 90, 0, 0};
+    uint8_t elem_c[8] = {10, 0, 0, 3, 1, 0, 0, 0};
+
+    (void)state;
+
+    assert_ok(bf_set_new(&set_a, NULL, key, ARRAY_SIZE(key)));
+    assert_ok(bf_set_add_elem(set_a, elem_a));
+    assert_ok(bf_set_add_elem(set_a, elem_b));
+
+    assert_ok(bf_set_new(&set_b, NULL, key, ARRAY_SIZE(key)));
+    assert_ok(bf_set_add_elem(set_b, elem_a));
+    assert_ok(bf_set_add_elem(set_b, elem_c));
+
+    assert_int_equal(bf_set_equal(set_a, set_b), 0);
+
+    assert_ok(bf_set_add_elem(set_b, elem_b));
+    assert_int_equal(bf_set_equal(set_a, set_b), 0);
+
+    assert_ok(bf_set_new(&set_diff_key, NULL, key2, ARRAY_SIZE(key2)));
+    assert_ok(bf_set_add_elem(set_diff_key, elem_a));
+    assert_ok(bf_set_add_elem(set_diff_key, elem_b));
+    assert_int_equal(bf_set_equal(set_a, set_diff_key), 0);
+
+    assert_ok(bf_set_new(&set_empty, NULL, key, ARRAY_SIZE(key)));
+    assert_int_equal(bf_set_equal(set_a, set_empty), 0);
+
+    assert_ok(bf_set_new(&set_empty2, NULL, key, ARRAY_SIZE(key)));
+    assert_int_equal(bf_set_equal(set_empty, set_empty2), 1);
+}
+
 int main(void)
 {
     const struct CMUnitTest tests[] = {
@@ -420,6 +485,8 @@ int main(void)
         cmocka_unit_test(remove_many_disjoint_sets),
         cmocka_unit_test(remove_many_mismatched_key_count),
         cmocka_unit_test(remove_many_mismatched_key_type),
+        cmocka_unit_test(equal_with_reordered_elements),
+        cmocka_unit_test(not_equal),
     };
 
     return cmocka_run_group_tests(tests, NULL, NULL);