cgen: add an error counter

For kfunc call errors, add a dedicated counter that can be read by the
daemon in the future. For now, this counter must be read via `bpftool`
and is not available to other clients.

Fixes #105

src/bpfilter/cgen/cgen.c

@@ -185,15 +185,23 @@ void bf_cgen_dump(const struct bf_cgen *cgen, prefix_t *prefix)
     bf_dump_prefix_pop(prefix);
 }
 
-int bf_cgen_get_counter(const struct bf_cgen *cgen, uint32_t counter_idx,
+int bf_cgen_get_counter(const struct bf_cgen *cgen,
+                        enum bf_counter_type counter_idx,
                         struct bf_counter *counter)
 {
     bf_assert(cgen && counter);
 
-    /* There are 1 more counter than number of rules. The last counter is
-     * dedicated to the policy. */
-    if (counter_idx > bf_list_size(&cgen->chain->rules))
-        return -EINVAL;
+    /* There are two more counter than rules. The special counters must
+     * be accessed via the specific values, to avoid confusion. */
+    enum bf_counter_type rule_count = bf_list_size(&cgen->chain->rules);
+    if (counter_idx == BF_COUNTER_POLICY) {
+      counter_idx = rule_count;
+    } else if (counter_idx == BF_COUNTER_ERRORS) {
+      counter_idx = rule_count+1;
+    } else if (counter_idx < 0 ||
+               counter_idx >= rule_count) {
+      return -EINVAL;
+    }
 
     return bf_program_get_counter(cgen->program, counter_idx, counter);
 }

src/bpfilter/cgen/cgen.h

@@ -127,12 +127,22 @@ int bf_cgen_unload(struct bf_cgen *cgen);
 
 void bf_cgen_dump(const struct bf_cgen *cgen, prefix_t *prefix);
 
+/**
+ * @enum bf_counter_type
+ *
+ * Special counter types for @ref bf_cgen_get_counter.
+ */
+enum bf_counter_type
+{
+    BF_COUNTER_POLICY = -1,
+    BF_COUNTER_ERRORS = -2,
+};
+
 /**
  * Get packets and bytes counter at a specific index.
  *
- * Counters are referenced by their index in the counters map. There are 1 more
- * counter in the map than the number of rules. This last counter (the last in
- * the map) is dedicated to the policy.
+ * Counters are referenced by their index in the counters map or the enum
+ * values defined by @ref bf_counter_type.
  *
  * The counter from all the program generated from @p cgen are summarised
  * together.
@@ -142,7 +152,8 @@ void bf_cgen_dump(const struct bf_cgen *cgen, prefix_t *prefix);
  *        correspond to a valid index, -E2BIG is returned.
  * @param counter Counter structure to fill with the counter values. Can't be
  *        NULL.
- * @return 0 on success, or a negative errno  value on failure.
+ * @return 0 on success, or a negative errno value on failure.
  */
-int bf_cgen_get_counter(const struct bf_cgen *cgen, uint32_t counter_idx,
+int bf_cgen_get_counter(const struct bf_cgen *cgen,
+                        enum bf_counter_type counter_idx,
                         struct bf_counter *counter);

src/bpfilter/cgen/program.c

@@ -891,6 +891,11 @@ int bf_program_generate(struct bf_program *program)
             bf_flavor_to_str(bf_hook_to_flavor(program->hook)),
             bf_front_to_str(program->front), bf_hook_to_str(program->hook));
 
+    /* Add 1 to the number of counters for the policy counter, and 1
+     * for the first reserved error slot. This must be done ahead of
+     * generation, as we will index into the error counters. */
+    program->num_counters = bf_list_size(&chain->rules) + 2;
+
     r = _bf_program_generate_runtime_init(program);
     if (r)
         return r;
@@ -936,9 +941,6 @@ int bf_program_generate(struct bf_program *program)
     if (r)
         return bf_err_r(r, "failed to generate function call fixups");
 
-    // Add 1 to the number of counters for the policy counter.
-    program->num_counters = bf_list_size(&chain->rules) + 1;
-
     return 0;
 }
 

src/bpfilter/cgen/stub.c

@@ -67,6 +67,15 @@ static int _bf_stub_make_ctx_dynptr(struct bf_program *program,
         _cleanup_bf_jmpctx_ struct bf_jmpctx _ =
             bf_jmpctx_get(program, BPF_JMP_IMM(BPF_JEQ, BF_REG_2, 0, 0));
 
+        // BF_ARG_1: index in counters map (last slot).
+        EMIT(program, BPF_MOV32_IMM(BF_ARG_1, program->num_counters-1));
+
+        // BF_ARG_2: packet size, from the context.
+        EMIT(program, BPF_LDX_MEM(BPF_DW, BF_ARG_2, BF_REG_CTX,
+                                  BF_PROG_CTX_OFF(pkt_size)));
+
+        EMIT_FIXUP_CALL(program, BF_FIXUP_FUNC_UPDATE_COUNTERS);
+
         if (bf_opts_is_verbose(BF_VERBOSE_BPF))
             EMIT_PRINT(program, "failed to create a new dynamic pointer");
 
@@ -117,6 +126,15 @@ int bf_stub_parse_l2_ethhdr(struct bf_program *program)
         _cleanup_bf_jmpctx_ struct bf_jmpctx _ =
             bf_jmpctx_get(program, BPF_JMP_IMM(BPF_JNE, BF_REG_RET, 0, 0));
 
+        // BF_ARG_1: index in counters map (last slot).
+        EMIT(program, BPF_MOV32_IMM(BF_ARG_1, program->num_counters-1));
+
+        // BF_ARG_2: packet size, from the context.
+        EMIT(program, BPF_LDX_MEM(BPF_DW, BF_ARG_2, BF_REG_CTX,
+                                  BF_PROG_CTX_OFF(pkt_size)));
+
+        EMIT_FIXUP_CALL(program, BF_FIXUP_FUNC_UPDATE_COUNTERS);
+
         if (bf_opts_is_verbose(BF_VERBOSE_BPF))
             EMIT_PRINT(program, "failed to create L2 dynamic pointer slice");
 
@@ -192,6 +210,15 @@ int bf_stub_parse_l3_hdr(struct bf_program *program)
         _cleanup_bf_jmpctx_ struct bf_jmpctx _ =
             bf_jmpctx_get(program, BPF_JMP_IMM(BPF_JNE, BF_REG_RET, 0, 0));
 
+        // BF_ARG_1: index in counters map (last slot).
+        EMIT(program, BPF_MOV32_IMM(BF_ARG_1, program->num_counters-1));
+
+        // BF_ARG_2: packet size, from the context.
+        EMIT(program, BPF_LDX_MEM(BPF_DW, BF_ARG_2, BF_REG_CTX,
+                                  BF_PROG_CTX_OFF(pkt_size)));
+
+        EMIT_FIXUP_CALL(program, BF_FIXUP_FUNC_UPDATE_COUNTERS);
+
         if (bf_opts_is_verbose(BF_VERBOSE_BPF))
             EMIT_PRINT(program, "failed to create L3 dynamic pointer slice");
 

src/bpfilter/xlate/ipt/ipt.c

@@ -603,7 +603,7 @@ int _bf_ipt_get_entries_handler(struct bf_request *request,
         struct ipt_entry *first_rule;
         struct ipt_entry *last_rule;
         struct bf_cgen *cgen;
-        uint32_t counter_idx;
+        enum bf_counter_type counter_idx;
 
         if (!(_bf_cache->valid_hooks & (1 << i))) {
             bf_dbg("ipt hook %d is not enabled, skipping", i);
@@ -613,12 +613,18 @@ int _bf_ipt_get_entries_handler(struct bf_request *request,
         first_rule = bf_ipt_entries_get_rule(entries, _bf_cache->hook_entry[i]);
         last_rule = bf_ipt_entries_get_rule(entries, _bf_cache->underflow[i]);
         cgen = bf_ctx_get_cgen(_bf_ipt_hook_to_bf_hook(i), BF_FRONT_IPT);
+        enum bf_counter_type rule_count = bf_list_size(&cgen->chain->rules);
 
         for (counter_idx = 0; first_rule <= last_rule;
              ++counter_idx, first_rule = ipt_get_next_rule(first_rule)) {
             struct bf_counter counter = {};
 
-            r = bf_cgen_get_counter(cgen, counter_idx, &counter);
+            /* Note that the policy is considered a rule, but we must access
+             * via the unambiguous counter enum rather than overflowing. */
+            bool is_policy = counter_idx == rule_count;
+            r = bf_cgen_get_counter(cgen,
+                                    is_policy ? BF_COUNTER_POLICY : counter_idx,
+                                    &counter);
             if (r) {
                 return bf_err_r(r, "failed to get IPT counter for index %u",
                                 counter_idx);
@@ -628,7 +634,7 @@ int _bf_ipt_get_entries_handler(struct bf_request *request,
             first_rule->counters.pcnt = counter.packets;
         }
 
-        if (counter_idx != bf_list_size(&cgen->chain->rules) + 1) {
+        if (counter_idx != rule_count+1) {
             /* We expect len(cgen->rules) + 1 as the policy is considered
              * a rule for iptables, but not for bpfilter. */
             return bf_err_r(-EINVAL, "invalid number of rules requested");