netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing
@ 2021-02-08  9:05 Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 1/6] xsk: add tracepoints for packet drops Ciara Loftus
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, Ciara Loftus

This series introduces tracing infrastructure for AF_XDP sockets (xsk).
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 or empty packet buffers are dropped in the kernel,
traces are generated which describe the reason for the packet drop, the netdev
and qid information of the xsk which encountered the drop, and some more
information depending on what type of drop was encountered that will tell
the user why the packet was dropped.  This information should help a user
troubleshoot packet drops by providing an extra level of detail which is not
available through use of simple counters

Example traces:
xsk_packet_drop: netdev: ve3213 qid 0 reason: packet too big: len 3000 max 2048 not_used 0
xsk_packet_drop: netdev: ve3213 qid 0 reason: invalid fill addr: addr 520192 not_used 0 not_used 0
xsk_packet_drop: netdev: ve9266 qid 0 reason: invalid tx desc: addr 0 len 4097 options 0

It was decided to use a single event 'xsk_packet_drop' to capture these three
drop types. This means that for some of them, there is some redundant information
in the trace marked as 'not_used'. An alternative to this would be to introduce 3
separate event types under xsk, each with their own appropriate trace format.
Suggestions are welcome on which approach would be better to take.

The event can be monitored using perf:
perf stat -a -e xsk:xsk_packet_drop

A selftest is added for each drop type. These tests provide the conditions to
trigger the traces and ensure that the appropriate traces are generated.

v4->v5:
* Removed whitespace and renamed struct name in if_xdp.h as suggested by Song.

v3->v4:
* Fixed selftest commits with correct logs
* Fixed warnings reported by W=1 build: trace argument types and print formatting

v2->v3:
* Removed some traces which traced events which were not technically drops eg.
when the rxq is full.
* Introduced traces for descriptor validation on RX and TX and selftests for both

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 23a2d70c7a2f28eb1a8f6bc19d68d23968cad0ce

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_PKT_TOO_BIG test
  selftests/bpf: XSK_TRACE_INVALID_FILLADDR test
  selftests/bpf: XSK_TRACE_INVALID_DESC_TX test

 MAINTAINERS                                |   1 +
 include/linux/bpf_trace.h                  |   1 +
 include/trace/events/xsk.h                 |  71 +++++++
 include/uapi/linux/if_xdp.h                |   6 +
 kernel/bpf/core.c                          |   1 +
 net/xdp/xsk.c                              |   7 +-
 net/xdp/xsk_buff_pool.c                    |   3 +
 net/xdp/xsk_queue.h                        |   4 +
 tools/include/uapi/linux/if_xdp.h          |   6 +
 tools/testing/selftests/bpf/test_xsk.sh    |  90 ++++++++-
 tools/testing/selftests/bpf/xdpxceiver.c   | 206 +++++++++++++++++++--
 tools/testing/selftests/bpf/xdpxceiver.h   |   9 +
 tools/testing/selftests/bpf/xsk_prereqs.sh |   3 +-
 13 files changed, 379 insertions(+), 29 deletions(-)
 create mode 100644 include/trace/events/xsk.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 1/6] xsk: add tracepoints for packet drops
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 2/6] selftests/bpf: restructure setting the packet count Ciara Loftus
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, Ciara Loftus

This commit introduces tracing infrastructure for AF_XDP sockets
(xsks) and a new trace event called 'xsk_packet_drop'. This trace
event is triggered when a packet cannot be processed by the socket
due to one of the following issues:
(1) packet exceeds the maximum permitted size.
(2) invalid fill descriptor address.
(3) invalid tx descriptor field.

The trace provides information about the error to the user. For
example the size vs permitted size is provided for (1). For (2)
and (3) the relevant descriptor fields are printed. This information
should help a user troubleshoot packet drops by providing this extra
level of detail which is not available through use of simple counters.

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        | 71 +++++++++++++++++++++++++++++++
 include/uapi/linux/if_xdp.h       |  6 +++
 kernel/bpf/core.c                 |  1 +
 net/xdp/xsk.c                     |  7 ++-
 net/xdp/xsk_buff_pool.c           |  3 ++
 net/xdp/xsk_queue.h               |  4 ++
 tools/include/uapi/linux/if_xdp.h |  6 +++
 9 files changed, 99 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..a45d447142a3
--- /dev/null
+++ b/include/trace/events/xsk.h
@@ -0,0 +1,71 @@
+/* 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(reason) \
+	__print_symbolic(reason, \
+			{ XSK_TRACE_DROP_PKT_TOO_BIG, "packet too big" }, \
+			{ XSK_TRACE_DROP_INVALID_FILLADDR, "invalid fill addr" }, \
+			{ XSK_TRACE_DROP_INVALID_TXD, "invalid tx desc" })
+
+#define print_val1(reason) \
+	__print_symbolic(reason, \
+			{ XSK_TRACE_DROP_PKT_TOO_BIG, "len" }, \
+			{ XSK_TRACE_DROP_INVALID_FILLADDR, "addr" }, \
+			{ XSK_TRACE_DROP_INVALID_TXD, "addr" })
+
+#define print_val2(reason) \
+	__print_symbolic(reason, \
+			{ XSK_TRACE_DROP_PKT_TOO_BIG, "max" }, \
+			{ XSK_TRACE_DROP_INVALID_FILLADDR, "not_used" }, \
+			{ XSK_TRACE_DROP_INVALID_TXD, "len" })
+
+#define print_val3(reason) \
+	__print_symbolic(reason, \
+			{ XSK_TRACE_DROP_PKT_TOO_BIG, "not_used" }, \
+			{ XSK_TRACE_DROP_INVALID_FILLADDR, "not_used" }, \
+			{ XSK_TRACE_DROP_INVALID_TXD, "options" })
+
+TRACE_EVENT(xsk_packet_drop,
+
+	TP_PROTO(char *name, u16 queue_id, u32 reason, u64 val1, u64 val2, u64 val3),
+
+	TP_ARGS(name, queue_id, reason, val1, val2, val3),
+
+	TP_STRUCT__entry(
+		__field(char *, name)
+		__field(u16, queue_id)
+		__field(u32, reason)
+		__field(u64, val1)
+		__field(u32, val2)
+		__field(u32, val3)
+	),
+
+	TP_fast_assign(
+		__entry->name = name;
+		__entry->queue_id = queue_id;
+		__entry->reason = reason;
+		__entry->val1 = val1;
+		__entry->val2 = val2;
+		__entry->val3 = val3;
+	),
+
+	TP_printk("netdev: %s qid %u reason: %s: %s %llu %s %u %s %u",
+		  __entry->name, __entry->queue_id, print_reason(__entry->reason),
+		  print_val1(__entry->reason), __entry->val1,
+		  print_val2(__entry->reason), __entry->val2,
+		  print_val3(__entry->reason), __entry->val3
+	)
+);
+
+#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..f7b5ebc2c00d 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -108,4 +108,10 @@ struct xdp_desc {
 
 /* UMEM descriptor is __u64 */
 
+enum xsk_trace_reasons {
+	XSK_TRACE_DROP_PKT_TOO_BIG,
+	XSK_TRACE_DROP_INVALID_FILLADDR,
+	XSK_TRACE_DROP_INVALID_TXD,
+};
+
 #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..689da22c8e4f 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>
@@ -189,9 +190,13 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
 	struct xdp_buff *xsk_xdp;
 	int err;
 	u32 len;
+	u32 max = xsk_pool_get_rx_frame_size(xs->pool);
 
 	len = xdp->data_end - xdp->data;
-	if (len > xsk_pool_get_rx_frame_size(xs->pool)) {
+	if (len > max) {
+		trace_xsk_packet_drop(xs->dev->name, xs->queue_id,
+				      XSK_TRACE_DROP_PKT_TOO_BIG,
+				      len, max, 0);
 		xs->rx_dropped++;
 		return -ENOSPC;
 	}
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 8de01aaac4a0..e0bd1bfd4324 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>
@@ -460,6 +461,8 @@ static struct xdp_buff_xsk *__xp_alloc(struct xsk_buff_pool *pool)
 		ok = pool->unaligned ? xp_check_unaligned(pool, &addr) :
 		     xp_check_aligned(pool, &addr);
 		if (!ok) {
+			trace_xsk_packet_drop(pool->netdev->name, pool->queue_id,
+					       XSK_TRACE_DROP_INVALID_FILLADDR, addr, 0, 0);
 			pool->fq->invalid_descs++;
 			xskq_cons_release(pool->fq);
 			continue;
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 2823b7c3302d..8e9ba3cfe286 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -6,6 +6,7 @@
 #ifndef _LINUX_XSK_QUEUE_H
 #define _LINUX_XSK_QUEUE_H
 
+#include <linux/bpf_trace.h>
 #include <linux/types.h>
 #include <linux/if_xdp.h>
 #include <net/xdp_sock.h>
@@ -175,6 +176,9 @@ static inline bool xskq_cons_is_valid_desc(struct xsk_queue *q,
 					   struct xsk_buff_pool *pool)
 {
 	if (!xp_validate_desc(pool, d)) {
+		trace_xsk_packet_drop(pool->netdev->name, pool->queue_id,
+				       XSK_TRACE_DROP_INVALID_TXD, d->addr,
+				       d->len, d->options);
 		q->invalid_descs++;
 		return false;
 	}
diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index a78a8096f4ce..f7b5ebc2c00d 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -108,4 +108,10 @@ struct xdp_desc {
 
 /* UMEM descriptor is __u64 */
 
+enum xsk_trace_reasons {
+	XSK_TRACE_DROP_PKT_TOO_BIG,
+	XSK_TRACE_DROP_INVALID_FILLADDR,
+	XSK_TRACE_DROP_INVALID_TXD,
+};
+
 #endif /* _LINUX_IF_XDP_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 2/6] selftests/bpf: restructure setting the packet count
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 1/6] xsk: add tracepoints for packet drops Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 3/6] selftests/bpf: add framework for xsk selftests Ciara Loftus
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, Ciara Loftus

Prior to this, the packet count was fixed at 10000 for every test.
Future tracing tests need to modify the count in order to ensure
the trace buffer does not become full. 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 3/6] selftests/bpf: add framework for xsk selftests
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 1/6] xsk: add tracepoints for packet drops Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 2/6] selftests/bpf: restructure setting the packet count Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 4/6] selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test Ciara Loftus
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 4/6] selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
                   ` (2 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH bpf-next v5 3/6] selftests/bpf: add framework for xsk selftests Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 5/6] selftests/bpf: XSK_TRACE_INVALID_FILLADDR test Ciara Loftus
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, 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 which look like so:

xsk_packet_drop: netdev: ve3213 qid 0 reason: packet too big: \
  len 60 max 1 not_used 0

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 | 102 ++++++++++++++++++++---
 tools/testing/selftests/bpf/xdpxceiver.h |   6 ++
 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..5b4e42ac8c20 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 PKT_TOO_BIG"
+
+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 PKT_TOO_BIG"
+
+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..dda965c3d9ec 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_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
@@ -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_PKT_TOO_BIG
  *
- * Total tests: 8
+ * Total tests: 10
  *
  * Flow:
  * -----
@@ -272,13 +275,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);
 
@@ -363,7 +376,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 +463,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 +516,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 +744,28 @@ 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), "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 +887,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 +955,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 +997,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,15 +1158,25 @@ 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);
 
 	if (opt_trace_enable) {
+		expected_traces = opt_pkt_count;
 		if (enable_disable_trace(1)) {
 			ksft_test_result_fail("ERROR: failed to enable tracing for trace test\n");
 			ksft_exit_xfail();
 		}
+		switch (opt_trace_code) {
+		case XSK_TRACE_DROP_PKT_TOO_BIG:
+			reason_str = "packet too big";
+			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..4cdb8fe81837 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -42,6 +42,8 @@
 #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_MAX_PKTS 100 /* limit size to avoid filling trace buffer */
 
 typedef __u32 u32;
 typedef __u16 u16;
@@ -66,11 +68,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 = -1;
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 5/6] selftests/bpf: XSK_TRACE_INVALID_FILLADDR test
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
                   ` (3 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH bpf-next v5 4/6] selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-08  9:05 ` [PATCH bpf-next v5 6/6] selftests/bpf: XSK_TRACE_INVALID_DESC_TX test Ciara Loftus
  2021-02-11  8:03 ` [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Magnus Karlsson
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, Ciara Loftus

This test supplies invalid addresses to the fill
queue at init. On RX, traces are expected which report
the invalid address which look like so:

xsk_packet_drop: netdev: ve3213 qid 0 reason: invalid fill addr: \
  addr 262144 not_used 0 not_used 0

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 | 29 ++++++++++++++++++------
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 5b4e42ac8c20..bc026da25d54 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 INVALID_FILLADDR"
+
+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 INVALID_FILLADDR"
+
+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 dda965c3d9ec..e63dc1c228ed 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_PKT_TOO_BIG
  *       Increase the headroom size and send packets. Validate traces.
+ *    f. Tracing - XSK_TRACE_DROP_INVALID_FILLADDR
+ *       Populate the fill queue with invalid addresses. 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_PKT_TOO_BIG
+ *    f. Tracing - XSK_TRACE_DROP_INVALID_FILLADDR
  *
- * Total tests: 10
+ * Total tests: 12
  *
  * Flow:
  * -----
@@ -276,7 +279,8 @@ 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,
+		.fill_size = opt_trace_code == XSK_TRACE_DROP_INVALID_FILLADDR ? opt_pkt_count :
+						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,
@@ -302,13 +306,21 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
 {
 	int ret, i;
 	u32 idx;
+	u32 num_addrs = XSK_RING_PROD__DEFAULT_NUM_DESCS;
+	u32 invalid = 0;
 
-	ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
-	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
+	if (opt_trace_code == XSK_TRACE_DROP_INVALID_FILLADDR) {
+		num_addrs = opt_pkt_count;
+		invalid = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
+	}
+
+	ret = xsk_ring_prod__reserve(&umem->fq, num_addrs, &idx);
+	if (ret != num_addrs)
 		exit_with_error(ret);
-	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
-		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * XSK_UMEM__DEFAULT_FRAME_SIZE;
-	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
+	for (i = 0; i < num_addrs; i++)
+		*xsk_ring_prod__fill_addr(&umem->fq, idx++) =
+			(i * XSK_UMEM__DEFAULT_FRAME_SIZE) + invalid;
+	xsk_ring_prod__submit(&umem->fq, num_addrs);
 }
 
 static int xsk_configure_socket(struct ifobject *ifobject)
@@ -1172,6 +1184,9 @@ int main(int argc, char **argv)
 		case XSK_TRACE_DROP_PKT_TOO_BIG:
 			reason_str = "packet too big";
 			break;
+		case XSK_TRACE_DROP_INVALID_FILLADDR:
+			reason_str = "invalid fill addr";
+			break;
 		default:
 			ksft_test_result_fail("ERROR: unsupported trace %i\n",
 						opt_trace_code);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v5 6/6] selftests/bpf: XSK_TRACE_INVALID_DESC_TX test
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
                   ` (4 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH bpf-next v5 5/6] selftests/bpf: XSK_TRACE_INVALID_FILLADDR test Ciara Loftus
@ 2021-02-08  9:05 ` Ciara Loftus
  2021-02-11  8:03 ` [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Magnus Karlsson
  6 siblings, 0 replies; 9+ messages in thread
From: Ciara Loftus @ 2021-02-08  9:05 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua
  Cc: daniel, song, Ciara Loftus

This test sets the length of tx descriptors to an invalid
value. When the kernel tries to transmit these descriptors,
error traces are expected which look like so:

xsk_packet_drop: netdev: ve9266 qid 0 reason: invalid tx desc: \
  addr 258048 len 4097 options 0

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 | 22 ++++++++++++++++++----
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index bc026da25d54..4e654eb0a595 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 INVALID_DESC_TX"
+
+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 INVALID_DESC_TX"
+
+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 e63dc1c228ed..6cf824a33fdc 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -37,6 +37,8 @@
  *       Increase the headroom size and send packets. Validate traces.
  *    f. Tracing - XSK_TRACE_DROP_INVALID_FILLADDR
  *       Populate the fill queue with invalid addresses. Validate traces.
+ *    g. Tracing - XSK_TRACE_DROP_INVALID_TXD
+ *       Populate the tx descriptors with invalid addresses. 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_PKT_TOO_BIG
  *    f. Tracing - XSK_TRACE_DROP_INVALID_FILLADDR
+ *    g. Tracing - XSK_TRACE_DROP_INVALID_TXD
  *
- * Total tests: 12
+ * Total tests: 14
  *
  * Flow:
  * -----
@@ -560,8 +563,11 @@ static inline void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
 	if (!xsk->outstanding_tx)
 		return;
 
-	if (!NEED_WAKEUP || xsk_ring_prod__needs_wakeup(&xsk->tx))
+	if (!NEED_WAKEUP || xsk_ring_prod__needs_wakeup(&xsk->tx)) {
 		kick_tx(xsk);
+		if (opt_trace_code == XSK_TRACE_DROP_INVALID_TXD)
+			xsk->outstanding_tx = 0;
+	}
 
 	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
 	if (rcvd) {
@@ -632,6 +638,7 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
 {
 	u32 idx;
 	unsigned int i;
+	bool invalid_tx_test = opt_trace_code == XSK_TRACE_DROP_INVALID_TXD;
 
 	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
 		complete_tx_only(xsk, batch_size);
@@ -640,7 +647,8 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
 		struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
 
 		tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
-		tx_desc->len = PKT_SIZE;
+		tx_desc->len = invalid_tx_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
+
 	}
 
 	xsk_ring_prod__submit(&xsk->tx, batch_size);
@@ -1014,7 +1022,10 @@ static void *worker_testapp_validate(void *arg)
 				rx_pkt(ifobject->xsk, fds);
 				worker_pkt_validate();
 			} else {
-				worker_trace_validate(tr_fp, ifobject->ifname);
+				worker_trace_validate(tr_fp,
+					opt_trace_code == XSK_TRACE_DROP_INVALID_TXD ?
+					ifdict[!ifobject->ifdict_index]->ifname :
+					ifobject->ifname);
 			}
 
 			if (sigvar)
@@ -1187,6 +1198,9 @@ int main(int argc, char **argv)
 		case XSK_TRACE_DROP_INVALID_FILLADDR:
 			reason_str = "invalid fill addr";
 			break;
+		case XSK_TRACE_DROP_INVALID_TXD:
+			reason_str = "invalid tx desc";
+			break;
 		default:
 			ksft_test_result_fail("ERROR: unsupported trace %i\n",
 						opt_trace_code);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing
  2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
                   ` (5 preceding siblings ...)
  2021-02-08  9:05 ` [PATCH bpf-next v5 6/6] selftests/bpf: XSK_TRACE_INVALID_DESC_TX test Ciara Loftus
@ 2021-02-11  8:03 ` Magnus Karlsson
  2021-02-11  8:18   ` Loftus, Ciara
  6 siblings, 1 reply; 9+ messages in thread
From: Magnus Karlsson @ 2021-02-11  8:03 UTC (permalink / raw)
  To: Ciara Loftus
  Cc: Network Development, bpf, Karlsson, Magnus,
	Björn Töpel, Weqaar Janjua, Daniel Borkmann, song

On Mon, Feb 8, 2021 at 10:39 AM Ciara Loftus <ciara.loftus@intel.com> wrote:
>
> This series introduces tracing infrastructure for AF_XDP sockets (xsk).
> 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 or empty packet buffers are dropped in the kernel,
> traces are generated which describe the reason for the packet drop, the netdev
> and qid information of the xsk which encountered the drop, and some more
> information depending on what type of drop was encountered that will tell
> the user why the packet was dropped.  This information should help a user
> troubleshoot packet drops by providing an extra level of detail which is not
> available through use of simple counters
>
> Example traces:
> xsk_packet_drop: netdev: ve3213 qid 0 reason: packet too big: len 3000 max 2048 not_used 0
> xsk_packet_drop: netdev: ve3213 qid 0 reason: invalid fill addr: addr 520192 not_used 0 not_used 0
> xsk_packet_drop: netdev: ve9266 qid 0 reason: invalid tx desc: addr 0 len 4097 options 0
>
> It was decided to use a single event 'xsk_packet_drop' to capture these three
> drop types. This means that for some of them, there is some redundant information
> in the trace marked as 'not_used'. An alternative to this would be to introduce 3
> separate event types under xsk, each with their own appropriate trace format.
> Suggestions are welcome on which approach would be better to take.
>
> The event can be monitored using perf:
> perf stat -a -e xsk:xsk_packet_drop
>
> A selftest is added for each drop type. These tests provide the conditions to
> trigger the traces and ensure that the appropriate traces are generated.

So what you have done now is to remove all the trace points that
provided no added information on top of the stats counters. The ones
you have left, provide extra information. The two  XSK_TRACE_INVALID_*
points provide the reason why the descriptor was dropped and
XSK_TRACE_DROP_PKT_TOO_BIG provides the size of the packet that was
dropped.

However, the XSK_TRACE_INVALID checks could be performed from user
space and the same data could be printed out from there. A developer
could add this, or we could have a verification mode in the ring
access functions in libbpf that could be turned on by the developer if
he sees the error counters in the kernel being increased. That only
leaves us with XSK_TRACE_DROP_PKT_TOO_BIG which cannot be tested by
user space since the ingress packet is being dropped by the kernel.
But this is unfortunately not enough to warrant this trace
infrastructure on its own, so I think we should drop this patch set.

But I would really like to salvage all your tests, because they are
needed. Instead of verifying the trace, could you please verify the
stats counters that are already there? A lot of your code will be
applicable for that case too. So my suggestion is that you drop this
patch set and  produce a new one that only focuses on selftests for
the XDP_STATISTICS getsockopt. You can add some of the tests you had
in your v2. What do you think?

Thank you.

> v4->v5:
> * Removed whitespace and renamed struct name in if_xdp.h as suggested by Song.
>
> v3->v4:
> * Fixed selftest commits with correct logs
> * Fixed warnings reported by W=1 build: trace argument types and print formatting
>
> v2->v3:
> * Removed some traces which traced events which were not technically drops eg.
> when the rxq is full.
> * Introduced traces for descriptor validation on RX and TX and selftests for both
>
> 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 23a2d70c7a2f28eb1a8f6bc19d68d23968cad0ce
>
> 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_PKT_TOO_BIG test
>   selftests/bpf: XSK_TRACE_INVALID_FILLADDR test
>   selftests/bpf: XSK_TRACE_INVALID_DESC_TX test
>
>  MAINTAINERS                                |   1 +
>  include/linux/bpf_trace.h                  |   1 +
>  include/trace/events/xsk.h                 |  71 +++++++
>  include/uapi/linux/if_xdp.h                |   6 +
>  kernel/bpf/core.c                          |   1 +
>  net/xdp/xsk.c                              |   7 +-
>  net/xdp/xsk_buff_pool.c                    |   3 +
>  net/xdp/xsk_queue.h                        |   4 +
>  tools/include/uapi/linux/if_xdp.h          |   6 +
>  tools/testing/selftests/bpf/test_xsk.sh    |  90 ++++++++-
>  tools/testing/selftests/bpf/xdpxceiver.c   | 206 +++++++++++++++++++--
>  tools/testing/selftests/bpf/xdpxceiver.h   |   9 +
>  tools/testing/selftests/bpf/xsk_prereqs.sh |   3 +-
>  13 files changed, 379 insertions(+), 29 deletions(-)
>  create mode 100644 include/trace/events/xsk.h
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing
  2021-02-11  8:03 ` [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Magnus Karlsson
@ 2021-02-11  8:18   ` Loftus, Ciara
  0 siblings, 0 replies; 9+ messages in thread
From: Loftus, Ciara @ 2021-02-11  8:18 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: Network Development, bpf, Karlsson, Magnus,
	Björn Töpel, Janjua, Weqaar A, Daniel Borkmann, song

> >
> > This series introduces tracing infrastructure for AF_XDP sockets (xsk).
> > 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 or empty packet buffers are dropped in the
> kernel,
> > traces are generated which describe the reason for the packet drop, the
> netdev
> > and qid information of the xsk which encountered the drop, and some
> more
> > information depending on what type of drop was encountered that will tell
> > the user why the packet was dropped.  This information should help a user
> > troubleshoot packet drops by providing an extra level of detail which is not
> > available through use of simple counters
> >
> > Example traces:
> > xsk_packet_drop: netdev: ve3213 qid 0 reason: packet too big: len 3000
> max 2048 not_used 0
> > xsk_packet_drop: netdev: ve3213 qid 0 reason: invalid fill addr: addr 520192
> not_used 0 not_used 0
> > xsk_packet_drop: netdev: ve9266 qid 0 reason: invalid tx desc: addr 0 len
> 4097 options 0
> >
> > It was decided to use a single event 'xsk_packet_drop' to capture these
> three
> > drop types. This means that for some of them, there is some redundant
> information
> > in the trace marked as 'not_used'. An alternative to this would be to
> introduce 3
> > separate event types under xsk, each with their own appropriate trace
> format.
> > Suggestions are welcome on which approach would be better to take.
> >
> > The event can be monitored using perf:
> > perf stat -a -e xsk:xsk_packet_drop
> >
> > A selftest is added for each drop type. These tests provide the conditions
> to
> > trigger the traces and ensure that the appropriate traces are generated.
> 
> So what you have done now is to remove all the trace points that
> provided no added information on top of the stats counters. The ones
> you have left, provide extra information. The two  XSK_TRACE_INVALID_*
> points provide the reason why the descriptor was dropped and
> XSK_TRACE_DROP_PKT_TOO_BIG provides the size of the packet that was
> dropped.
> 
> However, the XSK_TRACE_INVALID checks could be performed from user
> space and the same data could be printed out from there. A developer
> could add this, or we could have a verification mode in the ring
> access functions in libbpf that could be turned on by the developer if
> he sees the error counters in the kernel being increased. That only
> leaves us with XSK_TRACE_DROP_PKT_TOO_BIG which cannot be tested by
> user space since the ingress packet is being dropped by the kernel.
> But this is unfortunately not enough to warrant this trace
> infrastructure on its own, so I think we should drop this patch set.
> 
> But I would really like to salvage all your tests, because they are
> needed. Instead of verifying the trace, could you please verify the
> stats counters that are already there? A lot of your code will be
> applicable for that case too. So my suggestion is that you drop this
> patch set and  produce a new one that only focuses on selftests for
> the XDP_STATISTICS getsockopt. You can add some of the tests you had
> in your v2. What do you think?

Thanks for your feedback Magnus. I see your point. If it's possible to do the invalid desc checking in userspace then this probably doesn't belong in the kernel, and keeping the entire tracing infrastructure just for one trace would be too heavy.

I like your suggestion re stats selftests. I'll submit something along those lines.

Thanks,
Ciara

> 
> Thank you.
> 
> > v4->v5:
> > * Removed whitespace and renamed struct name in if_xdp.h as suggested
> by Song.
> >
> > v3->v4:
> > * Fixed selftest commits with correct logs
> > * Fixed warnings reported by W=1 build: trace argument types and print
> formatting
> >
> > v2->v3:
> > * Removed some traces which traced events which were not technically
> drops eg.
> > when the rxq is full.
> > * Introduced traces for descriptor validation on RX and TX and selftests for
> both
> >
> > 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
> 23a2d70c7a2f28eb1a8f6bc19d68d23968cad0ce
> >
> > 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_PKT_TOO_BIG test
> >   selftests/bpf: XSK_TRACE_INVALID_FILLADDR test
> >   selftests/bpf: XSK_TRACE_INVALID_DESC_TX test
> >
> >  MAINTAINERS                                |   1 +
> >  include/linux/bpf_trace.h                  |   1 +
> >  include/trace/events/xsk.h                 |  71 +++++++
> >  include/uapi/linux/if_xdp.h                |   6 +
> >  kernel/bpf/core.c                          |   1 +
> >  net/xdp/xsk.c                              |   7 +-
> >  net/xdp/xsk_buff_pool.c                    |   3 +
> >  net/xdp/xsk_queue.h                        |   4 +
> >  tools/include/uapi/linux/if_xdp.h          |   6 +
> >  tools/testing/selftests/bpf/test_xsk.sh    |  90 ++++++++-
> >  tools/testing/selftests/bpf/xdpxceiver.c   | 206 +++++++++++++++++++--
> >  tools/testing/selftests/bpf/xdpxceiver.h   |   9 +
> >  tools/testing/selftests/bpf/xsk_prereqs.sh |   3 +-
> >  13 files changed, 379 insertions(+), 29 deletions(-)
> >  create mode 100644 include/trace/events/xsk.h
> >
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-02-11  8:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08  9:05 [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 1/6] xsk: add tracepoints for packet drops Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 2/6] selftests/bpf: restructure setting the packet count Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 3/6] selftests/bpf: add framework for xsk selftests Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 4/6] selftests/bpf: XSK_TRACE_DROP_PKT_TOO_BIG test Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 5/6] selftests/bpf: XSK_TRACE_INVALID_FILLADDR test Ciara Loftus
2021-02-08  9:05 ` [PATCH bpf-next v5 6/6] selftests/bpf: XSK_TRACE_INVALID_DESC_TX test Ciara Loftus
2021-02-11  8:03 ` [PATCH bpf-next v5 0/6] AF_XDP Packet Drop Tracing Magnus Karlsson
2021-02-11  8:18   ` Loftus, Ciara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).