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.somarow...@intel.com>
Reviewed-by: Reshma Pattan <reshma.pat...@intel.com>
Reviewed-by: Anatoly Burakov <anatoly.bura...@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_ */
--
Thanks,
Anatoly
