[v5,1/4] test: add ring pmd based packet rx/tx for UT

Message ID 1532429671-1606-2-git-send-email-naga.sureshx.somarowthu@intel.com (mailing list archive)
State Superseded, archived
Headers
Series add unit tests for bitrate, latency and pdump libraries |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Naga Suresh Somarowthu July 24, 2018, 10:54 a.m. UTC
  Added ring pmd based packet rx/tx helper functions
for verifying Latency, Bitrate and pdump lib UTs.

Signed-off-by: Naga Suresh Somarowthu <naga.sureshx.somarowthu@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 test/test/Makefile                |   1 +
 test/test/sample_packet_forward.c | 115 ++++++++++++++++++++++++++++++++++++++
 test/test/sample_packet_forward.h |  41 ++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 test/test/sample_packet_forward.c
 create mode 100644 test/test/sample_packet_forward.h
  

Comments

Anatoly Burakov July 24, 2018, 11:21 a.m. UTC | #1
On 24-Jul-18 11:54 AM, Naga Suresh Somarowthu wrote:
> Added ring pmd based packet rx/tx helper functions
> for verifying Latency, Bitrate and pdump lib UTs.
> 
> Signed-off-by: Naga Suresh Somarowthu <naga.sureshx.somarowthu@intel.com>
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>

Why is it carrying my Reviewed-by: ? I do not recall sending it...

> ---
>   test/test/Makefile                |   1 +
>   test/test/sample_packet_forward.c | 115 ++++++++++++++++++++++++++++++++++++++
>   test/test/sample_packet_forward.h |  41 ++++++++++++++
>   3 files changed, 157 insertions(+)
>   create mode 100644 test/test/sample_packet_forward.c
>   create mode 100644 test/test/sample_packet_forward.h
> 
> diff --git a/test/test/Makefile b/test/test/Makefile
> index e6967bab6..9f7d398e4 100644

<snip>

> +/* Sample test to create virtual rings and tx,rx portid from rings */
> +int
> +test_ring_setup(struct rte_ring *ring[], uint16_t *portid)
> +{
> +	ring[0] = rte_ring_create("R0", RING_SIZE, rte_socket_id(),
> +				  RING_F_SP_ENQ | RING_F_SC_DEQ);
> +	if (ring[0] == NULL) {
> +		printf("%s() line %u: rte_ring_create R0 failed",
> +		       __func__, __LINE__);
> +		return TEST_FAILED;
> +	}
> +	*portid = rte_eth_from_rings("net_ringa", ring, NUM_QUEUES,
> +			ring, NUM_QUEUES, rte_socket_id());
> +
> +	return TEST_SUCCESS;
> +}

Previous comments have not been addressed.

You are always creating one ring, yet you create an array of rings (with 
a size of one). Why?

Even assuming you need to create multiple rings (which, as far as i can 
tell from other patches, you don't), why not just accept a 
pointer-to-pointer, like this:

int test_ring_setup(struct rte_ring **ring) {
    *ring = rte_ring_create();
}

// calling code
struct rte_ring *ring;
test_ring_setup(&ring);

Surely this would be more understandable?

(in fact, you do this with mempools - why not rings?)

Also, here and in lots of other places: why is this function returning 
TEST_SUCCESS? Is this an autotest? If not, then it shouldn't return 
these values.

> +
> +/* Sample test to free the mempool */
> +void
> +test_mp_free(struct rte_mempool *mp)
> +{
> +	rte_mempool_free(mp);
> +}
> +
> +/* Sample test to free the virtual rings */
> +void
> +test_ring_free(struct rte_ring *rxtx)
> +{
> +	rte_ring_free(rxtx);
> +}

<snip>

> +}
> diff --git a/test/test/sample_packet_forward.h b/test/test/sample_packet_forward.h
> new file mode 100644
> index 000000000..1293ee0a8
> --- /dev/null
> +++ b/test/test/sample_packet_forward.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef _SAMPLE_PACKET_FORWARD_H_
> +#define _SAMPLE_PACKET_FORWARD_H_
> +
> +/* MACROS to support virtual ring creation */
> +#define RING_SIZE 256
> +#define NUM_RINGS 1
> +#define NUM_QUEUES 1
> +#define NB_MBUF 512
> +
> +#define NUM_PACKETS 10

Some of the above #define's are not used in this patch. They probably 
should be added in later patches?

> +
> +/* Sample test to create virtual rings and tx,rx portid from rings */
> +int test_ring_setup(struct rte_ring *ring_server[], uint16_t *portid);
> +
> +/* Sample test to free the virtual rings */
> +void test_ring_free(struct rte_ring *rxtx);
> +
> +/* Sample test to forward packet using virtual port id */
> +int test_packet_forward(struct rte_mbuf **pbuf, uint16_t portid,
> +		uint16_t queue_id);
> +
> +/* sample test to allocate buffer for pkts */
> +int test_get_mbuf_from_pool(struct rte_mempool **mp, struct rte_mbuf **pbuf,
> +		char *poolname);
> +
> +/* Sample test to create the mempool */
> +int test_get_mempool(struct rte_mempool **mp, char *poolname);
> +
> +/* sample test to deallocate the allocated buffers */
> +void test_put_mbuf_to_pool(struct rte_mempool *mp, struct rte_mbuf **pbuf);
> +
> +/* Sample test to free the mempool */
> +void test_mp_free(struct rte_mempool *mp);
> +
> +/* Sample test to release the vdev */
> +void test_vdev_uninit(const char *vdev);
> +#endif				/* _SAMPLE_PACKET_FORWARD_H_ */
>
  

Patch

diff --git a/test/test/Makefile b/test/test/Makefile
index e6967bab6..9f7d398e4 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -165,6 +165,7 @@  SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c
 
 SRCS-y += virtual_pmd.c
 SRCS-y += packet_burst_generator.c
+SRCS-y += sample_packet_forward.c
 SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
 
 ifeq ($(CONFIG_RTE_LIBRTE_PMD_RING),y)
diff --git a/test/test/sample_packet_forward.c b/test/test/sample_packet_forward.c
new file mode 100644
index 000000000..8f9d7009a
--- /dev/null
+++ b/test/test/sample_packet_forward.c
@@ -0,0 +1,115 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include <string.h>
+#include <errno.h>
+#include <time.h>
+
+#include <rte_memcpy.h>
+#include <rte_common.h>
+#include <rte_eth_ring.h>
+#include <rte_ethdev.h>
+#include <rte_mbuf.h>
+#include <rte_bus_vdev.h>
+
+#include "sample_packet_forward.h"
+#include "test.h"
+
+/* Sample test to create virtual rings and tx,rx portid from rings */
+int
+test_ring_setup(struct rte_ring *ring[], uint16_t *portid)
+{
+	ring[0] = rte_ring_create("R0", RING_SIZE, rte_socket_id(),
+				  RING_F_SP_ENQ | RING_F_SC_DEQ);
+	if (ring[0] == NULL) {
+		printf("%s() line %u: rte_ring_create R0 failed",
+		       __func__, __LINE__);
+		return TEST_FAILED;
+	}
+	*portid = rte_eth_from_rings("net_ringa", ring, NUM_QUEUES,
+			ring, NUM_QUEUES, rte_socket_id());
+
+	return TEST_SUCCESS;
+}
+
+/* Sample test to free the mempool */
+void
+test_mp_free(struct rte_mempool *mp)
+{
+	rte_mempool_free(mp);
+}
+
+/* Sample test to free the virtual rings */
+void
+test_ring_free(struct rte_ring *rxtx)
+{
+	rte_ring_free(rxtx);
+}
+
+/* Sample test to release the vdev */
+void
+test_vdev_uninit(const char *vdev)
+{
+	rte_vdev_uninit(vdev);
+}
+
+/* sample test to deallocate the allocated buffers */
+int
+test_get_mempool(struct rte_mempool **mp, char *poolname)
+{
+	*mp = rte_pktmbuf_pool_create(poolname, NB_MBUF, 32, 0,
+			RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
+	if (*mp == NULL)
+		return TEST_FAILED;
+	return TEST_SUCCESS;
+}
+
+/* sample test to allocate buffer for pkts */
+int
+test_get_mbuf_from_pool(struct rte_mempool **mp, struct rte_mbuf **pbuf,
+		char *poolname)
+{
+	int ret = 0;
+
+	ret = test_get_mempool(mp, poolname);
+	if (ret < 0)
+		return TEST_FAILED;
+	if (rte_pktmbuf_alloc_bulk(*mp, pbuf, NUM_PACKETS) != 0) {
+		printf("%s() line %u: rte_pktmbuf_alloc_bulk failed", __func__,
+		       __LINE__);
+		return TEST_FAILED;
+	}
+	return TEST_SUCCESS;
+}
+
+/* sample test to deallocate the allocated buffers */
+void
+test_put_mbuf_to_pool(struct rte_mempool *mp, struct rte_mbuf **pbuf)
+{
+	int itr = 0;
+
+	for (itr = 0; itr < NUM_PACKETS; itr++)
+		rte_pktmbuf_free(pbuf[itr]);
+	rte_mempool_free(mp);
+}
+
+/* Sample test to forward packets using virtual portids */
+int
+test_packet_forward(struct rte_mbuf **pbuf, uint16_t portid, uint16_t queue_id)
+{
+	/* send and receive packet and check for stats update */
+	if (rte_eth_tx_burst(portid, queue_id, pbuf, NUM_PACKETS)
+			< NUM_PACKETS) {
+		printf("%s() line %u: Error sending packet to"
+		       " port %d\n", __func__, __LINE__, portid);
+		return TEST_FAILED;
+	}
+	if (rte_eth_rx_burst(portid, queue_id, pbuf, NUM_PACKETS)
+			< NUM_PACKETS) {
+		printf("%s() line %u: Error receiving packet from"
+		       " port %d\n", __func__, __LINE__, portid);
+		return TEST_FAILED;
+	}
+	return TEST_SUCCESS;
+}
diff --git a/test/test/sample_packet_forward.h b/test/test/sample_packet_forward.h
new file mode 100644
index 000000000..1293ee0a8
--- /dev/null
+++ b/test/test/sample_packet_forward.h
@@ -0,0 +1,41 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#ifndef _SAMPLE_PACKET_FORWARD_H_
+#define _SAMPLE_PACKET_FORWARD_H_
+
+/* MACROS to support virtual ring creation */
+#define RING_SIZE 256
+#define NUM_RINGS 1
+#define NUM_QUEUES 1
+#define NB_MBUF 512
+
+#define NUM_PACKETS 10
+
+/* Sample test to create virtual rings and tx,rx portid from rings */
+int test_ring_setup(struct rte_ring *ring_server[], uint16_t *portid);
+
+/* Sample test to free the virtual rings */
+void test_ring_free(struct rte_ring *rxtx);
+
+/* Sample test to forward packet using virtual port id */
+int test_packet_forward(struct rte_mbuf **pbuf, uint16_t portid,
+		uint16_t queue_id);
+
+/* sample test to allocate buffer for pkts */
+int test_get_mbuf_from_pool(struct rte_mempool **mp, struct rte_mbuf **pbuf,
+		char *poolname);
+
+/* Sample test to create the mempool */
+int test_get_mempool(struct rte_mempool **mp, char *poolname);
+
+/* sample test to deallocate the allocated buffers */
+void test_put_mbuf_to_pool(struct rte_mempool *mp, struct rte_mbuf **pbuf);
+
+/* Sample test to free the mempool */
+void test_mp_free(struct rte_mempool *mp);
+
+/* Sample test to release the vdev */
+void test_vdev_uninit(const char *vdev);
+#endif				/* _SAMPLE_PACKET_FORWARD_H_ */