[dpdk-dev] [PATCH 0/4] Add PCAP support to source and sink port

[dpdk-dev] [PATCH 0/4] Add PCAP support to source and sink port

[Fan Zhang]

This patchset adds feature to source and sink type port in librte_port
library, and to examples/ip_pipline. Originally, source/sink ports act
as input and output of NULL packets generator. This patchset enables
them read from and write to specific PCAP file, to generate and dump
packets.

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Fan Zhang (4):
  lib/librte_port: add PCAP file support to source port
  example/ip_pipeline: add PCAP file support
  lib/librte_port: add packet dumping to PCAP file support in sink port
  examples/ip_pipeline: add packets dumping to PCAP file support

 config/common_bsdapp                   |   1 +
 config/common_linuxapp                 |   1 +
 examples/ip_pipeline/app.h             |   4 +
 examples/ip_pipeline/config_parse.c    | 261 ++++++++++++++++++-
 examples/ip_pipeline/init.c            |  19 ++
 examples/ip_pipeline/pipeline_be.h     |   2 +
 lib/librte_port/Makefile               |   4 +
 lib/librte_port/rte_port_source_sink.c | 458 +++++++++++++++++++++++++++++++--
 lib/librte_port/rte_port_source_sink.h |  18 +-
 mk/rte.app.mk                          |   1 +
 10 files changed, 751 insertions(+), 18 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH 1/4] lib/librte_port: add PCAP file support to source port

[Fan Zhang]

Originally, source ports in librte_port is an input port used as packet
generator. Similar to Linux kernel /dev/zero character device, it
generates null packets. This patch adds optional PCAP file support to
source port: instead of sending NULL packets, the source port generates
packets copied from a PCAP file. To increase the performance, the packets
in the file are loaded to memory initially, and copied to mbufs in circular
manner. Users can enable or disable this feature by setting
CONFIG_RTE_PORT_PCAP compiler option "y" or "n".

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 config/common_bsdapp                   |   1 +
 config/common_linuxapp                 |   1 +
 lib/librte_port/Makefile               |   4 +
 lib/librte_port/rte_port_source_sink.c | 190 +++++++++++++++++++++++++++++++++
 lib/librte_port/rte_port_source_sink.h |   7 ++
 mk/rte.app.mk                          |   1 +
 6 files changed, 204 insertions(+)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index ed7c31c..1eb36ae 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -459,6 +459,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
 #
 CONFIG_RTE_LIBRTE_PORT=y
 CONFIG_RTE_PORT_STATS_COLLECT=n
+CONFIG_RTE_PORT_PCAP=n
 
 #
 # Compile librte_table
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 74bc515..776fcd3 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -476,6 +476,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
 #
 CONFIG_RTE_LIBRTE_PORT=y
 CONFIG_RTE_PORT_STATS_COLLECT=n
+CONFIG_RTE_PORT_PCAP=n
 
 #
 # Compile librte_table
diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 410053e..e3e4318 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -36,6 +36,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
 #
 LIB = librte_port.a
 
+ifeq ($(CONFIG_RTE_PORT_PCAP),y)
+LDLIBS += -lpcap
+endif
+
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 
diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index a06477e..44fc0d5 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -36,6 +36,11 @@
 #include <rte_mbuf.h>
 #include <rte_mempool.h>
 #include <rte_malloc.h>
+#include <rte_memcpy.h>
+
+#ifdef RTE_PORT_PCAP
+#include <pcap.h>
+#endif
 
 #include "rte_port_source_sink.h"
 
@@ -60,14 +65,158 @@ struct rte_port_source {
 	struct rte_port_in_stats stats;
 
 	struct rte_mempool *mempool;
+
+	/* PCAP buffers and indexes */
+	uint8_t **pkts;
+	uint8_t *pkt_buff;
+	uint32_t *pkt_len;
+	uint32_t n_pkts;
+	uint32_t pkt_index;
 };
 
+#ifdef RTE_PORT_PCAP
+
+/**
+ * Load PCAP file, allocate and copy packets in the file to memory
+ *
+ * @param p
+ *   Parameters for source port
+ * @param port
+ *   Handle to source port
+ * @param socket_id
+ *   Socket id where the memory is created
+ * @return
+ *   0 on SUCCESS
+ *   error code otherwise
+ */
+static int
+pcap_source_load(struct rte_port_source_params *p,
+		struct rte_port_source *port,
+		int socket_id)
+{
+	uint32_t n_pkts = 0;
+	uint32_t i;
+	uint32_t *pkt_len_aligns = NULL;
+	size_t total_buff_len = 0;
+	pcap_t *pcap_handle;
+	char pcap_errbuf[PCAP_ERRBUF_SIZE];
+	uint32_t max_len;
+	struct pcap_pkthdr pcap_hdr;
+	const uint8_t *pkt;
+	uint8_t *buff = NULL;
+	uint32_t pktmbuf_maxlen = (uint32_t)
+			(rte_pktmbuf_data_room_size(port->mempool) -
+			RTE_PKTMBUF_HEADROOM);
+
+	if (p->file_name == NULL)
+		return 0;
+
+	if (p->n_bytes_per_pkt == 0)
+		max_len = pktmbuf_maxlen;
+	else
+		max_len = RTE_MIN(p->n_bytes_per_pkt, pktmbuf_maxlen);
+
+	/* first time open, get packet number */
+	pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+	if (pcap_handle == NULL)
+		goto error_exit;
+
+	while ((pkt = pcap_next(pcap_handle, &pcap_hdr)) != NULL)
+		n_pkts++;
+
+	pcap_close(pcap_handle);
+
+	port->pkt_len = rte_zmalloc_socket("PCAP",
+		(sizeof(*port->pkt_len) * n_pkts), 0, socket_id);
+	if (port->pkt_len == NULL)
+		goto error_exit;
+
+	pkt_len_aligns = rte_malloc("PCAP",
+		(sizeof(*pkt_len_aligns) * n_pkts), 0);
+	if (pkt_len_aligns == NULL)
+		goto error_exit;
+
+	port->pkts = rte_zmalloc_socket("PCAP",
+		(sizeof(*port->pkts) * n_pkts), 0, socket_id);
+	if (port->pkts == NULL)
+		goto error_exit;
+
+	/* open 2nd time, get pkt_len */
+	pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+	if (pcap_handle == NULL)
+		goto error_exit;
+
+	for (i = 0; i < n_pkts; i++) {
+		pkt = pcap_next(pcap_handle, &pcap_hdr);
+		port->pkt_len[i] = RTE_MIN(max_len, pcap_hdr.len);
+		pkt_len_aligns[i] = RTE_CACHE_LINE_ROUNDUP(port->pkt_len[i]);
+		total_buff_len += pkt_len_aligns[i];
+	}
+
+	pcap_close(pcap_handle);
+
+	/* allocate a big trunk of data for pcap file load */
+	buff = rte_zmalloc_socket("PCAP",
+		total_buff_len, 0, socket_id);
+	if (buff == NULL)
+		goto error_exit;
+	port->pkt_buff = buff;
+
+	/* open file one last time to copy the pkt content */
+	pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+	if (pcap_handle == NULL)
+		goto error_exit;
+
+	for (i = 0; i < n_pkts; i++) {
+		pkt = pcap_next(pcap_handle, &pcap_hdr);
+		rte_memcpy(buff, pkt, port->pkt_len[i]);
+		port->pkts[i] = buff;
+		buff += pkt_len_aligns[i];
+	}
+
+	pcap_close(pcap_handle);
+
+	port->n_pkts = n_pkts;
+
+	rte_free(pkt_len_aligns);
+
+	return 0;
+
+error_exit:
+	if (pkt_len_aligns)
+		rte_free(pkt_len_aligns);
+	if (port->pkt_len)
+		rte_free(port->pkt_len);
+	if (port->pkts)
+		rte_free(port->pkts);
+	if (port->pkt_buff)
+		rte_free(port->pkt_buff);
+
+	return -1;
+}
+
+#else
+static int
+pcap_source_load(__rte_unused struct rte_port_source_params *p,
+		struct rte_port_source *port,
+		__rte_unused int socket_id)
+{
+	port->pkt_buff = NULL;
+	port->pkt_len = NULL;
+	port->pkts = NULL;
+	port->pkt_index = 0;
+
+	return 0;
+}
+#endif
+
 static void *
 rte_port_source_create(void *params, int socket_id)
 {
 	struct rte_port_source_params *p =
 			(struct rte_port_source_params *) params;
 	struct rte_port_source *port;
+	int status;
 
 	/* Check input arguments*/
 	if ((p == NULL) || (p->mempool == NULL)) {
@@ -86,16 +235,41 @@ rte_port_source_create(void *params, int socket_id)
 	/* Initialization */
 	port->mempool = (struct rte_mempool *) p->mempool;
 
+	/* pcap file load and initialization */
+	status = pcap_source_load(p, port, socket_id);
+	if (status < 0) {
+		RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP support\n",
+				__func__);
+		rte_free(port);
+		port = NULL;
+	} else {
+		if (port->pkt_buff != NULL) {
+			RTE_LOG(INFO, PORT, "Successfully load pcap file"
+				" %s with %u pkts\n",
+				p->file_name, port->n_pkts);
+		}
+	}
+
 	return port;
 }
 
 static int
 rte_port_source_free(void *port)
 {
+	struct rte_port_source *p =
+			(struct rte_port_source *)port;
+
 	/* Check input parameters */
 	if (port == NULL)
 		return 0;
 
+	if (p->pkt_len)
+		rte_free(p->pkt_len);
+	if (p->pkts)
+		rte_free(p->pkts);
+	if (p->pkt_buff)
+		rte_free(p->pkt_buff);
+
 	rte_free(port);
 
 	return 0;
@@ -115,6 +289,22 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 		rte_pktmbuf_reset(pkts[i]);
 	}
 
+	if (p->pkt_buff != NULL) {
+		for (i = 0; i < n_pkts; i++) {
+			uint8_t *pkt_data = rte_pktmbuf_mtod(pkts[i],
+					uint8_t *);
+
+			rte_memcpy(pkt_data, p->pkts[p->pkt_index],
+					p->pkt_len[p->pkt_index]);
+			pkts[i]->data_len = p->pkt_len[p->pkt_index];
+			pkts[i]->pkt_len = pkts[i]->data_len;
+
+			p->pkt_index++;
+			if (p->pkt_index >= p->n_pkts)
+				p->pkt_index = 0;
+		}
+	}
+
 	RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(p, n_pkts);
 
 	return n_pkts;
diff --git a/lib/librte_port/rte_port_source_sink.h b/lib/librte_port/rte_port_source_sink.h
index 0f9be79..6f39bec 100644
--- a/lib/librte_port/rte_port_source_sink.h
+++ b/lib/librte_port/rte_port_source_sink.h
@@ -53,6 +53,13 @@ extern "C" {
 struct rte_port_source_params {
 	/** Pre-initialized buffer pool */
 	struct rte_mempool *mempool;
+	/** The full path of the pcap file to read packets from */
+	char *file_name;
+	/** The number of bytes to be read from each packet in the
+	 *  pcap file. If this value is 0, the whole packet is read;
+	 *  if it is bigger than packet size, the generated packets
+	 *  will contain the whole packet */
+	uint32_t n_bytes_per_pkt;
 };
 
 /** source port operations */
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 8ecab41..39fdbe1 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -111,6 +111,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lxenstore
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD)      += -lgxio
 # QAT PMD has a dependency on libcrypto (from openssl) for calculating HMAC precomputes
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT)        += -lcrypto
+_LDLIBS-$(CONFIG_RTE_PORT_PCAP)        += -lpcap
 endif # CONFIG_RTE_BUILD_COMBINE_LIBS or not CONFIG_RTE_BUILD_SHARED_LIBS
 
 _LDLIBS-y += --start-group
-- 
2.5.0

[dpdk-dev] [PATCH 2/4] examples/ip_pipeline: add PCAP file support

[Fan Zhang]

This patch add PCAP file support to ip_pipeline. Input port type SOURCE
now supports loading specific PCAP file and sends the packets in it to
pipeline instance. The packets are then released by SINK output port. This
feature can be applied by specifying parameters in configuration file as
shown below;

[PIPELINE1]
type = PASS-THROUGH
core = 1
pktq_in = SOURCE0 SOURCE1
pktq_out = SINK0 SINK1
pcap_file_rd = /path/to/eth1.PCAP /path/to/eth2.PCAP
pcap_bytes_rd_per_pkt = 0 64

The configuration section "pcap_file_rd" contains full path and name of
the PCAP file to be loaded. If multiple SOURCEs exists, each shall have
its own PCAP file path listed in this section, separated by spaces.
Multiple SOURCE ports may share same PCAP file to be copied.

The configuration section "pcap_bytes_rd_per_pkt" contains integer value
and indicates the maximum number of bytes to be copied from each packet
in the PCAP file. If this value is "0", all packets in the file will be
copied fully; if the packet size is smaller than the assigned value, the
entire packet is copied. Same as "pcap_file_rd", every SOURCE shall have
its own maximum copy byte number.

To enable PCAP support to IP pipeline, the compiler option
CONFIG_RTE_PORT_PCAP must be set to 'y'. It is possible to disable PCAP
support by removing "pcap_file_rd" and "pcap_bytes_rd_per_pkt" lines
from the configuration file.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/ip_pipeline/app.h          |   2 +
 examples/ip_pipeline/config_parse.c | 102 +++++++++++++++++++++++++++++++++++-
 examples/ip_pipeline/init.c         |   8 +++
 3 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
index 6510d6d..9dbe668 100644
--- a/examples/ip_pipeline/app.h
+++ b/examples/ip_pipeline/app.h
@@ -148,6 +148,8 @@ struct app_pktq_source_params {
 	uint32_t parsed;
 	uint32_t mempool_id; /* Position in the app->mempool_params array */
 	uint32_t burst;
+	char *file_name; /* Full path of PCAP file to be copied to mbufs */
+	uint32_t n_bytes_per_pkt;
 };
 
 struct app_pktq_sink_params {
diff --git a/examples/ip_pipeline/config_parse.c b/examples/ip_pipeline/config_parse.c
index 1bedbe4..f0bed81 100644
--- a/examples/ip_pipeline/config_parse.c
+++ b/examples/ip_pipeline/config_parse.c
@@ -178,6 +178,8 @@ struct app_pktq_source_params default_source_params = {
 	.parsed = 0,
 	.mempool_id = 0,
 	.burst = 32,
+	.file_name = NULL,
+	.n_bytes_per_pkt = 0,
 };
 
 struct app_pktq_sink_params default_sink_params = {
@@ -924,6 +926,83 @@ parse_eal(struct app_params *app,
 }
 
 static int
+parse_pipeline_pcap_source(struct app_params *app,
+	struct app_pipeline_params *p,
+	const char *file_name, const char *cp_size)
+{
+	const char *next = NULL;
+	char *end;
+	uint32_t i;
+	int parse_file = 0;
+
+	if (file_name && !cp_size) {
+		next = file_name;
+		parse_file = 1; /* parse file path */
+	} else if (cp_size && !file_name) {
+		next = cp_size;
+		parse_file = 0; /* parse copy size */
+	} else
+		return -EINVAL;
+
+	char name[APP_PARAM_NAME_SIZE];
+	size_t name_len;
+
+	if (p->n_pktq_in == 0)
+		return -EINVAL;
+
+	for (i = 0; i < p->n_pktq_in; i++) {
+		if (p->pktq_in[i].type != APP_PKTQ_IN_SOURCE)
+			return -EINVAL;
+	}
+
+	i = 0;
+	while (*next != '\0') {
+		uint32_t id;
+
+		if (i >= p->n_pktq_in)
+			return -EINVAL;
+
+		id = p->pktq_in[i].id;
+
+		end = strchr(next, ' ');
+		if (!end)
+			name_len = strlen(next);
+		else
+			name_len = end - next;
+
+		if (name_len == 0 || name_len == sizeof(name))
+			return -EINVAL;
+
+		strncpy(name, next, name_len);
+		name[name_len] = '\0';
+		next += name_len;
+		if (*next != '\0')
+			next++;
+
+		if (parse_file) {
+			app->source_params[id].file_name = strdup(name);
+			if (app->source_params[id].file_name == NULL)
+				return -ENOMEM;
+		} else {
+			if (parser_read_uint32(
+					&app->source_params[id].n_bytes_per_pkt,
+					name) != 0) {
+				if (app->source_params[id].file_name != NULL)
+					free(app->source_params[id].file_name);
+				return -EINVAL;
+			}
+		}
+
+		i++;
+
+		if (i == p->n_pktq_in)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int
 parse_pipeline_pktq_in(struct app_params *app,
 	struct app_pipeline_params *p,
 	const char *value)
@@ -1169,6 +1248,12 @@ parse_pipeline(struct app_params *app,
 		else if (strcmp(ent->name, "timer_period") == 0)
 			ret = parser_read_uint32(&param->timer_period,
 				ent->value);
+		else if (strcmp(ent->name, "pcap_file_rd") == 0)
+			ret = parse_pipeline_pcap_source(app, param,
+					ent->value, NULL);
+		else if (strcmp(ent->name, "pcap_bytes_rd_per_pkt") == 0)
+			ret = parse_pipeline_pcap_source(app, param,
+					NULL, ent->value);
 		else {
 			APP_CHECK((param->n_args < APP_MAX_PIPELINE_ARGS),
 				"CFG: [%s] out of memory",
@@ -1700,8 +1785,18 @@ parse_source(struct app_params *app,
 			PARSER_IMPLICIT_PARAM_ADD_CHECK(idx, section_name);
 			param->mempool_id = idx;
 			ret = 0;
-		} else if (strcmp(ent->name, "burst") == 0)
+		} else if (strcmp(ent->name, "burst") == 0) {
 			ret = parser_read_uint32(&param->burst, ent->value);
+		} else if (strcmp(ent->name, "pcap_file_rd")) {
+			param->file_name = strdup(ent->value);
+			if (param->file_name != NULL)
+				ret = 0;
+			else
+				ret = -ENOMEM;
+		} else if (strcmp(ent->name, "pcap_bytes_rd_per_pkt") == 0) {
+			ret = parser_read_uint32(&param->n_bytes_per_pkt,
+					ent->value);
+		}
 
 		APP_CHECK(ret != -ESRCH,
 			"CFG: [%s] entry '%s': unknown entry\n",
@@ -2272,6 +2367,9 @@ save_source_params(struct app_params *app, FILE *f)
 			"mempool",
 			app->mempool_params[p->mempool_id].name);
 		fprintf(f, "%s = %" PRIu32 "\n", "burst", p->burst);
+		fprintf(f, "%s = %s\n", "pcap_file_rd", p->file_name);
+		fprintf(f, "%s = %" PRIu32 "\n",
+				"pcap_bytes_rd_per_pkt", p->n_bytes_per_pkt);
 		fputc('\n', f);
 	}
 }
@@ -2654,7 +2752,7 @@ app_config_args(struct app_params *app, int argc, char **argv)
 	optind = 0; /* reset getopt lib */
 
 	/* Check that mandatory args have been provided */
-	if (!p_present)
+	if (!p_present && (app->n_pktq_hwq_in != 0))
 		rte_panic("Error: PORT_MASK is not provided\n");
 
 	/* Check dependencies between args */
diff --git a/examples/ip_pipeline/init.c b/examples/ip_pipeline/init.c
index 186ca03..4a3a9a7 100644
--- a/examples/ip_pipeline/init.c
+++ b/examples/ip_pipeline/init.c
@@ -1154,6 +1154,14 @@ static void app_pipeline_params_get(struct app_params *app,
 			out->type = PIPELINE_PORT_IN_SOURCE;
 			out->params.source.mempool = app->mempool[mempool_id];
 			out->burst_size = app->source_params[in->id].burst;
+			if (app->source_params[in->id].file_name == NULL)
+				break;
+			out->params.source.file_name = strdup(
+				app->source_params[in->id].file_name);
+			out->params.source.n_bytes_per_pkt =
+				app->source_params[in->id].n_bytes_per_pkt;
+			if (out->params.source.file_name == NULL)
+				out->params.source.n_bytes_per_pkt = 0;
 			break;
 		default:
 			break;
-- 
2.5.0

[dpdk-dev] [PATCH 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port

[Fan Zhang]

Originally, sink ports in librte_port releases received mbufs back to
mempool. This patch adds optional packet dumping to PCAP feature in sink
port: the packets will be dumped to user defined PCAP file for storage or
debugging. The user may also choose the sink port's activity: either it
continuously dump the packets to the file, or stops at certain dumping

This feature shares same CONFIG_RTE_PORT_PCAP compiler option as source
port PCAP file support feature. Users can enable or disable this feature
by setting CONFIG_RTE_PORT_PCAP compiler option "y" or "n".

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 lib/librte_port/rte_port_source_sink.c | 268 +++++++++++++++++++++++++++++++--
 lib/librte_port/rte_port_source_sink.h |  11 +-
 2 files changed, 263 insertions(+), 16 deletions(-)

diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index 44fc0d5..2878014 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -37,6 +37,7 @@
 #include <rte_mempool.h>
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
+#include <rte_ether.h>
 
 #ifdef RTE_PORT_PCAP
 #include <pcap.h>
@@ -345,12 +346,183 @@ rte_port_source_stats_read(void *port,
 
 struct rte_port_sink {
 	struct rte_port_out_stats stats;
+
+	/* PCAP dumper handle and pkts number */
+	void *dumper;
+	uint32_t max_pkts;
+	uint32_t pkt_index;
+	uint32_t dump_finish;
 };
 
+#ifdef RTE_PORT_PCAP
+
+/**
+ * Open PCAP file for dumping packets to the file later
+ *
+ * @param port
+ *   Handle to sink port
+ * @param p
+ *   Sink port parameter
+ * @return
+ *   0 on SUCCESS
+ *   error code otherwise
+ */
+static int
+pcap_sink_open(struct rte_port_sink *port,
+		__rte_unused struct rte_port_sink_params *p)
+{
+	pcap_t *tx_pcap;
+	pcap_dumper_t *pcap_dumper;
+
+	if (p->file_name == NULL) {
+		port->dumper = NULL;
+		port->max_pkts = 0;
+		port->pkt_index = 0;
+		port->dump_finish = 0;
+		return 0;
+	}
+
+	/** Open a dead pcap handler for opening dumper file */
+	tx_pcap = pcap_open_dead(DLT_EN10MB, 65535);
+	if (tx_pcap == NULL)
+		return -1;
+
+	/* The dumper is created using the previous pcap_t reference */
+	pcap_dumper = pcap_dump_open(tx_pcap, p->file_name);
+	if (pcap_dumper == NULL)
+		return -1;
+
+	port->dumper = pcap_dumper;
+	port->max_pkts = p->max_n_pkts;
+	port->pkt_index = 0;
+	port->dump_finish = 0;
+
+	return 0;
+}
+
+uint8_t jumbo_pkt_buf[ETHER_MAX_JUMBO_FRAME_LEN];
+
+/**
+ * Dump a packet to PCAP dumper
+ *
+ * @param p
+ *   Handle to sink port
+ * @param mbuf
+ *   Handle to mbuf structure holding the packet
+ */
+static void
+pcap_sink_dump_pkt(struct rte_port_sink *port, struct rte_mbuf *mbuf)
+{
+	uint8_t *pcap_dumper = (uint8_t *)(port->dumper);
+	struct pcap_pkthdr pcap_hdr;
+	uint8_t *pkt;
+
+	/* Maximum num packets already reached */
+	if (port->dump_finish)
+		return;
+
+	pkt = rte_pktmbuf_mtod(mbuf, uint8_t *);
+
+	pcap_hdr.len = mbuf->pkt_len;
+	pcap_hdr.caplen = pcap_hdr.len;
+	gettimeofday(&(pcap_hdr.ts), NULL);
+
+	if (mbuf->nb_segs > 1) {
+		struct rte_mbuf *jumbo_mbuf;
+		uint32_t pkt_index = 0;
+
+		/* if packet size longer than ETHER_MAX_JUMBO_FRAME_LEN,
+		 * ignore it.
+		 */
+		if (mbuf->pkt_len > ETHER_MAX_JUMBO_FRAME_LEN)
+			return;
+
+		for (jumbo_mbuf = mbuf; jumbo_mbuf != NULL;
+				jumbo_mbuf = jumbo_mbuf->next) {
+			rte_memcpy(&jumbo_pkt_buf[pkt_index],
+					rte_pktmbuf_mtod(jumbo_mbuf, uint8_t *),
+					jumbo_mbuf->data_len);
+			pkt_index += jumbo_mbuf->data_len;
+		}
+
+		jumbo_pkt_buf[pkt_index] = '\0';
+
+		pkt = jumbo_pkt_buf;
+	}
+
+	pcap_dump(pcap_dumper, &pcap_hdr, pkt);
+
+	port->pkt_index++;
+
+	if ((port->max_pkts != 0) && (port->pkt_index >= port->max_pkts)) {
+		port->dump_finish = 1;
+		RTE_LOG(INFO, PORT, "Dumped %u packets to file\n",
+				port->pkt_index);
+	}
+
+}
+
+/**
+ * Flush pcap dumper
+ *
+ * @param dumper
+ *   Handle to pcap dumper
+ */
+
+static void
+pcap_sink_flush_pkt(void *dumper)
+{
+	pcap_dumper_t *pcap_dumper = (pcap_dumper_t *)dumper;
+
+	pcap_dump_flush(pcap_dumper);
+}
+
+/**
+ * Close a PCAP dumper handle
+ *
+ * @param dumper
+ *   Handle to pcap dumper
+ */
+static void
+pcap_sink_close(void *dumper)
+{
+	pcap_dumper_t *pcap_dumper = (pcap_dumper_t *)dumper;
+
+	pcap_dump_close(pcap_dumper);
+}
+
+#else
+
+static int
+pcap_sink_open(struct rte_port_sink *port,
+		__rte_unused struct rte_port_sink_params *p)
+{
+	port->dumper = NULL;
+	port->max_pkts = 0;
+	port->pkt_index = 0;
+	port->dump_finish = 0;
+
+	return 0;
+}
+
+static void
+pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
+		__rte_unused struct rte_mbuf *mbuf) {}
+
+static void
+pcap_sink_flush_pkt(__rte_unused void *dumper) {}
+
+static void
+pcap_sink_close(__rte_unused void *dumper) {}
+
+#endif
+
 static void *
 rte_port_sink_create(__rte_unused void *params, int socket_id)
 {
 	struct rte_port_sink *port;
+	struct rte_port_sink_params *p = params;
+	int status;
 
 	/* Memory allocation */
 	port = rte_zmalloc_socket("PORT", sizeof(*port),
@@ -360,6 +532,19 @@ rte_port_sink_create(__rte_unused void *params, int socket_id)
 		return NULL;
 	}
 
+	/* Try to open PCAP file for dumping, if possible */
+	status = pcap_sink_open(port, p);
+	if (status < 0) {
+		RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP support "
+				"support\n", __func__);
+		rte_free(port);
+		port = NULL;
+	} else {
+		if (port->dumper != NULL)
+			RTE_LOG(INFO, PORT, "Ready to dump packets to file "
+					"%s\n", p->file_name);
+	}
+
 	return port;
 }
 
@@ -369,6 +554,8 @@ rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
 	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
 
 	RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
+	if (p->dumper != NULL)
+		pcap_sink_dump_pkt(p, pkt);
 	rte_pktmbuf_free(pkt);
 	RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
 
@@ -387,21 +574,44 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 
 		RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, n_pkts);
 		RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
-		for (i = 0; i < n_pkts; i++) {
-			struct rte_mbuf *pkt = pkts[i];
-
-			rte_pktmbuf_free(pkt);
+		if (p->dumper) {
+			for (i = 0; i < n_pkts; i++) {
+				struct rte_mbuf *pkt = pkts[i];
+
+				pcap_sink_dump_pkt(p, pkt);
+				rte_pktmbuf_free(pkt);
+			}
+		} else {
+			for (i = 0; i < n_pkts; i++) {
+				struct rte_mbuf *pkt = pkts[i];
+
+				rte_pktmbuf_free(pkt);
+			}
 		}
 	} else {
-		for ( ; pkts_mask; ) {
-			uint32_t pkt_index = __builtin_ctzll(pkts_mask);
-			uint64_t pkt_mask = 1LLU << pkt_index;
-			struct rte_mbuf *pkt = pkts[pkt_index];
-
-			RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
-			RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
-			rte_pktmbuf_free(pkt);
-			pkts_mask &= ~pkt_mask;
+		if (p->dumper) {
+			for ( ; pkts_mask; ) {
+				uint32_t pkt_index = __builtin_ctzll(pkts_mask);
+				uint64_t pkt_mask = 1LLU << pkt_index;
+				struct rte_mbuf *pkt = pkts[pkt_index];
+
+				RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
+				RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
+				pcap_sink_dump_pkt(p, pkt);
+				rte_pktmbuf_free(pkt);
+				pkts_mask &= ~pkt_mask;
+			}
+		} else {
+			for ( ; pkts_mask; ) {
+				uint32_t pkt_index = __builtin_ctzll(pkts_mask);
+				uint64_t pkt_mask = 1LLU << pkt_index;
+				struct rte_mbuf *pkt = pkts[pkt_index];
+
+				RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
+				RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
+				rte_pktmbuf_free(pkt);
+				pkts_mask &= ~pkt_mask;
+			}
 		}
 	}
 
@@ -409,6 +619,34 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 }
 
 static int
+rte_port_sink_flush(void *port)
+{
+	struct rte_port_sink *p = (struct rte_port_sink *)port;
+
+	if (p->dumper != NULL)
+		pcap_sink_flush_pkt(p->dumper);
+
+	return 0;
+}
+
+static int
+rte_port_sink_free(void *port)
+{
+	struct rte_port_sink *p =
+			(struct rte_port_sink *)port;
+	/* Check input parameters */
+	if (p == NULL)
+		return 0;
+
+	if (p->dumper != NULL)
+		pcap_sink_close(p->dumper);
+
+	rte_free(p);
+
+	return 0;
+}
+
+static int
 rte_port_sink_stats_read(void *port, struct rte_port_out_stats *stats,
 		int clear)
 {
@@ -436,9 +674,9 @@ struct rte_port_in_ops rte_port_source_ops = {
 
 struct rte_port_out_ops rte_port_sink_ops = {
 	.f_create = rte_port_sink_create,
-	.f_free = NULL,
+	.f_free = rte_port_sink_free,
 	.f_tx = rte_port_sink_tx,
 	.f_tx_bulk = rte_port_sink_tx_bulk,
-	.f_flush = NULL,
+	.f_flush = rte_port_sink_flush,
 	.f_stats = rte_port_sink_stats_read,
 };
diff --git a/lib/librte_port/rte_port_source_sink.h b/lib/librte_port/rte_port_source_sink.h
index 6f39bec..bc88fdd 100644
--- a/lib/librte_port/rte_port_source_sink.h
+++ b/lib/librte_port/rte_port_source_sink.h
@@ -65,7 +65,16 @@ struct rte_port_source_params {
 /** source port operations */
 extern struct rte_port_in_ops rte_port_source_ops;
 
-/** sink port parameters: NONE */
+/** sink port parameters */
+struct rte_port_sink_params {
+	/** The full path of the pcap file to write the packets to */
+	char *file_name;
+	/** The maximum number of packets write to the pcap file.
+	 *  If this value is 0, the "infinite" write will be carried
+	 *  out.
+	 */
+	uint32_t max_n_pkts;
+};
 
 /** sink port operations */
 extern struct rte_port_out_ops rte_port_sink_ops;
-- 
2.5.0

[dpdk-dev] [PATCH 4/4] examples/ip_pipeline: add packets dumping to PCAP file support

[Fan Zhang]

This patch add packet dumping feature to ip_pipeline. Output port type
SINK now supports dumping packets to PCAP file before releasing mbuf back
to mempool. This feature can be applied by specifying parameters in
configuration file as shown below:

[PIPELINE1]
type = PASS-THROUGH
core = 1
pktq_in = SOURCE0 SOURCE1
pktq_out = SINK0 SINK1
pcap_file_wr = /path/to/eth1.pcap /path/to/eth2.pcap
pcap_n_pkt_wr = 80 0

The configuration section "pcap_file_wr" contains full path and name of
the PCAP file which the packets will be dumped to. If multiple SINKs
exists, each shall have its own PCAP file path listed in this section,
separated by spaces. Multiple SINK ports shall NOT share same PCAP file to
be dumped.

The configuration section "pcap_n_pkt_wr" contains integer value(s)
and indicates the maximum number of packets to be dumped to the PCAP file.
If this value is "0", the "infinite" dumping mode will be used. If this
value is N (N > 0), the dumping will be finished when the number of
packets dumped to the file reaches N.

To enable PCAP dumping support to IP pipeline, the compiler option
CONFIG_RTE_PORT_PCAP must be set to 'y'. It is possible to disable this
feature by removing "pcap_file_wr" and "pcap_n_pkt_wr" lines from the
configuration file.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/ip_pipeline/app.h          |   2 +
 examples/ip_pipeline/config_parse.c | 159 ++++++++++++++++++++++++++++++++++++
 examples/ip_pipeline/init.c         |  11 +++
 examples/ip_pipeline/pipeline_be.h  |   2 +
 4 files changed, 174 insertions(+)

diff --git a/examples/ip_pipeline/app.h b/examples/ip_pipeline/app.h
index 9dbe668..144fab8 100644
--- a/examples/ip_pipeline/app.h
+++ b/examples/ip_pipeline/app.h
@@ -155,6 +155,8 @@ struct app_pktq_source_params {
 struct app_pktq_sink_params {
 	char *name;
 	uint8_t parsed;
+	char *file_name; /* Full path of PCAP file to be copied to mbufs */
+	uint32_t n_pkts_to_dump;
 };
 
 struct app_msgq_params {
diff --git a/examples/ip_pipeline/config_parse.c b/examples/ip_pipeline/config_parse.c
index f0bed81..9f5b974 100644
--- a/examples/ip_pipeline/config_parse.c
+++ b/examples/ip_pipeline/config_parse.c
@@ -184,6 +184,8 @@ struct app_pktq_source_params default_source_params = {
 
 struct app_pktq_sink_params default_sink_params = {
 	.parsed = 0,
+	.file_name = NULL,
+	.n_pkts_to_dump = 0,
 };
 
 struct app_msgq_params default_msgq_params = {
@@ -1003,6 +1005,83 @@ parse_pipeline_pcap_source(struct app_params *app,
 }
 
 static int
+parse_pipeline_pcap_sink(struct app_params *app,
+	struct app_pipeline_params *p,
+	const char *file_name, const char *n_pkts_to_dump)
+{
+	const char *next = NULL;
+	char *end;
+	uint32_t i;
+	int parse_file = 0;
+
+	if (file_name && !n_pkts_to_dump) {
+		next = file_name;
+		parse_file = 1; /* parse file path */
+	} else if (n_pkts_to_dump && !file_name) {
+		next = n_pkts_to_dump;
+		parse_file = 0; /* parse copy size */
+	} else
+		return -EINVAL;
+
+	char name[APP_PARAM_NAME_SIZE];
+	size_t name_len;
+
+	if (p->n_pktq_out == 0)
+		return -EINVAL;
+
+	for (i = 0; i < p->n_pktq_out; i++) {
+		if (p->pktq_out[i].type != APP_PKTQ_OUT_SINK)
+			return -EINVAL;
+	}
+
+	i = 0;
+	while (*next != '\0') {
+		uint32_t id;
+
+		if (i >= p->n_pktq_out)
+			return -EINVAL;
+
+		id = p->pktq_out[i].id;
+
+		end = strchr(next, ' ');
+		if (!end)
+			name_len = strlen(next);
+		else
+			name_len = end - next;
+
+		if (name_len == 0 || name_len == sizeof(name))
+			return -EINVAL;
+
+		strncpy(name, next, name_len);
+		name[name_len] = '\0';
+		next += name_len;
+		if (*next != '\0')
+			next++;
+
+		if (parse_file) {
+			app->sink_params[id].file_name = strdup(name);
+			if (app->sink_params[id].file_name == NULL)
+				return -ENOMEM;
+		} else {
+			if (parser_read_uint32(
+				&app->sink_params[id].n_pkts_to_dump,
+				name) != 0) {
+				if (app->sink_params[id].file_name != NULL)
+					free(app->sink_params[id].file_name);
+				return -EINVAL;
+			}
+		}
+
+		i++;
+
+		if (i == p->n_pktq_out)
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int
 parse_pipeline_pktq_in(struct app_params *app,
 	struct app_pipeline_params *p,
 	const char *value)
@@ -1254,6 +1333,12 @@ parse_pipeline(struct app_params *app,
 		else if (strcmp(ent->name, "pcap_bytes_rd_per_pkt") == 0)
 			ret = parse_pipeline_pcap_source(app, param,
 					NULL, ent->value);
+		else if (strcmp(ent->name, "pcap_file_wr") == 0)
+			ret = parse_pipeline_pcap_sink(app, param,
+					ent->value, NULL);
+		else if (strcmp(ent->name, "pcap_n_pkt_wr") == 0)
+			ret = parse_pipeline_pcap_sink(app, param,
+					NULL, ent->value);
 		else {
 			APP_CHECK((param->n_args < APP_MAX_PIPELINE_ARGS),
 				"CFG: [%s] out of memory",
@@ -1813,6 +1898,58 @@ parse_source(struct app_params *app,
 }
 
 static void
+parse_sink(struct app_params *app,
+	const char *section_name,
+	struct rte_cfgfile *cfg)
+{
+	struct app_pktq_sink_params *param;
+	struct rte_cfgfile_entry *entries;
+	int n_entries, ret, i;
+	ssize_t param_idx;
+
+	n_entries = rte_cfgfile_section_num_entries(cfg, section_name);
+	PARSE_ERROR_SECTION_NO_ENTRIES((n_entries > 0), section_name);
+
+	entries = malloc(n_entries * sizeof(struct rte_cfgfile_entry));
+	PARSE_ERROR_MALLOC(entries != NULL);
+
+	rte_cfgfile_section_entries(cfg, section_name, entries, n_entries);
+
+	param_idx = APP_PARAM_ADD(app->sink_params, section_name);
+	PARSER_PARAM_ADD_CHECK(param_idx, app->sink_params, section_name);
+
+	param = &app->sink_params[param_idx];
+	param->parsed = 1;
+
+	for (i = 0; i < n_entries; i++) {
+		struct rte_cfgfile_entry *ent = &entries[i];
+
+		ret = -ESRCH;
+		if (strcmp(ent->name, "pcap_file_wr")) {
+			param->file_name = strdup(ent->value);
+			if (param->file_name != NULL)
+				ret = 0;
+			else
+				ret = -ENOMEM;
+		} else if (strcmp(ent->name, "pcap_n_pkt_wr")) {
+			ret = parser_read_uint32(&param->n_pkts_to_dump,
+					ent->value);
+		}
+		APP_CHECK(ret != -ESRCH,
+			"CFG: [%s] entry '%s': unknown entry\n",
+			section_name,
+			ent->name);
+		APP_CHECK(ret == 0,
+			"CFG: [%s] entry '%s': Invalid value '%s'\n",
+			section_name,
+			ent->name,
+			ent->value);
+	}
+
+	free(entries);
+}
+
+static void
 parse_msgq_req_pipeline(struct app_params *app,
 	const char *section_name,
 	struct rte_cfgfile *cfg)
@@ -1971,6 +2108,7 @@ static const struct config_section cfg_file_scheme[] = {
 	{"SWQ", 1, parse_swq},
 	{"TM", 1, parse_tm},
 	{"SOURCE", 1, parse_source},
+	{"SINK", 1, parse_sink},
 	{"MSGQ-REQ-PIPELINE", 1, parse_msgq_req_pipeline},
 	{"MSGQ-RSP-PIPELINE", 1, parse_msgq_rsp_pipeline},
 	{"MSGQ", 1, parse_msgq},
@@ -2375,6 +2513,26 @@ save_source_params(struct app_params *app, FILE *f)
 }
 
 static void
+save_sink_params(struct app_params *app, FILE *f)
+{
+	struct app_pktq_sink_params *p;
+	size_t i, count;
+
+	count = RTE_DIM(app->sink_params);
+	for (i = 0; i < count; i++) {
+		p = &app->sink_params[i];
+		if (!APP_PARAM_VALID(p))
+			continue;
+
+		fprintf(f, "[%s]\n", p->name);
+		fprintf(f, "%s = %s\n", "pcap_file_wr", p->file_name);
+		fprintf(f, "%s = %" PRIu32 "\n",
+				"pcap_n_pkt_wr", p->n_pkts_to_dump);
+		fputc('\n', f);
+	}
+}
+
+static void
 save_msgq_params(struct app_params *app, FILE *f)
 {
 	struct app_msgq_params *p;
@@ -2552,6 +2710,7 @@ app_config_save(struct app_params *app, const char *file_name)
 	save_swq_params(app, file);
 	save_tm_params(app, file);
 	save_source_params(app, file);
+	save_sink_params(app, file);
 	save_msgq_params(app, file);
 
 	fclose(file);
diff --git a/examples/ip_pipeline/init.c b/examples/ip_pipeline/init.c
index 4a3a9a7..6e41964 100644
--- a/examples/ip_pipeline/init.c
+++ b/examples/ip_pipeline/init.c
@@ -1285,6 +1285,16 @@ static void app_pipeline_params_get(struct app_params *app,
 		}
 		case APP_PKTQ_OUT_SINK:
 			out->type = PIPELINE_PORT_OUT_SINK;
+			if (app->sink_params[in->id].file_name != NULL) {
+				out->params.sink.file_name = strdup(
+					app->sink_params[in->id].file_name);
+				if (out->params.sink.file_name == NULL) {
+					out->params.sink.max_n_pkts = 0;
+					break;
+				}
+				out->params.sink.max_n_pkts =
+					app->sink_params[in->id].n_pkts_to_dump;
+			}
 			break;
 		default:
 			break;
@@ -1322,6 +1332,7 @@ app_init_pipelines(struct app_params *app)
 
 		APP_LOG(app, HIGH, "Initializing %s ...", params->name);
 
+		memset(&pp, 0, sizeof(pp));
 		ptype = app_pipeline_type_find(app, params->type);
 		if (ptype == NULL)
 			rte_panic("Init error: Unknown pipeline type \"%s\"\n",
diff --git a/examples/ip_pipeline/pipeline_be.h b/examples/ip_pipeline/pipeline_be.h
index 0ba00f6..b8f229e 100644
--- a/examples/ip_pipeline/pipeline_be.h
+++ b/examples/ip_pipeline/pipeline_be.h
@@ -137,6 +137,7 @@ struct pipeline_port_out_params {
 		struct rte_port_ring_writer_ipv4_ras_params ring_ipv4_ras;
 		struct rte_port_ring_writer_ipv6_ras_params ring_ipv6_ras;
 		struct rte_port_sched_writer_params sched;
+		struct rte_port_sink_params sink;
 	} params;
 };
 
@@ -163,6 +164,7 @@ pipeline_port_out_params_convert(struct pipeline_port_out_params  *p)
 	case PIPELINE_PORT_OUT_SCHED_WRITER:
 		return (void *) &p->params.sched;
 	case PIPELINE_PORT_OUT_SINK:
+		return (void *) &p->params.sink;
 	default:
 		return NULL;
 	}
-- 
2.5.0

Re: [dpdk-dev] [PATCH 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port

[Panu Matilainen]

On 01/27/2016 07:39 PM, Fan Zhang wrote:
> Originally, sink ports in librte_port releases received mbufs back to
> mempool. This patch adds optional packet dumping to PCAP feature in sink
> port: the packets will be dumped to user defined PCAP file for storage or
> debugging. The user may also choose the sink port's activity: either it
> continuously dump the packets to the file, or stops at certain dumping
>
> This feature shares same CONFIG_RTE_PORT_PCAP compiler option as source
> port PCAP file support feature. Users can enable or disable this feature
> by setting CONFIG_RTE_PORT_PCAP compiler option "y" or "n".
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---
>   lib/librte_port/rte_port_source_sink.c | 268 +++++++++++++++++++++++++++++++--
>   lib/librte_port/rte_port_source_sink.h |  11 +-
>   2 files changed, 263 insertions(+), 16 deletions(-)
>
[...]
> +#ifdef RTE_PORT_PCAP
> +
> +/**
> + * Open PCAP file for dumping packets to the file later
> + *
> + * @param port
> + *   Handle to sink port
> + * @param p
> + *   Sink port parameter
> + * @return
> + *   0 on SUCCESS
> + *   error code otherwise
> + */
[...]
> +
> +#else
> +
> +static int
> +pcap_sink_open(struct rte_port_sink *port,
> +		__rte_unused struct rte_port_sink_params *p)
> +{
> +	port->dumper = NULL;
> +	port->max_pkts = 0;
> +	port->pkt_index = 0;
> +	port->dump_finish = 0;
> +
> +	return 0;
> +}

Shouldn't this just return -ENOTSUP instead of success when the pcap 
feature is not built in?

> +
> +static void
> +pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
> +		__rte_unused struct rte_mbuf *mbuf) {}
> +
> +static void
> +pcap_sink_flush_pkt(__rte_unused void *dumper) {}
> +
> +static void
> +pcap_sink_close(__rte_unused void *dumper) {}
> +
> +#endif
> +
>   static void *
>   rte_port_sink_create(__rte_unused void *params, int socket_id)
>   {
>   	struct rte_port_sink *port;
> +	struct rte_port_sink_params *p = params;
> +	int status;
>
>   	/* Memory allocation */
>   	port = rte_zmalloc_socket("PORT", sizeof(*port),
> @@ -360,6 +532,19 @@ rte_port_sink_create(__rte_unused void *params, int socket_id)
>   		return NULL;
>   	}
>
> +	/* Try to open PCAP file for dumping, if possible */
> +	status = pcap_sink_open(port, p);
> +	if (status < 0) {
> +		RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP support "
> +				"support\n", __func__);
> +		rte_free(port);
> +		port = NULL;
> +	} else {
> +		if (port->dumper != NULL)
> +			RTE_LOG(INFO, PORT, "Ready to dump packets to file "
> +					"%s\n", p->file_name);
> +	}
> +
>   	return port;
>   }
>
> @@ -369,6 +554,8 @@ rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
>   	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
>
>   	RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
> +	if (p->dumper != NULL)
> +		pcap_sink_dump_pkt(p, pkt);
>   	rte_pktmbuf_free(pkt);
>   	RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
>
> @@ -387,21 +574,44 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
>
>   		RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, n_pkts);
>   		RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
> -		for (i = 0; i < n_pkts; i++) {
> -			struct rte_mbuf *pkt = pkts[i];
> -
> -			rte_pktmbuf_free(pkt);
> +		if (p->dumper) {
> +			for (i = 0; i < n_pkts; i++) {
> +				struct rte_mbuf *pkt = pkts[i];
> +
> +				pcap_sink_dump_pkt(p, pkt);
> +				rte_pktmbuf_free(pkt);
> +			}
> +		} else {
> +			for (i = 0; i < n_pkts; i++) {
> +				struct rte_mbuf *pkt = pkts[i];
> +
> +				rte_pktmbuf_free(pkt);
> +			}
>   		}
>   	} else {
> -		for ( ; pkts_mask; ) {
> -			uint32_t pkt_index = __builtin_ctzll(pkts_mask);
> -			uint64_t pkt_mask = 1LLU << pkt_index;
> -			struct rte_mbuf *pkt = pkts[pkt_index];
> -
> -			RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
> -			RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
> -			rte_pktmbuf_free(pkt);
> -			pkts_mask &= ~pkt_mask;
> +		if (p->dumper) {
> +			for ( ; pkts_mask; ) {
> +				uint32_t pkt_index = __builtin_ctzll(pkts_mask);
> +				uint64_t pkt_mask = 1LLU << pkt_index;
> +				struct rte_mbuf *pkt = pkts[pkt_index];
> +
> +				RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
> +				RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
> +				pcap_sink_dump_pkt(p, pkt);
> +				rte_pktmbuf_free(pkt);
> +				pkts_mask &= ~pkt_mask;
> +			}
> +		} else {
> +			for ( ; pkts_mask; ) {
> +				uint32_t pkt_index = __builtin_ctzll(pkts_mask);
> +				uint64_t pkt_mask = 1LLU << pkt_index;
> +				struct rte_mbuf *pkt = pkts[pkt_index];
> +
> +				RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
> +				RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
> +				rte_pktmbuf_free(pkt);
> +				pkts_mask &= ~pkt_mask;
> +			}

These add quite a fair chunk of nearly identical duplicate code, which 
could be easily avoided with an _ops-style function pointer between say, 
null_sink_dump_pkt() which is a no-op function and pcap_sink_dump_pkt().

	- Panu -

Re: [dpdk-dev] [PATCH 1/4] lib/librte_port: add PCAP file support to source port

[Panu Matilainen]

On 01/27/2016 07:39 PM, Fan Zhang wrote:
> Originally, source ports in librte_port is an input port used as packet
> generator. Similar to Linux kernel /dev/zero character device, it
> generates null packets. This patch adds optional PCAP file support to
> source port: instead of sending NULL packets, the source port generates
> packets copied from a PCAP file. To increase the performance, the packets
> in the file are loaded to memory initially, and copied to mbufs in circular
> manner. Users can enable or disable this feature by setting
> CONFIG_RTE_PORT_PCAP compiler option "y" or "n".
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---
>   config/common_bsdapp                   |   1 +
>   config/common_linuxapp                 |   1 +
>   lib/librte_port/Makefile               |   4 +
>   lib/librte_port/rte_port_source_sink.c | 190 +++++++++++++++++++++++++++++++++
>   lib/librte_port/rte_port_source_sink.h |   7 ++
>   mk/rte.app.mk                          |   1 +
>   6 files changed, 204 insertions(+)
>
[...]
> +#ifdef RTE_PORT_PCAP
> +
> +/**
> + * Load PCAP file, allocate and copy packets in the file to memory
> + *
> + * @param p
> + *   Parameters for source port
> + * @param port
> + *   Handle to source port
> + * @param socket_id
> + *   Socket id where the memory is created
> + * @return
> + *   0 on SUCCESS
> + *   error code otherwise
> + */
> +static int
> +pcap_source_load(struct rte_port_source_params *p,
> +		struct rte_port_source *port,
> +		int socket_id)
> +{
[...]
> +#else
> +static int
> +pcap_source_load(__rte_unused struct rte_port_source_params *p,
> +		struct rte_port_source *port,
> +		__rte_unused int socket_id)
> +{
> +	port->pkt_buff = NULL;
> +	port->pkt_len = NULL;
> +	port->pkts = NULL;
> +	port->pkt_index = 0;
> +
> +	return 0;
> +}
> +#endif

Same as in patch 3/4, shouldn't this return -ENOTSUP when pcap support 
is not built in, instead of success?

[...]

> diff --git a/lib/librte_port/rte_port_source_sink.h b/lib/librte_port/rte_port_source_sink.h
> index 0f9be79..6f39bec 100644
> --- a/lib/librte_port/rte_port_source_sink.h
> +++ b/lib/librte_port/rte_port_source_sink.h
> @@ -53,6 +53,13 @@ extern "C" {
>   struct rte_port_source_params {
>   	/** Pre-initialized buffer pool */
>   	struct rte_mempool *mempool;
> +	/** The full path of the pcap file to read packets from */
> +	char *file_name;
> +	/** The number of bytes to be read from each packet in the
> +	 *  pcap file. If this value is 0, the whole packet is read;
> +	 *  if it is bigger than packet size, the generated packets
> +	 *  will contain the whole packet */
> +	uint32_t n_bytes_per_pkt;
>   };

This is a likely ABI-break. It "only" appends to the struct, which might 
in some cases be okay but only when there's no sensible use for the 
struct within arrays or embedded in structs. The ip_pipeline example for 
example embeds struct rte_port_source_params within another struct which 
is could be thought of as an indication that other applications might be 
doing this as well.

An ABI break for librte_port has not been announced for 2.3 so you'd 
need to announce the intent to do so in 2.4 now, and then either wait 
till post 2.3 or wrap this in CONFIG_RTE_NEXT_ABI.

	- Panu -

Re: [dpdk-dev] [PATCH 1/4] lib/librte_port: add PCAP file support to source port

[Zhang, Roy Fan]

Hi Panu,

Thank you for the careful review and comments.

On 28/01/2016 11:54, Panu Matilainen wrote:
> On 01/27/2016 07:39 PM, Fan Zhang wrote:
>> Originally, source ports in librte_port is an input port used as packet
>> generator. Similar to Linux kernel /dev/zero character device, it
>> generates null packets. This patch adds optional PCAP file support to
>> source port: instead of sending NULL packets, the source port generates
>> packets copied from a PCAP file. To increase the performance, the 
>> packets
>> in the file are loaded to memory initially, and copied to mbufs in 
>> circular
>> manner. Users can enable or disable this feature by setting
>> CONFIG_RTE_PORT_PCAP compiler option "y" or "n".
>>
>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>> ---
>>   config/common_bsdapp                   |   1 +
>>   config/common_linuxapp                 |   1 +
>>   lib/librte_port/Makefile               |   4 +
>>   lib/librte_port/rte_port_source_sink.c | 190 
>> +++++++++++++++++++++++++++++++++
>>   lib/librte_port/rte_port_source_sink.h |   7 ++
>>   mk/rte.app.mk                          |   1 +
>>   6 files changed, 204 insertions(+)
>>
> [...]
>> +#ifdef RTE_PORT_PCAP
>> +
>> +/**
>> + * Load PCAP file, allocate and copy packets in the file to memory
>> + *
>> + * @param p
>> + *   Parameters for source port
>> + * @param port
>> + *   Handle to source port
>> + * @param socket_id
>> + *   Socket id where the memory is created
>> + * @return
>> + *   0 on SUCCESS
>> + *   error code otherwise
>> + */
>> +static int
>> +pcap_source_load(struct rte_port_source_params *p,
>> +        struct rte_port_source *port,
>> +        int socket_id)
>> +{
> [...]
>> +#else
>> +static int
>> +pcap_source_load(__rte_unused struct rte_port_source_params *p,
>> +        struct rte_port_source *port,
>> +        __rte_unused int socket_id)
>> +{
>> +    port->pkt_buff = NULL;
>> +    port->pkt_len = NULL;
>> +    port->pkts = NULL;
>> +    port->pkt_index = 0;
>> +
>> +    return 0;
>> +}
>> +#endif
>
> Same as in patch 3/4, shouldn't this return -ENOTSUP when pcap support 
> is not built in, instead of success?

Thank you for the suggestion. I will make the change in V2.

> [...]
>
>> diff --git a/lib/librte_port/rte_port_source_sink.h 
>> b/lib/librte_port/rte_port_source_sink.h
>> index 0f9be79..6f39bec 100644
>> --- a/lib/librte_port/rte_port_source_sink.h
>> +++ b/lib/librte_port/rte_port_source_sink.h
>> @@ -53,6 +53,13 @@ extern "C" {
>>   struct rte_port_source_params {
>>       /** Pre-initialized buffer pool */
>>       struct rte_mempool *mempool;
>> +    /** The full path of the pcap file to read packets from */
>> +    char *file_name;
>> +    /** The number of bytes to be read from each packet in the
>> +     *  pcap file. If this value is 0, the whole packet is read;
>> +     *  if it is bigger than packet size, the generated packets
>> +     *  will contain the whole packet */
>> +    uint32_t n_bytes_per_pkt;
>>   };
>
> This is a likely ABI-break. It "only" appends to the struct, which 
> might in some cases be okay but only when there's no sensible use for 
> the struct within arrays or embedded in structs. The ip_pipeline 
> example for example embeds struct rte_port_source_params within 
> another struct which is could be thought of as an indication that 
> other applications might be doing this as well.
>
> An ABI break for librte_port has not been announced for 2.3 so you'd 
> need to announce the intent to do so in 2.4 now, and then either wait 
> till post 2.3 or wrap this in CONFIG_RTE_NEXT_ABI.
>     - Panu -

Before Submitting the patch I have run validate-abi script, the 
validation results showed "compatible" on all libraries.

Also, the patch's added line in the rte_port_source_sink.c was ensured 
that, if the CONFIG_RTE_PORT_PCAP compiler option is set to "n" (by 
default), the added read-only elements in the new rte_source_params 
structure won't be touched.
CONFIG_RTE_PORT_PCAP compiler option lies in config/comm_bsdapp and 
config/com_linuxapp, and is freshly added in this patch.

If an application is built on top of latest release version of 
rte_port_source_sink library, it shall work fine with the new library if 
the new CONFIG_RTE_PORT_PCAP opition is left the default "n".
ip_pipeline example works fine this way.

I hope this changes your mind on breaking the ABI.

Best regards,
Fan

Re: [dpdk-dev] [PATCH 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port

[Zhang, Roy Fan]

Hi Panu,

Thank you again for careful review and comments.

On 28/01/2016 11:43, Panu Matilainen wrote:
> On 01/27/2016 07:39 PM, Fan Zhang wrote:
>> Originally, sink ports in librte_port releases received mbufs back to
>> mempool. This patch adds optional packet dumping to PCAP feature in sink
>> port: the packets will be dumped to user defined PCAP file for 
>> storage or
>> debugging. The user may also choose the sink port's activity: either it
>> continuously dump the packets to the file, or stops at certain dumping
>>
>> This feature shares same CONFIG_RTE_PORT_PCAP compiler option as source
>> port PCAP file support feature. Users can enable or disable this feature
>> by setting CONFIG_RTE_PORT_PCAP compiler option "y" or "n".
>>
>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>> ---
>>   lib/librte_port/rte_port_source_sink.c | 268 
>> +++++++++++++++++++++++++++++++--
>>   lib/librte_port/rte_port_source_sink.h |  11 +-
>>   2 files changed, 263 insertions(+), 16 deletions(-)
>>
> [...]
>> +#ifdef RTE_PORT_PCAP
>> +
>> +/**
>> + * Open PCAP file for dumping packets to the file later
>> + *
>> + * @param port
>> + *   Handle to sink port
>> + * @param p
>> + *   Sink port parameter
>> + * @return
>> + *   0 on SUCCESS
>> + *   error code otherwise
>> + */
> [...]
>> +
>> +#else
>> +
>> +static int
>> +pcap_sink_open(struct rte_port_sink *port,
>> +        __rte_unused struct rte_port_sink_params *p)
>> +{
>> +    port->dumper = NULL;
>> +    port->max_pkts = 0;
>> +    port->pkt_index = 0;
>> +    port->dump_finish = 0;
>> +
>> +    return 0;
>> +}
>
> Shouldn't this just return -ENOTSUP instead of success when the pcap 
> feature is not built in?

I agree, I will modify the code in v2.

>> +
>> +static void
>> +pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
>> +        __rte_unused struct rte_mbuf *mbuf) {}
>> +
>> +static void
>> +pcap_sink_flush_pkt(__rte_unused void *dumper) {}
>> +
>> +static void
>> +pcap_sink_close(__rte_unused void *dumper) {}
>> +
>> +#endif
>> +
>>   static void *
>>   rte_port_sink_create(__rte_unused void *params, int socket_id)
>>   {
>>       struct rte_port_sink *port;
>> +    struct rte_port_sink_params *p = params;
>> +    int status;
>>
>>       /* Memory allocation */
>>       port = rte_zmalloc_socket("PORT", sizeof(*port),
>> @@ -360,6 +532,19 @@ rte_port_sink_create(__rte_unused void *params, 
>> int socket_id)
>>           return NULL;
>>       }
>>
>> +    /* Try to open PCAP file for dumping, if possible */
>> +    status = pcap_sink_open(port, p);
>> +    if (status < 0) {
>> +        RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP support "
>> +                "support\n", __func__);
>> +        rte_free(port);
>> +        port = NULL;
>> +    } else {
>> +        if (port->dumper != NULL)
>> +            RTE_LOG(INFO, PORT, "Ready to dump packets to file "
>> +                    "%s\n", p->file_name);
>> +    }
>> +
>>       return port;
>>   }
>>
>> @@ -369,6 +554,8 @@ rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
>>       __rte_unused struct rte_port_sink *p = (struct rte_port_sink *) 
>> port;
>>
>>       RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
>> +    if (p->dumper != NULL)
>> +        pcap_sink_dump_pkt(p, pkt);
>>       rte_pktmbuf_free(pkt);
>>       RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
>>
>> @@ -387,21 +574,44 @@ rte_port_sink_tx_bulk(void *port, struct 
>> rte_mbuf **pkts,
>>
>>           RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, n_pkts);
>>           RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
>> -        for (i = 0; i < n_pkts; i++) {
>> -            struct rte_mbuf *pkt = pkts[i];
>> -
>> -            rte_pktmbuf_free(pkt);
>> +        if (p->dumper) {
>> +            for (i = 0; i < n_pkts; i++) {
>> +                struct rte_mbuf *pkt = pkts[i];
>> +
>> +                pcap_sink_dump_pkt(p, pkt);
>> +                rte_pktmbuf_free(pkt);
>> +            }
>> +        } else {
>> +            for (i = 0; i < n_pkts; i++) {
>> +                struct rte_mbuf *pkt = pkts[i];
>> +
>> +                rte_pktmbuf_free(pkt);
>> +            }
>>           }
>>       } else {
>> -        for ( ; pkts_mask; ) {
>> -            uint32_t pkt_index = __builtin_ctzll(pkts_mask);
>> -            uint64_t pkt_mask = 1LLU << pkt_index;
>> -            struct rte_mbuf *pkt = pkts[pkt_index];
>> -
>> -            RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
>> -            RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
>> -            rte_pktmbuf_free(pkt);
>> -            pkts_mask &= ~pkt_mask;
>> +        if (p->dumper) {
>> +            for ( ; pkts_mask; ) {
>> +                uint32_t pkt_index = __builtin_ctzll(pkts_mask);
>> +                uint64_t pkt_mask = 1LLU << pkt_index;
>> +                struct rte_mbuf *pkt = pkts[pkt_index];
>> +
>> +                RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
>> +                RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
>> +                pcap_sink_dump_pkt(p, pkt);
>> +                rte_pktmbuf_free(pkt);
>> +                pkts_mask &= ~pkt_mask;
>> +            }
>> +        } else {
>> +            for ( ; pkts_mask; ) {
>> +                uint32_t pkt_index = __builtin_ctzll(pkts_mask);
>> +                uint64_t pkt_mask = 1LLU << pkt_index;
>> +                struct rte_mbuf *pkt = pkts[pkt_index];
>> +
>> +                RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
>> +                RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
>> +                rte_pktmbuf_free(pkt);
>> +                pkts_mask &= ~pkt_mask;
>> +            }
>
> These add quite a fair chunk of nearly identical duplicate code, which 
> could be easily avoided with an _ops-style function pointer between 
> say, null_sink_dump_pkt() which is a no-op function and 
> pcap_sink_dump_pkt().

The reason of doing this is to avoid using if/else in for loop to speed 
up a little. But your suggestion is quite nice, I will think better way 
like this and modify the code.

>     - Panu -

Best regards,
Fan