[PATCH bpf-next v2 0/6] AF_XDP Packet Drop Tracing

[PATCH bpf-next v2 0/6] AF_XDP Packet Drop Tracing

[Ciara Loftus]

This series introduces tracing infrastructure for AF_XDP sockets (xsks).
A trace event 'xsk_packet_drop' is created which can be enabled by toggling

/sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable

When enabled and packets are dropped in the kernel, traces are generated
which describe the reason for the packet drop as well as the netdev and
qid information of the xsk which encountered the drop.

Example traces:

507.588563: xsk_packet_drop: netdev: eth0 qid 0 reason: rxq full
507.588567: xsk_packet_drop: netdev: eth0 qid 0 reason: packet too big
507.588568: xsk_packet_drop: netdev: eth0 qid 0 reason: fq empty

The event can also be monitored using perf:

perf stat -a -e xsk:xsk_packet_drop

Three selftests are added which each ensure the appropriate traces are
generated for the following scenarios:
* rx queue full
* packet too big
* fill queue empty

v1->v2:
* Rebase on top of Björn's cleanup series.
* Fixed packet count for trace tests which don't need EOT frame.

This series applies on commit 190d1c921ad0862da14807e1670f54020f48e889

Ciara Loftus (6):
  xsk: add tracepoints for packet drops
  selftests/bpf: restructure setting the packet count
  selftests/bpf: add framework for xsk selftests
  selftests/bpf: XSK_TRACE_DROP_RXQ_FULL test
  selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test
  selftests/bpf: XSK_TRACE_DROP_FQ_EMPTY test

 MAINTAINERS                                |   1 +
 include/linux/bpf_trace.h                  |   1 +
 include/trace/events/xsk.h                 |  45 +++++
 include/uapi/linux/if_xdp.h                |   8 +
 kernel/bpf/core.c                          |   1 +
 net/xdp/xsk.c                              |   5 +
 net/xdp/xsk_buff_pool.c                    |   8 +-
 tools/include/uapi/linux/if_xdp.h          |   8 +
 tools/testing/selftests/bpf/test_xsk.sh    |  90 +++++++++-
 tools/testing/selftests/bpf/xdpxceiver.c   | 192 +++++++++++++++++++--
 tools/testing/selftests/bpf/xdpxceiver.h   |  10 ++
 tools/testing/selftests/bpf/xsk_prereqs.sh |   3 +-
 12 files changed, 348 insertions(+), 24 deletions(-)
 create mode 100644 include/trace/events/xsk.h

-- 
2.17.1

[PATCH bpf-next v2 1/6] xsk: add tracepoints for packet drops

[Ciara Loftus]

This commit introduces static perf tracepoints for AF_XDP which
report information about packet drops, such as the netdev, qid and
high level reason for the drop. The tracepoint can be
enabled/disabled by toggling
/sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 MAINTAINERS                       |  1 +
 include/linux/bpf_trace.h         |  1 +
 include/trace/events/xsk.h        | 45 +++++++++++++++++++++++++++++++
 include/uapi/linux/if_xdp.h       |  8 ++++++
 kernel/bpf/core.c                 |  1 +
 net/xdp/xsk.c                     |  5 ++++
 net/xdp/xsk_buff_pool.c           |  8 +++++-
 tools/include/uapi/linux/if_xdp.h |  8 ++++++
 8 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 include/trace/events/xsk.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1df56a32d2df..efe6662d4198 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19440,6 +19440,7 @@ S:	Maintained
 F:	Documentation/networking/af_xdp.rst
 F:	include/net/xdp_sock*
 F:	include/net/xsk_buff_pool.h
+F:	include/trace/events/xsk.h
 F:	include/uapi/linux/if_xdp.h
 F:	include/uapi/linux/xdp_diag.h
 F:	include/net/netns/xdp.h
diff --git a/include/linux/bpf_trace.h b/include/linux/bpf_trace.h
index ddf896abcfb6..477d29b6c2c1 100644
--- a/include/linux/bpf_trace.h
+++ b/include/linux/bpf_trace.h
@@ -3,5 +3,6 @@
 #define __LINUX_BPF_TRACE_H__
 
 #include <trace/events/xdp.h>
+#include <trace/events/xsk.h>
 
 #endif /* __LINUX_BPF_TRACE_H__ */
diff --git a/include/trace/events/xsk.h b/include/trace/events/xsk.h
new file mode 100644
index 000000000000..4f5629ba1b0f
--- /dev/null
+++ b/include/trace/events/xsk.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2021 Intel Corporation. */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM xsk
+
+#if !defined(_TRACE_XSK_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_XSK_H
+
+#include <linux/if_xdp.h>
+#include <linux/tracepoint.h>
+
+#define print_reason(val) \
+	__print_symbolic(val, \
+			{ XSK_TRACE_DROP_RXQ_FULL, "rxq full" }, \
+			{ XSK_TRACE_DROP_PKT_TOO_BIG, "packet too big" }, \
+			{ XSK_TRACE_DROP_FQ_EMPTY, "fq empty" }, \
+			{ XSK_TRACE_DROP_POOL_EMPTY, "xskb pool empty" }, \
+			{ XSK_TRACE_DROP_DRV_ERR_TX, "driver error on tx" })
+
+TRACE_EVENT(xsk_packet_drop,
+
+	TP_PROTO(char *name, u16 queue_id, u32 reason),
+
+	TP_ARGS(name, queue_id, reason),
+
+	TP_STRUCT__entry(
+		__field(char *, name)
+		__field(u16, queue_id)
+		__field(u32, reason)
+	),
+
+	TP_fast_assign(
+		__entry->name = name;
+		__entry->queue_id = queue_id;
+		__entry->reason = reason;
+	),
+
+	TP_printk("netdev: %s qid %u reason: %s", __entry->name,
+			__entry->queue_id, print_reason(__entry->reason))
+);
+
+#endif /* _TRACE_XSK_H */
+
+#include <trace/define_trace.h>
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
index a78a8096f4ce..5f1b8bf99bb5 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -108,4 +108,12 @@ struct xdp_desc {
 
 /* UMEM descriptor is __u64 */
 
+enum xdp_trace_reasons {
+	XSK_TRACE_DROP_RXQ_FULL,
+	XSK_TRACE_DROP_PKT_TOO_BIG,
+	XSK_TRACE_DROP_FQ_EMPTY,
+	XSK_TRACE_DROP_POOL_EMPTY,
+	XSK_TRACE_DROP_DRV_ERR_TX,
+};
+
 #endif /* _LINUX_IF_XDP_H */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5bbd4884ff7a..442b0d7f9bf8 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2362,3 +2362,4 @@ EXPORT_SYMBOL(bpf_stats_enabled_key);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception);
 EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_bulk_tx);
+EXPORT_TRACEPOINT_SYMBOL_GPL(xsk_packet_drop);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 4faabd1ecfd1..9b850716630b 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -11,6 +11,7 @@
 
 #define pr_fmt(fmt) "AF_XDP: %s: " fmt, __func__
 
+#include <linux/bpf_trace.h>
 #include <linux/if_xdp.h>
 #include <linux/init.h>
 #include <linux/sched/mm.h>
@@ -158,6 +159,7 @@ static int __xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 	addr = xp_get_handle(xskb);
 	err = xskq_prod_reserve_desc(xs->rx, addr, len);
 	if (err) {
+		trace_xsk_packet_drop(xs->dev->name, xs->queue_id, XSK_TRACE_DROP_RXQ_FULL);
 		xs->rx_queue_full++;
 		return err;
 	}
@@ -192,6 +194,7 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
 
 	len = xdp->data_end - xdp->data;
 	if (len > xsk_pool_get_rx_frame_size(xs->pool)) {
+		trace_xsk_packet_drop(xs->dev->name, xs->queue_id, XSK_TRACE_DROP_PKT_TOO_BIG);
 		xs->rx_dropped++;
 		return -ENOSPC;
 	}
@@ -516,6 +519,8 @@ static int xsk_generic_xmit(struct sock *sk)
 		if (err == NET_XMIT_DROP) {
 			/* SKB completed but not sent */
 			err = -EBUSY;
+			trace_xsk_packet_drop(xs->dev->name, xs->queue_id,
+					      XSK_TRACE_DROP_DRV_ERR_TX);
 			goto out;
 		}
 
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 8de01aaac4a0..d3c1ca83c75d 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <linux/bpf_trace.h>
 #include <net/xsk_buff_pool.h>
 #include <net/xdp_sock.h>
 #include <net/xdp_sock_drv.h>
@@ -445,8 +446,11 @@ static struct xdp_buff_xsk *__xp_alloc(struct xsk_buff_pool *pool)
 	u64 addr;
 	bool ok;
 
-	if (pool->free_heads_cnt == 0)
+	if (pool->free_heads_cnt == 0) {
+		trace_xsk_packet_drop(pool->netdev->name, pool->queue_id,
+				      XSK_TRACE_DROP_POOL_EMPTY);
 		return NULL;
+	}
 
 	xskb = pool->free_heads[--pool->free_heads_cnt];
 
@@ -454,6 +458,8 @@ static struct xdp_buff_xsk *__xp_alloc(struct xsk_buff_pool *pool)
 		if (!xskq_cons_peek_addr_unchecked(pool->fq, &addr)) {
 			pool->fq->queue_empty_descs++;
 			xp_release(xskb);
+			trace_xsk_packet_drop(pool->netdev->name, pool->queue_id,
+					      XSK_TRACE_DROP_FQ_EMPTY);
 			return NULL;
 		}
 
diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index a78a8096f4ce..5f1b8bf99bb5 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -108,4 +108,12 @@ struct xdp_desc {
 
 /* UMEM descriptor is __u64 */
 
+enum xdp_trace_reasons {
+	XSK_TRACE_DROP_RXQ_FULL,
+	XSK_TRACE_DROP_PKT_TOO_BIG,
+	XSK_TRACE_DROP_FQ_EMPTY,
+	XSK_TRACE_DROP_POOL_EMPTY,
+	XSK_TRACE_DROP_DRV_ERR_TX,
+};
+
 #endif /* _LINUX_IF_XDP_H */
-- 
2.17.1

[PATCH bpf-next v2 2/6] selftests/bpf: restructure setting the packet count

[Ciara Loftus]

Prior to this, the packet count was fixed at 10000
for every test. Future tracing tests need to modify
the count, so make it possible to set the count from
test_xsk.h using the -C opt.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 17 +++++++++--------
 tools/testing/selftests/bpf/xsk_prereqs.sh |  3 +--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 88a7483eaae4..2b4a4f42b220 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -82,6 +82,7 @@ do
 done
 
 TEST_NAME="PREREQUISITES"
+DEFAULTPKTS=10000
 
 URANDOM=/dev/urandom
 [ ! -e "${URANDOM}" ] && { echo "${URANDOM} not found. Skipping tests."; test_exit 1 1; }
@@ -154,7 +155,7 @@ TEST_NAME="SKB NOPOLL"
 
 vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
 
-params=("-S")
+params=("-S" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -166,7 +167,7 @@ TEST_NAME="SKB POLL"
 
 vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
 
-params=("-S" "-p")
+params=("-S" "-p" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -178,7 +179,7 @@ TEST_NAME="DRV NOPOLL"
 
 vethXDPnative ${VETH0} ${VETH1} ${NS1}
 
-params=("-N")
+params=("-N" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -190,7 +191,7 @@ TEST_NAME="DRV POLL"
 
 vethXDPnative ${VETH0} ${VETH1} ${NS1}
 
-params=("-N" "-p")
+params=("-N" "-p" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -202,7 +203,7 @@ TEST_NAME="SKB SOCKET TEARDOWN"
 
 vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
 
-params=("-S" "-T")
+params=("-S" "-T" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -214,7 +215,7 @@ TEST_NAME="DRV SOCKET TEARDOWN"
 
 vethXDPnative ${VETH0} ${VETH1} ${NS1}
 
-params=("-N" "-T")
+params=("-N" "-T" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -226,7 +227,7 @@ TEST_NAME="SKB BIDIRECTIONAL SOCKETS"
 
 vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
 
-params=("-S" "-B")
+params=("-S" "-B" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
@@ -238,7 +239,7 @@ TEST_NAME="DRV BIDIRECTIONAL SOCKETS"
 
 vethXDPnative ${VETH0} ${VETH1} ${NS1}
 
-params=("-N" "-B")
+params=("-N" "-B" "-C" "${DEFAULTPKTS}")
 execxdpxceiver params
 
 retval=$?
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 9d54c4645127..41dd713d14df 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -15,7 +15,6 @@ NC='\033[0m'
 STACK_LIM=131072
 SPECFILE=veth.spec
 XSKOBJ=xdpxceiver
-NUMPKTS=10000
 
 validate_root_exec()
 {
@@ -131,5 +130,5 @@ execxdpxceiver()
 			copy[$index]=${!current}
 		done
 
-	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS}
+	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]}
 }
-- 
2.17.1

[PATCH bpf-next v2 3/6] selftests/bpf: add framework for xsk selftests

[Ciara Loftus]

This commit introduces framework to the xsk selftests
for testing the xsk_packet_drop traces. The '-t' or
'--trace-enable' args enable the trace, and disable
it on exit unless it was already enabled before the
test.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 65 ++++++++++++++++++++++--
 tools/testing/selftests/bpf/xdpxceiver.h |  3 ++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 99ea6cf069e6..e6e0d42d8074 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -72,6 +72,7 @@
 typedef __u16 __sum16;
 #include <linux/if_link.h>
 #include <linux/if_ether.h>
+#include <linux/if_xdp.h>
 #include <linux/ip.h>
 #include <linux/udp.h>
 #include <arpa/inet.h>
@@ -108,7 +109,8 @@ static void __exit_with_error(int error, const char *file, const char *func, int
 #define print_ksft_result(void)\
 	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB", opt_poll ? "POLL" :\
 			       "NOPOLL", opt_teardown ? "Socket Teardown" : "",\
-			       opt_bidi ? "Bi-directional Sockets" : ""))
+			       opt_bidi ? "Bi-directional Sockets" : "",\
+			       opt_trace_enable ? "Trace enabled" : ""))
 
 static void pthread_init_mutex(void)
 {
@@ -342,6 +344,7 @@ static struct option long_options[] = {
 	{"bidi", optional_argument, 0, 'B'},
 	{"debug", optional_argument, 0, 'D'},
 	{"tx-pkt-count", optional_argument, 0, 'C'},
+	{"trace-enable", optional_argument, 0, 't'},
 	{0, 0, 0, 0}
 };
 
@@ -359,7 +362,8 @@ static void usage(const char *prog)
 	    "  -T, --tear-down      Tear down sockets by repeatedly recreating them\n"
 	    "  -B, --bidi           Bi-directional sockets test\n"
 	    "  -D, --debug          Debug mode - dump packets L2 - L5\n"
-	    "  -C, --tx-pkt-count=n Number of packets to send\n";
+	    "  -C, --tx-pkt-count=n Number of packets to send\n"
+	    "  -t, --trace-enable   Enable trace\n";
 	ksft_print_msg(str, prog);
 }
 
@@ -446,7 +450,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:q:pSNcTBDC:", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:q:pSNcTBDC:t", long_options, &option_index);
 
 		if (c == -1)
 			break;
@@ -497,6 +501,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'C':
 			opt_pkt_count = atoi(optarg);
 			break;
+		case 't':
+			opt_trace_enable = 1;
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -803,6 +810,48 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs, pthread_mut
 		exit_with_error(ret);
 }
 
+static int enable_disable_trace(bool enable)
+{
+	FILE *en_fp;
+	int val;
+	int read, ret = 0;
+
+	en_fp = fopen(TRACE_ENABLE_FILE, "r+");
+	if (en_fp == NULL) {
+		ksft_print_msg("Error opening %s\n", TRACE_ENABLE_FILE);
+		return -1;
+	}
+
+	/* Read current value */
+	read = fscanf(en_fp, "%i", &val);
+	if (read != 1) {
+		ksft_print_msg("Error reading from %s\n", TRACE_ENABLE_FILE);
+		ret = -1;
+		goto out_close;
+	}
+
+	if (val != enable) {
+		char w[2];
+
+		snprintf(w, 2, "%d", enable);
+		if (fputs(w, en_fp) == EOF) {
+			ksft_print_msg("Error writing to %s\n", TRACE_ENABLE_FILE);
+			ret = -1;
+		} else {
+			ksft_print_msg("Trace %s\n", enable == 1 ? "enabled" : "disabled");
+		}
+	}
+
+	/* If we are enabling the trace, flag to restore it to its original state (off) on exit */
+	reset_trace = enable;
+
+out_close:
+	fclose(en_fp);
+
+	return ret;
+}
+
+
 static void *worker_testapp_validate(void *arg)
 {
 	struct udphdr *udp_hdr =
@@ -1041,6 +1090,13 @@ int main(int argc, char **argv)
 
 	init_iface_config(ifaceconfig);
 
+	if (opt_trace_enable) {
+		if (enable_disable_trace(1)) {
+			ksft_test_result_fail("ERROR: failed to enable tracing for trace test\n");
+			ksft_exit_xfail();
+		}
+	}
+
 	pthread_init_mutex();
 
 	ksft_set_plan(1);
@@ -1057,6 +1113,9 @@ int main(int argc, char **argv)
 	for (int i = 0; i < MAX_INTERFACES; i++)
 		free(ifdict[i]);
 
+	if (reset_trace)
+		enable_disable_trace(0);
+
 	pthread_destroy_mutex();
 
 	ksft_exit_pass();
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 0e9f9b7e61c2..5308b501eecc 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -41,6 +41,7 @@
 #define BATCH_SIZE 64
 #define POLL_TMOUT 1000
 #define NEED_WAKEUP true
+#define TRACE_ENABLE_FILE "/sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable"
 
 typedef __u32 u32;
 typedef __u16 u16;
@@ -64,6 +65,8 @@ static int opt_poll;
 static int opt_teardown;
 static int opt_bidi;
 static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
+static int opt_trace_enable;
+static int reset_trace;
 static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
 static u32 pkt_counter;
 static u32 prev_pkt = -1;
-- 
2.17.1

[PATCH bpf-next v2 4/6] selftests/bpf: XSK_TRACE_DROP_RXQ_FULL test

[Ciara Loftus]

This test limits the size of the RXQ and does not read
from it. Packets are transmitted. Traces are expected
which report packet drops for packets which could not
fit in the limited size rxq. The test validates that
these traces were successfully generated.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh  |  25 ++++++
 tools/testing/selftests/bpf/xdpxceiver.c | 101 ++++++++++++++++++++---
 tools/testing/selftests/bpf/xdpxceiver.h |   7 ++
 3 files changed, 123 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 2b4a4f42b220..a085ef0602a7 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -83,6 +83,7 @@ done
 
 TEST_NAME="PREREQUISITES"
 DEFAULTPKTS=10000
+TRACEPKTS=64
 
 URANDOM=/dev/urandom
 [ ! -e "${URANDOM}" ] && { echo "${URANDOM} not found. Skipping tests."; test_exit 1 1; }
@@ -246,6 +247,30 @@ retval=$?
 test_status $retval "${TEST_NAME}"
 statusList+=($retval)
 
+### TEST 10
+TEST_NAME="SKB TRACE DROP RXQ_FULL"
+
+vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
+
+params=("-S" "-t" "0" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
+### TEST 11
+TEST_NAME="DRV TRACE DROP RXQ_FULL"
+
+vethXDPnative ${VETH0} ${VETH1} ${NS1}
+
+params=("-N" "-t" "0" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
 ## END TESTS
 
 cleanup_exit ${VETH0} ${VETH1} ${NS1}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index e6e0d42d8074..56bf87d41ab5 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -33,6 +33,8 @@
  *       Configure sockets as bi-directional tx/rx sockets, sets up fill and
  *       completion rings on each socket, tx/rx in both directions. Only nopoll
  *       mode is used
+ *    e. Tracing - XSK_TRACE_DROP_RXQ_FULL
+ *       Reduce the RXQ size and do not read from it. Validate traces.
  *
  * 2. AF_XDP DRV/Native mode
  *    Works on any netdevice with XDP_REDIRECT support, driver dependent. Processes
@@ -44,8 +46,9 @@
  *    d. Bi-directional sockets
  *    - Only copy mode is supported because veth does not currently support
  *      zero-copy mode
+ *    e. Tracing - XSK_TRACE_DROP_RXQ_FULL
  *
- * Total tests: 8
+ * Total tests: 10
  *
  * Flow:
  * -----
@@ -310,7 +313,9 @@ static int xsk_configure_socket(struct ifobject *ifobject)
 		exit_with_error(errno);
 
 	ifobject->xsk->umem = ifobject->umem;
-	cfg.rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+	cfg.rx_size = opt_trace_enable && (opt_trace_code == XSK_TRACE_DROP_RXQ_FULL) ?
+						TRACE_RXQ_FULL_RXQ_SIZE :
+						XSK_RING_CONS__DEFAULT_NUM_DESCS;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
 	cfg.xdp_flags = opt_xdp_flags;
@@ -363,7 +368,7 @@ static void usage(const char *prog)
 	    "  -B, --bidi           Bi-directional sockets test\n"
 	    "  -D, --debug          Debug mode - dump packets L2 - L5\n"
 	    "  -C, --tx-pkt-count=n Number of packets to send\n"
-	    "  -t, --trace-enable   Enable trace\n";
+	    "  -t, --trace-enable=n Enable trace and execute test 'n'\n";
 	ksft_print_msg(str, prog);
 }
 
@@ -450,7 +455,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:q:pSNcTBDC:t", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:q:pSNcTBDC:t:", long_options, &option_index);
 
 		if (c == -1)
 			break;
@@ -503,6 +508,7 @@ static void parse_command_line(int argc, char **argv)
 			break;
 		case 't':
 			opt_trace_enable = 1;
+			opt_trace_code = atoi(optarg);
 			break;
 		default:
 			usage(basename(argv[0]));
@@ -730,6 +736,29 @@ static void worker_pkt_dump(void)
 	}
 }
 
+static void worker_trace_validate(FILE *fp, char *ifname)
+{
+	char trace_str[128];
+	char *line = NULL;
+	size_t len = 0;
+	int ret = 0;
+
+	snprintf(trace_str, sizeof(trace_str),
+		"xsk_packet_drop: netdev: %s qid 0 reason: %s",
+		ifname, reason_str);
+
+	while (trace_counter != expected_traces) {
+		while ((ret = getline(&line, &len, fp)) == EOF)
+			continue;
+		if (strstr(line, trace_str) != NULL)
+			trace_counter++;
+	}
+
+	sigvar = 1;
+
+	fclose(fp);
+}
+
 static void worker_pkt_validate(void)
 {
 	u32 payloadseqnum = -2;
@@ -851,6 +880,25 @@ static int enable_disable_trace(bool enable)
 	return ret;
 }
 
+static FILE *get_eof_trace(void)
+{
+	FILE *ret_fp;
+	char *line = NULL;
+	size_t len = 0;
+	int ret = 0;
+
+	ret_fp = fopen(TRACE_FILE, "r");
+	if (ret_fp == NULL) {
+		ksft_print_msg("Error opening %s\n", TRACE_FILE);
+		return NULL;
+	}
+
+	/* Go to end of file and record the file pointer */
+	while ((ret = getline(&line, &len, ret_fp)) != EOF)
+		;
+
+	return ret_fp;
+}
 
 static void *worker_testapp_validate(void *arg)
 {
@@ -900,12 +948,22 @@ static void *worker_testapp_validate(void *arg)
 		}
 
 		ksft_print_msg("Sending %d packets on interface %s\n",
-			       (opt_pkt_count - 1), ifobject->ifname);
+			       (opt_trace_enable ? opt_pkt_count : opt_pkt_count - 1),
+			       ifobject->ifname);
 		tx_only_all(ifobject);
 	} else if (ifobject->fv.vector == rx) {
 		struct pollfd fds[MAX_SOCKS] = { };
+		FILE *tr_fp = NULL;
 		int ret;
 
+		if (opt_trace_enable) {
+			tr_fp = get_eof_trace();
+			if (tr_fp == NULL) {
+				ksft_print_msg("Error getting EOF of trace\n");
+				exit_with_error(-1);
+			}
+		}
+
 		if (!bidi_pass)
 			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
 
@@ -932,15 +990,22 @@ static void *worker_testapp_validate(void *arg)
 				if (ret <= 0)
 					continue;
 			}
-			rx_pkt(ifobject->xsk, fds);
-			worker_pkt_validate();
+
+			if (!opt_trace_enable) {
+				rx_pkt(ifobject->xsk, fds);
+				worker_pkt_validate();
+			} else {
+				worker_trace_validate(tr_fp, ifobject->ifname);
+			}
 
 			if (sigvar)
 				break;
 		}
 
-		ksft_print_msg("Received %d packets on interface %s\n",
-			       pkt_counter, ifobject->ifname);
+		ksft_print_msg("%s %d packets on interface %s\n",
+			       opt_trace_enable ? "Traced" : "Received",
+			       opt_trace_enable ? trace_counter : pkt_counter,
+			       ifobject->ifname);
 
 		if (opt_teardown)
 			ksft_print_msg("Destroying socket\n");
@@ -1086,7 +1151,7 @@ int main(int argc, char **argv)
 
 	parse_command_line(argc, argv);
 
-	num_frames = ++opt_pkt_count;
+	num_frames = opt_trace_enable ? opt_pkt_count : ++opt_pkt_count;
 
 	init_iface_config(ifaceconfig);
 
@@ -1095,6 +1160,22 @@ int main(int argc, char **argv)
 			ksft_test_result_fail("ERROR: failed to enable tracing for trace test\n");
 			ksft_exit_xfail();
 		}
+		switch (opt_trace_code) {
+		case XSK_TRACE_DROP_RXQ_FULL:
+			if (opt_pkt_count <= TRACE_RXQ_FULL_RXQ_SIZE ||
+				(opt_pkt_count > TRACE_MAX_PKTS)) {
+				ksft_test_result_fail("ERROR: invalid packet count %i\n",
+						      opt_pkt_count);
+				ksft_exit_xfail();
+			}
+			expected_traces = opt_pkt_count - TRACE_RXQ_FULL_RXQ_SIZE;
+			reason_str = "rxq full";
+			break;
+		default:
+			ksft_test_result_fail("ERROR: unsupported trace %i\n",
+						opt_trace_code);
+			ksft_exit_xfail();
+		}
 	}
 
 	pthread_init_mutex();
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 5308b501eecc..f0e50df8b4e0 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -42,6 +42,9 @@
 #define POLL_TMOUT 1000
 #define NEED_WAKEUP true
 #define TRACE_ENABLE_FILE "/sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable"
+#define TRACE_FILE "/sys/kernel/debug/tracing/trace"
+#define TRACE_RXQ_FULL_RXQ_SIZE 32 /* must be smaller than fq */
+#define TRACE_MAX_PKTS 100 /* limit size to avoid filling trace buffer */
 
 typedef __u32 u32;
 typedef __u16 u16;
@@ -66,11 +69,15 @@ static int opt_teardown;
 static int opt_bidi;
 static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
 static int opt_trace_enable;
+static int opt_trace_code;
 static int reset_trace;
 static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
 static u32 pkt_counter;
+static u32 trace_counter;
 static u32 prev_pkt = -1;
 static int sigvar;
+static int expected_traces;
+static const char *reason_str;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
-- 
2.17.1

[PATCH bpf-next v2 5/6] selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test

[Ciara Loftus]

This test increases the UMEM headroom to a size that
will cause packets to be dropped. Traces which report
these drops are expected. The test validates that these
traces were successfully generated.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh  | 24 ++++++++++++++++++++++++
 tools/testing/selftests/bpf/xdpxceiver.c | 21 +++++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index a085ef0602a7..95ceee151de1 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -271,6 +271,30 @@ retval=$?
 test_status $retval "${TEST_NAME}"
 statusList+=($retval)
 
+### TEST 12
+TEST_NAME="SKB TRACE DROP PKT_TOO_BIG"
+
+vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
+
+params=("-S" "-t" "1" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
+### TEST 13
+TEST_NAME="DRV TRACE DROP PKT_TOO_BIG"
+
+vethXDPnative ${VETH0} ${VETH1} ${NS1}
+
+params=("-N" "-t" "1" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
 ## END TESTS
 
 cleanup_exit ${VETH0} ${VETH1} ${NS1}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 56bf87d41ab5..bee10bb686fc 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -35,6 +35,8 @@
  *       mode is used
  *    e. Tracing - XSK_TRACE_DROP_RXQ_FULL
  *       Reduce the RXQ size and do not read from it. Validate traces.
+ *    f. Tracing - XSK_TRACE_DROP_PKT_TOO_BIG
+ *       Increase the headroom size and send packets. Validate traces.
  *
  * 2. AF_XDP DRV/Native mode
  *    Works on any netdevice with XDP_REDIRECT support, driver dependent. Processes
@@ -47,8 +49,9 @@
  *    - Only copy mode is supported because veth does not currently support
  *      zero-copy mode
  *    e. Tracing - XSK_TRACE_DROP_RXQ_FULL
+ *    f. Tracing - XSK_TRACE_DROP_PKT_TOO_BIG
  *
- * Total tests: 10
+ * Total tests: 12
  *
  * Flow:
  * -----
@@ -275,13 +278,23 @@ static void gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
 static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size)
 {
 	int ret;
+	struct xsk_umem_config cfg = {
+		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
+		.comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
+		.frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
+		.frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM,
+		.flags = XSK_UMEM__DEFAULT_FLAGS
+	};
+
+	if (opt_trace_code == XSK_TRACE_DROP_PKT_TOO_BIG)
+		cfg.frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE - XDP_PACKET_HEADROOM - 1;
 
 	data->umem = calloc(1, sizeof(struct xsk_umem_info));
 	if (!data->umem)
 		exit_with_error(errno);
 
 	ret = xsk_umem__create(&data->umem->umem, buffer, size,
-			       &data->umem->fq, &data->umem->cq, NULL);
+			       &data->umem->fq, &data->umem->cq, &cfg);
 	if (ret)
 		exit_with_error(ret);
 
@@ -1171,6 +1184,10 @@ int main(int argc, char **argv)
 			expected_traces = opt_pkt_count - TRACE_RXQ_FULL_RXQ_SIZE;
 			reason_str = "rxq full";
 			break;
+		case XSK_TRACE_DROP_PKT_TOO_BIG:
+			expected_traces = opt_pkt_count;
+			reason_str = "packet too big";
+			break;
 		default:
 			ksft_test_result_fail("ERROR: unsupported trace %i\n",
 						opt_trace_code);
-- 
2.17.1

[PATCH bpf-next v2 6/6] selftests/bpf: XSK_TRACE_DROP_FQ_EMPTY test

[Ciara Loftus]

This test skips the populating of the fill queue which
causes packet drops and traces reporting the drop. The
test validates that these traces were successfully
generated.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh  | 24 ++++++++++++++++++++++++
 tools/testing/selftests/bpf/xdpxceiver.c | 13 +++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 95ceee151de1..997ba0aa79db 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -295,6 +295,30 @@ retval=$?
 test_status $retval "${TEST_NAME}"
 statusList+=($retval)
 
+### TEST 14
+TEST_NAME="SKB TRACE DROP FQ_EMPTY"
+
+vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
+
+params=("-S" "-t" "2" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
+### TEST 15
+TEST_NAME="DRV TRACE DROP FQ_EMPTY"
+
+vethXDPnative ${VETH0} ${VETH1} ${NS1}
+
+params=("-N" "-t" "2" "-C" "${TRACEPKTS}")
+execxdpxceiver params
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
+
 ## END TESTS
 
 cleanup_exit ${VETH0} ${VETH1} ${NS1}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index bee10bb686fc..49c2d42b5882 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -37,6 +37,8 @@
  *       Reduce the RXQ size and do not read from it. Validate traces.
  *    f. Tracing - XSK_TRACE_DROP_PKT_TOO_BIG
  *       Increase the headroom size and send packets. Validate traces.
+ *    g. Tracing - XSK_TRACE_DROP_FQ_EMPTY
+ *       Do not populate the fill queue and send packets. Validate traces.
  *
  * 2. AF_XDP DRV/Native mode
  *    Works on any netdevice with XDP_REDIRECT support, driver dependent. Processes
@@ -50,8 +52,9 @@
  *      zero-copy mode
  *    e. Tracing - XSK_TRACE_DROP_RXQ_FULL
  *    f. Tracing - XSK_TRACE_DROP_PKT_TOO_BIG
+ *    g. Tracing - XSK_TRACE_DROP_FQ_EMPTY
  *
- * Total tests: 12
+ * Total tests: 14
  *
  * Flow:
  * -----
@@ -981,7 +984,9 @@ static void *worker_testapp_validate(void *arg)
 			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
 
 		ksft_print_msg("Interface [%s] vector [Rx]\n", ifobject->ifname);
-		xsk_populate_fill_ring(ifobject->umem);
+		if (opt_trace_code != XSK_TRACE_DROP_FQ_EMPTY)
+			xsk_populate_fill_ring(ifobject->umem);
+
 
 		TAILQ_INIT(&head);
 		if (debug_pkt_dump) {
@@ -1188,6 +1193,10 @@ int main(int argc, char **argv)
 			expected_traces = opt_pkt_count;
 			reason_str = "packet too big";
 			break;
+		case XSK_TRACE_DROP_FQ_EMPTY:
+			expected_traces = opt_pkt_count;
+			reason_str = "fq empty";
+			break;
 		default:
 			ksft_test_result_fail("ERROR: unsupported trace %i\n",
 						opt_trace_code);
-- 
2.17.1

Re: [PATCH bpf-next v2 0/6] AF_XDP Packet Drop Tracing

[Toke Høiland-Jørgensen]

Ciara Loftus <ciara.loftus@intel.com> writes:

> This series introduces tracing infrastructure for AF_XDP sockets (xsks).
> A trace event 'xsk_packet_drop' is created which can be enabled by toggling
>
> /sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable
>
> When enabled and packets are dropped in the kernel, traces are generated
> which describe the reason for the packet drop as well as the netdev and
> qid information of the xsk which encountered the drop.
>
> Example traces:
>
> 507.588563: xsk_packet_drop: netdev: eth0 qid 0 reason: rxq full
> 507.588567: xsk_packet_drop: netdev: eth0 qid 0 reason: packet too big
> 507.588568: xsk_packet_drop: netdev: eth0 qid 0 reason: fq empty
>
> The event can also be monitored using perf:
>
> perf stat -a -e xsk:xsk_packet_drop

Would it make sense to also hook this up to drop_monitor?

-Toke

Re: [PATCH bpf-next v2 1/6] xsk: add tracepoints for packet drops

[Daniel Borkmann]

On 1/26/21 8:52 AM, Ciara Loftus wrote:
> This commit introduces static perf tracepoints for AF_XDP which
> report information about packet drops, such as the netdev, qid and
> high level reason for the drop. The tracepoint can be
> enabled/disabled by toggling
> /sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable

Could you add a rationale to the commit log on why xsk diag stats dump
is insufficient here given you add tracepoints to most locations where
we also bump the counters already? And given diag infra also exposes the
ifindex, queue id, etc, why troubleshooting the xsk socket via ss tool
is not sufficient?

[...]
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 4faabd1ecfd1..9b850716630b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -11,6 +11,7 @@
>   
>   #define pr_fmt(fmt) "AF_XDP: %s: " fmt, __func__
>   
> +#include <linux/bpf_trace.h>
>   #include <linux/if_xdp.h>
>   #include <linux/init.h>
>   #include <linux/sched/mm.h>
> @@ -158,6 +159,7 @@ static int __xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>   	addr = xp_get_handle(xskb);
>   	err = xskq_prod_reserve_desc(xs->rx, addr, len);
>   	if (err) {
> +		trace_xsk_packet_drop(xs->dev->name, xs->queue_id, XSK_TRACE_DROP_RXQ_FULL);
>   		xs->rx_queue_full++;
>   		return err;

I presume if this will be triggered under stress you'll likely also spam
your trace event log w/ potentially mio of msgs per sec?

>   	}
> @@ -192,6 +194,7 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
>   
>   	len = xdp->data_end - xdp->data;
>   	if (len > xsk_pool_get_rx_frame_size(xs->pool)) {
> +		trace_xsk_packet_drop(xs->dev->name, xs->queue_id, XSK_TRACE_DROP_PKT_TOO_BIG);
>   		xs->rx_dropped++;
>   		return -ENOSPC;
>   	}
> @@ -516,6 +519,8 @@ static int xsk_generic_xmit(struct sock *sk)
>   		if (err == NET_XMIT_DROP) {
>   			/* SKB completed but not sent */
>   			err = -EBUSY;
> +			trace_xsk_packet_drop(xs->dev->name, xs->queue_id,
> +					      XSK_TRACE_DROP_DRV_ERR_TX);

Is there a reason to not bump error counter here?

>   			goto out;
>   		}
>   
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 8de01aaac4a0..d3c1ca83c75d 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -1,5 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0
>   

RE: [PATCH bpf-next v2 0/6] AF_XDP Packet Drop Tracing

[Loftus, Ciara]

> 
> Ciara Loftus <ciara.loftus@intel.com> writes:
> 
> > This series introduces tracing infrastructure for AF_XDP sockets (xsks).
> > A trace event 'xsk_packet_drop' is created which can be enabled by
> toggling
> >
> > /sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable
> >
> > When enabled and packets are dropped in the kernel, traces are generated
> > which describe the reason for the packet drop as well as the netdev and
> > qid information of the xsk which encountered the drop.
> >
> > Example traces:
> >
> > 507.588563: xsk_packet_drop: netdev: eth0 qid 0 reason: rxq full
> > 507.588567: xsk_packet_drop: netdev: eth0 qid 0 reason: packet too big
> > 507.588568: xsk_packet_drop: netdev: eth0 qid 0 reason: fq empty
> >
> > The event can also be monitored using perf:
> >
> > perf stat -a -e xsk:xsk_packet_drop
> 
> Would it make sense to also hook this up to drop_monitor?
> 
> -Toke

Thanks for bring that to my attention Toke. I think it makes sense.
I put together a quick prototype and it looks like a good fit.
Neil what do you think?
Perhaps as a follow up patch to the series in question however?

Ciara

RE: [PATCH bpf-next v2 1/6] xsk: add tracepoints for packet drops

[Loftus, Ciara]

> On 1/26/21 8:52 AM, Ciara Loftus wrote:
> > This commit introduces static perf tracepoints for AF_XDP which
> > report information about packet drops, such as the netdev, qid and
> > high level reason for the drop. The tracepoint can be
> > enabled/disabled by toggling
> > /sys/kernel/debug/tracing/events/xsk/xsk_packet_drop/enable
> 
> Could you add a rationale to the commit log on why xsk diag stats dump
> is insufficient here given you add tracepoints to most locations where
> we also bump the counters already? And given diag infra also exposes the
> ifindex, queue id, etc, why troubleshooting the xsk socket via ss tool
> is not sufficient?

Thanks for your feedback Daniel.

The stats tell us that there is *a* problem whereas the traces will shed
light on what that problem is. eg. The XSK_TRACE_DROP_PKT_TOO_BIG
trace tells us we dropped a packet on RX due to it being too big vs. ss
would just tell us the packet was dropped.
I will add this rationale in the v3.

> 
> [...]
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 4faabd1ecfd1..9b850716630b 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -11,6 +11,7 @@
> >
> >   #define pr_fmt(fmt) "AF_XDP: %s: " fmt, __func__
> >
> > +#include <linux/bpf_trace.h>
> >   #include <linux/if_xdp.h>
> >   #include <linux/init.h>
> >   #include <linux/sched/mm.h>
> > @@ -158,6 +159,7 @@ static int __xsk_rcv_zc(struct xdp_sock *xs, struct
> xdp_buff *xdp, u32 len)
> >   	addr = xp_get_handle(xskb);
> >   	err = xskq_prod_reserve_desc(xs->rx, addr, len);
> >   	if (err) {
> > +		trace_xsk_packet_drop(xs->dev->name, xs->queue_id,
> XSK_TRACE_DROP_RXQ_FULL);
> >   		xs->rx_queue_full++;
> >   		return err;
> 
> I presume if this will be triggered under stress you'll likely also spam
> your trace event log w/ potentially mio of msgs per sec?

You are correct. After some consideration I'm going to drop this
trace and some others in the next rev which are not technically
guaranteed drops and could end up spamming the log under
stress as you mentioned.

> 
> >   	}
> > @@ -192,6 +194,7 @@ static int __xsk_rcv(struct xdp_sock *xs, struct
> xdp_buff *xdp)
> >
> >   	len = xdp->data_end - xdp->data;
> >   	if (len > xsk_pool_get_rx_frame_size(xs->pool)) {
> > +		trace_xsk_packet_drop(xs->dev->name, xs->queue_id,
> XSK_TRACE_DROP_PKT_TOO_BIG);
> >   		xs->rx_dropped++;
> >   		return -ENOSPC;
> >   	}
> > @@ -516,6 +519,8 @@ static int xsk_generic_xmit(struct sock *sk)
> >   		if (err == NET_XMIT_DROP) {
> >   			/* SKB completed but not sent */
> >   			err = -EBUSY;
> > +			trace_xsk_packet_drop(xs->dev->name, xs-
> >queue_id,
> > +					      XSK_TRACE_DROP_DRV_ERR_TX);
> 
> Is there a reason to not bump error counter here?

This too falls into the not-technically-a-drop category and will be
removed in the next rev. I think a stat would be useful though.
I'll draw up a separate patch. Thanks for the suggestion.

> 
> >   			goto out;
> >   		}
> >
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 8de01aaac4a0..d3c1ca83c75d 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -1,5 +1,6 @@
> >   // SPDX-License-Identifier: GPL-2.0
> >

Re: [PATCH bpf-next v2 1/6] xsk: add tracepoints for packet drops

[Daniel Borkmann]

On 1/27/21 3:10 PM, Loftus, Ciara wrote:
[...]
> 
> Thanks for your feedback Daniel.
> 
> The stats tell us that there is *a* problem whereas the traces will shed
> light on what that problem is. eg. The XSK_TRACE_DROP_PKT_TOO_BIG
> trace tells us we dropped a packet on RX due to it being too big vs. ss
> would just tell us the packet was dropped.

But wouldn't that just be a matter of extending struct xdp_diag_stats +
xsk_diag_put_stats() with more fine-grained counters? Just wondering given
you add the trace_xsk_packet_drop() tracepoints at locations where we
bump most of these counters already for ss tool. I guess this was my confusion -
as far as I can see only the two XSK_TRACE_DROP_{PKT_TOO_BIG,DRV_ERR_TX} are
not covered in xdp_diag_stats. Is there any other reason that the diag is
not sufficient for your use case?

Thanks,
Daniel