[dpdk-dev] [PATCH 0/5][pktgen] fixes and minor features

[dpdk-dev] [PATCH 0/5][pktgen] fixes and minor features

[Rafal Kozik]

Hello Keith,

This patch set contain two fixes for issues that were noticed when running
DPDK in debug configuration. There are also three minor features that
may be useful. Those patches are independent from each other.

I am kindly asking, if those changes are appropriate
to be merged to Pktgen?

Best regards,
Rafal

Rafal Kozik (5):
  [pktgen] fix race condition in start
  [pktgen] different PCAP per each queue
  [pktgen] use constants for PCAP creation
  [pktgen] expose number of missed Rx packets
  [pktgen] fix cleanup of not sent packets

 app/lpktgenlib.c      |  1 +
 app/pktgen-cmds.c     |  4 ++--
 app/pktgen-main.c     | 40 ++++++++++++++++++++++++++++++----------
 app/pktgen-pcap.c     | 34 +++++++++++++++++-----------------
 app/pktgen-port-cfg.c |  8 +++++++-
 app/pktgen-port-cfg.h |  1 +
 app/pktgen.c          |  7 +++----
 7 files changed, 61 insertions(+), 34 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/5] [pktgen] fix race condition in start

[Rafal Kozik]

Flag SENDING_PACKETS must be set last.
When it is set before SEND_FOREVER flag, Tx lcores start to call
pktgen_send_pkts. But as current_tx_count is 0 and SEND_FOREVER
is not yet set, SENDING_PACKETS will be cleared.
This causes in some cases that start command will no take effect.

Fixes: f5f901fdf52b ("Update for 1.8 rte_mbuf changes.")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
 app/pktgen-cmds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pktgen-cmds.c b/app/pktgen-cmds.c
index 95f6ef1..655f6bd 100644
--- a/app/pktgen-cmds.c
+++ b/app/pktgen-cmds.c
@@ -1412,10 +1412,10 @@ pktgen_start_transmitting(port_info_t *info)
 		rte_atomic64_set(&info->current_tx_count,
 				 rte_atomic64_read(&info->transmit_count));
 
-		pktgen_set_port_flags(info, SENDING_PACKETS);
-
 		if (rte_atomic64_read(&info->current_tx_count) == 0)
 			pktgen_set_port_flags(info, SEND_FOREVER);
+
+		pktgen_set_port_flags(info, SENDING_PACKETS);
 	}
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH 2/5] [pktgen] different PCAP per each queue

[Rafal Kozik]

In some test scenario it is necessary to configure different flow per each
Tx queue. This patch adds possibility to provide list of PCAPs files.

If number of files is smaller than number of queues, first PCAP file
is used for the queues which number is above the number of files.
To achieve uniform throughput, average length of packets in each PCAP
needs to be the same.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
 app/pktgen-main.c     | 40 ++++++++++++++++++++++++++++++----------
 app/pktgen-pcap.c     | 12 ++++++------
 app/pktgen-port-cfg.c |  8 +++++++-
 app/pktgen-port-cfg.h |  1 +
 4 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/app/pktgen-main.c b/app/pktgen-main.c
index b90da0c..68ffa13 100644
--- a/app/pktgen-main.c
+++ b/app/pktgen-main.c
@@ -81,6 +81,7 @@ pktgen_usage(const char *prgname)
 	printf(
 		"Usage: %s [EAL options] -- [-h] [-v] [-P] [-G] [-T] [-f cmd_file] [-l log_file] [-s P:PCAP_file] [-m <string>]\n"
 		"  -s P:file    PCAP packet stream file, 'P' is the port number\n"
+		"  -s P:file0,file1,... list of PCAP packet stream files per queue, 'P' is the port number\n"
 		"  -f filename  Command file (.pkt) to execute or a Lua script (.lua) file\n"
 		"  -l filename  Write log to filename\n"
 		"  -P           Enable PROMISCUOUS mode on all ports\n"
@@ -142,10 +143,10 @@ pktgen_usage(const char *prgname)
 static int
 pktgen_parse_args(int argc, char **argv)
 {
-	int opt, ret, port;
+	int opt, ret, port, q;
 	char **argvopt;
 	int option_index;
-	char *prgname = argv[0], *p;
+	char *prgname = argv[0], *p, *pc;
 	static struct option lgopts[] = {
 		{"crc-strip", 0, 0, 0},
 		{NULL, 0, 0, 0}
@@ -187,14 +188,27 @@ pktgen_parse_args(int argc, char **argv)
 		case 's':	/* Read a PCAP packet capture file (stream) */
 			port = strtol(optarg, NULL, 10);
 			p = strchr(optarg, ':');
-			if ( (p == NULL) ||
-			     (pktgen.info[port].pcap =
-				      _pcap_open(++p, port)) == NULL) {
-				pktgen_log_error(
-					"Invalid PCAP filename (%s) must include port number as P:filename",
-					optarg);
-				pktgen_usage(prgname);
-				return -1;
+			pc = strchr(optarg, ',');
+			if (p == NULL)
+				goto pcap_err;
+			if (pc == NULL) {
+				pktgen.info[port].pcap = _pcap_open(++p, port);
+				if (pktgen.info[port].pcap == NULL)
+					goto pcap_err;
+			} else {
+				q = 0;
+				while (p != NULL && q < NUM_Q) {
+					p++;
+					pc = strchr(p, ',');
+					if (pc != NULL)
+						*pc = '\0';
+					pktgen.info[port].pcaps[q] = _pcap_open(p, port);
+					if (pktgen.info[port].pcaps[q] == NULL)
+						goto pcap_err;
+					p = pc;
+					q++;
+				}
+				pktgen.info[port].pcap = pktgen.info[port].pcaps[0];
 			}
 			break;
 		case 'P':	/* Enable promiscuous mode on the ports */
@@ -257,6 +271,12 @@ pktgen_parse_args(int argc, char **argv)
 	ret = optind - 1;
 	optind = 1;	/* reset getopt lib */
 	return ret;
+
+pcap_err:
+	pktgen_log_error("Invalid PCAP filename (%s) must include port number as P:filename",
+			 optarg);
+	pktgen_usage(prgname);
+	return -1;
 }
 
 #define MAX_BACKTRACE	32
diff --git a/app/pktgen-pcap.c b/app/pktgen-pcap.c
index dffdd13..e7ef9a2 100644
--- a/app/pktgen-pcap.c
+++ b/app/pktgen-pcap.c
@@ -343,9 +343,9 @@ pktgen_pcap_parse(pcap_info_t *pcap, port_info_t *info, unsigned qid)
 	/* If count is greater then zero then we allocate and create the PCAP mbuf pool. */
 	if (elt_count > 0) {
 		/* Create the average size packet */
-		info->pcap->pkt_size    = (pkt_sizes / elt_count);
-		info->pcap->pkt_count   = elt_count;
-		info->pcap->pkt_idx     = 0;
+		pcap->pkt_size    = (pkt_sizes / elt_count);
+		pcap->pkt_count   = elt_count;
+		pcap->pkt_idx     = 0;
 
 		_pcap_rewind(pcap);
 
@@ -374,15 +374,15 @@ pktgen_pcap_parse(pcap_info_t *pcap, port_info_t *info, unsigned qid)
 		scrn_printf(0, 0, "\r");
 		if (info->q[qid].pcap_mp == NULL)
 			pktgen_log_panic("Cannot init port %d for %d PCAP packets",
-					 info->pid, info->pcap->pkt_count);
+					 info->pid, pcap->pkt_count);
 
-		data_size = (info->pcap->pkt_count * DEFAULT_MBUF_SIZE);
+		data_size = (pcap->pkt_count * DEFAULT_MBUF_SIZE);
 		scrn_printf(0, 0,
 		        "    Create: %-*s - Number of MBUFs %6u for %5d packets                 = %6u KB\n",
 		        16,
 		        name,
 		        elt_count,
-		        info->pcap->pkt_count,
+		        pcap->pkt_count,
 		        (data_size + 1023) / 1024);
 		pktgen.mem_used         += data_size;
 		pktgen.total_mem_used   += data_size;
diff --git a/app/pktgen-port-cfg.c b/app/pktgen-port-cfg.c
index a1da13c..91ce694 100644
--- a/app/pktgen-port-cfg.c
+++ b/app/pktgen-port-cfg.c
@@ -318,7 +318,13 @@ pktgen_config_ports(void)
 				pktgen_log_panic("Cannot init port %d for Special TX mbufs", pid);
 
 			/* Setup the PCAP file for each port */
-			if (pktgen.info[pid].pcap != NULL)
+			if (pktgen.info[pid].pcaps[q] != NULL) {
+				if (pktgen_pcap_parse(pktgen.info[pid].pcaps[q], info, q) == -1) {
+					pktgen_log_panic(
+						"Cannot load PCAP file for port %d queue %d",
+						pid, q);
+				}
+			} else if (pktgen.info[pid].pcap != NULL)
 				if (pktgen_pcap_parse(pktgen.info[pid].pcap, info, q) == -1)
 					pktgen_log_panic("Cannot load PCAP file for port %d", pid);
 
diff --git a/app/pktgen-port-cfg.h b/app/pktgen-port-cfg.h
index 89019e4..381ddfe 100644
--- a/app/pktgen-port-cfg.h
+++ b/app/pktgen-port-cfg.h
@@ -214,6 +214,7 @@ typedef struct port_info_s {
 	int32_t rx_tapfd;	/**< Rx Tap file descriptor */
 	int32_t tx_tapfd;	/**< Tx Tap file descriptor */
 	pcap_info_t *pcap;	/**< PCAP information header */
+	pcap_info_t *pcaps[NUM_Q];	/**< Per Tx queue PCAP information headers */
 	uint64_t pcap_cycles;	/**< number of cycles for pcap sending */
 
 	int32_t pcap_result;	/**< PCAP result of filter compile */
-- 
2.7.4

[dpdk-dev] [PATCH 3/5] [pktgen] use constants for PCAP creation

[Rafal Kozik]

Instead of magic number and DPDK constants use, constants defined
in PKTGEN. It allows to easily adjust packets size for users needs.
For example jumbo packets could be supported by just increasing
DEFAULT_MBUF_SIZE and MAX_PKT_SIZE.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
 app/pktgen-pcap.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/app/pktgen-pcap.c b/app/pktgen-pcap.c
index e7ef9a2..7f380f6 100644
--- a/app/pktgen-pcap.c
+++ b/app/pktgen-pcap.c
@@ -43,7 +43,7 @@ pktgen_print_pcap(uint16_t pid)
 	pcap_info_t *pcap;
 	pcaprec_hdr_t pcap_hdr;
 	char buff[64];
-	char pkt_buff[2048];
+	char pkt_buff[DEFAULT_MBUF_SIZE];
 
 	pktgen_display_set_color("top.page");
 	display_topline("<PCAP Page>");
@@ -216,7 +216,7 @@ pktgen_pcap_mbuf_ctor(struct rte_mempool *mp,
 	uint32_t mbuf_size, buf_len, priv_size = 0;
 	pcaprec_hdr_t hdr;
 	ssize_t len = -1;
-	char buffer[2048];
+	char buffer[DEFAULT_MBUF_SIZE];
 	pcap_info_t *pcap = (pcap_info_t *)opaque_arg;
 
 #if RTE_VERSION >= RTE_VERSION_NUM(16, 7, 0, 0)
@@ -274,10 +274,10 @@ pktgen_pcap_mbuf_ctor(struct rte_mempool *mp,
 		len = hdr.incl_len;
 
 		/* Adjust the packet length if not a valid size. */
-		if (len < (ETHER_MIN_LEN - 4) )
-			len = (ETHER_MIN_LEN - 4);
-		else if (len > (ETHER_MAX_LEN - 4) )
-			len = (ETHER_MAX_LEN - 4);
+		if (len < MIN_PKT_SIZE)
+			len = MIN_PKT_SIZE;
+		else if (len > MAX_PKT_SIZE)
+			len = MAX_PKT_SIZE;
 
 		m->data_len = len;
 		m->pkt_len  = len;
@@ -309,7 +309,7 @@ pktgen_pcap_parse(pcap_info_t *pcap, port_info_t *info, unsigned qid)
 	pcaprec_hdr_t hdr;
 	uint32_t elt_count, data_size, len, i;
 	uint64_t pkt_sizes = 0;
-	char buffer[2048];
+	char buffer[DEFAULT_MBUF_SIZE];
 	char name[RTE_MEMZONE_NAMESIZE];
 
 	if ( (pcap == NULL) || (info == NULL) )
@@ -327,10 +327,10 @@ pktgen_pcap_parse(pcap_info_t *pcap, port_info_t *info, unsigned qid)
 		/* Skip any jumbo packets or packets that are too small */
 		len = hdr.incl_len;
 
-		if (len < (ETHER_MIN_LEN - 4) )
-			len = (ETHER_MIN_LEN - 4);
-		else if (len > (ETHER_MAX_LEN - 4) )
-			len = (ETHER_MAX_LEN - 4);
+		if (len < MIN_PKT_SIZE)
+			len = MIN_PKT_SIZE;
+		else if (len > MAX_PKT_SIZE)
+			len = MAX_PKT_SIZE;
 
 		elt_count++;
 
-- 
2.7.4

[dpdk-dev] [PATCH 4/5] [pktgen] expose number of missed Rx packets

[Rafal Kozik]

Expose number of missed Rx packets from DPDK NIC statistics to Lua API.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
 app/lpktgenlib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/lpktgenlib.c b/app/lpktgenlib.c
index 9a6307c..e9c7241 100644
--- a/app/lpktgenlib.c
+++ b/app/lpktgenlib.c
@@ -2888,6 +2888,7 @@ port_stats(lua_State *L, port_info_t *info, char *type)
 	setf_integer(L, "ierrors", stats.ierrors);
 	setf_integer(L, "oerrors", stats.oerrors);
 	setf_integer(L, "rx_nombuf", stats.rx_nombuf);
+	setf_integer(L, "imissed", stats.imissed);
 
 	if (strcmp(type, "rate") == 0) {
 		setf_integer(L, "pkts_rx", stats.ipackets);
-- 
2.7.4

[dpdk-dev] [PATCH 5/5] [pktgen] fix cleanup of not sent packets

[Rafal Kozik]

Not sent packets are copied to the beginning of array to be reused in next
iteration. But as in some cases more than half of mbuf could not be sent,
source and destination locations would overlap. In such case rte_memcpy
cannot be used.
One of side effects is double sending the same mbuf and as a consequence
double returning it to the mempool.

Fixes: 2bf1eecea240 ("fixup code for 18.05 and cleanup")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
---
 app/pktgen.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/pktgen.c b/app/pktgen.c
index f7eee07..c1039a6 100644
--- a/app/pktgen.c
+++ b/app/pktgen.c
@@ -272,11 +272,10 @@ _send_burst_fast(port_info_t *info, uint16_t qid)
 {
 	struct mbuf_table   *mtab = &info->q[qid].tx_mbufs;
 	struct rte_mbuf **pkts;
-	uint32_t ret, cnt, sav, retry;
+	uint32_t ret, cnt, retry, i;
 
 	cnt = mtab->len;
 	mtab->len = 0;
-	sav = cnt;
 
 	pkts = mtab->m_table;
 
@@ -299,8 +298,8 @@ _send_burst_fast(port_info_t *info, uint16_t qid)
 		}
 	}
 	if (cnt) {
-		rte_memcpy(&mtab->m_table[0], &mtab->m_table[sav - cnt],
-		           sizeof(char *) * cnt);
+		for (i = 0; i < cnt; i++)
+			mtab->m_table[i] = pkts[i];
 		mtab->len = cnt;
 	}
 }
-- 
2.7.4