[dpdk-dev] [PATCH] port: add file descriptor SWX port

[dpdk-dev] [PATCH] port: add file descriptor SWX port

[Venkata Suresh Kumar P]

Add the file descriptor input/output port type for the SWX pipeline.

Signed-off-by: Venkata Suresh Kumar P <venkata.suresh.kumar.p@intel.com>
Signed-off-by: Churchill Khangar <churchill.khangar@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Depends-on: patch-87498 ("port: add ring SWX port")
---
 doc/api/doxy-api-index.md         |   1 +
 examples/pipeline/cli.c           | 142 +++++++++++++++++-
 examples/pipeline/obj.c           | 110 ++++++++++++++
 examples/pipeline/obj.h           |  18 +++
 lib/librte_port/meson.build       |   2 +
 lib/librte_port/rte_swx_port_fd.c | 299 ++++++++++++++++++++++++++++++++++++++
 lib/librte_port/rte_swx_port_fd.h |  57 ++++++++
 lib/librte_port/version.map       |   2 +
 8 files changed, 629 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_port/rte_swx_port_fd.c
 create mode 100644 lib/librte_port/rte_swx_port_fd.h

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index befc9bc..cf3b245 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -187,6 +187,7 @@ The public API headers are grouped by topics:
     [port]             (@ref rte_swx_port.h),
     [ethdev]           (@ref rte_swx_port_ethdev.h),
     [ring]             (@ref rte_swx_port_ring.h),
+    [fd]               (@ref rte_swx_port_fd.h),
     [src/sink]         (@ref rte_swx_port_source_sink.h)
   * SWX table:
     [table]            (@ref rte_swx_table.h),
diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
index cacfb28..4ef3ff4 100644
--- a/examples/pipeline/cli.c
+++ b/examples/pipeline/cli.c
@@ -12,6 +12,7 @@
 #include <rte_swx_port_ethdev.h>
 #include <rte_swx_port_ring.h>
 #include <rte_swx_port_source_sink.h>
+#include <rte_swx_port_fd.h>
 #include <rte_swx_pipeline.h>
 #include <rte_swx_ctl.h>
 
@@ -493,6 +494,33 @@
 	}
 }
 
+static const char cmd_tap_help[] =
+"tap <tap_name>\n";
+
+static void
+cmd_tap(char **tokens,
+	uint32_t n_tokens,
+	char *out,
+	size_t out_size,
+	void *obj)
+{
+	struct tap *tap;
+	char *name;
+
+	if (n_tokens < 2) {
+		snprintf(out, out_size, MSG_ARG_MISMATCH, tokens[0]);
+		return;
+	}
+	name = tokens[1];
+
+	tap = tap_create(obj, name);
+	if (tap == NULL) {
+		snprintf(out, out_size, MSG_CMD_FAIL, tokens[0]);
+		return;
+	}
+}
+
+
 static const char cmd_pipeline_create_help[] =
 "pipeline <pipeline_name> create <numa_node>\n";
 
@@ -530,7 +558,8 @@
 "pipeline <pipeline_name> port in <port_id>\n"
 "   link <link_name> rxq <queue_id> bsz <burst_size>\n"
 "   ring <ring_name> bsz <burst_size>\n"
-"   | source <mempool_name> <file_name>\n";
+"   | source <mempool_name> <file_name>\n"
+"   | tap <tap_name> mempool <mempool_name> mtu <mtu> bsz <burst_size>\n";
 
 static void
 cmd_pipeline_port_in(char **tokens,
@@ -678,6 +707,68 @@
 			port_id,
 			"source",
 			&params);
+	} else if (strcmp(tokens[t0], "tap") == 0) {
+		struct rte_swx_port_fd_reader_params params;
+		struct tap *tap;
+		struct mempool *mp;
+
+		if (n_tokens < t0 + 8) {
+			snprintf(out, out_size, MSG_ARG_MISMATCH,
+				"pipeline port in tap");
+			return;
+		}
+
+		tap = tap_find(obj, tokens[t0 + 1]);
+		if (!tap) {
+			snprintf(out, out_size, MSG_ARG_INVALID,
+				"tap_name");
+			return;
+		}
+		params.fd = tap->fd;
+
+		if (strcmp(tokens[t0 + 2], "mempool") != 0) {
+			snprintf(out, out_size, MSG_ARG_NOT_FOUND,
+				"mempool");
+			return;
+		}
+
+		mp = mempool_find(obj, tokens[t0 + 3]);
+		if (!mp) {
+			snprintf(out, out_size, MSG_ARG_INVALID,
+				"mempool_name");
+			return;
+		}
+		params.mempool = mp->m;
+
+		if (strcmp(tokens[t0 + 4], "mtu") != 0) {
+			snprintf(out, out_size, MSG_ARG_NOT_FOUND,
+				"mtu");
+			return;
+		}
+
+		if (parser_read_uint32(&params.mtu, tokens[t0 + 5]) != 0) {
+			snprintf(out, out_size, MSG_ARG_INVALID, "mtu");
+			return;
+		}
+
+		if (strcmp(tokens[t0 + 6], "bsz") != 0) {
+			snprintf(out, out_size, MSG_ARG_NOT_FOUND, "bsz");
+			return;
+		}
+
+		if (parser_read_uint32(&params.burst_size, tokens[t0 + 7])) {
+			snprintf(out, out_size, MSG_ARG_INVALID,
+				"burst_size");
+			return;
+		}
+
+		t0 += 8;
+
+		status = rte_swx_pipeline_port_in_config(p->p,
+			port_id,
+			"fd",
+			&params);
+
 	} else {
 		snprintf(out, out_size, MSG_ARG_INVALID, tokens[0]);
 		return;
@@ -698,7 +789,8 @@
 "pipeline <pipeline_name> port out <port_id>\n"
 "   link <link_name> txq <txq_id> bsz <burst_size>\n"
 "   ring <ring_name> bsz <burst_size>\n"
-"   | sink <file_name> | none\n";
+"   | sink <file_name> | none\n"
+"   | tap <tap_name> bsz <burst_size>\n";
 
 static void
 cmd_pipeline_port_out(char **tokens,
@@ -832,6 +924,41 @@
 			port_id,
 			"sink",
 			&params);
+	} else if (strcmp(tokens[t0], "tap") == 0) {
+		struct rte_swx_port_fd_writer_params params;
+		struct tap *tap;
+
+		if (n_tokens < t0 + 4) {
+			snprintf(out, out_size, MSG_ARG_MISMATCH,
+				"pipeline port out tap");
+			return;
+		}
+
+		tap = tap_find(obj, tokens[t0 + 1]);
+		if (!tap) {
+			snprintf(out, out_size, MSG_ARG_INVALID,
+				"tap_name");
+			return;
+		}
+		params.fd = tap->fd;
+
+		if (strcmp(tokens[t0 + 2], "bsz") != 0) {
+			snprintf(out, out_size, MSG_ARG_NOT_FOUND, "bsz");
+			return;
+		}
+
+		if (parser_read_uint32(&params.burst_size, tokens[t0 + 3])) {
+			snprintf(out, out_size, MSG_ARG_INVALID,
+				"burst_size");
+			return;
+		}
+
+		t0 += 4;
+
+		status = rte_swx_pipeline_port_out_config(p->p,
+			port_id,
+			"fd",
+			&params);
 	} else {
 		snprintf(out, out_size, MSG_ARG_INVALID, tokens[0]);
 		return;
@@ -1303,6 +1430,7 @@
 			"List of commands:\n"
 			"\tmempool\n"
 			"\tlink\n"
+			"\ttap\n"
 			"\tpipeline create\n"
 			"\tpipeline port in\n"
 			"\tpipeline port out\n"
@@ -1329,6 +1457,11 @@
 		return;
 	}
 
+	if (strcmp(tokens[0], "tap") == 0) {
+		snprintf(out, out_size, "\n%s\n", cmd_tap_help);
+		return;
+	}
+
 	if ((strcmp(tokens[0], "pipeline") == 0) &&
 		(n_tokens == 2) && (strcmp(tokens[1], "create") == 0)) {
 		snprintf(out, out_size, "\n%s\n", cmd_pipeline_create_help);
@@ -1434,6 +1567,11 @@
 		return;
 	}
 
+	if (strcmp(tokens[0], "tap") == 0) {
+		cmd_tap(tokens, n_tokens, out, out_size, obj);
+		return;
+	}
+
 	if (strcmp(tokens[0], "pipeline") == 0) {
 		if ((n_tokens >= 3) &&
 			(strcmp(tokens[2], "create") == 0)) {
diff --git a/examples/pipeline/obj.c b/examples/pipeline/obj.c
index 154d832..fdd63fe 100644
--- a/examples/pipeline/obj.c
+++ b/examples/pipeline/obj.c
@@ -4,6 +4,14 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <netinet/in.h>
+#ifdef RTE_EXEC_ENV_LINUX
+#include <linux/if.h>
+#include <linux/if_tun.h>
+#endif
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include <rte_mempool.h>
 #include <rte_mbuf.h>
@@ -11,6 +19,7 @@
 #include <rte_swx_port_ethdev.h>
 #include <rte_swx_port_ring.h>
 #include <rte_swx_port_source_sink.h>
+#include <rte_swx_port_fd.h>
 #include <rte_swx_table_em.h>
 #include <rte_swx_pipeline.h>
 #include <rte_swx_ctl.h>
@@ -33,6 +42,11 @@
 TAILQ_HEAD(ring_list, ring);
 
 /*
+ * tap
+ */
+TAILQ_HEAD(tap_list, tap);
+
+/*
  * pipeline
  */
 TAILQ_HEAD(pipeline_list, pipeline);
@@ -45,6 +59,7 @@ struct obj {
 	struct link_list link_list;
 	struct ring_list ring_list;
 	struct pipeline_list pipeline_list;
+	struct tap_list tap_list;
 };
 
 /*
@@ -422,6 +437,88 @@ struct ring *
 }
 
 /*
+ * tap
+ */
+#define TAP_DEV                                            "/dev/net/tun"
+
+struct tap *
+tap_find(struct obj *obj, const char *name)
+{
+	struct tap *tap;
+
+	if (!obj || !name)
+		return NULL;
+
+	TAILQ_FOREACH(tap, &obj->tap_list, node)
+		if (strcmp(tap->name, name) == 0)
+			return tap;
+
+	return NULL;
+}
+
+struct tap *
+tap_next(struct obj *obj, struct tap *tap)
+{
+	return (tap == NULL) ?
+		TAILQ_FIRST(&obj->tap_list) : TAILQ_NEXT(tap, node);
+}
+
+#ifndef RTE_EXEC_ENV_LINUX
+
+struct tap *
+tap_create(struct obj *obj __rte_unused, const char *name __rte_unused)
+{
+	return NULL;
+}
+
+#else
+
+struct tap *
+tap_create(struct obj *obj, const char *name)
+{
+	struct tap *tap;
+	struct ifreq ifr;
+	int fd, status;
+
+	/* Check input params */
+	if ((name == NULL) ||
+		tap_find(obj, name))
+		return NULL;
+
+	/* Resource create */
+	fd = open(TAP_DEV, O_RDWR | O_NONBLOCK);
+	if (fd < 0)
+		return NULL;
+
+	memset(&ifr, 0, sizeof(ifr));
+	ifr.ifr_flags = IFF_TAP | IFF_NO_PI; /* No packet information */
+	strlcpy(ifr.ifr_name, name, IFNAMSIZ);
+
+	status = ioctl(fd, TUNSETIFF, (void *) &ifr);
+	if (status < 0) {
+		close(fd);
+		return NULL;
+	}
+
+	/* Node allocation */
+	tap = calloc(1, sizeof(struct tap));
+	if (tap == NULL) {
+		close(fd);
+		return NULL;
+	}
+	/* Node fill in */
+	strlcpy(tap->name, name, sizeof(tap->name));
+	tap->fd = fd;
+
+	/* Node add to list */
+	TAILQ_INSERT_TAIL(&obj->tap_list, tap, node);
+
+	return tap;
+}
+
+#endif
+
+/*
  * pipeline
  */
 #ifndef PIPELINE_MSGQ_SIZE
@@ -483,6 +580,18 @@ struct pipeline *
 	if (status)
 		goto error;
 
+	status = rte_swx_pipeline_port_in_type_register(p,
+		"fd",
+		&rte_swx_port_fd_reader_ops);
+	if (status)
+		goto error;
+
+	status = rte_swx_pipeline_port_out_type_register(p,
+		"fd",
+		&rte_swx_port_fd_writer_ops);
+	if (status)
+		goto error;
+
 	status = rte_swx_pipeline_table_type_register(p,
 		"exact",
 		RTE_SWX_TABLE_MATCH_EXACT,
@@ -541,6 +650,7 @@ struct obj *
 	TAILQ_INIT(&obj->link_list);
 	TAILQ_INIT(&obj->ring_list);
 	TAILQ_INIT(&obj->pipeline_list);
+	TAILQ_INIT(&obj->tap_list);
 
 	return obj;
 }
diff --git a/examples/pipeline/obj.h b/examples/pipeline/obj.h
index 1aab2a3..b921610 100644
--- a/examples/pipeline/obj.h
+++ b/examples/pipeline/obj.h
@@ -126,6 +126,24 @@ struct ring *
 ring_find(struct obj *obj, const char *name);
 
 /*
+ * tap
+ */
+struct tap {
+	TAILQ_ENTRY(tap) node;
+	char name[NAME_SIZE];
+	int fd;
+};
+
+struct tap *
+tap_find(struct obj *obj, const char *name);
+
+struct tap *
+tap_next(struct obj *obj, struct tap *tap);
+
+struct tap *
+tap_create(struct obj *obj, const char *name);
+
+/*
  * pipeline
  */
 struct pipeline {
diff --git a/lib/librte_port/meson.build b/lib/librte_port/meson.build
index 9fcd62c..dd02657 100644
--- a/lib/librte_port/meson.build
+++ b/lib/librte_port/meson.build
@@ -14,6 +14,7 @@ sources = files(
 	'rte_swx_port_ethdev.c',
 	'rte_swx_port_ring.c',
 	'rte_swx_port_source_sink.c',
+	'rte_swx_port_fd.c',
 	)
 headers = files(
 	'rte_port_ethdev.h',
@@ -30,6 +31,7 @@ headers = files(
 	'rte_swx_port_ethdev.h',
 	'rte_swx_port_ring.h',
 	'rte_swx_port_source_sink.h',
+	'rte_swx_port_fd.h',
 	)
 deps += ['ethdev', 'sched', 'ip_frag', 'cryptodev', 'eventdev']
 
diff --git a/lib/librte_port/rte_swx_port_fd.c b/lib/librte_port/rte_swx_port_fd.c
new file mode 100644
index 0000000..5848bbb
--- /dev/null
+++ b/lib/librte_port/rte_swx_port_fd.c
@@ -0,0 +1,299 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2016 Intel Corporation
+ */
+#include <string.h>
+#include <stdint.h>
+#include <unistd.h>
+
+#include <rte_mbuf.h>
+#include <rte_malloc.h>
+#include <rte_hexdump.h>
+
+#include "rte_swx_port_fd.h"
+
+#ifndef TRACE_LEVEL
+#define TRACE_LEVEL 0
+#endif
+
+#if TRACE_LEVEL
+#define TRACE(...) printf(__VA_ARGS__)
+#else
+#define TRACE(...)
+#endif
+
+/*
+ * FD Reader
+ */
+struct reader {
+	struct {
+		int fd;
+		uint32_t mtu;
+		uint32_t burst_size;
+		struct rte_mempool *mempool;
+	} params;
+
+	struct rte_swx_port_in_stats stats;
+	struct rte_mbuf **pkts;
+	uint32_t n_pkts;
+	uint32_t pos;
+};
+
+static void *
+reader_create(void *args)
+{
+	struct rte_swx_port_fd_reader_params *conf = args;
+	struct reader *p;
+
+	/* Check input parameters. */
+	if (!conf || conf->fd < 0 || conf->mtu == 0 || !conf->mempool)
+		return NULL;
+
+	/* Memory allocation. */
+	p = calloc(1, sizeof(struct reader));
+	if (!p)
+		return NULL;
+
+	p->pkts = calloc(conf->burst_size, sizeof(struct rte_mbuf *));
+	if (!p->pkts) {
+		free(p);
+		return NULL;
+	}
+
+	/* Initialization. */
+	p->params.fd = conf->fd;
+	p->params.mtu = conf->mtu;
+	p->params.burst_size = conf->burst_size;
+	p->params.mempool = conf->mempool;
+
+	return p;
+}
+
+static void
+reader_free(void *port)
+{
+	struct reader *p = port;
+	uint32_t i;
+
+	if (!p)
+		return;
+
+	for (i = 0; i < p->n_pkts; i++)
+		rte_pktmbuf_free(p->pkts[i]);
+
+	free(p->pkts);
+	free(p);
+}
+
+static int
+reader_pkt_rx(void *port, struct rte_swx_pkt *pkt)
+{
+	struct reader *p = port;
+	struct rte_mbuf *m;
+	void *pkt_data;
+	ssize_t n_bytes;
+	uint32_t i, j;
+
+	if (p->n_pkts == p->pos) {
+		if (rte_pktmbuf_alloc_bulk(p->params.mempool, p->pkts, p->params.burst_size) != 0)
+			return 0;
+
+		for (i = 0; i < p->params.burst_size; i++) {
+			m = p->pkts[i];
+			pkt_data = rte_pktmbuf_mtod(m, void *);
+			n_bytes = read(p->params.fd, pkt_data, (size_t) p->params.mtu);
+
+			if (n_bytes <= 0)
+				break;
+
+			m->data_len = n_bytes;
+			m->pkt_len = n_bytes;
+
+			p->stats.n_pkts++;
+			p->stats.n_bytes += n_bytes;
+		}
+
+		for (j = i; j < p->params.burst_size; j++)
+			rte_pktmbuf_free(p->pkts[j]);
+
+		p->n_pkts = i;
+		p->pos = 0;
+
+		if (!p->n_pkts)
+			return 0;
+	}
+
+	m = p->pkts[p->pos++];
+	pkt->handle = m;
+	pkt->pkt = m->buf_addr;
+	pkt->offset = m->data_off;
+	pkt->length = m->pkt_len;
+
+	TRACE("[FD %u] Pkt %d (%u bytes at offset %u)\n",
+		(uint32_t)p->params.fd,
+		p->pos - 1,
+		pkt->length,
+		pkt->offset);
+
+	if (TRACE_LEVEL)
+		rte_hexdump(stdout, NULL,
+			&((uint8_t *)m->buf_addr)[m->data_off], m->data_len);
+
+	return 1;
+}
+
+static void
+reader_stats_read(void *port, struct rte_swx_port_in_stats *stats)
+{
+	struct reader *p = port;
+
+	memcpy(stats, &p->stats, sizeof(p->stats));
+}
+
+/*
+ * FD Writer
+ */
+struct writer {
+	struct {
+		int fd;
+		uint32_t mtu;
+		uint32_t burst_size;
+		struct rte_mempool *mempool;
+	} params;
+
+	struct rte_swx_port_out_stats stats;
+	struct rte_mbuf **pkts;
+	uint32_t n_pkts;
+};
+
+static void *
+writer_create(void *args)
+{
+	struct rte_swx_port_fd_writer_params *conf = args;
+	struct writer *p;
+
+	/* Check input parameters. */
+	if (!conf)
+		return NULL;
+
+	/* Memory allocation. */
+	p = calloc(1, sizeof(struct writer));
+	if (!p)
+		return NULL;
+
+
+	p->pkts = calloc(conf->burst_size, sizeof(struct rte_mbuf *));
+	if (!p->pkts) {
+		free(p);
+		return NULL;
+	}
+
+	/* Initialization. */
+	p->params.fd = conf->fd;
+	p->params.burst_size = conf->burst_size;
+
+	return p;
+}
+
+static void
+__writer_flush(struct writer *p)
+{
+	struct rte_mbuf *pkt;
+	void *pkt_data;
+	size_t n_bytes;
+	ssize_t ret;
+	uint32_t i;
+
+	for (i = 0; i < p->n_pkts; i++) {
+		pkt = p->pkts[i];
+		pkt_data = rte_pktmbuf_mtod(pkt, void*);
+		n_bytes = rte_pktmbuf_data_len(pkt);
+
+		ret = write(p->params.fd, pkt_data, n_bytes);
+		if (ret < 0)
+			break;
+	}
+
+	TRACE("[FD %u] %u packets out\n",
+		(uint32_t)p->params.fd,
+		p->n_pkts);
+
+	for (i = 0; i < p->n_pkts; i++)
+		rte_pktmbuf_free(p->pkts[i]);
+
+	p->n_pkts = 0;
+}
+
+static void
+writer_pkt_tx(void *port, struct rte_swx_pkt *pkt)
+{
+	struct writer *p = port;
+	struct rte_mbuf *m = pkt->handle;
+
+	TRACE("[FD %u] Pkt %u (%u bytes at offset %u)\n",
+		(uint32_t)p->params.fd,
+		p->n_pkts - 1,
+		pkt->length,
+		pkt->offset);
+
+	if (TRACE_LEVEL)
+		rte_hexdump(stdout, NULL, &pkt->pkt[pkt->offset], pkt->length);
+
+	m->pkt_len = pkt->length;
+	m->data_len = (uint16_t)pkt->length;
+	m->data_off = (uint16_t)pkt->offset;
+
+	p->stats.n_pkts++;
+	p->stats.n_bytes += pkt->length;
+
+	p->pkts[p->n_pkts++] = m;
+	if (p->n_pkts == p->params.burst_size)
+		__writer_flush(p);
+}
+
+static void
+writer_flush(void *port)
+{
+	struct writer *p = port;
+
+	if (p->n_pkts)
+		__writer_flush(p);
+}
+
+static void
+writer_free(void *port)
+{
+	struct writer *p = port;
+
+	if (!p)
+		return;
+
+	writer_flush(p);
+	free(p->pkts);
+	free(p);
+}
+
+static void
+writer_stats_read(void *port, struct rte_swx_port_out_stats *stats)
+{
+	struct writer *p = port;
+
+	memcpy(stats, &p->stats, sizeof(p->stats));
+}
+
+/*
+ * Summary of port operations
+ */
+struct rte_swx_port_in_ops rte_swx_port_fd_reader_ops = {
+	.create = reader_create,
+	.free = reader_free,
+	.pkt_rx = reader_pkt_rx,
+	.stats_read = reader_stats_read,
+};
+
+struct rte_swx_port_out_ops rte_swx_port_fd_writer_ops = {
+	.create = writer_create,
+	.free = writer_free,
+	.pkt_tx = writer_pkt_tx,
+	.flush = writer_flush,
+	.stats_read = writer_stats_read,
+};
diff --git a/lib/librte_port/rte_swx_port_fd.h b/lib/librte_port/rte_swx_port_fd.h
new file mode 100644
index 0000000..b1a8581
--- /dev/null
+++ b/lib/librte_port/rte_swx_port_fd.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2016 Intel Corporation
+ */
+
+#ifndef __INCLUDE_RTE_SWX_PORT_FD_H__
+#define __INCLUDE_RTE_SWX_PORT_FD_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @file
+ * RTE SWX FD Input and Output Ports
+ *
+ ***/
+
+#include <stdint.h>
+
+#include <rte_mempool.h>
+#include "rte_swx_port.h"
+
+/** fd_reader port parameters */
+struct rte_swx_port_fd_reader_params {
+	/** File descriptor. Must be valid and opened in non-blocking mode. */
+	int fd;
+
+	/** Maximum Transfer Unit (MTU) */
+	uint32_t mtu;
+
+	/** Pre-initialized buffer pool */
+	struct rte_mempool *mempool;
+
+	/** RX burst size */
+	uint32_t burst_size;
+};
+
+/** fd_reader port operations */
+extern struct rte_swx_port_in_ops rte_swx_port_fd_reader_ops;
+
+/** fd_writer port parameters */
+struct rte_swx_port_fd_writer_params {
+	/** File descriptor. Must be valid and opened in non-blocking mode. */
+	int fd;
+
+	/** TX burst size */
+	uint32_t burst_size;
+};
+
+/** fd_writer port operations */
+extern struct rte_swx_port_out_ops rte_swx_port_fd_writer_ops;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/lib/librte_port/version.map b/lib/librte_port/version.map
index d8be68c..8819818 100644
--- a/lib/librte_port/version.map
+++ b/lib/librte_port/version.map
@@ -48,4 +48,6 @@ EXPERIMENTAL {
 	#added in 21.02
 	rte_swx_port_ring_reader_ops;
 	rte_swx_port_ring_writer_ops;
+	rte_swx_port_fd_reader_ops;
+	rte_swx_port_fd_writer_ops;
 };
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Thomas Monjalon]

19/03/2021 13:02, Venkata Suresh Kumar P:
> Add the file descriptor input/output port type for the SWX pipeline.

I think it deserves a bit more explanation about what is FD I/O port.

>  /*
> + * tap
> + */
> +#define TAP_DEV                                            "/dev/net/tun"

Spaces are free :)


> +#ifndef TRACE_LEVEL
> +#define TRACE_LEVEL 0
> +#endif
> +
> +#if TRACE_LEVEL
> +#define TRACE(...) printf(__VA_ARGS__)
> +#else
> +#define TRACE(...)
> +#endif

Would you consider rte_trace?


> --- /dev/null
> +++ b/lib/librte_port/rte_swx_port_fd.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2016 Intel Corporation

I guess you did not create it in 2016.

> + */
> +
> +#ifndef __INCLUDE_RTE_SWX_PORT_FD_H__
> +#define __INCLUDE_RTE_SWX_PORT_FD_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * @file
> + * RTE SWX FD Input and Output Ports
> + *
> + ***/

Useless blank line.

[...]
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif

A comment after such a far #endif is better:
	/* __INCLUDE_RTE_SWX_PORT_FD_H__ */

> --- a/lib/librte_port/version.map
> +++ b/lib/librte_port/version.map
> @@ -48,4 +48,6 @@ EXPERIMENTAL {
>         #added in 21.02

In 21.05

>         rte_swx_port_ring_reader_ops;
>         rte_swx_port_ring_writer_ops;
> +       rte_swx_port_fd_reader_ops;
> +       rte_swx_port_fd_writer_ops;

Please sort in alphabetical order.

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[P, Venkata Suresh Kumar]

Thanks a lot for reviewing the code and providing your comments.

I have addressed below comments in V2 patch.

Thanks & Regards,
Suresh.

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net> 
Sent: Tuesday, March 23, 2021 9:52 PM
To: P, Venkata Suresh Kumar <venkata.suresh.kumar.p@intel.com>
Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Khangar, Churchill <churchill.khangar@intel.com>; Jangra, Yogesh <yogesh.jangra@intel.com>
Subject: Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

19/03/2021 13:02, Venkata Suresh Kumar P:
> Add the file descriptor input/output port type for the SWX pipeline.

I think it deserves a bit more explanation about what is FD I/O port. -- [Suresh] - Addressed in V2 patch

>  /*
> + * tap
> + */
> +#define TAP_DEV                                            "/dev/net/tun"

Spaces are free :) -- [Suresh] - Addressed in V2 patch


> +#ifndef TRACE_LEVEL
> +#define TRACE_LEVEL 0
> +#endif
> +
> +#if TRACE_LEVEL
> +#define TRACE(...) printf(__VA_ARGS__) #else #define TRACE(...) 
> +#endif

Would you consider rte_trace?

> --- /dev/null
> +++ b/lib/librte_port/rte_swx_port_fd.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2016 Intel Corporation

I guess you did not create it in 2016. -- [Suresh] - Addressed in V2 patch

> + */
> +
> +#ifndef __INCLUDE_RTE_SWX_PORT_FD_H__ #define 
> +__INCLUDE_RTE_SWX_PORT_FD_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * @file
> + * RTE SWX FD Input and Output Ports
> + *
> + ***/

Useless blank line. -- [Suresh] - Addressed in V2 patch

[...]
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif

A comment after such a far #endif is better: -- [Suresh] - Addressed in V2 patch
	/* __INCLUDE_RTE_SWX_PORT_FD_H__ */

> --- a/lib/librte_port/version.map
> +++ b/lib/librte_port/version.map
> @@ -48,4 +48,6 @@ EXPERIMENTAL {
>         #added in 21.02

In 21.05

>         rte_swx_port_ring_reader_ops;
>         rte_swx_port_ring_writer_ops;
> +       rte_swx_port_fd_reader_ops;
> +       rte_swx_port_fd_writer_ops;

Please sort in alphabetical order. -- [Suresh] - Addressed in V2 patch

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Stephen Hemminger]

On Fri, 19 Mar 2021 08:02:06 -0400
Venkata Suresh Kumar P <venkata.suresh.kumar.p@intel.com> wrote:

> +static void
> +reader_stats_read(void *port, struct rte_swx_port_in_stats *stats)
> +{
> +	struct reader *p = port;
> +
> +	memcpy(stats, &p->stats, sizeof(p->stats));
> +}

Why not use structure assignment (which is type safe) instead of memcpy?

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Stephen Hemminger]

On Fri, 19 Mar 2021 08:02:06 -0400
Venkata Suresh Kumar P <venkata.suresh.kumar.p@intel.com> wrote:

> +static void
> +__writer_flush(struct writer *p)
> +{
> +	struct rte_mbuf *pkt;
> +	void *pkt_data;
> +	size_t n_bytes;
> +	ssize_t ret;
> +	uint32_t i;
> +
> +	for (i = 0; i < p->n_pkts; i++) {
> +		pkt = p->pkts[i];
> +		pkt_data = rte_pktmbuf_mtod(pkt, void*);
> +		n_bytes = rte_pktmbuf_data_len(pkt);
> +
> +		ret = write(p->params.fd, pkt_data, n_bytes);
> +		if (ret < 0)
> +			break;
> +	}
> +
> +	TRACE("[FD %u] %u packets out\n",
> +		(uint32_t)p->params.fd,
> +		p->n_pkts);
> +
> +	for (i = 0; i < p->n_pkts; i++)
> +		rte_pktmbuf_free(p->pkts[i]);

Loop can be replaced by:
	rte_pktmbuf_free_bulk(p->pkts, p->n_pkts);

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Stephen Hemminger]

On Fri, 19 Mar 2021 08:02:06 -0400
Venkata Suresh Kumar P <venkata.suresh.kumar.p@intel.com> wrote:

> +/*
> + * FD Reader
> + */
> +struct reader {
> +	struct {
> +		int fd;
> +		uint32_t mtu;
> +		uint32_t burst_size;
> +		struct rte_mempool *mempool;
> +	} params;
> +
> +	struct rte_swx_port_in_stats stats;
> +	struct rte_mbuf **pkts;
> +	uint32_t n_pkts;
> +	uint32_t pos;
> +};
> +

You could save one allocation (and get better cache locality)
by using a dynamic array here.

struct reader {
	struct {
		int fd;
		uint32_t mtu;
		uint32_t burst_size;
		struct rte_mempool *mempool;
	} params;

	struct rte_swx_port_in_stats stats;

	uint32_t n_pkts;
	uint32_t pos;
	struct rte_mbuf *pkts[];
};

Then change the allocation to size it based on number of packets.

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Thomas Monjalon]

23/03/2021 19:07, P, Venkata Suresh Kumar:
> Thanks a lot for reviewing the code and providing your comments.
> 
> I have addressed below comments in V2 patch.

OK thanks.

What about the question about rte_trace?
Opinions?



> From: Thomas Monjalon <thomas@monjalon.net> 
> 19/03/2021 13:02, Venkata Suresh Kumar P:
> > Add the file descriptor input/output port type for the SWX pipeline.
> 
> I think it deserves a bit more explanation about what is FD I/O port. -- [Suresh] - Addressed in V2 patch
> 
> >  /*
> > + * tap
> > + */
> > +#define TAP_DEV                                            "/dev/net/tun"
> 
> Spaces are free :) -- [Suresh] - Addressed in V2 patch
> 
> 
> > +#ifndef TRACE_LEVEL
> > +#define TRACE_LEVEL 0
> > +#endif
> > +
> > +#if TRACE_LEVEL
> > +#define TRACE(...) printf(__VA_ARGS__) #else #define TRACE(...) 
> > +#endif
> 
> Would you consider rte_trace?
> 
> > --- /dev/null
> > +++ b/lib/librte_port/rte_swx_port_fd.h
> > @@ -0,0 +1,57 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2016 Intel Corporation
> 
> I guess you did not create it in 2016. -- [Suresh] - Addressed in V2 patch
> 
> > + */
> > +
> > +#ifndef __INCLUDE_RTE_SWX_PORT_FD_H__ #define 
> > +__INCLUDE_RTE_SWX_PORT_FD_H__
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * @file
> > + * RTE SWX FD Input and Output Ports
> > + *
> > + ***/
> 
> Useless blank line. -- [Suresh] - Addressed in V2 patch
> 
> [...]
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> 
> A comment after such a far #endif is better: -- [Suresh] - Addressed in V2 patch
> 	/* __INCLUDE_RTE_SWX_PORT_FD_H__ */
> 
> > --- a/lib/librte_port/version.map
> > +++ b/lib/librte_port/version.map
> > @@ -48,4 +48,6 @@ EXPERIMENTAL {
> >         #added in 21.02
> 
> In 21.05
> 
> >         rte_swx_port_ring_reader_ops;
> >         rte_swx_port_ring_writer_ops;
> > +       rte_swx_port_fd_reader_ops;
> > +       rte_swx_port_fd_writer_ops;
> 
> Please sort in alphabetical order. -- [Suresh] - Addressed in V2 patch

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, March 23, 2021 6:39 PM
> To: P, Venkata Suresh Kumar <venkata.suresh.kumar.p@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Khangar, Churchill <churchill.khangar@intel.com>; Jangra,
> Yogesh <yogesh.jangra@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port
> 
> 23/03/2021 19:07, P, Venkata Suresh Kumar:
> > Thanks a lot for reviewing the code and providing your comments.
> >
> > I have addressed below comments in V2 patch.
> 
> OK thanks.
> 
> What about the question about rte_trace?
> Opinions?
> 

Hi Thomas,

All rte_swx_port ports are currently following this pattern, so it makes sense to have this one do the same for now.

I am not that familiar with the (relatively new) rte_trace mechanism, so I am not sure if it has any run-time performance (I am assuming that it doesn't). We will take the AR to take a look at rte_trace and come back with a patch to convert traces for all ports to rte_trace, most likely in the 21.05 time frame. Is this OK for you?

Regards,
Cristian

> 
> 
> > From: Thomas Monjalon <thomas@monjalon.net>
> > 19/03/2021 13:02, Venkata Suresh Kumar P:
> > > Add the file descriptor input/output port type for the SWX pipeline.
> >
> > I think it deserves a bit more explanation about what is FD I/O port. --
> [Suresh] - Addressed in V2 patch
> >
> > >  /*
> > > + * tap
> > > + */
> > > +#define TAP_DEV                                            "/dev/net/tun"
> >
> > Spaces are free :) -- [Suresh] - Addressed in V2 patch
> >
> >
> > > +#ifndef TRACE_LEVEL
> > > +#define TRACE_LEVEL 0
> > > +#endif
> > > +
> > > +#if TRACE_LEVEL
> > > +#define TRACE(...) printf(__VA_ARGS__) #else #define TRACE(...)
> > > +#endif
> >
> > Would you consider rte_trace?
> >
> > > --- /dev/null
> > > +++ b/lib/librte_port/rte_swx_port_fd.h
> > > @@ -0,0 +1,57 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause
> > > + * Copyright(c) 2016 Intel Corporation
> >
> > I guess you did not create it in 2016. -- [Suresh] - Addressed in V2 patch
> >
> > > + */
> > > +
> > > +#ifndef __INCLUDE_RTE_SWX_PORT_FD_H__ #define
> > > +__INCLUDE_RTE_SWX_PORT_FD_H__
> > > +
> > > +#ifdef __cplusplus
> > > +extern "C" {
> > > +#endif
> > > +
> > > +/**
> > > + * @file
> > > + * RTE SWX FD Input and Output Ports
> > > + *
> > > + ***/
> >
> > Useless blank line. -- [Suresh] - Addressed in V2 patch
> >
> > [...]
> > > +
> > > +#ifdef __cplusplus
> > > +}
> > > +#endif
> > > +
> > > +#endif
> >
> > A comment after such a far #endif is better: -- [Suresh] - Addressed in V2
> patch
> > 	/* __INCLUDE_RTE_SWX_PORT_FD_H__ */
> >
> > > --- a/lib/librte_port/version.map
> > > +++ b/lib/librte_port/version.map
> > > @@ -48,4 +48,6 @@ EXPERIMENTAL {
> > >         #added in 21.02
> >
> > In 21.05
> >
> > >         rte_swx_port_ring_reader_ops;
> > >         rte_swx_port_ring_writer_ops;
> > > +       rte_swx_port_fd_reader_ops;
> > > +       rte_swx_port_fd_writer_ops;
> >
> > Please sort in alphabetical order. -- [Suresh] - Addressed in V2 patch
> 
> 
> 

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Thomas Monjalon]

23/03/2021 19:56, Dumitrescu, Cristian:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 23/03/2021 19:07, P, Venkata Suresh Kumar:
> > > Thanks a lot for reviewing the code and providing your comments.
> > >
> > > I have addressed below comments in V2 patch.
> > 
> > OK thanks.
> > 
> > What about the question about rte_trace?
> > Opinions?
> 
> Hi Thomas,
> 
> All rte_swx_port ports are currently following this pattern,
> so it makes sense to have this one do the same for now.
> 
> I am not that familiar with the (relatively new) rte_trace mechanism, so I am not sure if it has any run-time performance (I am assuming that it doesn't). We will take the AR to take a look at rte_trace and come back with a patch to convert traces for all ports to rte_trace, most likely in the 21.05 time frame. Is this OK for you?

Yes, would be great.
In general we should avoid such #ifdef in DPDK code.
Maybe rte_trace can help, maybe rte_log levels would suffice.
Note there is RTE_LOG_DP_LEVEL for logging in datapath.

Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, March 23, 2021 7:56 PM
> To: P, Venkata Suresh Kumar <venkata.suresh.kumar.p@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; Khangar, Churchill <churchill.khangar@intel.com>; Jangra,
> Yogesh <yogesh.jangra@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] port: add file descriptor SWX port
> 
> 23/03/2021 19:56, Dumitrescu, Cristian:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 23/03/2021 19:07, P, Venkata Suresh Kumar:
> > > > Thanks a lot for reviewing the code and providing your comments.
> > > >
> > > > I have addressed below comments in V2 patch.
> > >
> > > OK thanks.
> > >
> > > What about the question about rte_trace?
> > > Opinions?
> >
> > Hi Thomas,
> >
> > All rte_swx_port ports are currently following this pattern,
> > so it makes sense to have this one do the same for now.
> >
> > I am not that familiar with the (relatively new) rte_trace mechanism, so I
> am not sure if it has any run-time performance (I am assuming that it
> doesn't). We will take the AR to take a look at rte_trace and come back with a
> patch to convert traces for all ports to rte_trace, most likely in the 21.05 time
> frame. Is this OK for you?
> 
> Yes, would be great.
> In general we should avoid such #ifdef in DPDK code.
> Maybe rte_trace can help, maybe rte_log levels would suffice.
> Note there is RTE_LOG_DP_LEVEL for logging in datapath.
> 

Yes, we will look at rte_trace for 21.05 and come back with a resolution. Thanks, Thomas!