[PATCH 1/2] pdump: avoid integer issues

[PATCH 1/2] pdump: avoid integer issues

[Stephen Hemminger]

There are warnings from Coverity and other tools that handling
of intermediate bursts may wraparound. Shouldn't be possible but
use unsigned int to avoid any issues, and just as fast.

Coverity issue: 499471
Fixes: 0dea03ef2e8c ("pdump: remove use of VLA")

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pdump/rte_pdump.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index 039a4013dd..c3d0ffa779 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -27,7 +27,7 @@ RTE_LOG_REGISTER_DEFAULT(pdump_logtype, NOTICE);
 /* Used for the multi-process communication */
 #define PDUMP_MP	"mp_pdump"
 
-#define PDUMP_BURST_SIZE	32
+#define PDUMP_BURST_SIZE	32u
 
 /* Overly generous timeout for secondary to respond */
 #define MP_TIMEOUT_S 5
@@ -139,13 +139,11 @@ pdump_cb_release(struct pdump_rxtx_cbs *cbs)
 static void
 pdump_copy_burst(uint16_t port_id, uint16_t queue_id,
 		 enum rte_pcapng_direction direction,
-		 struct rte_mbuf **pkts, uint16_t nb_pkts,
+		 struct rte_mbuf **pkts, unsigned int nb_pkts,
 		 const struct pdump_rxtx_cbs *cbs,
 		 struct rte_pdump_stats *stats)
 {
-	unsigned int i;
-	int ring_enq;
-	uint16_t d_pkts = 0;
+	unsigned int i, ring_enq, d_pkts = 0;
 	struct rte_mbuf *dup_bufs[PDUMP_BURST_SIZE]; /* duplicated packets */
 	struct rte_ring *ring;
 	struct rte_mempool *mp;
@@ -188,7 +186,7 @@ pdump_copy_burst(uint16_t port_id, uint16_t queue_id,
 			dup_bufs[d_pkts++] = p;
 	}
 
-	if (unlikely(d_pkts == 0))
+	if (d_pkts == 0)
 		return;
 
 	rte_atomic_fetch_add_explicit(&stats->accepted, d_pkts, rte_memory_order_relaxed);
@@ -210,10 +208,10 @@ pdump_copy(uint16_t port_id, uint16_t queue_id,
 	   const struct pdump_rxtx_cbs *cbs,
 	   struct rte_pdump_stats *stats)
 {
-	uint16_t offs = 0;
+	unsigned int offs = 0;
 
 	do {
-		uint16_t n = RTE_MIN(nb_pkts - offs, PDUMP_BURST_SIZE);
+		unsigned int n = RTE_MIN(nb_pkts - offs, PDUMP_BURST_SIZE);
 
 		pdump_copy_burst(port_id, queue_id, direction, &pkts[offs], n, cbs, stats);
 		offs += n;
-- 
2.51.0

[PATCH 2/2] pdump: fix race in pdump disabling

[Stephen Hemminger]

There is a race where the request to disable pdump may get ahead
of the handling of pdump requests in dumpcap. The fix is to do
local removal of callbacks before forwarding same to secondary.

To reproduce:
   1. Start testpmd and start traffic
   2. Start dumpcap to capture
   3. Interrupt dumpcap with ^C

Testpmd will show missing response and dumpcap will show error:
EAL: Cannot find action: mp_pdump

Only reproducible if additional logging not enabled.

Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pdump/rte_pdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index c3d0ffa779..ac94efe7ff 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -576,12 +576,12 @@ __pdump_request(void *param)
 	PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
 
 	ret = set_pdump_rxtx_cbs(req);
-	ret = pdump_send_response(req, ret, bundle->peer);
 
 	/* Primary process is responsible for broadcasting request to all secondaries */
 	if (ret == 0)
 		pdump_request_to_secondary(req);
 
+	pdump_send_response(req, ret, bundle->peer);
 	free(bundle);
 }
 
-- 
2.51.0