DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/2] pdump: cleanups
@ 2019-11-08  4:38 Stephen Hemminger
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08  4:38 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

These are a couple of small cleanups for 19.10 which
came out of work on pcapng support. Full pcapng support
and BPF are planned for DPDK 20.02.

Stephen Hemminger (2):
  pdump: use new pktmbuf copy function
  pdump: use dynamic logtype

 lib/librte_pdump/rte_pdump.c | 137 ++++++++++-------------------------
 1 file changed, 39 insertions(+), 98 deletions(-)

-- 
2.20.1


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

* [dpdk-dev] [PATCH 1/2] pdump: use new pktmbuf copy function
  2019-11-08  4:38 [dpdk-dev] [PATCH 0/2] pdump: cleanups Stephen Hemminger
@ 2019-11-08  4:38 ` Stephen Hemminger
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype Stephen Hemminger
  2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
  2 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08  4:38 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The rte_pktmbuf_copy handles varying size mbuf pools correctly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pdump/rte_pdump.c | 69 +-----------------------------------
 1 file changed, 1 insertion(+), 68 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index ac94fea93656..7816c7254615 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -64,73 +64,6 @@ static struct pdump_rxtx_cbs {
 } rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
 tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
 
-static inline int
-pdump_pktmbuf_copy_data(struct rte_mbuf *seg, const struct rte_mbuf *m)
-{
-	if (rte_pktmbuf_tailroom(seg) < m->data_len) {
-		RTE_LOG(ERR, PDUMP,
-			"User mempool: insufficient data_len of mbuf\n");
-		return -EINVAL;
-	}
-
-	seg->port = m->port;
-	seg->vlan_tci = m->vlan_tci;
-	seg->hash = m->hash;
-	seg->tx_offload = m->tx_offload;
-	seg->ol_flags = m->ol_flags;
-	seg->packet_type = m->packet_type;
-	seg->vlan_tci_outer = m->vlan_tci_outer;
-	seg->data_len = m->data_len;
-	seg->pkt_len = seg->data_len;
-	rte_memcpy(rte_pktmbuf_mtod(seg, void *),
-			rte_pktmbuf_mtod(m, void *),
-			rte_pktmbuf_data_len(seg));
-
-	return 0;
-}
-
-static inline struct rte_mbuf *
-pdump_pktmbuf_copy(struct rte_mbuf *m, struct rte_mempool *mp)
-{
-	struct rte_mbuf *m_dup, *seg, **prev;
-	uint32_t pktlen;
-	uint16_t nseg;
-
-	m_dup = rte_pktmbuf_alloc(mp);
-	if (unlikely(m_dup == NULL))
-		return NULL;
-
-	seg = m_dup;
-	prev = &seg->next;
-	pktlen = m->pkt_len;
-	nseg = 0;
-
-	do {
-		nseg++;
-		if (pdump_pktmbuf_copy_data(seg, m) < 0) {
-			if (seg != m_dup)
-				rte_pktmbuf_free_seg(seg);
-			rte_pktmbuf_free(m_dup);
-			return NULL;
-		}
-		*prev = seg;
-		prev = &seg->next;
-	} while ((m = m->next) != NULL &&
-			(seg = rte_pktmbuf_alloc(mp)) != NULL);
-
-	*prev = NULL;
-	m_dup->nb_segs = nseg;
-	m_dup->pkt_len = pktlen;
-
-	/* Allocation of new indirect segment failed */
-	if (unlikely(seg == NULL)) {
-		rte_pktmbuf_free(m_dup);
-		return NULL;
-	}
-
-	__rte_mbuf_sanity_check(m_dup, 1);
-	return m_dup;
-}
 
 static inline void
 pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
@@ -148,7 +81,7 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
 	ring = cbs->ring;
 	mp = cbs->mp;
 	for (i = 0; i < nb_pkts; i++) {
-		p = pdump_pktmbuf_copy(pkts[i], mp);
+		p = rte_pktmbuf_copy(pkts[i], mp, 0, UINT32_MAX);
 		if (p)
 			dup_bufs[d_pkts++] = p;
 	}
-- 
2.20.1


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

* [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype
  2019-11-08  4:38 [dpdk-dev] [PATCH 0/2] pdump: cleanups Stephen Hemminger
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
@ 2019-11-08  4:38 ` Stephen Hemminger
  2019-11-08  8:13   ` David Marchand
  2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08  4:38 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The logtype USER1 should not be overloaded for library function.
Instead use a dynamic log type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pdump/rte_pdump.c | 68 ++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 7816c7254615..ae25e2a064a4 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -13,8 +13,12 @@
 #include "rte_pdump.h"
 
 #define DEVICE_ID_SIZE 64
-/* Macros for printing using RTE_LOG */
-#define RTE_LOGTYPE_PDUMP RTE_LOGTYPE_USER1
+
+/* Macro for printing using RTE_LOG */
+static int pdump_logtype;
+#define PDUMP_LOG(level, fmt, args...)				\
+	rte_log(RTE_LOG_ ## level, pdump_logtype, "%s(): " fmt,	\
+		__func__, ## args)
 
 /* Used for the multi-process communication */
 #define PDUMP_MP	"mp_pdump"
@@ -88,7 +92,7 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
 
 	ring_enq = rte_ring_enqueue_burst(ring, (void *)dup_bufs, d_pkts, NULL);
 	if (unlikely(ring_enq < d_pkts)) {
-		RTE_LOG(DEBUG, PDUMP,
+		PDUMP_LOG(DEBUG,
 			"only %d of packets enqueued to ring\n", ring_enq);
 		do {
 			rte_pktmbuf_free(dup_bufs[ring_enq]);
@@ -127,7 +131,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 		cbs = &rx_cbs[port][qid];
 		if (cbs && operation == ENABLE) {
 			if (cbs->cb) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add rx callback for port=%d "
 					"and queue=%d, callback already exists\n",
 					port, qid);
@@ -138,7 +142,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			cbs->cb = rte_eth_add_first_rx_callback(port, qid,
 								pdump_rx, cbs);
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add rx callback, errno=%d\n",
 					rte_errno);
 				return rte_errno;
@@ -148,7 +152,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			int ret;
 
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to delete non existing rx "
 					"callback for port=%d and queue=%d\n",
 					port, qid);
@@ -156,7 +160,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			}
 			ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
 			if (ret < 0) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to remove rx callback, errno=%d\n",
 					-ret);
 				return ret;
@@ -182,7 +186,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 		cbs = &tx_cbs[port][qid];
 		if (cbs && operation == ENABLE) {
 			if (cbs->cb) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add tx callback for port=%d "
 					"and queue=%d, callback already exists\n",
 					port, qid);
@@ -193,7 +197,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			cbs->cb = rte_eth_add_tx_callback(port, qid, pdump_tx,
 								cbs);
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add tx callback, errno=%d\n",
 					rte_errno);
 				return rte_errno;
@@ -203,7 +207,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			int ret;
 
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to delete non existing tx "
 					"callback for port=%d and queue=%d\n",
 					port, qid);
@@ -211,7 +215,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			}
 			ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
 			if (ret < 0) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to remove tx callback, errno=%d\n",
 					-ret);
 				return ret;
@@ -240,7 +244,7 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		ret = rte_eth_dev_get_port_by_name(p->data.en_v1.device,
 				&port);
 		if (ret < 0) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"failed to get port id for device id=%s\n",
 				p->data.en_v1.device);
 			return -EINVAL;
@@ -252,7 +256,7 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		ret = rte_eth_dev_get_port_by_name(p->data.dis_v1.device,
 				&port);
 		if (ret < 0) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"failed to get port id for device id=%s\n",
 				p->data.dis_v1.device);
 			return -EINVAL;
@@ -277,18 +281,18 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		nb_rx_q = dev_info.nb_rx_queues;
 		nb_tx_q = dev_info.nb_tx_queues;
 		if (nb_rx_q == 0 && flags & RTE_PDUMP_FLAG_RX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"number of rx queues cannot be 0\n");
 			return -EINVAL;
 		}
 		if (nb_tx_q == 0 && flags & RTE_PDUMP_FLAG_TX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"number of tx queues cannot be 0\n");
 			return -EINVAL;
 		}
 		if ((nb_tx_q == 0 || nb_rx_q == 0) &&
 			flags == RTE_PDUMP_FLAG_RXTX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"both tx&rx queues must be non zero\n");
 			return -EINVAL;
 		}
@@ -324,7 +328,7 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 
 	/* recv client requests */
 	if (mp_msg->len_param != sizeof(*cli_req)) {
-		RTE_LOG(ERR, PDUMP, "failed to recv from client\n");
+		PDUMP_LOG(ERR, "failed to recv from client\n");
 		resp->err_value = -EINVAL;
 	} else {
 		cli_req = (const struct pdump_request *)mp_msg->param;
@@ -337,8 +341,8 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 	mp_resp.len_param = sizeof(*resp);
 	mp_resp.num_fds = 0;
 	if (rte_mp_reply(&mp_resp, peer) < 0) {
-		RTE_LOG(ERR, PDUMP, "failed to send to client:%s, %s:%d\n",
-			strerror(rte_errno), __func__, __LINE__);
+		PDUMP_LOG(ERR, "failed to send to client:%s\n",
+			  strerror(rte_errno));
 		return -1;
 	}
 
@@ -366,19 +370,18 @@ static int
 pdump_validate_ring_mp(struct rte_ring *ring, struct rte_mempool *mp)
 {
 	if (ring == NULL || mp == NULL) {
-		RTE_LOG(ERR, PDUMP, "NULL ring or mempool are passed %s:%d\n",
-			__func__, __LINE__);
+		PDUMP_LOG(ERR, "NULL ring or mempool\n");
 		rte_errno = EINVAL;
 		return -1;
 	}
 	if (mp->flags & MEMPOOL_F_SP_PUT || mp->flags & MEMPOOL_F_SC_GET) {
-		RTE_LOG(ERR, PDUMP, "mempool with either SP or SC settings"
+		PDUMP_LOG(ERR, "mempool with either SP or SC settings"
 		" is not valid for pdump, should have MP and MC settings\n");
 		rte_errno = EINVAL;
 		return -1;
 	}
 	if (ring->prod.single || ring->cons.single) {
-		RTE_LOG(ERR, PDUMP, "ring with either SP or SC settings"
+		PDUMP_LOG(ERR, "ring with either SP or SC settings"
 		" is not valid for pdump, should have MP and MC settings\n");
 		rte_errno = EINVAL;
 		return -1;
@@ -392,7 +395,7 @@ pdump_validate_flags(uint32_t flags)
 {
 	if (flags != RTE_PDUMP_FLAG_RX && flags != RTE_PDUMP_FLAG_TX &&
 		flags != RTE_PDUMP_FLAG_RXTX) {
-		RTE_LOG(ERR, PDUMP,
+		PDUMP_LOG(ERR,
 			"invalid flags, should be either rx/tx/rxtx\n");
 		rte_errno = EINVAL;
 		return -1;
@@ -407,17 +410,15 @@ pdump_validate_port(uint16_t port, char *name)
 	int ret = 0;
 
 	if (port >= RTE_MAX_ETHPORTS) {
-		RTE_LOG(ERR, PDUMP, "Invalid port id %u, %s:%d\n", port,
-			__func__, __LINE__);
+		PDUMP_LOG(ERR, "Invalid port id %u\n", port);
 		rte_errno = EINVAL;
 		return -1;
 	}
 
 	ret = rte_eth_dev_get_name_by_port(port, name);
 	if (ret < 0) {
-		RTE_LOG(ERR, PDUMP,
-			"port id to name mapping failed for port id=%u, %s:%d\n",
-			port, __func__, __LINE__);
+		PDUMP_LOG(ERR, "port %u to name mapping failed\n",
+			  port);
 		rte_errno = EINVAL;
 		return -1;
 	}
@@ -472,7 +473,7 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	}
 
 	if (ret < 0)
-		RTE_LOG(ERR, PDUMP,
+		PDUMP_LOG(ERR,
 			"client request for pdump enable/disable failed\n");
 	return ret;
 }
@@ -559,3 +560,10 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 
 	return ret;
 }
+
+RTE_INIT(pdump_log)
+{
+	pdump_logtype = rte_log_register("lib.pdump");
+	if (pdump_logtype >= 0)
+		rte_log_set_level(pdump_logtype, RTE_LOG_NOTICE);
+}
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype Stephen Hemminger
@ 2019-11-08  8:13   ` David Marchand
  2019-11-08 16:19     ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2019-11-08  8:13 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Fri, Nov 8, 2019 at 5:39 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The logtype USER1 should not be overloaded for library function.
> Instead use a dynamic log type.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

This patch is incomplete.
$ git grep -w RTE_LOG lib/librte_pdump/
lib/librte_pdump/rte_pdump.c:/* Macro for printing using RTE_LOG */
lib/librte_pdump/rte_pdump.c:                   RTE_LOG(ERR, PDUMP,

== Build lib/librte_pdump
  CC rte_pdump.o
In file included from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_bus.h:24:0,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_eal.h:21,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_lcore.h:16,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/generic/rte_spinlock.h:21,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_spinlock.h:12,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_mempool.h:44,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_mbuf.h:38,
                 from
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/lib/librte_pdump/rte_pdump.c:6:
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/lib/librte_pdump/rte_pdump.c:
In function ‘set_pdump_rxtx_cbs’:
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_log.h:337:4:
error: ‘RTE_LOGTYPE_PDUMP’ undeclared (first use in this function);
did you mean ‘RTE_LOGTYPE_PMD’?
    RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
    ^
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/lib/librte_pdump/rte_pdump.c:275:4:
note: in expansion of macro ‘RTE_LOG’
    RTE_LOG(ERR, PDUMP,
    ^~~~~~~
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/x86_64-native-linuxapp-gcc/include/rte_log.h:337:4:
note: each undeclared identifier is reported only once for each
function it appears in
    RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
    ^
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/lib/librte_pdump/rte_pdump.c:275:4:
note: in expansion of macro ‘RTE_LOG’
    RTE_LOG(ERR, PDUMP,
    ^~~~~~~
/home-local/jenkins-local/jenkins-agent/workspace/Apply-Custom-Patch-Set/dpdk/lib/librte_pdump/rte_pdump.c:
At top level:



-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype
  2019-11-08  8:13   ` David Marchand
@ 2019-11-08 16:19     ` Stephen Hemminger
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08 16:19 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

On Fri, 8 Nov 2019 09:13:56 +0100
David Marchand <david.marchand@redhat.com> wrote:

> On Fri, Nov 8, 2019 at 5:39 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > The logtype USER1 should not be overloaded for library function.
> > Instead use a dynamic log type.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> This patch is incomplete.
> $ git grep -w RTE_LOG lib/librte_pdump/
> lib/librte_pdump/rte_pdump.c:/* Macro for printing using RTE_LOG */
> lib/librte_pdump/rte_pdump.c:                   RTE_LOG(ERR, PDUMP,

Thanks missed one that got added by later code.
> 


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

* [dpdk-dev] [PATCH v2 0/2] pdump: cleanups
  2019-11-08  4:38 [dpdk-dev] [PATCH 0/2] pdump: cleanups Stephen Hemminger
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
  2019-11-08  4:38 ` [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype Stephen Hemminger
@ 2019-11-08 16:47 ` Stephen Hemminger
  2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
                     ` (2 more replies)
  2 siblings, 3 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

These are a couple of small cleanups for 19.10 which
came out of work on pcapng support. Full pcapng support
and BPF are planned for DPDK 20.02.

Stephen Hemminger (2):
  pdump: use new pktmbuf copy function
  pdump: use dynamic logtype

 lib/librte_pdump/rte_pdump.c | 139 ++++++++++-------------------------
 1 file changed, 40 insertions(+), 99 deletions(-)

v2 - replace new log message from info_get as well

-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 1/2] pdump: use new pktmbuf copy function
  2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
@ 2019-11-08 16:47   ` Stephen Hemminger
  2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 2/2] pdump: use dynamic logtype Stephen Hemminger
  2019-11-12 20:37   ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups David Marchand
  2 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The rte_pktmbuf_copy handles varying size mbuf pools correctly.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pdump/rte_pdump.c | 69 +-----------------------------------
 1 file changed, 1 insertion(+), 68 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index ac94fea93656..7816c7254615 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -64,73 +64,6 @@ static struct pdump_rxtx_cbs {
 } rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
 tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
 
-static inline int
-pdump_pktmbuf_copy_data(struct rte_mbuf *seg, const struct rte_mbuf *m)
-{
-	if (rte_pktmbuf_tailroom(seg) < m->data_len) {
-		RTE_LOG(ERR, PDUMP,
-			"User mempool: insufficient data_len of mbuf\n");
-		return -EINVAL;
-	}
-
-	seg->port = m->port;
-	seg->vlan_tci = m->vlan_tci;
-	seg->hash = m->hash;
-	seg->tx_offload = m->tx_offload;
-	seg->ol_flags = m->ol_flags;
-	seg->packet_type = m->packet_type;
-	seg->vlan_tci_outer = m->vlan_tci_outer;
-	seg->data_len = m->data_len;
-	seg->pkt_len = seg->data_len;
-	rte_memcpy(rte_pktmbuf_mtod(seg, void *),
-			rte_pktmbuf_mtod(m, void *),
-			rte_pktmbuf_data_len(seg));
-
-	return 0;
-}
-
-static inline struct rte_mbuf *
-pdump_pktmbuf_copy(struct rte_mbuf *m, struct rte_mempool *mp)
-{
-	struct rte_mbuf *m_dup, *seg, **prev;
-	uint32_t pktlen;
-	uint16_t nseg;
-
-	m_dup = rte_pktmbuf_alloc(mp);
-	if (unlikely(m_dup == NULL))
-		return NULL;
-
-	seg = m_dup;
-	prev = &seg->next;
-	pktlen = m->pkt_len;
-	nseg = 0;
-
-	do {
-		nseg++;
-		if (pdump_pktmbuf_copy_data(seg, m) < 0) {
-			if (seg != m_dup)
-				rte_pktmbuf_free_seg(seg);
-			rte_pktmbuf_free(m_dup);
-			return NULL;
-		}
-		*prev = seg;
-		prev = &seg->next;
-	} while ((m = m->next) != NULL &&
-			(seg = rte_pktmbuf_alloc(mp)) != NULL);
-
-	*prev = NULL;
-	m_dup->nb_segs = nseg;
-	m_dup->pkt_len = pktlen;
-
-	/* Allocation of new indirect segment failed */
-	if (unlikely(seg == NULL)) {
-		rte_pktmbuf_free(m_dup);
-		return NULL;
-	}
-
-	__rte_mbuf_sanity_check(m_dup, 1);
-	return m_dup;
-}
 
 static inline void
 pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
@@ -148,7 +81,7 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
 	ring = cbs->ring;
 	mp = cbs->mp;
 	for (i = 0; i < nb_pkts; i++) {
-		p = pdump_pktmbuf_copy(pkts[i], mp);
+		p = rte_pktmbuf_copy(pkts[i], mp, 0, UINT32_MAX);
 		if (p)
 			dup_bufs[d_pkts++] = p;
 	}
-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 2/2] pdump: use dynamic logtype
  2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
  2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
@ 2019-11-08 16:47   ` Stephen Hemminger
  2019-11-12 20:37   ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups David Marchand
  2 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-11-08 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The logtype USER1 should not be overloaded for library function.
Instead use a dynamic log type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pdump/rte_pdump.c | 70 ++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index 7816c7254615..8a01ac51071a 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -13,8 +13,12 @@
 #include "rte_pdump.h"
 
 #define DEVICE_ID_SIZE 64
-/* Macros for printing using RTE_LOG */
-#define RTE_LOGTYPE_PDUMP RTE_LOGTYPE_USER1
+
+/* Macro for printing using RTE_LOG */
+static int pdump_logtype;
+#define PDUMP_LOG(level, fmt, args...)				\
+	rte_log(RTE_LOG_ ## level, pdump_logtype, "%s(): " fmt,	\
+		__func__, ## args)
 
 /* Used for the multi-process communication */
 #define PDUMP_MP	"mp_pdump"
@@ -88,7 +92,7 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
 
 	ring_enq = rte_ring_enqueue_burst(ring, (void *)dup_bufs, d_pkts, NULL);
 	if (unlikely(ring_enq < d_pkts)) {
-		RTE_LOG(DEBUG, PDUMP,
+		PDUMP_LOG(DEBUG,
 			"only %d of packets enqueued to ring\n", ring_enq);
 		do {
 			rte_pktmbuf_free(dup_bufs[ring_enq]);
@@ -127,7 +131,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 		cbs = &rx_cbs[port][qid];
 		if (cbs && operation == ENABLE) {
 			if (cbs->cb) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add rx callback for port=%d "
 					"and queue=%d, callback already exists\n",
 					port, qid);
@@ -138,7 +142,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			cbs->cb = rte_eth_add_first_rx_callback(port, qid,
 								pdump_rx, cbs);
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add rx callback, errno=%d\n",
 					rte_errno);
 				return rte_errno;
@@ -148,7 +152,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			int ret;
 
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to delete non existing rx "
 					"callback for port=%d and queue=%d\n",
 					port, qid);
@@ -156,7 +160,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			}
 			ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
 			if (ret < 0) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to remove rx callback, errno=%d\n",
 					-ret);
 				return ret;
@@ -182,7 +186,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 		cbs = &tx_cbs[port][qid];
 		if (cbs && operation == ENABLE) {
 			if (cbs->cb) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add tx callback for port=%d "
 					"and queue=%d, callback already exists\n",
 					port, qid);
@@ -193,7 +197,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			cbs->cb = rte_eth_add_tx_callback(port, qid, pdump_tx,
 								cbs);
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to add tx callback, errno=%d\n",
 					rte_errno);
 				return rte_errno;
@@ -203,7 +207,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			int ret;
 
 			if (cbs->cb == NULL) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to delete non existing tx "
 					"callback for port=%d and queue=%d\n",
 					port, qid);
@@ -211,7 +215,7 @@ pdump_register_tx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue,
 			}
 			ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
 			if (ret < 0) {
-				RTE_LOG(ERR, PDUMP,
+				PDUMP_LOG(ERR,
 					"failed to remove tx callback, errno=%d\n",
 					-ret);
 				return ret;
@@ -240,7 +244,7 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		ret = rte_eth_dev_get_port_by_name(p->data.en_v1.device,
 				&port);
 		if (ret < 0) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"failed to get port id for device id=%s\n",
 				p->data.en_v1.device);
 			return -EINVAL;
@@ -252,7 +256,7 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		ret = rte_eth_dev_get_port_by_name(p->data.dis_v1.device,
 				&port);
 		if (ret < 0) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"failed to get port id for device id=%s\n",
 				p->data.dis_v1.device);
 			return -EINVAL;
@@ -268,7 +272,7 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 
 		ret = rte_eth_dev_info_get(port, &dev_info);
 		if (ret != 0) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"Error during getting device (port %u) info: %s\n",
 				port, strerror(-ret));
 			return ret;
@@ -277,18 +281,18 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 		nb_rx_q = dev_info.nb_rx_queues;
 		nb_tx_q = dev_info.nb_tx_queues;
 		if (nb_rx_q == 0 && flags & RTE_PDUMP_FLAG_RX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"number of rx queues cannot be 0\n");
 			return -EINVAL;
 		}
 		if (nb_tx_q == 0 && flags & RTE_PDUMP_FLAG_TX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"number of tx queues cannot be 0\n");
 			return -EINVAL;
 		}
 		if ((nb_tx_q == 0 || nb_rx_q == 0) &&
 			flags == RTE_PDUMP_FLAG_RXTX) {
-			RTE_LOG(ERR, PDUMP,
+			PDUMP_LOG(ERR,
 				"both tx&rx queues must be non zero\n");
 			return -EINVAL;
 		}
@@ -324,7 +328,7 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 
 	/* recv client requests */
 	if (mp_msg->len_param != sizeof(*cli_req)) {
-		RTE_LOG(ERR, PDUMP, "failed to recv from client\n");
+		PDUMP_LOG(ERR, "failed to recv from client\n");
 		resp->err_value = -EINVAL;
 	} else {
 		cli_req = (const struct pdump_request *)mp_msg->param;
@@ -337,8 +341,8 @@ pdump_server(const struct rte_mp_msg *mp_msg, const void *peer)
 	mp_resp.len_param = sizeof(*resp);
 	mp_resp.num_fds = 0;
 	if (rte_mp_reply(&mp_resp, peer) < 0) {
-		RTE_LOG(ERR, PDUMP, "failed to send to client:%s, %s:%d\n",
-			strerror(rte_errno), __func__, __LINE__);
+		PDUMP_LOG(ERR, "failed to send to client:%s\n",
+			  strerror(rte_errno));
 		return -1;
 	}
 
@@ -366,19 +370,18 @@ static int
 pdump_validate_ring_mp(struct rte_ring *ring, struct rte_mempool *mp)
 {
 	if (ring == NULL || mp == NULL) {
-		RTE_LOG(ERR, PDUMP, "NULL ring or mempool are passed %s:%d\n",
-			__func__, __LINE__);
+		PDUMP_LOG(ERR, "NULL ring or mempool\n");
 		rte_errno = EINVAL;
 		return -1;
 	}
 	if (mp->flags & MEMPOOL_F_SP_PUT || mp->flags & MEMPOOL_F_SC_GET) {
-		RTE_LOG(ERR, PDUMP, "mempool with either SP or SC settings"
+		PDUMP_LOG(ERR, "mempool with either SP or SC settings"
 		" is not valid for pdump, should have MP and MC settings\n");
 		rte_errno = EINVAL;
 		return -1;
 	}
 	if (ring->prod.single || ring->cons.single) {
-		RTE_LOG(ERR, PDUMP, "ring with either SP or SC settings"
+		PDUMP_LOG(ERR, "ring with either SP or SC settings"
 		" is not valid for pdump, should have MP and MC settings\n");
 		rte_errno = EINVAL;
 		return -1;
@@ -392,7 +395,7 @@ pdump_validate_flags(uint32_t flags)
 {
 	if (flags != RTE_PDUMP_FLAG_RX && flags != RTE_PDUMP_FLAG_TX &&
 		flags != RTE_PDUMP_FLAG_RXTX) {
-		RTE_LOG(ERR, PDUMP,
+		PDUMP_LOG(ERR,
 			"invalid flags, should be either rx/tx/rxtx\n");
 		rte_errno = EINVAL;
 		return -1;
@@ -407,17 +410,15 @@ pdump_validate_port(uint16_t port, char *name)
 	int ret = 0;
 
 	if (port >= RTE_MAX_ETHPORTS) {
-		RTE_LOG(ERR, PDUMP, "Invalid port id %u, %s:%d\n", port,
-			__func__, __LINE__);
+		PDUMP_LOG(ERR, "Invalid port id %u\n", port);
 		rte_errno = EINVAL;
 		return -1;
 	}
 
 	ret = rte_eth_dev_get_name_by_port(port, name);
 	if (ret < 0) {
-		RTE_LOG(ERR, PDUMP,
-			"port id to name mapping failed for port id=%u, %s:%d\n",
-			port, __func__, __LINE__);
+		PDUMP_LOG(ERR, "port %u to name mapping failed\n",
+			  port);
 		rte_errno = EINVAL;
 		return -1;
 	}
@@ -472,7 +473,7 @@ pdump_prepare_client_request(char *device, uint16_t queue,
 	}
 
 	if (ret < 0)
-		RTE_LOG(ERR, PDUMP,
+		PDUMP_LOG(ERR,
 			"client request for pdump enable/disable failed\n");
 	return ret;
 }
@@ -559,3 +560,10 @@ rte_pdump_disable_by_deviceid(char *device_id, uint16_t queue,
 
 	return ret;
 }
+
+RTE_INIT(pdump_log)
+{
+	pdump_logtype = rte_log_register("lib.pdump");
+	if (pdump_logtype >= 0)
+		rte_log_set_level(pdump_logtype, RTE_LOG_NOTICE);
+}
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v2 0/2] pdump: cleanups
  2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
  2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
  2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 2/2] pdump: use dynamic logtype Stephen Hemminger
@ 2019-11-12 20:37   ` David Marchand
  2 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2019-11-12 20:37 UTC (permalink / raw)
  To: Pattan, Reshma, Stephen Hemminger; +Cc: dev

On Fri, Nov 8, 2019 at 5:47 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> These are a couple of small cleanups for 19.10 which

Stephen, we might be a bit late for this release, but it is still 19.11 :-)

> came out of work on pcapng support. Full pcapng support
> and BPF are planned for DPDK 20.02.
>
> Stephen Hemminger (2):
>   pdump: use new pktmbuf copy function
>   pdump: use dynamic logtype
>
>  lib/librte_pdump/rte_pdump.c | 139 ++++++++++-------------------------
>  1 file changed, 40 insertions(+), 99 deletions(-)
>
> v2 - replace new log message from info_get as well

Overall looks good.
Reshma, review please.


Thanks.

-- 
David Marchand


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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08  4:38 [dpdk-dev] [PATCH 0/2] pdump: cleanups Stephen Hemminger
2019-11-08  4:38 ` [dpdk-dev] [PATCH 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
2019-11-08  4:38 ` [dpdk-dev] [PATCH 2/2] pdump: use dynamic logtype Stephen Hemminger
2019-11-08  8:13   ` David Marchand
2019-11-08 16:19     ` Stephen Hemminger
2019-11-08 16:47 ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups Stephen Hemminger
2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 1/2] pdump: use new pktmbuf copy function Stephen Hemminger
2019-11-08 16:47   ` [dpdk-dev] [PATCH v2 2/2] pdump: use dynamic logtype Stephen Hemminger
2019-11-12 20:37   ` [dpdk-dev] [PATCH v2 0/2] pdump: cleanups David Marchand

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git