diff mbox

[3/5] netmap cleanup and update to changes in socket example

Message ID 1397034218-2971-4-git-send-email-ciprian.barbu@linaro.org
State Accepted
Headers show

Commit Message

Ciprian Barbu April 9, 2014, 9:03 a.m. UTC
Reworked the netmap pktio example to support any number of threads of execution
and multiple interfaces to work on. The threads are no longer associated with
pktios because of using odp_schedule that gets packets from any pktio.
General cleanup changes were made, setting hardcoded values to defines, renaming
some variables, modifying some comments.

Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
---
 .gitignore                                         |   2 +-
 platform/linux-generic/include/odp_packet_netmap.h |   7 +-
 platform/linux-generic/source/odp_packet_netmap.c  |  67 ++++++-----
 test/packet_netmap/Makefile                        |   2 +-
 test/packet_netmap/odp_example_pktio_netmap.c      | 132 +++++++++++++--------
 5 files changed, 118 insertions(+), 92 deletions(-)
diff mbox

Patch

diff --git a/.gitignore b/.gitignore
index ae6eb64..6f95db0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,7 +10,7 @@  obj/
 build/
 test/example/odp_example
 test/packet/odp_packet
-test/packet_netmap/odp_packet
+test/packet_netmap/odp_packet_netmap
 test/api_test/odp_atomic
 test/api_test/odp_shm
 test/api_test/odp_ring
diff --git a/platform/linux-generic/include/odp_packet_netmap.h b/platform/linux-generic/include/odp_packet_netmap.h
index 960ccbf..eae936a 100644
--- a/platform/linux-generic/include/odp_packet_netmap.h
+++ b/platform/linux-generic/include/odp_packet_netmap.h
@@ -29,17 +29,14 @@ 
 typedef struct {
 	odp_buffer_pool_t pool;
 	size_t max_frame_len; /**< max frame len = buf_size - sizeof(pkt_hdr) */
-	size_t l2_offset; /**< l2 hdr start offset from start of pkt payload */
+	size_t frame_offset; /**< frame start offset from start of pkt buf */
 	size_t buf_size; /**< size of buffer payload in 'pool' */
+	int netmap_mode;
 	struct nm_desc_t *nm_desc;
 	struct netmap_ring *rxring;
 	struct netmap_ring *txring;
-
-	/********************************/
 	odp_queue_t tx_access; /* Used for exclusive access to send packets */
 	uint32_t if_flags;
-	uint32_t if_reqcap;
-	uint32_t if_curcap;
 	char ifname[32];
 } pkt_netmap_t;
 
diff --git a/platform/linux-generic/source/odp_packet_netmap.c b/platform/linux-generic/source/odp_packet_netmap.c
index 1cbd84c..f5cc500 100644
--- a/platform/linux-generic/source/odp_packet_netmap.c
+++ b/platform/linux-generic/source/odp_packet_netmap.c
@@ -33,6 +33,7 @@ 
 
 #include <helper/odp_eth.h>
 #include <helper/odp_ip.h>
+#include <helper/odp_packet_helper.h>
 
 #define NETMAP_WITH_LIBS
 #include <odp_packet_netmap.h>
@@ -49,6 +50,9 @@ 
 #define ETHBUF_ALIGN(buf_ptr) ((uint8_t *)ODP_ALIGN_ROUNDUP_PTR((buf_ptr), \
 				sizeof(uint32_t)) + ETHBUF_OFFSET)
 
+#define ETH_PROMISC  1 /* TODO: maybe this should be exported to the user */
+#define WAITLINK_TMO 2
+
 static int nm_do_ioctl(pkt_netmap_t * const pkt_nm, unsigned long cmd,
 		       int subcmd)
 {
@@ -105,12 +109,9 @@  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, char *netdev,
 	char qname[ODP_QUEUE_NAME_LEN];
 	char ifname[32];
 	odp_packet_t pkt;
-	odp_buffer_t buf;
 	odp_buffer_t token;
 	uint8_t *pkt_buf;
-	int wait_link = 2;
 	uint16_t ringid;
-	int promisc = 1; /* TODO: maybe this should be exported to the user */
 	uint8_t *l2_hdr;
 	int ret;
 
@@ -118,21 +119,22 @@  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, char *netdev,
 		return -1;
 	pkt_nm->pool = pool;
 
-	buf = odp_buffer_alloc(pool);
-	if (!odp_buffer_is_valid(buf))
+	pkt = odp_packet_alloc(pool);
+	if (!odp_packet_is_valid(pkt))
 		return -1;
 
-	pkt = odp_packet_from_buffer(buf);
 	pkt_buf = odp_packet_buf_addr(pkt);
 	l2_hdr = ETHBUF_ALIGN(pkt_buf);
 	/* Store eth buffer offset for buffers from this pool */
-	pkt_nm->l2_offset = (uintptr_t)l2_hdr - (uintptr_t)pkt_buf;
+	pkt_nm->frame_offset = (uintptr_t)l2_hdr - (uintptr_t)pkt_buf;
 	/* pkt buffer size */
-	pkt_nm->buf_size = odp_buffer_size(buf);
+	pkt_nm->buf_size = odp_packet_buf_size(pkt);
 	/* max frame len taking into account the l2-offset */
-	pkt_nm->max_frame_len = pkt_nm->buf_size - pkt_nm->l2_offset;
+	pkt_nm->max_frame_len = pkt_nm->buf_size - pkt_nm->frame_offset;
+	/* save netmap_mode for later use */
+	pkt_nm->netmap_mode = nm_params->netmap_mode;
 
-	odp_buffer_free(buf);
+	odp_packet_free(pkt);
 
 	if (nm_params->netmap_mode == ODP_NETMAP_MODE_SW)
 		ringid = NETMAP_SW_RING;
@@ -172,7 +174,7 @@  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, char *netdev,
 			ODP_DBG("%s is down, bringing up...\n", pkt_nm->ifname);
 			pkt_nm->if_flags |= IFF_UP;
 		}
-		if (promisc) {
+		if (ETH_PROMISC) {
 			pkt_nm->if_flags |= IFF_PROMISC;
 			nm_do_ioctl(pkt_nm, SIOCSIFFLAGS, 0);
 		}
@@ -183,9 +185,10 @@  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, char *netdev,
 		ret = nm_do_ioctl(pkt_nm, SIOCETHTOOL, ETHTOOL_STSO);
 		if (ret)
 			ODP_DBG("ETHTOOL_STSO not supported\n");
-		/* Not sure why this is needed but the netmap example bridge
-		 * uses it; it is possible that this causes some problem at
-		 * startup, should be investigated further */
+		/* TODO: This seems to cause the app to not receive frames
+		 * first time it is launched after netmap driver is inserted.
+		 * Should be investigated further.
+		 */
 		/*
 		nm_do_ioctl(pkt_nm, SIOCETHTOOL, ETHTOOL_SRXCSUM);
 		*/
@@ -211,7 +214,7 @@  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, char *netdev,
 	odp_queue_enq(pkt_nm->tx_access, token);
 
 	ODP_DBG("Wait for link to come up\n");
-	sleep(wait_link);
+	sleep(WAITLINK_TMO);
 	ODP_DBG("Done\n");
 
 	return 0;
@@ -233,11 +236,10 @@  int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 	struct netmap_ring *rxring;
 	int fd;
 	unsigned nb_rx = 0;
-	uint32_t limit, rx, cur;
+	uint32_t limit, rx;
 	odp_packet_t pkt = ODP_PACKET_INVALID;
-
 #ifdef NETMAP_BLOCKING_IO
-	struct pollfd fds[2];
+	struct pollfd fds[1];
 	int ret;
 #endif
 
@@ -249,8 +251,6 @@  int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 
 	rxring = pkt_nm->rxring;
 	while (nb_rx < len) {
-		odp_buffer_t buf;
-
 #ifdef NETMAP_BLOCKING_IO
 		ret = poll(&fds[0], 1, 50);
 		if (ret <= 0 || (fds[0].revents & POLLERR))
@@ -270,17 +270,16 @@  int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 
 		ODP_DBG("receiving %d frames out of %u\n", limit, len);
 
-		cur = rxring->cur;
 		for (rx = 0; rx < limit; rx++) {
 			struct netmap_slot *rslot;
 			char *p;
-			uint16_t payload_len;
+			uint16_t frame_len;
 			uint8_t *pkt_buf;
 			uint8_t *l2_hdr;
+			uint32_t cur;
 
 			if (odp_likely(pkt == ODP_PACKET_INVALID)) {
-				buf = odp_buffer_alloc(pkt_nm->pool);
-				pkt = odp_packet_from_buffer(buf);
+				pkt = odp_packet_alloc(pkt_nm->pool);
 				if (odp_unlikely(pkt == ODP_PACKET_INVALID))
 					break;
 			}
@@ -288,27 +287,27 @@  int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 			cur = rxring->cur;
 			rslot = &rxring->slot[cur];
 			p = NETMAP_BUF(rxring, rslot->buf_idx);
-			payload_len = rslot->len;
+			frame_len = rslot->len;
 
 			rxring->head = nm_ring_next(rxring, cur);
-			rxring->cur  = nm_ring_next(rxring, cur);
+			rxring->cur = rxring->head;
 
-			if (payload_len > pkt_nm->max_frame_len) {
+			if (frame_len > pkt_nm->max_frame_len) {
 				ODP_ERR("Data partially lost %u %lu!\n",
-					payload_len, pkt_nm->max_frame_len);
-				payload_len = pkt_nm->max_frame_len;
+					frame_len, pkt_nm->max_frame_len);
+				frame_len = pkt_nm->max_frame_len;
 			}
 
 			pkt_buf = odp_packet_buf_addr(pkt);
-			l2_hdr = pkt_buf + pkt_nm->l2_offset;
+			l2_hdr = pkt_buf + pkt_nm->frame_offset;
 
 			/* For now copy the data in the mbuf,
 			   worry about zero-copy later */
-			memcpy(l2_hdr, p, payload_len);
+			memcpy(l2_hdr, p, frame_len);
 
 			/* Initialize, parse and set packet header data */
 			odp_packet_init(pkt);
-			odp_packet_parse(pkt, payload_len, pkt_nm->l2_offset);
+			odp_packet_parse(pkt, frame_len, pkt_nm->frame_offset);
 
 			pkt_table[nb_rx] = pkt;
 			pkt = ODP_PACKET_INVALID;
@@ -361,7 +360,7 @@  int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 		txbuf = NETMAP_BUF(txring, slot->buf_idx);
 
 		pkt = pkt_table[i];
-		frame = odp_packet_l2(pkt);
+		frame = odp_packet_start(pkt);
 		frame_len = odp_packet_get_len(pkt);
 
 		memcpy(txbuf, frame, frame_len);
@@ -379,7 +378,7 @@  int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
 		ODP_DBG("===> sent %03u frames to netmap adapter\n", limit);
 
 	for (i = 0; i < len; i++)
-		odp_buffer_free(pkt_table[i]);
+		odp_packet_free(pkt_table[i]);
 
 	return limit;
 }
diff --git a/test/packet_netmap/Makefile b/test/packet_netmap/Makefile
index a8b8532..5c4abfc 100644
--- a/test/packet_netmap/Makefile
+++ b/test/packet_netmap/Makefile
@@ -4,7 +4,7 @@ 
 # SPDX-License-Identifier:     BSD-3-Clause
 
 ODP_ROOT = ../..
-ODP_APP  = odp_packet
+ODP_APP  = odp_packet_netmap
 
 EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
 
diff --git a/test/packet_netmap/odp_example_pktio_netmap.c b/test/packet_netmap/odp_example_pktio_netmap.c
index 7c60467..283abe4 100644
--- a/test/packet_netmap/odp_example_pktio_netmap.c
+++ b/test/packet_netmap/odp_example_pktio_netmap.c
@@ -21,8 +21,8 @@ 
 #include <arpa/inet.h>
 
 #include <odp.h>
-#include <odp_debug.h>
 #include <helper/odp_linux.h>
+#include <helper/odp_packet_helper.h>
 #include <helper/odp_eth.h>
 #include <helper/odp_ip.h>
 #include <helper/odp_packet_helper.h>
@@ -30,6 +30,7 @@ 
 #include <odp_pktio_netmap.h>
 
 #define MAX_WORKERS            32
+#define MAX_IFS                16
 #define SHM_PKT_POOL_SIZE      (512*2048)
 #define SHM_PKT_POOL_BUF_SIZE  1856
 #define MAX_PKT_BURST          16
@@ -73,8 +74,8 @@  typedef struct {
 	char *pktio_dev;	/**< Interface name to use */
 	int netmap_mode;	/**< Either poll the hardware rings or the
 				     rings associated with the host stack */
-	odp_queue_t bridge_q;   /**< Related pktio_entry */
-} thread_args_t;
+	odp_queue_t bridge_q;   /**< Connect the network stack with the NIC */
+} pktio_info_t;
 
 /**
  * Grouping of both parsed CL args and thread specific args - alloc together
@@ -82,16 +83,18 @@  typedef struct {
 typedef struct {
 	/** Application (parsed) arguments */
 	appl_args_t appl;
-	/** Thread specific arguments */
-	thread_args_t thread[MAX_WORKERS];
-	/** Lookup table */
-	unsigned char pktio_tbl[ODP_CONFIG_PKTIO_ENTRIES];
+	/** pktio entries: one for SW ring and one for HW ring */
+	pktio_info_t pktios[2 * MAX_IFS];
+	/** TODO: find a way to associate private data with pktios */
+	/** Lookup table: find pktio_info_t based on pktio id */
+	pktio_info_t *pktio_lt[ODP_CONFIG_PKTIO_ENTRIES];
 } args_t;
 
 /** Global pointer to args */
 static args_t *args;
 
 /* helper funcs */
+static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len);
 static void swap_pkt_addrs(odp_packet_t pkt_tbl[], unsigned len);
 static void parse_args(int argc, char *argv[], appl_args_t *appl_args);
 static void print_info(char *progname, appl_args_t *appl_args);
@@ -106,16 +109,15 @@  static void *pktio_queue_thread(void *arg)
 {
 	int thr;
 	odp_buffer_pool_t pkt_pool;
-	thread_args_t *thr_args;
 	odp_packet_t pkt;
 	odp_buffer_t buf;
 	unsigned long pkt_cnt = 0;
+	unsigned long err_cnt = 0;
 
-	thr = odp_thread_id();
-	thr_args = arg;
+	(void)arg;
 
-	printf("Pktio thread [%02i] starts, pktio_dev:%s\n", thr,
-	       thr_args->pktio_dev);
+	thr = odp_thread_id();
+	printf("Pktio thread [%02i] starts\n", thr);
 
 	/* Lookup the packet pool */
 	pkt_pool = odp_buffer_pool_lookup("packet_pool");
@@ -128,12 +130,19 @@  static void *pktio_queue_thread(void *arg)
 	for (;;) {
 		odp_pktio_t pktio_tmp;
 		odp_queue_t outq_def;
-		int pktio_nr;
+		pktio_info_t *pktio_info;
 
 		/* Use schedule to get buf from any input queue */
 		buf = odp_schedule(NULL);
 
 		pkt = odp_packet_from_buffer(buf);
+
+		/* Drop packets with errors */
+		if (odp_unlikely(drop_err_pkts(&pkt, 1) == 0)) {
+			ODP_ERR("Drop frame - err_cnt:%lu\n", ++err_cnt);
+			continue;
+		}
+
 		pktio_tmp = odp_pktio_get_input(pkt);
 		if (pktio_tmp == ODP_PKTIO_INVALID) {
 			ODP_ERR("[%02i] Error: invalid pktio\n", thr);
@@ -149,10 +158,10 @@  static void *pktio_queue_thread(void *arg)
 		}
 
 		/* Lookup the thread associated with the entry */
-		pktio_nr = args->pktio_tbl[pktio_tmp];
+		pktio_info = args->pktio_lt[pktio_tmp];
 
 		/* Send back packets arrived on physical interface */
-		if (args->thread[pktio_nr].netmap_mode == ODP_NETMAP_MODE_HW) {
+		if (pktio_info->netmap_mode == ODP_NETMAP_MODE_HW) {
 			odp_packet_t pkt_copy;
 
 			pkt_copy = odp_packet_alloc(pkt_pool);
@@ -167,7 +176,7 @@  static void *pktio_queue_thread(void *arg)
 			}
 		}
 
-		odp_queue_enq(args->thread[pktio_nr].bridge_q, buf);
+		odp_queue_enq(pktio_info->bridge_q, buf);
 
 		/* Print packet counts every once in a while */
 		if (odp_unlikely(pkt_cnt++ % 100000 == 0)) {
@@ -238,9 +247,7 @@  int main(int argc, char *argv[])
 	}
 	odp_buffer_pool_print(pool);
 
-	/* Create and init worker threads */
-	memset(thread_tbl, 0, sizeof(thread_tbl));
-	for (i = 0; i < num_workers; ++i) {
+	for (i = 0; i < 2 * args->appl.if_count; ++i) {
 		odp_pktio_params_t params;
 		netmap_params_t *nm_params = &params.nm_params;
 		char inq_name[ODP_QUEUE_NAME_LEN];
@@ -248,21 +255,12 @@  int main(int argc, char *argv[])
 		odp_queue_param_t qparam;
 		odp_pktio_t pktio;
 		int ret;
-		int if_idx;
 
-		if (i == 2)
-			break;
-
-		/* In netmap mode there will be one thread polling the physical
-		 * interface and one polling the host stack for that interface
+		/* Create a pktio polling the hardware rings and one that polls
+		 * the software ring associated with the physical interface
 		 */
-		if_idx = i < 2 * args->appl.if_count ? i / 2 : -1;
 
-		if (if_idx == -1) {
-			args->thread[i].pktio_dev = NULL;
-			continue;
-		}
-		args->thread[i].pktio_dev = args->appl.ifs[if_idx].if_name;
+		args->pktios[i].pktio_dev = args->appl.ifs[i / 2].if_name;
 		memset(nm_params, 0, sizeof(*nm_params));
 		nm_params->type = ODP_PKTIO_TYPE_NETMAP;
 		if (i % 2) {
@@ -272,7 +270,7 @@  int main(int argc, char *argv[])
 			nm_params->netmap_mode = ODP_NETMAP_MODE_HW;
 			nm_params->ringid = 0;
 		}
-		pktio = odp_pktio_open(args->thread[i].pktio_dev,
+		pktio = odp_pktio_open(args->pktios[i].pktio_dev,
 				       pool, &params);
 		/* Open a packet IO instance for this thread */
 		if (pktio == ODP_PKTIO_INVALID) {
@@ -280,9 +278,11 @@  int main(int argc, char *argv[])
 			return -1;
 		}
 
-		args->thread[i].pktio = pktio;
-		/* Save pktio id in the lookup table */
-		args->pktio_tbl[pktio] = i;
+		args->pktios[i].pktio = pktio;
+		args->pktios[i].pool = pool;
+		args->pktios[i].netmap_mode = nm_params->netmap_mode;
+		/* Save pktio_info in the lookup table */
+		args->pktio_lt[pktio] = &args->pktios[i];
 		/*
 		 * Create and set the default INPUT queue associated with the
 		 * 'pktio' resource
@@ -317,28 +317,25 @@  int main(int argc, char *argv[])
 			odp_pktio_t pktio_bridge;
 			odp_queue_t outq_def;
 
-			pktio_bridge = args->thread[i-1].pktio;
+			pktio_bridge = args->pktios[i-1].pktio;
 			outq_def = odp_pktio_outq_getdef(pktio_bridge);
-			args->thread[i].bridge_q = outq_def;
+			args->pktios[i].bridge_q = outq_def;
 
-			pktio_bridge = args->thread[i].pktio;
+			pktio_bridge = args->pktios[i].pktio;
 			outq_def = odp_pktio_outq_getdef(pktio_bridge);
-			args->thread[i-1].bridge_q = outq_def;
+			args->pktios[i-1].bridge_q = outq_def;
 		}
-
-		args->thread[i].pool = pool;
 	}
 
+	memset(thread_tbl, 0, sizeof(thread_tbl));
 	for (i = 0; i < num_workers; ++i) {
-		if (i == 2)
-			break;
 
 		/*
 		 * Create threads one-by-one instead of all-at-once,
 		 * because each thread might get different arguments
 		 */
 		odp_linux_pthread_create(thread_tbl, 1, i, pktio_queue_thread,
-					 &args->thread[i]);
+					 NULL);
 	}
 
 	/* Master thread waits for other threads to exit */
@@ -350,6 +347,37 @@  int main(int argc, char *argv[])
 }
 
 /**
+ * Drop packets which input parsing marked as containing errors.
+ *
+ * Frees packets with error and modifies pkt_tbl[] to only contain packets with
+ * no detected errors.
+ *
+ * @param pkt_tbl  Array of packet
+ * @param len      Length of pkt_tbl[]
+ *
+ * @return Number of packets with no detected error
+ */
+static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
+{
+	odp_packet_t pkt;
+	unsigned pkt_cnt = len;
+	unsigned i, j;
+
+	for (i = 0, j = 0; i < len; ++i) {
+		pkt = pkt_tbl[i];
+
+		if (odp_unlikely(odp_packet_error(pkt))) {
+			odp_packet_free(pkt); /* Drop */
+			pkt_cnt--;
+		} else if (odp_unlikely(i != j++)) {
+			pkt_tbl[j] = pkt;
+		}
+	}
+
+	return pkt_cnt;
+}
+
+/**
  * Swap eth src<->dst and IP src<->dst addresses
  *
  * @param pkt_tbl  Array of packets
@@ -366,19 +394,21 @@  static void swap_pkt_addrs(odp_packet_t pkt_tbl[], unsigned len)
 
 	for (i = 0; i < len; ++i) {
 		pkt = pkt_tbl[i];
-		eth = (odp_ethhdr_t *)odp_packet_l2(pkt);
+		if (odp_packet_inflag_eth(pkt)) {
+			eth = (odp_ethhdr_t *)odp_packet_l2(pkt);
 
-		if (odp_be_to_cpu_16(eth->type) == ODP_ETHTYPE_IPV4) {
 			tmp_addr = eth->dst;
 			eth->dst = eth->src;
 			eth->src = tmp_addr;
 
-			/* IPv4 */
-			ip = (odp_ipv4hdr_t *)odp_packet_l3(pkt);
+			if (odp_packet_inflag_ipv4(pkt)) {
+				/* IPv4 */
+				ip = (odp_ipv4hdr_t *)odp_packet_l3(pkt);
 
-			ip_tmp_addr  = ip->src_addr;
-			ip->src_addr = ip->dst_addr;
-			ip->dst_addr = ip_tmp_addr;
+				ip_tmp_addr  = ip->src_addr;
+				ip->src_addr = ip->dst_addr;
+				ip->dst_addr = ip_tmp_addr;
+			}
 		}
 	}
 }