[PATCH bpf-next 0/3] samples/bpf: improve xdpsock application

This small series improves/fixes three things in the xdpsock sample
application. Details can be found in the individual commit messages,
but a brief summary follows:

Patch 1: fix one packet sending in xdpsock
Patch 2: fix possible deadlock in xdpsock
Patch 3: add quiet option to xdpsock

This patch has been applied against commit 8081ede1f731 ("perf: Stop using deprecated bpf_program__title()")

Thanks: Magnus

Magnus Karlsson (3):
  samples/bpf: fix one packet sending in xdpsock
  samples/bpf: fix possible deadlock in xdpsock
  samples/bpf: add quiet option to xdpsock

 samples/bpf/xdpsock_user.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

--
2.7.4

From: Magnus Karlsson <magnus.karlsson@intel.com>

Fix a possible deadlock in the l2fwd application in xdpsock that can
occur when there is no space in the Tx ring. There are two ways to get
the kernel to consume entries in the Tx ring: calling sendto() to make
it send packets and freeing entries from the completion ring, as the
kernel will not send a packet if there is no space for it to add a
completion entry in the completion ring. The Tx loop in l2fwd only
used to call sendto(). This patches adds cleaning the completion ring
in that loop.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 samples/bpf/xdpsock_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index b6175cb..b60bf4e 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c

@@ -1125,6 +1125,7 @@ static void l2fwd(struct xsk_socket_info *xsk, struct pollfd *fds)
 	while (ret != rcvd) {
 		if (ret < 0)
 			exit_with_error(-ret);
+		complete_tx_l2fwd(xsk, fds);
 		if (xsk_ring_prod__needs_wakeup(&xsk->tx))
 			kick_tx(xsk);
 		ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);

-- 
2.7.4

From: Magnus Karlsson <magnus.karlsson@intel.com>

Fix the sending of a single packet (or small burst) in xdpsock when
executing in copy mode. Currently, the l2fwd application in xdpsock
only transmits the packets after a batch of them has been received,
which might be confusing if you only send one packet and expect that
it is returned pronto. Fix this by calling sendto() more often and add
a comment in the code that states that this can be optimized if
needed.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Reported-by: Tirthendu Sarkar <redacted>
---
 samples/bpf/xdpsock_user.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 4cead34..b6175cb 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c

@@ -897,6 +897,14 @@ static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk,
 	if (!xsk->outstanding_tx)
 		return;
 
+	/* In copy mode, Tx is driven by a syscall so we need to use e.g. sendto() to
+	 * really send the packets. In zero-copy mode we do not have to do this, since Tx
+	 * is driven by the NAPI loop. So as an optimization, we do not have to call
+	 * sendto() all the time in zero-copy mode for l2fwd.
+	 */
+	if (opt_xdp_bind_flags & XDP_COPY)
+		kick_tx(xsk);
+
 	ndescs = (xsk->outstanding_tx > opt_batch_size) ? opt_batch_size :
 		xsk->outstanding_tx;
 

-- 
2.7.4

From: Magnus Karlsson <magnus.karlsson@intel.com>

Add a quiet option (-Q) that disables the statistics print outs of
xdpsock. This is good to have when measuring 0% loss rate performance
as it will be quite terrible if the application uses printfs.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 samples/bpf/xdpsock_user.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index b60bf4e..b220173 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c

@@ -78,6 +78,7 @@ static int opt_pkt_count;
 static u16 opt_pkt_size = MIN_PKT_SIZE;
 static u32 opt_pkt_fill_pattern = 0x12345678;
 static bool opt_extra_stats;
+static bool opt_quiet;
 static int opt_poll;
 static int opt_interval = 1;
 static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;

@@ -718,6 +719,7 @@ static struct option long_options[] = {
 	{"tx-pkt-size", required_argument, 0, 's'},
 	{"tx-pkt-pattern", required_argument, 0, 'P'},
 	{"extra-stats", no_argument, 0, 'x'},
+	{"quiet", no_argument, 0, 'Q'},
 	{0, 0, 0, 0}
 };
 

@@ -753,6 +755,7 @@ static void usage(const char *prog)
 		"			Min size: %d, Max size %d.\n"
 		"  -P, --tx-pkt-pattern=nPacket fill pattern. Default: 0x%x\n"
 		"  -x, --extra-stats	Display extra statistics.\n"
+		"  -Q, --quiet          Do not display any stats.\n"
 		"\n";
 	fprintf(stderr, str, prog, XSK_UMEM__DEFAULT_FRAME_SIZE,
 		opt_batch_size, MIN_PKT_SIZE, MIN_PKT_SIZE,

@@ -768,7 +771,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "Frtli:q:pSNn:czf:muMd:b:C:s:P:x",
+		c = getopt_long(argc, argv, "Frtli:q:pSNn:czf:muMd:b:C:s:P:xQ",
 				long_options, &option_index);
 		if (c == -1)
 			break;

@@ -852,6 +855,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'x':
 			opt_extra_stats = 1;
 			break;
+		case 'Q':
+			opt_quiet = 1;
+			break;
 		default:
 			usage(basename(argv[0]));
 		}

@@ -1286,9 +1292,11 @@ int main(int argc, char **argv)
 
 	setlocale(LC_ALL, "");
 
-	ret = pthread_create(&pt, NULL, poller, NULL);
-	if (ret)
-		exit_with_error(ret);
+	if (!opt_quiet) {
+		ret = pthread_create(&pt, NULL, poller, NULL);
+		if (ret)
+			exit_with_error(ret);
+	}
 
 	prev_time = get_nsecs();
 	start_time = prev_time;

@@ -1302,7 +1310,8 @@ int main(int argc, char **argv)
 
 	benchmark_done = true;
 
-	pthread_join(pt, NULL);
+	if (!opt_quiet)
+		pthread_join(pt, NULL);
 
 	xdpsock_cleanup();
 

-- 
2.7.4

On Thu, Sep 10, 2020 at 1:31 AM Magnus Karlsson
[off-list ref] wrote:

This small series improves/fixes three things in the xdpsock sample
application. Details can be found in the individual commit messages,
but a brief summary follows:

Patch 1: fix one packet sending in xdpsock
Patch 2: fix possible deadlock in xdpsock
Patch 3: add quiet option to xdpsock

This patch has been applied against commit 8081ede1f731 ("perf: Stop using deprecated bpf_program__title()")

Thanks: Magnus

Applied. Thanks