DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] bug fix and code clean-up
@ 2016-04-01 13:41 Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 1/3] port: fix source port parameter check Fan Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Fan Zhang @ 2016-04-01 13:41 UTC (permalink / raw)
  To: dev

This patchset fixes d4b4213 and eb5f411, plus code clean-up of port library.

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

Fan Zhang (3):
  port: fix source port parameter check
  port: fix sink port parameter check
  port: code clean-up

 lib/librte_port/Makefile               |   5 -
 lib/librte_port/rte_port_source_sink.c | 322 +++++++++++++--------------------
 mk/rte.app.mk                          |   2 -
 3 files changed, 122 insertions(+), 207 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH 1/3] port: fix source port parameter check
  2016-04-01 13:41 [dpdk-dev] [PATCH 0/3] bug fix and code clean-up Fan Zhang
@ 2016-04-01 13:41 ` Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 2/3] port: fix sink " Fan Zhang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Fan Zhang @ 2016-04-01 13:41 UTC (permalink / raw)
  To: dev

Fixes: d4b4213 ("port: add pcap file source")

This patch fixes source port parameter checking logic. Originally, if user
set field "file_name" with meaning value but leave PCAP support feature
disabled, the program simply ignores this field without notifying the
user.

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 | 128 +++++++++++++--------------------
 1 file changed, 50 insertions(+), 78 deletions(-)

diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index 05620d6..5c24cea 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -37,10 +37,10 @@
 #include <rte_mempool.h>
 #include <rte_malloc.h>
 
-#ifdef RTE_NEXT_ABI
-
 #include <rte_memcpy.h>
 
+#ifdef RTE_NEXT_ABI
+
 #ifdef RTE_PORT_PCAP
 #include <rte_ether.h>
 #include <pcap.h>
@@ -74,39 +74,24 @@ struct rte_port_source {
 
 	struct rte_mempool *mempool;
 
-#ifdef RTE_NEXT_ABI
-	/* PCAP buffers and indexes */
+	/* PCAP buffers and indices */
 	uint8_t **pkts;
 	uint8_t *pkt_buff;
 	uint32_t *pkt_len;
 	uint32_t n_pkts;
 	uint32_t pkt_index;
-#endif
 };
 
 #ifdef RTE_NEXT_ABI
 
 #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,
+pcap_source_load(struct rte_port_source *port,
+		const char *file_name,
+		uint32_t n_bytes_per_pkt,
 		int socket_id)
 {
-	uint32_t status = 0;
 	uint32_t n_pkts = 0;
 	uint32_t i;
 	uint32_t *pkt_len_aligns = NULL;
@@ -121,18 +106,16 @@ pcap_source_load(struct rte_port_source_params *p,
 			(rte_pktmbuf_data_room_size(port->mempool) -
 			RTE_PKTMBUF_HEADROOM);
 
-	if (p->file_name == NULL)
-		return 0;
-
-	if (p->n_bytes_per_pkt == 0)
+	if (n_bytes_per_pkt == 0)
 		max_len = pktmbuf_maxlen;
 	else
-		max_len = RTE_MIN(p->n_bytes_per_pkt, pktmbuf_maxlen);
+		max_len = RTE_MIN(n_bytes_per_pkt, pktmbuf_maxlen);
 
 	/* first time open, get packet number */
-	pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+	pcap_handle = pcap_open_offline(file_name, pcap_errbuf);
 	if (pcap_handle == NULL) {
-		status = -ENOENT;
+		RTE_LOG(ERR, PORT, "Failed to open pcap file "
+			"'%s' for reading\n", file_name);
 		goto error_exit;
 	}
 
@@ -144,28 +127,29 @@ pcap_source_load(struct rte_port_source_params *p,
 	port->pkt_len = rte_zmalloc_socket("PCAP",
 		(sizeof(*port->pkt_len) * n_pkts), 0, socket_id);
 	if (port->pkt_len == NULL) {
-		status = -ENOMEM;
+		RTE_LOG(ERR, PORT, "No enough memory\n");
 		goto error_exit;
 	}
 
 	pkt_len_aligns = rte_malloc("PCAP",
 		(sizeof(*pkt_len_aligns) * n_pkts), 0);
 	if (pkt_len_aligns == NULL) {
-		status = -ENOMEM;
+		RTE_LOG(ERR, PORT, "No enough memory\n");
 		goto error_exit;
 	}
 
 	port->pkts = rte_zmalloc_socket("PCAP",
 		(sizeof(*port->pkts) * n_pkts), 0, socket_id);
 	if (port->pkts == NULL) {
-		status = -ENOMEM;
+		RTE_LOG(ERR, PORT, "No enough memory\n");
 		goto error_exit;
 	}
 
 	/* open 2nd time, get pkt_len */
-	pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+	pcap_handle = pcap_open_offline(file_name, pcap_errbuf);
 	if (pcap_handle == NULL) {
-		status = -ENOENT;
+		RTE_LOG(ERR, PORT, "Failed to open pcap file "
+			"'%s' for reading\n", file_name);
 		goto error_exit;
 	}
 
@@ -183,16 +167,17 @@ pcap_source_load(struct rte_port_source_params *p,
 	buff = rte_zmalloc_socket("PCAP",
 		total_buff_len, 0, socket_id);
 	if (buff == NULL) {
-		status = -ENOMEM;
+		RTE_LOG(ERR, PORT, "No enough memory\n");
 		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);
+	pcap_handle = pcap_open_offline(file_name, pcap_errbuf);
 	if (pcap_handle == NULL) {
-		status = -ENOENT;
+		RTE_LOG(ERR, PORT, "Failed to open pcap file "
+			"'%s' for reading\n", file_name);
 		goto error_exit;
 	}
 
@@ -209,6 +194,10 @@ pcap_source_load(struct rte_port_source_params *p,
 
 	rte_free(pkt_len_aligns);
 
+	RTE_LOG(INFO, PORT, "Successfully load pcap file "
+		"'%s' with %u pkts\n",
+		file_name, port->n_pkts);
+
 	return 0;
 
 error_exit:
@@ -221,25 +210,30 @@ error_exit:
 	if (port->pkt_buff)
 		rte_free(port->pkt_buff);
 
-	return status;
+	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;
+#define PCAP_SOURCE_LOAD(port, file_name, n_bytes, socket_id)	\
+	pcap_source_load(port, file_name, n_bytes, socket_id)
+
+#else /* RTE_PORT_PCAP */
+
+#define PCAP_SOURCE_LOAD(port, file_name, n_bytes, socket_id)	\
+({								\
+	int _ret = 0;						\
+								\
+	if (file_name) {					\
+		RTE_LOG(ERR, PORT, "Source port field "		\
+			"\"file_name\" is not NULL.\n");	\
+		_ret = -1;					\
+	}							\
+								\
+	_ret;							\
+})
 
-	return -ENOTSUP;
-}
 #endif /* RTE_PORT_PCAP */
 
-#endif
+#endif /* RTE_NEXT_ABI */
 
 static void *
 rte_port_source_create(void *params, int socket_id)
@@ -267,36 +261,14 @@ rte_port_source_create(void *params, int socket_id)
 
 #ifdef RTE_NEXT_ABI
 
-	/* pcap file load and initialization */
-	int status = pcap_source_load(p, port, socket_id);
+	if (p->file_name) {
+		int status = PCAP_SOURCE_LOAD(port, p->file_name,
+			p->n_bytes_per_pkt, socket_id);
 
-	if (status == 0) {
-		if (port->pkt_buff != NULL) {
-			RTE_LOG(INFO, PORT, "Successfully load pcap file "
-				"'%s' with %u pkts\n",
-				p->file_name, port->n_pkts);
-		}
-	} else if (status != -ENOTSUP) {
-		/* ENOTSUP is not treated as error */
-		switch (status) {
-		case -ENOENT:
-			RTE_LOG(ERR, PORT, "%s: Failed to open pcap file "
-				"'%s' for reading\n",
-				__func__, p->file_name);
-			break;
-		case -ENOMEM:
-			RTE_LOG(ERR, PORT, "%s: Not enough memory\n",
-				__func__);
-			break;
-		default:
-			RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP "
-				"support for unknown reason\n",
-				__func__);
-			break;
+		if (status < 0) {
+			rte_free(port);
+			port = NULL;
 		}
-
-		rte_free(port);
-		port = NULL;
 	}
 
 #endif
-- 
2.5.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH 2/3] port: fix sink port parameter check
  2016-04-01 13:41 [dpdk-dev] [PATCH 0/3] bug fix and code clean-up Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 1/3] port: fix source port parameter check Fan Zhang
@ 2016-04-01 13:41 ` Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 3/3] port: code clean-up Fan Zhang
  2016-04-01 20:53 ` [dpdk-dev] [PATCH 0/3] bug fix and " Thomas Monjalon
  3 siblings, 0 replies; 8+ messages in thread
From: Fan Zhang @ 2016-04-01 13:41 UTC (permalink / raw)
  To: dev

Fixes: eb5f411 ("port: add pcap file dump")

This patch fixes sink port parameter checking logic. Originally, if user
set field "file_name" with meaning value but leave PCAP support feature
disabled, the program simply ignores this field without notifying the
user.

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 | 100 ++++++++++++++-------------------
 1 file changed, 43 insertions(+), 57 deletions(-)

diff --git a/lib/librte_port/rte_port_source_sink.c b/lib/librte_port/rte_port_source_sink.c
index 5c24cea..ca40a59 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -385,47 +385,37 @@ struct rte_port_sink {
 
 #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)
+	const char *file_name,
+	uint32_t max_n_pkts)
 {
 	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 -ENOENT;
+	if (tx_pcap == NULL) {
+		RTE_LOG(ERR, PORT, "Cannot open pcap dead handler\n");
+		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 -ENOENT;
+	pcap_dumper = pcap_dump_open(tx_pcap, file_name);
+	if (pcap_dumper == NULL) {
+		RTE_LOG(ERR, PORT, "Failed to open pcap file "
+			"\"%s\" for writing\n", file_name);
+		return -1;
+	}
 
 	port->dumper = pcap_dumper;
-	port->max_pkts = p->max_n_pkts;
+	port->max_pkts = max_n_pkts;
 	port->pkt_index = 0;
 	port->dump_finish = 0;
 
+	RTE_LOG(INFO, PORT, "Ready to dump packets to file \"%s\"\n",
+		file_name);
+
 	return 0;
 }
 
@@ -520,19 +510,23 @@ pcap_sink_close(void *dumper)
 	pcap_dump_close(pcap_dumper);
 }
 
-#else
+#define PCAP_SINK_OPEN(port, file_name, max_n_pkts)		\
+	pcap_sink_open(port, file_name, max_n_pkts)
 
-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;
+#else
 
-	return -ENOTSUP;
-}
+#define PCAP_SINK_OPEN(port, file_name, max_n_pkts)		\
+({								\
+	int _ret = 0;						\
+								\
+	if (file_name) {					\
+		RTE_LOG(ERR, PORT, "Sink port field "		\
+			"\"file_name\" is not NULL.\n");	\
+		_ret = -1;					\
+	}							\
+								\
+	_ret;							\
+})
 
 static void
 pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
@@ -547,11 +541,10 @@ pcap_sink_close(__rte_unused void *dumper) {}
 #endif
 
 static void *
-rte_port_sink_create(__rte_unused void *params, int socket_id)
+rte_port_sink_create(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),
@@ -561,24 +554,17 @@ 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) {
-		if (port->dumper != NULL)
-			RTE_LOG(INFO, PORT, "Ready to dump packets "
-				"to file %s\n", p->file_name);
-
-	} else if (status != -ENOTSUP) {
-		if (status == -ENOENT)
-			RTE_LOG(ERR, PORT, "%s: Failed to open pcap file "
-				"%s for writing\n", __func__,
-				p->file_name);
-		else
-			RTE_LOG(ERR, PORT, "%s: Failed to enable pcap "
-				"support for unknown reason\n", __func__);
-
-		rte_free(port);
-		port = NULL;
+	if (!p)
+		return port;
+
+	if (p->file_name) {
+		int status = PCAP_SINK_OPEN(port, p->file_name,
+			p->max_n_pkts);
+
+		if (status < 0) {
+			rte_free(port);
+			port = NULL;
+		}
 	}
 
 	return port;
-- 
2.5.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH 3/3] port: code clean-up
  2016-04-01 13:41 [dpdk-dev] [PATCH 0/3] bug fix and code clean-up Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 1/3] port: fix source port parameter check Fan Zhang
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 2/3] port: fix sink " Fan Zhang
@ 2016-04-01 13:41 ` Fan Zhang
  2016-04-01 13:56   ` Thomas Monjalon
  2016-04-01 20:53 ` [dpdk-dev] [PATCH 0/3] bug fix and " Thomas Monjalon
  3 siblings, 1 reply; 8+ messages in thread
From: Fan Zhang @ 2016-04-01 13:41 UTC (permalink / raw)
  To: dev

This patch clean-up the code in librte_port. The clean-up includes the
following:

* Clearer error message display.
* Remove unnecessary RTE_NEXT_ABI macro warping.
* Remove __rte_unused attribute

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 lib/librte_port/Makefile               |   5 --
 lib/librte_port/rte_port_source_sink.c | 102 +++++++++++----------------------
 mk/rte.app.mk                          |   2 -
 3 files changed, 33 insertions(+), 76 deletions(-)

diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 0b31c04..2c0ccbe 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -35,15 +35,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
 # library name
 #
 LIB = librte_port.a
-
-ifeq ($(CONFIG_RTE_NEXT_ABI),y)
-
 ifeq ($(CONFIG_RTE_PORT_PCAP),y)
 LDLIBS += -lpcap
 endif
 
-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 ca40a59..056c975 100644
--- a/lib/librte_port/rte_port_source_sink.c
+++ b/lib/librte_port/rte_port_source_sink.c
@@ -36,7 +36,6 @@
 #include <rte_mbuf.h>
 #include <rte_mempool.h>
 #include <rte_malloc.h>
-
 #include <rte_memcpy.h>
 
 #ifdef RTE_NEXT_ABI
@@ -286,15 +285,12 @@ rte_port_source_free(void *port)
 	if (p == NULL)
 		return 0;
 
-#ifdef RTE_NEXT_ABI
-
 	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);
-#endif
 
 	rte_free(p);
 
@@ -315,8 +311,6 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 		rte_pktmbuf_reset(pkts[i]);
 	}
 
-#ifdef RTE_NEXT_ABI
-
 	if (p->pkt_buff != NULL) {
 		for (i = 0; i < n_pkts; i++) {
 			uint8_t *pkt_data = rte_pktmbuf_mtod(pkts[i],
@@ -333,8 +327,6 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
 		}
 	}
 
-#endif
-
 	RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(p, n_pkts);
 
 	return n_pkts;
@@ -419,21 +411,12 @@ pcap_sink_open(struct rte_port_sink *port,
 	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)
+pcap_sink_write_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 jumbo_pkt_buf[ETHER_MAX_JUMBO_FRAME_LEN];
 	uint8_t *pkt;
 
 	/* Maximum num packets already reached */
@@ -481,37 +464,23 @@ pcap_sink_dump_pkt(struct rte_port_sink *port, struct rte_mbuf *mbuf)
 
 }
 
-/**
- * Flush pcap dumper
- *
- * @param dumper
- *   Handle to pcap dumper
- */
+#define PCAP_SINK_OPEN(port, file_name, max_n_pkts)		\
+	pcap_sink_open(port, file_name, max_n_pkts)
 
-static void
-pcap_sink_flush_pkt(void *dumper)
-{
-	pcap_dumper_t *pcap_dumper = (pcap_dumper_t *)dumper;
+#define PCAP_SINK_WRITE_PKT(port, mbuf)				\
+	pcap_sink_write_pkt(port, mbuf)
 
-	pcap_dump_flush(pcap_dumper);
-}
+#define PCAP_SINK_FLUSH_PKT(dumper)				\
+do {								\
+	if (dumper)						\
+		pcap_dump_flush((pcap_dumper_t *)dumper);	\
+} while (0)
 
-/**
- * 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);
-}
-
-#define PCAP_SINK_OPEN(port, file_name, max_n_pkts)		\
-	pcap_sink_open(port, file_name, max_n_pkts)
+#define PCAP_SINK_CLOSE(dumper)					\
+do {								\
+	if (dumper)						\
+		pcap_dump_close((pcap_dumper_t *)dumper);	\
+} while (0)
 
 #else
 
@@ -528,15 +497,11 @@ pcap_sink_close(void *dumper)
 	_ret;							\
 })
 
-static void
-pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
-		__rte_unused struct rte_mbuf *mbuf) {}
+#define PCAP_SINK_WRITE_PKT(port, mbuf) {}
 
-static void
-pcap_sink_flush_pkt(__rte_unused void *dumper) {}
+#define PCAP_SINK_FLUSH_PKT(dumper)
 
-static void
-pcap_sink_close(__rte_unused void *dumper) {}
+#define PCAP_SINK_CLOSE(dumper)
 
 #endif
 
@@ -573,11 +538,11 @@ rte_port_sink_create(void *params, int socket_id)
 static int
 rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
 {
-	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+	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);
+		PCAP_SINK_WRITE_PKT(p, pkt);
 	rte_pktmbuf_free(pkt);
 	RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
 
@@ -588,7 +553,7 @@ static int
 rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 	uint64_t pkts_mask)
 {
-	__rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+	struct rte_port_sink *p = (struct rte_port_sink *) port;
 
 	if ((pkts_mask & (pkts_mask + 1)) == 0) {
 		uint64_t n_pkts = __builtin_popcountll(pkts_mask);
@@ -598,11 +563,8 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 		RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
 
 		if (p->dumper) {
-			for (i = 0; i < n_pkts; i++) {
-				struct rte_mbuf *pkt = pkts[i];
-
-				pcap_sink_dump_pkt(p, pkt);
-			}
+			for (i = 0; i < n_pkts; i++)
+				PCAP_SINK_WRITE_PKT(p, pkts[i]);
 		}
 
 		for (i = 0; i < n_pkts; i++) {
@@ -619,7 +581,7 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
 			for ( ; dump_pkts_mask; ) {
 				pkt_index = __builtin_ctzll(
 					dump_pkts_mask);
-				pcap_sink_dump_pkt(p, pkts[pkt_index]);
+				PCAP_SINK_WRITE_PKT(p, pkts[pkt_index]);
 				dump_pkts_mask &= ~(1LLU << pkt_index);
 			}
 		}
@@ -642,10 +604,13 @@ 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;
+	struct rte_port_sink *p =
+			(struct rte_port_sink *)port;
 
-	if (p->dumper != NULL)
-		pcap_sink_flush_pkt(p->dumper);
+	if (p == NULL)
+		return 0;
+
+	PCAP_SINK_FLUSH_PKT(p->dumper);
 
 	return 0;
 }
@@ -655,12 +620,11 @@ 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);
+	PCAP_SINK_CLOSE(p->dumper);
 
 	rte_free(p);
 
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 179aac4..c66e491 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -92,9 +92,7 @@ endif
 ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
 endif
-ifeq ($(CONFIG_RTE_NEXT_ABI),y)
 _LDLIBS-$(CONFIG_RTE_PORT_PCAP)             += -lpcap
-endif
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lpcap
 _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -libverbs
-- 
2.5.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] port: code clean-up
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 3/3] port: code clean-up Fan Zhang
@ 2016-04-01 13:56   ` Thomas Monjalon
  2016-04-01 14:04     ` Zhang, Roy Fan
  2016-04-01 14:06     ` Zhang, Roy Fan
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Monjalon @ 2016-04-01 13:56 UTC (permalink / raw)
  To: Fan Zhang; +Cc: dev

2016-04-01 14:41, Fan Zhang:
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -92,9 +92,7 @@ endif
>  ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
>  endif
> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
>  _LDLIBS-$(CONFIG_RTE_PORT_PCAP)             += -lpcap
> -endif

Why removing NEXT_ABI here?
The pcap port cannot work without it because of 

+#ifdef RTE_NEXT_ABI
+
+	/** 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;
+
+#endif

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] port: code clean-up
  2016-04-01 13:56   ` Thomas Monjalon
@ 2016-04-01 14:04     ` Zhang, Roy Fan
  2016-04-01 14:06     ` Zhang, Roy Fan
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Roy Fan @ 2016-04-01 14:04 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

The reason for removing RTE_NEXT_ABI here is caused by sink port, which 
is not wrapped by RTE_NEXT_ABI macro.
If the user disable RTE_NEXT_ABI but enable RTE_PORT_PCAP, the original 
code will cause the compile error as pcap library is missing.

Regards,
Fan

On 01/04/2016 14:56, Thomas Monjalon wrote:
> 2016-04-01 14:41, Fan Zhang:
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -92,9 +92,7 @@ endif
>>   ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
>>   endif
>> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
>>   _LDLIBS-$(CONFIG_RTE_PORT_PCAP)             += -lpcap
>> -endif
> Why removing NEXT_ABI here?
> The pcap port cannot work without it because of
>
> +#ifdef RTE_NEXT_ABI
> +
> +	/** 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;
> +
> +#endif
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] port: code clean-up
  2016-04-01 13:56   ` Thomas Monjalon
  2016-04-01 14:04     ` Zhang, Roy Fan
@ 2016-04-01 14:06     ` Zhang, Roy Fan
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Roy Fan @ 2016-04-01 14:06 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

The reason for removing RTE_NEXT_ABI here is caused by sink port, which is not wrapped by RTE_NEXT_ABI macro.
If the user disable RTE_NEXT_ABI but enable RTE_PORT_PCAP, the original code will cause the compile error as pcap library is missing.

Regards,
Fan

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Friday, April 1, 2016 2:56 PM
To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 3/3] port: code clean-up

2016-04-01 14:41, Fan Zhang:
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -92,9 +92,7 @@ endif
>  ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
>  endif
> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
>  _LDLIBS-$(CONFIG_RTE_PORT_PCAP)             += -lpcap
> -endif

Why removing NEXT_ABI here?
The pcap port cannot work without it because of 

+#ifdef RTE_NEXT_ABI
+
+	/** 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;
+
+#endif

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH 0/3] bug fix and code clean-up
  2016-04-01 13:41 [dpdk-dev] [PATCH 0/3] bug fix and code clean-up Fan Zhang
                   ` (2 preceding siblings ...)
  2016-04-01 13:41 ` [dpdk-dev] [PATCH 3/3] port: code clean-up Fan Zhang
@ 2016-04-01 20:53 ` Thomas Monjalon
  3 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2016-04-01 20:53 UTC (permalink / raw)
  To: Fan Zhang; +Cc: dev

2016-04-01 14:41, Fan Zhang:
> This patchset fixes d4b4213 and eb5f411, plus code clean-up of port library.
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 
> Fan Zhang (3):
>   port: fix source port parameter check
>   port: fix sink port parameter check
>   port: code clean-up

Applied, thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-04-01 20:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 13:41 [dpdk-dev] [PATCH 0/3] bug fix and code clean-up Fan Zhang
2016-04-01 13:41 ` [dpdk-dev] [PATCH 1/3] port: fix source port parameter check Fan Zhang
2016-04-01 13:41 ` [dpdk-dev] [PATCH 2/3] port: fix sink " Fan Zhang
2016-04-01 13:41 ` [dpdk-dev] [PATCH 3/3] port: code clean-up Fan Zhang
2016-04-01 13:56   ` Thomas Monjalon
2016-04-01 14:04     ` Zhang, Roy Fan
2016-04-01 14:06     ` Zhang, Roy Fan
2016-04-01 20:53 ` [dpdk-dev] [PATCH 0/3] bug fix and " Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).