[RFC,4/5] tap: get errors from kernel on bpf load failure

Message ID 20240105222909.139674-5-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series BPF infrastructure enhancements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Jan. 5, 2024, 10:28 p.m. UTC
  The bpf load kernel API can provide some useful diagnostics
on failure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/tap/tap_bpf_api.c | 44 +++++++++++++++++++++++++++--------
 drivers/net/tap/tap_flow.c    | 16 ++++++++-----
 drivers/net/tap/tap_flow.h    |  4 ++--
 3 files changed, 46 insertions(+), 18 deletions(-)
  

Patch

diff --git a/drivers/net/tap/tap_bpf_api.c b/drivers/net/tap/tap_bpf_api.c
index c754c167a764..29223b7f0ea7 100644
--- a/drivers/net/tap/tap_bpf_api.c
+++ b/drivers/net/tap/tap_bpf_api.c
@@ -15,8 +15,10 @@ 
 #include <tap_bpf.h>
 #include <tap_bpf_insns.h>
 
-static int bpf_load(enum bpf_prog_type type, const struct bpf_insn *insns,
-		    size_t insns_cnt, const char *license);
+static int bpf_load(enum bpf_prog_type type,
+		    const struct bpf_insn *insns, size_t insns_cnt,
+		    char *log_buf, size_t log_size,
+		    const char *license);
 
 /**
  * Load BPF program (section cls_q) into the kernel and return a bpf fd
@@ -24,15 +26,22 @@  static int bpf_load(enum bpf_prog_type type, const struct bpf_insn *insns,
  * @param queue_idx
  *   Queue index matching packet cb
  *
+ * @param log_buf
+ *   Buffer to place resulting error message (optional)
+ *
+ * @param log_size
+ *   Size of log_buf
+ *
  * @return
  *   -1 if the BPF program couldn't be loaded. An fd (int) otherwise.
  */
-int tap_flow_bpf_cls_q(__u32 queue_idx)
+int tap_flow_bpf_cls_q(__u32 queue_idx, char *log_buf, size_t log_size)
 {
 	cls_q_insns[1].imm = queue_idx;
 
 	return bpf_load(BPF_PROG_TYPE_SCHED_CLS,
 			cls_q_insns, RTE_DIM(cls_q_insns),
+			log_buf, log_size,
 			"Dual BSD/GPL");
 }
 
@@ -45,16 +54,23 @@  int tap_flow_bpf_cls_q(__u32 queue_idx)
  * @param[in] map_fd
  *   BPF RSS map file descriptor
  *
+ * @param log_buf
+ *   Buffer to place resulting error message (optional)
+ *
+ * @param log_size
+ *   Size of log_buf
+ *
  * @return
  *   -1 if the BPF program couldn't be loaded. An fd (int) otherwise.
  */
-int tap_flow_bpf_calc_l3_l4_hash(__u32 key_idx, int map_fd)
+int tap_flow_bpf_calc_l3_l4_hash(__u32 key_idx, int map_fd, char *log_buf, size_t log_size)
 {
 	l3_l4_hash_insns[4].imm = key_idx;
 	l3_l4_hash_insns[9].imm = map_fd;
 
 	return bpf_load(BPF_PROG_TYPE_SCHED_ACT,
 			l3_l4_hash_insns, RTE_DIM(l3_l4_hash_insns),
+			log_buf, log_size,
 			"Dual BSD/GPL");
 }
 
@@ -105,6 +121,12 @@  static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
  * @param[in] insns_cnt
  *   Number of BPF instructions (size of array)
  *
+ * @param[out] log_buf
+ *   Space for log message
+ *
+ * @param[in] log_size
+ *   Number of characters available in log_buf
+ *
  * @param[in] license
  *   License string that must be acknowledged by the kernel
  *
@@ -112,9 +134,8 @@  static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
  *   -1 if the BPF program couldn't be loaded, fd (file descriptor) otherwise
  */
 static int bpf_load(enum bpf_prog_type type,
-		  const struct bpf_insn *insns,
-		  size_t insns_cnt,
-		  const char *license)
+		    const struct bpf_insn *insns, size_t insns_cnt,
+		    char *log_buf, size_t log_size, const char *license)
 {
 	union bpf_attr attr = {};
 
@@ -122,9 +143,12 @@  static int bpf_load(enum bpf_prog_type type,
 	attr.insn_cnt = (__u32)insns_cnt;
 	attr.insns = ptr_to_u64(insns);
 	attr.license = ptr_to_u64(license);
-	attr.log_buf = ptr_to_u64(NULL);
-	attr.log_level = 0;
-	attr.kern_version = 0;
+
+	if (log_size > 0) {
+		attr.log_level = 2;
+		attr.log_buf = ptr_to_u64(log_buf);
+		attr.log_size = log_size;
+	}
 
 	return sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
 }
diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index ed4d42f92f9f..897d71acbad1 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -18,6 +18,8 @@ 
 #include <tap_tcmsgs.h>
 #include <tap_rss.h>
 
+#define BPF_LOG_BUFSIZ	1024
+
 #ifndef HAVE_TC_FLOWER
 /*
  * For kernels < 4.2, this enum is not defined. Runtime checks will be made to
@@ -1885,11 +1887,13 @@  static int rss_enable(struct pmd_internals *pmd,
 	 * the correct queue.
 	 */
 	for (i = 0; i < pmd->dev->data->nb_rx_queues; i++) {
-		pmd->bpf_fd[i] = tap_flow_bpf_cls_q(i);
+		char log_buf[BPF_LOG_BUFSIZ];
+
+		pmd->bpf_fd[i] = tap_flow_bpf_cls_q(i, log_buf, sizeof(log_buf));
 		if (pmd->bpf_fd[i] < 0) {
 			TAP_LOG(ERR,
-				"Failed to load BPF section %s for queue %d",
-				SEC_NAME_CLS_Q, i);
+				"Failed to load BPF section %s for queue %u: %s",
+				SEC_NAME_CLS_Q, i, log_buf);
 			rte_flow_error_set(
 				error, ENOTSUP, RTE_FLOW_ERROR_TYPE_HANDLE,
 				NULL,
@@ -2074,6 +2078,7 @@  static int rss_add_actions(struct rte_flow *flow, struct pmd_internals *pmd,
 	/* 4096 is the maximum number of instructions for a BPF program */
 	unsigned int i;
 	int err;
+	char log_buf[BPF_LOG_BUFSIZ];
 	struct rss_key rss_entry = { .hash_fields = 0,
 				     .key_size = 0 };
 
@@ -2124,9 +2129,8 @@  static int rss_add_actions(struct rte_flow *flow, struct pmd_internals *pmd,
 	/*
 	 * Load bpf rules to calculate hash for this key_idx
 	 */
-
-	flow->bpf_fd[SEC_L3_L4] =
-		tap_flow_bpf_calc_l3_l4_hash(flow->key_idx, pmd->map_fd);
+	flow->bpf_fd[SEC_L3_L4] = tap_flow_bpf_calc_l3_l4_hash(flow->key_idx, pmd->map_fd,
+							       log_buf, sizeof(log_buf));
 	if (flow->bpf_fd[SEC_L3_L4] < 0) {
 		TAP_LOG(ERR,
 			"Failed to load BPF section %s (%d): %s",
diff --git a/drivers/net/tap/tap_flow.h b/drivers/net/tap/tap_flow.h
index 240fbc3dfaef..fb30b495fda1 100644
--- a/drivers/net/tap/tap_flow.h
+++ b/drivers/net/tap/tap_flow.h
@@ -57,8 +57,8 @@  int tap_flow_implicit_destroy(struct pmd_internals *pmd,
 int tap_flow_implicit_flush(struct pmd_internals *pmd,
 			    struct rte_flow_error *error);
 
-int tap_flow_bpf_cls_q(__u32 queue_idx);
-int tap_flow_bpf_calc_l3_l4_hash(__u32 key_idx, int map_fd);
+int tap_flow_bpf_cls_q(__u32 queue_idx, char *log_buf, size_t log_size);
+int tap_flow_bpf_calc_l3_l4_hash(__u32 key_idx, int map_fd, char *log_buf, size_t log_size);
 int tap_flow_bpf_rss_map_create(unsigned int key_size, unsigned int value_size,
 			unsigned int max_entries);
 int tap_flow_bpf_update_rss_elem(int fd, void *key, void *value);