[PATCH bpf-next 00/12] Various cleanups/fixes for AF_XDP selftests

[PATCH bpf-next 00/12] Various cleanups/fixes for AF_XDP selftests

[Björn Töpel]

This series is a number of fixes/cleanups, mainly to improve the
readability of the xdpxceiver selftest application.

Details in each commit!


Cheers,
Björn

Björn Töpel (12):
  selftests/bpf: remove a lot of ifobject casting
  selftests/bpf: remove unused enums
  selftests/bpf: fix style warnings
  selftests/bpf: remove memory leak
  selftests/bpf: improve readability of xdpxceiver/worker_pkt_validate()
  selftests/bpf: remove casting by introduce local variable
  selftests/bpf: change type from void * to struct ifaceconfigobj *
  selftests/bpf: change type from void * to struct generic_data *
  selftests/bpf: define local variables at the beginning of a block
  selftests/bpf: avoid heap allocation
  selftests/bpf: consistent malloc/calloc usage
  selftests/bpf: avoid useless void *-casts

 tools/testing/selftests/bpf/xdpxceiver.c | 219 +++++++++++------------
 tools/testing/selftests/bpf/xdpxceiver.h |   2 -
 2 files changed, 105 insertions(+), 116 deletions(-)


base-commit: 443edcefb8213155c0da22c4a999f4a49858fa39
-- 
2.27.0

[PATCH bpf-next 01/12] selftests/bpf: remove a lot of ifobject casting

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Instead of passing void * all over the place, let us pass the actual
type (ifobject) and remove the void-ptr-to-type-ptr casting.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 88 ++++++++++++------------
 1 file changed, 43 insertions(+), 45 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 1e722ee76b1f..cd1dd2b7458f 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -224,14 +224,14 @@ static inline u16 udp_csum(u32 saddr, u32 daddr, u32 len, u8 proto, u16 *udp_pkt
 	return csum_tcpudp_magic(saddr, daddr, len, proto, csum);
 }
 
-static void gen_eth_hdr(void *data, struct ethhdr *eth_hdr)
+static void gen_eth_hdr(struct ifobject *ifobject, struct ethhdr *eth_hdr)
 {
-	memcpy(eth_hdr->h_dest, ((struct ifobject *)data)->dst_mac, ETH_ALEN);
-	memcpy(eth_hdr->h_source, ((struct ifobject *)data)->src_mac, ETH_ALEN);
+	memcpy(eth_hdr->h_dest, ifobject->dst_mac, ETH_ALEN);
+	memcpy(eth_hdr->h_source, ifobject->src_mac, ETH_ALEN);
 	eth_hdr->h_proto = htons(ETH_P_IP);
 }
 
-static void gen_ip_hdr(void *data, struct iphdr *ip_hdr)
+static void gen_ip_hdr(struct ifobject *ifobject, struct iphdr *ip_hdr)
 {
 	ip_hdr->version = IP_PKT_VER;
 	ip_hdr->ihl = 0x5;
@@ -241,15 +241,15 @@ static void gen_ip_hdr(void *data, struct iphdr *ip_hdr)
 	ip_hdr->frag_off = 0;
 	ip_hdr->ttl = IPDEFTTL;
 	ip_hdr->protocol = IPPROTO_UDP;
-	ip_hdr->saddr = ((struct ifobject *)data)->src_ip;
-	ip_hdr->daddr = ((struct ifobject *)data)->dst_ip;
+	ip_hdr->saddr = ifobject->src_ip;
+	ip_hdr->daddr = ifobject->dst_ip;
 	ip_hdr->check = 0;
 }
 
-static void gen_udp_hdr(void *data, void *arg, struct udphdr *udp_hdr)
+static void gen_udp_hdr(void *data, struct ifobject *ifobject, struct udphdr *udp_hdr)
 {
-	udp_hdr->source = htons(((struct ifobject *)arg)->src_port);
-	udp_hdr->dest = htons(((struct ifobject *)arg)->dst_port);
+	udp_hdr->source = htons(ifobject->src_port);
+	udp_hdr->dest = htons(ifobject->dst_port);
 	udp_hdr->len = htons(UDP_PKT_SIZE);
 	memset32_htonl(pkt_data + PKT_HDR_SIZE,
 		       htonl(((struct generic_data *)data)->seqnum), UDP_PKT_DATA_SIZE);
@@ -628,28 +628,27 @@ static inline int get_batch_size(int pkt_cnt)
 	return opt_pkt_count - pkt_cnt;
 }
 
-static void complete_tx_only_all(void *arg)
+static void complete_tx_only_all(struct ifobject *ifobject)
 {
 	bool pending;
 
 	do {
 		pending = false;
-		if (((struct ifobject *)arg)->xsk->outstanding_tx) {
-			complete_tx_only(((struct ifobject *)
-					  arg)->xsk, BATCH_SIZE);
-			pending = !!((struct ifobject *)arg)->xsk->outstanding_tx;
+		if (ifobject->xsk->outstanding_tx) {
+			complete_tx_only(ifobject->xsk, BATCH_SIZE);
+			pending = !!ifobject->xsk->outstanding_tx;
 		}
 	} while (pending);
 }
 
-static void tx_only_all(void *arg)
+static void tx_only_all(struct ifobject *ifobject)
 {
 	struct pollfd fds[MAX_SOCKS] = { };
 	u32 frame_nb = 0;
 	int pkt_cnt = 0;
 	int ret;
 
-	fds[0].fd = xsk_socket__fd(((struct ifobject *)arg)->xsk->xsk);
+	fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
 	fds[0].events = POLLOUT;
 
 	while ((opt_pkt_count && pkt_cnt < opt_pkt_count) || !opt_pkt_count) {
@@ -664,12 +663,12 @@ static void tx_only_all(void *arg)
 				continue;
 		}
 
-		tx_only(((struct ifobject *)arg)->xsk, &frame_nb, batch_size);
+		tx_only(ifobject->xsk, &frame_nb, batch_size);
 		pkt_cnt += batch_size;
 	}
 
 	if (opt_pkt_count)
-		complete_tx_only_all(arg);
+		complete_tx_only_all(ifobject);
 }
 
 static void worker_pkt_dump(void)
@@ -780,14 +779,14 @@ static void worker_pkt_validate(void)
 	}
 }
 
-static void thread_common_ops(void *arg, void *bufs, pthread_mutex_t *mutexptr,
+static void thread_common_ops(struct ifobject *ifobject, void *bufs, pthread_mutex_t *mutexptr,
 			      atomic_int *spinningptr)
 {
 	int ctr = 0;
 	int ret;
 
-	xsk_configure_umem((struct ifobject *)arg, bufs, num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE);
-	ret = xsk_configure_socket((struct ifobject *)arg);
+	xsk_configure_umem(ifobject, bufs, num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE);
+	ret = xsk_configure_socket(ifobject);
 
 	/* Retry Create Socket if it fails as xsk_socket__create()
 	 * is asynchronous
@@ -798,9 +797,8 @@ static void thread_common_ops(void *arg, void *bufs, pthread_mutex_t *mutexptr,
 	pthread_mutex_lock(mutexptr);
 	while (ret && ctr < SOCK_RECONF_CTR) {
 		atomic_store(spinningptr, 1);
-		xsk_configure_umem((struct ifobject *)arg,
-				   bufs, num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE);
-		ret = xsk_configure_socket((struct ifobject *)arg);
+		xsk_configure_umem(ifobject, bufs, num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE);
+		ret = xsk_configure_socket(ifobject);
 		usleep(USLEEP_MAX);
 		ctr++;
 	}
@@ -818,6 +816,7 @@ static void *worker_testapp_validate(void *arg)
 	struct generic_data *data = (struct generic_data *)malloc(sizeof(struct generic_data));
 	struct iphdr *ip_hdr = (struct iphdr *)(pkt_data + sizeof(struct ethhdr));
 	struct ethhdr *eth_hdr = (struct ethhdr *)pkt_data;
+	struct ifobject *ifobject = (struct ifobject *)arg;
 	void *bufs = NULL;
 
 	pthread_attr_setstacksize(&attr, THREAD_STACK);
@@ -828,49 +827,48 @@ static void *worker_testapp_validate(void *arg)
 		if (bufs == MAP_FAILED)
 			exit_with_error(errno);
 
-		if (strcmp(((struct ifobject *)arg)->nsname, ""))
-			switch_namespace(((struct ifobject *)arg)->ifdict_index);
+		if (strcmp(ifobject->nsname, ""))
+			switch_namespace(ifobject->ifdict_index);
 	}
 
-	if (((struct ifobject *)arg)->fv.vector == tx) {
+	if (ifobject->fv.vector == tx) {
 		int spinningrxctr = 0;
 
 		if (!bidi_pass)
-			thread_common_ops(arg, bufs, &sync_mutex_tx, &spinning_tx);
+			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_tx);
 
 		while (atomic_load(&spinning_rx) && spinningrxctr < SOCK_RECONF_CTR) {
 			spinningrxctr++;
 			usleep(USLEEP_MAX);
 		}
 
-		ksft_print_msg("Interface [%s] vector [Tx]\n", ((struct ifobject *)arg)->ifname);
+		ksft_print_msg("Interface [%s] vector [Tx]\n", ifobject->ifname);
 		for (int i = 0; i < num_frames; i++) {
 			/*send EOT frame */
 			if (i == (num_frames - 1))
 				data->seqnum = -1;
 			else
 				data->seqnum = i;
-			gen_udp_hdr((void *)data, (void *)arg, udp_hdr);
-			gen_ip_hdr((void *)arg, ip_hdr);
+			gen_udp_hdr((void *)data, ifobject, udp_hdr);
+			gen_ip_hdr(ifobject, ip_hdr);
 			gen_udp_csum(udp_hdr, ip_hdr);
-			gen_eth_hdr((void *)arg, eth_hdr);
-			gen_eth_frame(((struct ifobject *)arg)->umem,
-				      i * XSK_UMEM__DEFAULT_FRAME_SIZE);
+			gen_eth_hdr(ifobject, eth_hdr);
+			gen_eth_frame(ifobject->umem, i * XSK_UMEM__DEFAULT_FRAME_SIZE);
 		}
 
 		free(data);
 		ksft_print_msg("Sending %d packets on interface %s\n",
-			       (opt_pkt_count - 1), ((struct ifobject *)arg)->ifname);
-		tx_only_all(arg);
-	} else if (((struct ifobject *)arg)->fv.vector == rx) {
+			       (opt_pkt_count - 1), ifobject->ifname);
+		tx_only_all(ifobject);
+	} else if (ifobject->fv.vector == rx) {
 		struct pollfd fds[MAX_SOCKS] = { };
 		int ret;
 
 		if (!bidi_pass)
-			thread_common_ops(arg, bufs, &sync_mutex_tx, &spinning_rx);
+			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
 
-		ksft_print_msg("Interface [%s] vector [Rx]\n", ((struct ifobject *)arg)->ifname);
-		xsk_populate_fill_ring(((struct ifobject *)arg)->umem);
+		ksft_print_msg("Interface [%s] vector [Rx]\n", ifobject->ifname);
+		xsk_populate_fill_ring(ifobject->umem);
 
 		TAILQ_INIT(&head);
 		if (debug_pkt_dump) {
@@ -879,7 +877,7 @@ static void *worker_testapp_validate(void *arg)
 				exit_with_error(errno);
 		}
 
-		fds[0].fd = xsk_socket__fd(((struct ifobject *)arg)->xsk->xsk);
+		fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
 		fds[0].events = POLLIN;
 
 		pthread_mutex_lock(&sync_mutex);
@@ -892,7 +890,7 @@ static void *worker_testapp_validate(void *arg)
 				if (ret <= 0)
 					continue;
 			}
-			rx_pkt(((struct ifobject *)arg)->xsk, fds);
+			rx_pkt(ifobject->xsk, fds);
 			worker_pkt_validate();
 
 			if (sigvar)
@@ -900,15 +898,15 @@ static void *worker_testapp_validate(void *arg)
 		}
 
 		ksft_print_msg("Received %d packets on interface %s\n",
-			       pkt_counter, ((struct ifobject *)arg)->ifname);
+			       pkt_counter, ifobject->ifname);
 
 		if (opt_teardown)
 			ksft_print_msg("Destroying socket\n");
 	}
 
 	if (!opt_bidi || (opt_bidi && bidi_pass)) {
-		xsk_socket__delete(((struct ifobject *)arg)->xsk->xsk);
-		(void)xsk_umem__delete(((struct ifobject *)arg)->umem->umem);
+		xsk_socket__delete(ifobject->xsk->xsk);
+		(void)xsk_umem__delete(ifobject->umem->umem);
 	}
 	pthread_exit(NULL);
 }
-- 
2.27.0

[PATCH bpf-next 02/12] selftests/bpf: remove unused enums

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

The enums undef and bidi are not used. Remove them.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 61f595b6f200..0e9f9b7e61c2 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -92,8 +92,6 @@ struct flow_vector {
 	enum fvector {
 		tx,
 		rx,
-		bidi,
-		undef,
 	} vector;
 };
 
-- 
2.27.0

[PATCH bpf-next 03/12] selftests/bpf: fix style warnings

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Silence three checkpatch style warnings.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index cd1dd2b7458f..77d1bda37afa 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -734,10 +734,10 @@ static void worker_pkt_validate(void)
 			break;
 		/*do not increment pktcounter if !(tos=0x9 and ipv4) */
 		if ((((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-				       sizeof(struct ethhdr)))->version == IP_PKT_VER)
-		    && (((struct iphdr *)(pkt_node_rx_q->pkt_frame + sizeof(struct ethhdr)))->tos ==
+				       sizeof(struct ethhdr)))->version == IP_PKT_VER) &&
+		    (((struct iphdr *)(pkt_node_rx_q->pkt_frame + sizeof(struct ethhdr)))->tos ==
 			IP_PKT_TOS)) {
-			payloadseqnum = *((uint32_t *) (pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
+			payloadseqnum = *((uint32_t *)(pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
 			if (debug_pkt_dump && payloadseqnum != EOT) {
 				pkt_obj = (struct pkt_frame *)malloc(sizeof(struct pkt_frame));
 				pkt_obj->payload = (char *)malloc(PKT_SIZE);
@@ -767,10 +767,10 @@ static void worker_pkt_validate(void)
 		} else {
 			ksft_print_msg("Invalid frame received: ");
 			ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n",
-				((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-				       sizeof(struct ethhdr)))->version,
-				((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-				       sizeof(struct ethhdr)))->tos);
+				       ((struct iphdr *)(pkt_node_rx_q->pkt_frame +
+							 sizeof(struct ethhdr)))->version,
+				       ((struct iphdr *)(pkt_node_rx_q->pkt_frame +
+							 sizeof(struct ethhdr)))->tos);
 			TAILQ_REMOVE(&head, pkt_node_rx_q, pkt_nodes);
 			free(pkt_node_rx_q->pkt_frame);
 			free(pkt_node_rx_q);
-- 
2.27.0

[PATCH bpf-next 04/12] selftests/bpf: remove memory leak

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

The allocated entry is immediately overwritten by an assignment. Fix
that.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 77d1bda37afa..9f40d310805a 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -728,7 +728,6 @@ static void worker_pkt_validate(void)
 	u32 payloadseqnum = -2;
 
 	while (1) {
-		pkt_node_rx_q = malloc(sizeof(struct pkt));
 		pkt_node_rx_q = TAILQ_LAST(&head, head_s);
 		if (!pkt_node_rx_q)
 			break;
-- 
2.27.0

[PATCH bpf-next 05/12] selftests/bpf: improve readability of xdpxceiver/worker_pkt_validate()

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Introduce a local variable to get rid of lot of casting. Move common
code outside the if/else-clause.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 29 ++++++++++--------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 9f40d310805a..ab2ed7b85f9e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -726,16 +726,17 @@ static void worker_pkt_dump(void)
 static void worker_pkt_validate(void)
 {
 	u32 payloadseqnum = -2;
+	struct iphdr *iphdr;
 
 	while (1) {
 		pkt_node_rx_q = TAILQ_LAST(&head, head_s);
 		if (!pkt_node_rx_q)
 			break;
+
+		iphdr = (struct iphdr *)(pkt_node_rx_q->pkt_frame + sizeof(struct ethhdr));
+
 		/*do not increment pktcounter if !(tos=0x9 and ipv4) */
-		if ((((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-				       sizeof(struct ethhdr)))->version == IP_PKT_VER) &&
-		    (((struct iphdr *)(pkt_node_rx_q->pkt_frame + sizeof(struct ethhdr)))->tos ==
-			IP_PKT_TOS)) {
+		if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
 			payloadseqnum = *((uint32_t *)(pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
 			if (debug_pkt_dump && payloadseqnum != EOT) {
 				pkt_obj = (struct pkt_frame *)malloc(sizeof(struct pkt_frame));
@@ -757,24 +758,18 @@ static void worker_pkt_validate(void)
 				ksft_exit_xfail();
 			}
 
-			TAILQ_REMOVE(&head, pkt_node_rx_q, pkt_nodes);
-			free(pkt_node_rx_q->pkt_frame);
-			free(pkt_node_rx_q);
-			pkt_node_rx_q = NULL;
 			prev_pkt = payloadseqnum;
 			pkt_counter++;
 		} else {
 			ksft_print_msg("Invalid frame received: ");
-			ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n",
-				       ((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-							 sizeof(struct ethhdr)))->version,
-				       ((struct iphdr *)(pkt_node_rx_q->pkt_frame +
-							 sizeof(struct ethhdr)))->tos);
-			TAILQ_REMOVE(&head, pkt_node_rx_q, pkt_nodes);
-			free(pkt_node_rx_q->pkt_frame);
-			free(pkt_node_rx_q);
-			pkt_node_rx_q = NULL;
+			ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
+				       iphdr->tos);
 		}
+
+		TAILQ_REMOVE(&head, pkt_node_rx_q, pkt_nodes);
+		free(pkt_node_rx_q->pkt_frame);
+		free(pkt_node_rx_q);
+		pkt_node_rx_q = NULL;
 	}
 }
 
-- 
2.27.0

[PATCH bpf-next 06/12] selftests/bpf: remove casting by introduce local variable

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Let us use a local variable in nsswitchthread(), so we can remove a
lot of casting for better readability.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index ab2ed7b85f9e..bea006ad8e17 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -382,21 +382,19 @@ static bool switch_namespace(int idx)
 
 static void *nsswitchthread(void *args)
 {
-	if (switch_namespace(((struct targs *)args)->idx)) {
-		ifdict[((struct targs *)args)->idx]->ifindex =
-		    if_nametoindex(ifdict[((struct targs *)args)->idx]->ifname);
-		if (!ifdict[((struct targs *)args)->idx]->ifindex) {
-			ksft_test_result_fail
-			    ("ERROR: [%s] interface \"%s\" does not exist\n",
-			     __func__, ifdict[((struct targs *)args)->idx]->ifname);
-			((struct targs *)args)->retptr = false;
+	struct targs *targs = args;
+
+	targs->retptr = false;
+
+	if (switch_namespace(targs->idx)) {
+		ifdict[targs->idx]->ifindex = if_nametoindex(ifdict[targs->idx]->ifname);
+		if (!ifdict[targs->idx]->ifindex) {
+			ksft_test_result_fail("ERROR: [%s] interface \"%s\" does not exist\n",
+					      __func__, ifdict[targs->idx]->ifname);
 		} else {
-			ksft_print_msg("Interface found: %s\n",
-				       ifdict[((struct targs *)args)->idx]->ifname);
-			((struct targs *)args)->retptr = true;
+			ksft_print_msg("Interface found: %s\n", ifdict[targs->idx]->ifname);
+			targs->retptr = true;
 		}
-	} else {
-		((struct targs *)args)->retptr = false;
 	}
 	pthread_exit(NULL);
 }
-- 
2.27.0

[PATCH bpf-next 07/12] selftests/bpf: change type from void * to struct ifaceconfigobj *

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Instead of casting from void *, let us use the actual type in
init_iface_config().

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 28 ++++++++++++------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index bea006ad8e17..c2cfc0b6d19e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -981,25 +981,25 @@ static void testapp_sockets(void)
 	print_ksft_result();
 }
 
-static void init_iface_config(void *ifaceconfig)
+static void init_iface_config(struct ifaceconfigobj *ifaceconfig)
 {
 	/*Init interface0 */
 	ifdict[0]->fv.vector = tx;
-	memcpy(ifdict[0]->dst_mac, ((struct ifaceconfigobj *)ifaceconfig)->dst_mac, ETH_ALEN);
-	memcpy(ifdict[0]->src_mac, ((struct ifaceconfigobj *)ifaceconfig)->src_mac, ETH_ALEN);
-	ifdict[0]->dst_ip = ((struct ifaceconfigobj *)ifaceconfig)->dst_ip.s_addr;
-	ifdict[0]->src_ip = ((struct ifaceconfigobj *)ifaceconfig)->src_ip.s_addr;
-	ifdict[0]->dst_port = ((struct ifaceconfigobj *)ifaceconfig)->dst_port;
-	ifdict[0]->src_port = ((struct ifaceconfigobj *)ifaceconfig)->src_port;
+	memcpy(ifdict[0]->dst_mac, ifaceconfig->dst_mac, ETH_ALEN);
+	memcpy(ifdict[0]->src_mac, ifaceconfig->src_mac, ETH_ALEN);
+	ifdict[0]->dst_ip = ifaceconfig->dst_ip.s_addr;
+	ifdict[0]->src_ip = ifaceconfig->src_ip.s_addr;
+	ifdict[0]->dst_port = ifaceconfig->dst_port;
+	ifdict[0]->src_port = ifaceconfig->src_port;
 
 	/*Init interface1 */
 	ifdict[1]->fv.vector = rx;
-	memcpy(ifdict[1]->dst_mac, ((struct ifaceconfigobj *)ifaceconfig)->src_mac, ETH_ALEN);
-	memcpy(ifdict[1]->src_mac, ((struct ifaceconfigobj *)ifaceconfig)->dst_mac, ETH_ALEN);
-	ifdict[1]->dst_ip = ((struct ifaceconfigobj *)ifaceconfig)->src_ip.s_addr;
-	ifdict[1]->src_ip = ((struct ifaceconfigobj *)ifaceconfig)->dst_ip.s_addr;
-	ifdict[1]->dst_port = ((struct ifaceconfigobj *)ifaceconfig)->src_port;
-	ifdict[1]->src_port = ((struct ifaceconfigobj *)ifaceconfig)->dst_port;
+	memcpy(ifdict[1]->dst_mac, ifaceconfig->src_mac, ETH_ALEN);
+	memcpy(ifdict[1]->src_mac, ifaceconfig->dst_mac, ETH_ALEN);
+	ifdict[1]->dst_ip = ifaceconfig->src_ip.s_addr;
+	ifdict[1]->src_ip = ifaceconfig->dst_ip.s_addr;
+	ifdict[1]->dst_port = ifaceconfig->src_port;
+	ifdict[1]->src_port = ifaceconfig->dst_port;
 }
 
 int main(int argc, char **argv)
@@ -1038,7 +1038,7 @@ int main(int argc, char **argv)
 
 	num_frames = ++opt_pkt_count;
 
-	init_iface_config((void *)ifaceconfig);
+	init_iface_config(ifaceconfig);
 
 	pthread_init_mutex();
 
-- 
2.27.0

[PATCH bpf-next 08/12] selftests/bpf: change type from void * to struct generic_data *

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Instead of casting from void *, let us use the actual type in
gen_udp_hdr().

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index c2cfc0b6d19e..993ce9b7aa76 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -246,13 +246,13 @@ static void gen_ip_hdr(struct ifobject *ifobject, struct iphdr *ip_hdr)
 	ip_hdr->check = 0;
 }
 
-static void gen_udp_hdr(void *data, struct ifobject *ifobject, struct udphdr *udp_hdr)
+static void gen_udp_hdr(struct generic_data *data, struct ifobject *ifobject,
+			struct udphdr *udp_hdr)
 {
 	udp_hdr->source = htons(ifobject->src_port);
 	udp_hdr->dest = htons(ifobject->dst_port);
 	udp_hdr->len = htons(UDP_PKT_SIZE);
-	memset32_htonl(pkt_data + PKT_HDR_SIZE,
-		       htonl(((struct generic_data *)data)->seqnum), UDP_PKT_DATA_SIZE);
+	memset32_htonl(pkt_data + PKT_HDR_SIZE, htonl(data->seqnum), UDP_PKT_DATA_SIZE);
 }
 
 static void gen_udp_csum(struct udphdr *udp_hdr, struct iphdr *ip_hdr)
@@ -841,7 +841,7 @@ static void *worker_testapp_validate(void *arg)
 				data->seqnum = -1;
 			else
 				data->seqnum = i;
-			gen_udp_hdr((void *)data, ifobject, udp_hdr);
+			gen_udp_hdr(data, ifobject, udp_hdr);
 			gen_ip_hdr(ifobject, ip_hdr);
 			gen_udp_csum(udp_hdr, ip_hdr);
 			gen_eth_hdr(ifobject, eth_hdr);
-- 
2.27.0

[PATCH bpf-next 09/12] selftests/bpf: define local variables at the beginning of a block

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Use C89 rules for variable definition.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 993ce9b7aa76..34bdcae9b908 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -567,9 +567,11 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
 	}
 
 	for (i = 0; i < rcvd; i++) {
-		u64 addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
-		(void)xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++)->len;
-		u64 orig = xsk_umem__extract_addr(addr);
+		u64 addr, orig;
+
+		addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
+		xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
+		orig = xsk_umem__extract_addr(addr);
 
 		addr = xsk_umem__add_offset_to_addr(addr);
 		pkt_node_rx = malloc(sizeof(struct pkt) + PKT_SIZE);
@@ -905,6 +907,8 @@ static void *worker_testapp_validate(void *arg)
 
 static void testapp_validate(void)
 {
+	struct timespec max_wait = { 0, 0 };
+
 	pthread_attr_init(&attr);
 	pthread_attr_setstacksize(&attr, THREAD_STACK);
 
@@ -929,8 +933,6 @@ static void testapp_validate(void)
 			exit_with_error(errno);
 	}
 
-	struct timespec max_wait = { 0, 0 };
-
 	if (clock_gettime(CLOCK_REALTIME, &max_wait))
 		exit_with_error(errno);
 	max_wait.tv_sec += TMOUT_SEC;
-- 
2.27.0

[PATCH bpf-next 10/12] selftests/bpf: avoid heap allocation

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

The data variable is only used locally. Instead of using the heap,
stick to using the stack.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 34bdcae9b908..2da59b142c03 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -807,10 +807,10 @@ static void *worker_testapp_validate(void *arg)
 {
 	struct udphdr *udp_hdr =
 	    (struct udphdr *)(pkt_data + sizeof(struct ethhdr) + sizeof(struct iphdr));
-	struct generic_data *data = (struct generic_data *)malloc(sizeof(struct generic_data));
 	struct iphdr *ip_hdr = (struct iphdr *)(pkt_data + sizeof(struct ethhdr));
 	struct ethhdr *eth_hdr = (struct ethhdr *)pkt_data;
 	struct ifobject *ifobject = (struct ifobject *)arg;
+	struct generic_data data;
 	void *bufs = NULL;
 
 	pthread_attr_setstacksize(&attr, THREAD_STACK);
@@ -840,17 +840,16 @@ static void *worker_testapp_validate(void *arg)
 		for (int i = 0; i < num_frames; i++) {
 			/*send EOT frame */
 			if (i == (num_frames - 1))
-				data->seqnum = -1;
+				data.seqnum = -1;
 			else
-				data->seqnum = i;
-			gen_udp_hdr(data, ifobject, udp_hdr);
+				data.seqnum = i;
+			gen_udp_hdr(&data, ifobject, udp_hdr);
 			gen_ip_hdr(ifobject, ip_hdr);
 			gen_udp_csum(udp_hdr, ip_hdr);
 			gen_eth_hdr(ifobject, eth_hdr);
 			gen_eth_frame(ifobject->umem, i * XSK_UMEM__DEFAULT_FRAME_SIZE);
 		}
 
-		free(data);
 		ksft_print_msg("Sending %d packets on interface %s\n",
 			       (opt_pkt_count - 1), ifobject->ifname);
 		tx_only_all(ifobject);
-- 
2.27.0

[PATCH bpf-next 11/12] selftests/bpf: consistent malloc/calloc usage

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

Use calloc instead of malloc where it makes sense, and avoid C++-style
void *-cast.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 2da59b142c03..a64e2a929e70 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -411,7 +411,7 @@ static int validate_interfaces(void)
 		if (strcmp(ifdict[i]->nsname, "")) {
 			struct targs *targs;
 
-			targs = (struct targs *)malloc(sizeof(struct targs));
+			targs = malloc(sizeof(*targs));
 			if (!targs)
 				exit_with_error(errno);
 
@@ -578,7 +578,7 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
 		if (!pkt_node_rx)
 			exit_with_error(errno);
 
-		pkt_node_rx->pkt_frame = (char *)malloc(PKT_SIZE);
+		pkt_node_rx->pkt_frame = malloc(PKT_SIZE);
 		if (!pkt_node_rx->pkt_frame)
 			exit_with_error(errno);
 
@@ -739,8 +739,8 @@ static void worker_pkt_validate(void)
 		if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
 			payloadseqnum = *((uint32_t *)(pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
 			if (debug_pkt_dump && payloadseqnum != EOT) {
-				pkt_obj = (struct pkt_frame *)malloc(sizeof(struct pkt_frame));
-				pkt_obj->payload = (char *)malloc(PKT_SIZE);
+				pkt_obj = malloc(sizeof(*pkt_obj));
+				pkt_obj->payload = malloc(PKT_SIZE);
 				memcpy(pkt_obj->payload, pkt_node_rx_q->pkt_frame, PKT_SIZE);
 				pkt_buf[payloadseqnum] = pkt_obj;
 			}
@@ -865,7 +865,7 @@ static void *worker_testapp_validate(void *arg)
 
 		TAILQ_INIT(&head);
 		if (debug_pkt_dump) {
-			pkt_buf = malloc(sizeof(struct pkt_frame **) * num_frames);
+			pkt_buf = calloc(num_frames, sizeof(*pkt_buf));
 			if (!pkt_buf)
 				exit_with_error(errno);
 		}
@@ -1017,7 +1017,7 @@ int main(int argc, char **argv)
 	u16 UDP_DST_PORT = 2020;
 	u16 UDP_SRC_PORT = 2121;
 
-	ifaceconfig = (struct ifaceconfigobj *)malloc(sizeof(struct ifaceconfigobj));
+	ifaceconfig = malloc(sizeof(struct ifaceconfigobj));
 	memcpy(ifaceconfig->dst_mac, MAC1, ETH_ALEN);
 	memcpy(ifaceconfig->src_mac, MAC2, ETH_ALEN);
 	inet_aton(IP1, &ifaceconfig->dst_ip);
@@ -1026,7 +1026,7 @@ int main(int argc, char **argv)
 	ifaceconfig->src_port = UDP_SRC_PORT;
 
 	for (int i = 0; i < MAX_INTERFACES; i++) {
-		ifdict[i] = (struct ifobject *)malloc(sizeof(struct ifobject));
+		ifdict[i] = malloc(sizeof(struct ifobject));
 		if (!ifdict[i])
 			exit_with_error(errno);
 
-- 
2.27.0

[PATCH bpf-next 12/12] selftests/bpf: avoid useless void *-casts

[Björn Töpel]

From: Björn Töpel <bjorn.topel@intel.com>

There is no need to cast to void * when the argument is void *. Avoid
cluttering of code.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a64e2a929e70..99ea6cf069e6 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -416,7 +416,7 @@ static int validate_interfaces(void)
 				exit_with_error(errno);
 
 			targs->idx = i;
-			if (pthread_create(&ns_thread, NULL, nsswitchthread, (void *)targs))
+			if (pthread_create(&ns_thread, NULL, nsswitchthread, targs))
 				exit_with_error(errno);
 
 			pthread_join(ns_thread, NULL);
@@ -923,12 +923,12 @@ static void testapp_validate(void)
 
 	/*Spawn RX thread */
 	if (!opt_bidi || (opt_bidi && !bidi_pass)) {
-		if (pthread_create(&t0, &attr, worker_testapp_validate, (void *)ifdict[1]))
+		if (pthread_create(&t0, &attr, worker_testapp_validate, ifdict[1]))
 			exit_with_error(errno);
 	} else if (opt_bidi && bidi_pass) {
 		/*switch Tx/Rx vectors */
 		ifdict[0]->fv.vector = rx;
-		if (pthread_create(&t0, &attr, worker_testapp_validate, (void *)ifdict[0]))
+		if (pthread_create(&t0, &attr, worker_testapp_validate, ifdict[0]))
 			exit_with_error(errno);
 	}
 
@@ -943,12 +943,12 @@ static void testapp_validate(void)
 
 	/*Spawn TX thread */
 	if (!opt_bidi || (opt_bidi && !bidi_pass)) {
-		if (pthread_create(&t1, &attr, worker_testapp_validate, (void *)ifdict[0]))
+		if (pthread_create(&t1, &attr, worker_testapp_validate, ifdict[0]))
 			exit_with_error(errno);
 	} else if (opt_bidi && bidi_pass) {
 		/*switch Tx/Rx vectors */
 		ifdict[1]->fv.vector = tx;
-		if (pthread_create(&t1, &attr, worker_testapp_validate, (void *)ifdict[1]))
+		if (pthread_create(&t1, &attr, worker_testapp_validate, ifdict[1]))
 			exit_with_error(errno);
 	}
 
-- 
2.27.0

Re: [PATCH bpf-next 00/12] Various cleanups/fixes for AF_XDP selftests

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Fri, 22 Jan 2021 16:47:13 +0100 you wrote:
> This series is a number of fixes/cleanups, mainly to improve the
> readability of the xdpxceiver selftest application.
> 
> Details in each commit!
> 
> 
> Cheers,
> Björn
> 
> [...]

Here is the summary with links:
  - [bpf-next,01/12] selftests/bpf: remove a lot of ifobject casting
    https://git.kernel.org/bpf/bpf-next/c/7140ef14007e
  - [bpf-next,02/12] selftests/bpf: remove unused enums
    https://git.kernel.org/bpf/bpf-next/c/449f0874fd4e
  - [bpf-next,03/12] selftests/bpf: fix style warnings
    https://git.kernel.org/bpf/bpf-next/c/a86072838b67
  - [bpf-next,04/12] selftests/bpf: remove memory leak
    https://git.kernel.org/bpf/bpf-next/c/4896d7e37ea5
  - [bpf-next,05/12] selftests/bpf: improve readability of xdpxceiver/worker_pkt_validate()
    https://git.kernel.org/bpf/bpf-next/c/8a9cba7ea858
  - [bpf-next,06/12] selftests/bpf: remove casting by introduce local variable
    https://git.kernel.org/bpf/bpf-next/c/0b50bd48cfe7
  - [bpf-next,07/12] selftests/bpf: change type from void * to struct ifaceconfigobj *
    https://git.kernel.org/bpf/bpf-next/c/124000e48b7e
  - [bpf-next,08/12] selftests/bpf: change type from void * to struct generic_data *
    https://git.kernel.org/bpf/bpf-next/c/59a4a87e4b26
  - [bpf-next,09/12] selftests/bpf: define local variables at the beginning of a block
    https://git.kernel.org/bpf/bpf-next/c/829725ec7bf5
  - [bpf-next,10/12] selftests/bpf: avoid heap allocation
    https://git.kernel.org/bpf/bpf-next/c/93dd4a06c0e3
  - [bpf-next,11/12] selftests/bpf: consistent malloc/calloc usage
    https://git.kernel.org/bpf/bpf-next/c/d08a17d6de20
  - [bpf-next,12/12] selftests/bpf: avoid useless void *-casts
    https://git.kernel.org/bpf/bpf-next/c/095af986525a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html