[0/2] Refactor Logging Infrastructure

src/bfcli/lexer.l

@@ -101,7 +101,7 @@ log             { BEGIN(STATE_LOG_OPTS); return LOG; }
     [0-9a-zA-Z]+(,[0-9a-zA-Z]+)* {
         BEGIN(INITIAL);
         yylval.sval = strdup(yytext);
-        return LOG_HEADERS;
+        return LOG_OPTS;
     }
 }
 

src/bfcli/parser.y

@@ -103,7 +103,7 @@
 %token SET
 %token LOG COUNTER MARK
 %token REDIRECT_TOKEN
-%token <sval> LOG_HEADERS
+%token <sval> LOG_OPTS
 %token <sval> SET_TYPE
 %token <sval> SET_RAW_PAYLOAD
 %token <sval> STRING
@@ -320,7 +320,7 @@ rule            : RULE matchers rule_options rule_verdict
                 }
                 ;
 
-rule_option     : LOG LOG_HEADERS
+rule_option     : LOG LOG_OPTS
                 {
                     _cleanup_free_ char *in = $2;
                     char *tmp = in;
@@ -329,12 +329,12 @@ rule_option     : LOG LOG_HEADERS
                     uint8_t log = 0;
 
                     while ((token = strtok_r(tmp, ",", &saveptr))) {
-                        enum bf_pkthdr header;
+                        enum bf_log_opt opt;
 
-                        if (bf_pkthdr_from_str(token, &header) < 0)
-                            bf_parse_err("unknown packet header '%s'", token);
+                        if (bf_log_opt_from_str(token, &opt) < 0)
+                            bf_parse_err("unknown log option '%s'", token);
 
-                        log |= BF_FLAG(header);
+                        log |= BF_FLAG(opt);
 
                         tmp = NULL;
                     }

src/bfcli/print.c

@@ -265,12 +265,12 @@ void bfc_chain_dump(struct bf_chain *chain, struct bf_hookopts *hookopts,
 
             (void)fprintf(stdout, "        log ");
 
-            for (enum bf_pkthdr hdr = 0; hdr < _BF_PKTHDR_MAX; ++hdr) {
-                if (!(log & BF_FLAG(hdr)))
+            for (enum bf_log_opt opt = 0; opt < _BF_LOG_OPT_MAX; ++opt) {
+                if (!(log & BF_FLAG(opt)))
                     continue;
 
-                log &= ~BF_FLAG(hdr);
-                (void)fprintf(stdout, "%s%s", bf_pkthdr_to_str(hdr),
+                log &= ~BF_FLAG(opt);
+                (void)fprintf(stdout, "%s%s", bf_log_opt_to_str(opt),
                               log ? "," : "\n");
             }
         }
@@ -373,17 +373,18 @@ static void _bf_chain_log_header(const struct bf_log *log)
         bf_logger_get_color(BF_COLOR_LIGHT_CYAN, BF_STYLE_NORMAL), time_str,
         time.tv_nsec / BF_TIME_US,
         bf_logger_get_color(BF_COLOR_RESET, BF_STYLE_RESET), log->rule_id,
-        bf_logger_get_color(BF_COLOR_DEFAULT, BF_STYLE_BOLD), log->pkt_size,
+        bf_logger_get_color(BF_COLOR_DEFAULT, BF_STYLE_BOLD),
+        log->payload.pkt.pkt_size,
         bf_logger_get_color(BF_COLOR_RESET, BF_STYLE_RESET),
         bf_verdict_to_str((enum bf_verdict)log->verdict));
 }
 
 static void _bf_chain_log_l2(const struct bf_log *log)
 {
-    struct ethhdr *ethhdr = (void *)log->l2hdr;
+    struct ethhdr *ethhdr = (void *)log->payload.pkt.l2hdr;
     const char *ethertype;
 
-    if (!(log->headers & (1 << BF_PKTHDR_LINK))) {
+    if (!(log->payload.pkt.headers & (1 << BF_LOG_OPT_LINK))) {
         (void)fprintf(stdout, "  Ethernet  : <unknown header>\n");
         return;
     }
@@ -418,14 +419,14 @@ static void _bf_chain_log_l3(const struct bf_log *log)
     char dst_addr[INET6_ADDRSTRLEN];
     const char *protocol;
 
-    if (!(log->headers & (1 << BF_PKTHDR_INTERNET))) {
+    if (!(log->payload.pkt.headers & (1 << BF_LOG_OPT_INTERNET))) {
         (void)fprintf(stdout, "  Internet  : <unknown header>\n");
         return;
     }
 
     switch (log->l3_proto) {
     case ETH_P_IP:
-        iphdr = (struct iphdr *)&log->l3hdr[0];
+        iphdr = (struct iphdr *)&log->payload.pkt.l3hdr[0];
 
         inet_ntop(AF_INET, &iphdr->saddr, src_addr, sizeof(src_addr));
         inet_ntop(AF_INET, &iphdr->daddr, dst_addr, sizeof(dst_addr));
@@ -451,7 +452,7 @@ static void _bf_chain_log_l3(const struct bf_log *log)
         break;
 
     case ETH_P_IPV6:
-        ipv6hdr = (struct ipv6hdr *)log->l3hdr;
+        ipv6hdr = (struct ipv6hdr *)log->payload.pkt.l3hdr;
 
         inet_ntop(AF_INET6, &ipv6hdr->saddr, src_addr, sizeof(src_addr));
         inet_ntop(AF_INET6, &ipv6hdr->daddr, dst_addr, sizeof(dst_addr));
@@ -490,14 +491,14 @@ static void _bf_chain_log_l4(const struct bf_log *log)
     struct udphdr *udphdr;
     const char *tcp_flags_str;
 
-    if (!(log->headers & (1 << BF_PKTHDR_TRANSPORT))) {
+    if (!(log->payload.pkt.headers & (1 << BF_LOG_OPT_TRANSPORT))) {
         (void)fprintf(stdout, "  Transport : <unknown header>\n");
         return;
     }
 
     switch (log->l4_proto) {
     case IPPROTO_TCP:
-        tcphdr = (struct tcphdr *)log->l4hdr;
+        tcphdr = (struct tcphdr *)log->payload.pkt.l4hdr;
         tcp_flags_str = _bf_tcp_flags_to_str(tcphdr);
 
         (void)fprintf(stdout, "  TCP       : %s%-5u%s → %s%-5u%s",
@@ -522,7 +523,7 @@ static void _bf_chain_log_l4(const struct bf_log *log)
         break;
 
     case IPPROTO_UDP:
-        udphdr = (struct udphdr *)log->l4hdr;
+        udphdr = (struct udphdr *)log->payload.pkt.l4hdr;
 
         (void)fprintf(stdout, "  UDP       : %s%-5u%s → %s%-5u%s [len=%u]\n",
                       bf_logger_get_color(BF_COLOR_LIGHT_YELLOW, BF_STYLE_BOLD),
@@ -535,7 +536,7 @@ static void _bf_chain_log_l4(const struct bf_log *log)
         break;
 
     case IPPROTO_ICMP:
-        icmphdr = (struct icmphdr *)log->l4hdr;
+        icmphdr = (struct icmphdr *)log->payload.pkt.l4hdr;
 
         (void)fprintf(stdout, "  ICMP      : type=%-3u code=%-3u",
                       icmphdr->type, icmphdr->code);
@@ -550,7 +551,7 @@ static void _bf_chain_log_l4(const struct bf_log *log)
         break;
 
     case IPPROTO_ICMPV6:
-        icmp6hdr = (struct icmp6hdr *)log->l4hdr;
+        icmp6hdr = (struct icmp6hdr *)log->payload.pkt.l4hdr;
 
         (void)fprintf(stdout, "  ICMPv6    : type=%-3u code=%-3u",
                       icmp6hdr->icmp6_type, icmp6hdr->icmp6_code);
@@ -573,12 +574,15 @@ static void _bf_chain_log_l4(const struct bf_log *log)
 
 void bfc_print_log(const struct bf_log *log)
 {
+    if (log->log_type != BF_LOG_TYPE_PACKET)
+        return;
+
     _bf_chain_log_header(log);
 
-    if (log->req_headers & (1 << BF_PKTHDR_LINK))
+    if (log->payload.pkt.req_headers & (1 << BF_LOG_OPT_LINK))
         _bf_chain_log_l2(log);
-    if (log->req_headers & (1 << BF_PKTHDR_INTERNET))
+    if (log->payload.pkt.req_headers & (1 << BF_LOG_OPT_INTERNET))
         _bf_chain_log_l3(log);
-    if (log->req_headers & (1 << BF_PKTHDR_TRANSPORT))
+    if (log->payload.pkt.req_headers & (1 << BF_LOG_OPT_TRANSPORT))
         _bf_chain_log_l4(log);
 }

src/libbpfilter/CMakeLists.txt

@@ -109,7 +109,7 @@ bf_target_add_elfstubs(libbpfilter
         "parse_ipv6_eh"
         "parse_ipv6_nh"
         "update_counters"
-        "log"
+        "pkt_log"
         "flow_hash"
 )
 

src/libbpfilter/bpf/pkt_log.bpf.c

@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ */
+
+#include <linux/bpf.h>
+
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+#include <stddef.h>
+
+#include "cgen/runtime.h"
+
+__u8 bf_pkt_log(struct bf_runtime *ctx, __u32 rule_id, __u8 headers,
+                __u32 verdict, __u32 l3_l4_proto)
+{
+    struct bf_log *log;
+    __u16 l3_proto = (__u16)(l3_l4_proto >> 16);
+    __u8 l4_proto = (__u8)l3_l4_proto;
+
+    log = bpf_ringbuf_reserve(ctx->log_map, sizeof(struct bf_log), 0);
+    if (!log) {
+        bpf_printk("failed to reserve %d bytes in ringbuf",
+                   sizeof(struct bf_log));
+        return 1;
+    }
+
+    log->ts = bpf_ktime_get_ns();
+    log->rule_id = rule_id;
+    log->verdict = verdict;
+    log->l3_proto = bpf_ntohs(l3_proto);
+    log->l4_proto = l4_proto;
+    log->log_type = BF_LOG_TYPE_PACKET;
+    log->payload.pkt.pkt_size = ctx->pkt_size;
+    log->payload.pkt.req_headers = headers;
+    log->payload.pkt.headers = 0;
+
+    if (headers & (1 << BF_LOG_OPT_LINK) && ctx->l2_hdr &&
+        ctx->l2_size <= BF_L2_SLICE_LEN) {
+        bpf_probe_read_kernel(log->payload.pkt.l2hdr, ctx->l2_size,
+                              ctx->l2_hdr);
+        log->payload.pkt.headers |= (1 << BF_LOG_OPT_LINK);
+    }
+
+    if (headers & (1 << BF_LOG_OPT_INTERNET) && ctx->l3_hdr &&
+        ctx->l3_size <= BF_L3_SLICE_LEN) {
+        bpf_probe_read_kernel(log->payload.pkt.l3hdr, ctx->l3_size,
+                              ctx->l3_hdr);
+        log->payload.pkt.headers |= (1 << BF_LOG_OPT_INTERNET);
+    }
+
+    if (headers & (1 << BF_LOG_OPT_TRANSPORT) && ctx->l4_hdr &&
+        ctx->l4_size <= BF_L4_SLICE_LEN) {
+        bpf_probe_read_kernel(log->payload.pkt.l4hdr, ctx->l4_size,
+                              ctx->l4_hdr);
+        log->payload.pkt.headers |= (1 << BF_LOG_OPT_TRANSPORT);
+    }
+
+    bpf_ringbuf_submit(log, 0);
+
+    return 0;
+}

src/libbpfilter/cgen/program.c

@@ -357,7 +357,7 @@ static int _bf_program_generate_rule(struct bf_program *program,
         EMIT(program, BPF_ALU64_IMM(BPF_LSH, BPF_REG_5, 16));
         EMIT(program, BPF_ALU64_REG(BPF_OR, BPF_REG_5, BPF_REG_8));
 
-        EMIT_FIXUP_ELFSTUB(program, BF_ELFSTUB_LOG);
+        EMIT_FIXUP_ELFSTUB(program, BF_ELFSTUB_PKT_LOG);
     }
 
     if (rule->counters) {

src/libbpfilter/include/bpfilter/elfstub.h

@@ -122,7 +122,7 @@ enum bf_elfstub_id
     /**
      * Log user-requested packet headers to a ring buffer.
      *
-     * `__u8 bf_log(struct bf_runtime *ctx, __u32 rule_id, __u8 headers, __u32 verdict, __u32 l3_l4_proto)`
+     * `__u8 bf_pkt_log(struct bf_runtime *ctx, __u32 rule_id, __u8 headers, __u32 verdict, __u32 l3_l4_proto)`
      *
      * **Parameters**
      * - `ctx`: address of the `bf_runtime` context of the program.
@@ -133,7 +133,7 @@ enum bf_elfstub_id
      *
      * **Return** 0 on success, or 1 on error.
      */
-    BF_ELFSTUB_LOG,
+    BF_ELFSTUB_PKT_LOG,
 
     /**
      * Calculate flow hash from packet 5-tuple + IPv6 flow label.

src/libbpfilter/include/bpfilter/rule.h

@@ -19,28 +19,42 @@
 #define BF_RULE_MARK_MASK (0x00000000ffffffffULL)
 
 /**
- * @brief Return the string representation of a `bf_pkthdr` enumeration value.
+ * @brief Return the string representation of a `bf_log_type` enumeration value.
  *
- * @param hdr `bf_pkthdr` enumeration value.
- * @return A pointer to the C-string representation of `hdr`.
+ * @param log_type `bf_log_type` enumeration value.
+ * @return A pointer to the C-string representation of `log_type`.
  */
-const char *bf_pkthdr_to_str(enum bf_pkthdr hdr);
+const char *bf_log_type_to_str(enum bf_log_type log_type);
 
 /**
- * @brief Return the `bf_pkthdr` enumeration value corresponding to a string.
+ * @brief Return the `bf_log_type` enumeration value corresponding to a string.
  *
- * @pre
- * - `str` is a non-NULL pointer to a C-string.
- * - `hdr != NULL`
- * @post
- * - On failure, `hdr` is unchanged.
+ * @param str String to get the corresponding `bf_log_type` enumeration value
+ *        for.
+ * @param log_type On success, contains the `bf_log_type` enumeration value
+ *        corresponding to `str`. Can't be NULL.
+ * @return 0 on success, or a negative error value on failure.
+ */
+int bf_log_type_from_str(const char *str, enum bf_log_type *log_type);
+
+/**
+ * @brief Return the string representation of a `bf_log_opt` enumeration value.
+ *
+ * @param opt `bf_log_opt` enumeration value.
+ * @return A pointer to the C-string representation of `opt`.
+ */
+const char *bf_log_opt_to_str(enum bf_log_opt opt);
+
+/**
+ * @brief Return the `bf_log_opt` enumeration value corresponding to a string.
  *
- * @param str String to get the corresponding `bf_pkthdr` enumeration value for.
- * @param hdr On success, contains the `bf_pkthdr` enumeration value
- *        corresponding to `str`.
+ * @param str String to get the corresponding `bf_log_opt` enumeration value
+ *        for.
+ * @param opt On success, contains the `bf_log_opt` enumeration value
+ *        corresponding to `str`. Can't be NULL.
  * @return 0 on success, or a negative error value on failure.
  */
-int bf_pkthdr_from_str(const char *str, enum bf_pkthdr *hdr);
+int bf_log_opt_from_str(const char *str, enum bf_log_opt *opt);
 
 #define _free_bf_rule_ __attribute__((__cleanup__(bf_rule_free)))
 
@@ -77,8 +91,8 @@ struct bf_rule
 };
 
 static_assert(
-    _BF_PKTHDR_MAX < 8,
-    "bf_pkthdr has more than 8 values, it won't fit in bf_rule.log's 8 bits");
+    _BF_LOG_OPT_MAX <= 8,
+    "bf_log_opt has more than 8 values, it won't fit in bf_rule.log's 8 bits");
 
 /**
  * Allocated and initialise a new rule.