From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Naga Suresh Somarowthu <naga.sureshx.somarowthu@intel.com>, dev@dpdk.org
Cc: remy.horton@intel.com, reshma.pattan@intel.com
Subject: Re: [dpdk-dev] [PATCH v5 1/4] test: add ring pmd based packet rx/tx for UT
Date: Tue, 24 Jul 2018 12:21:18 +0100 [thread overview]
Message-ID: <5bb45e92-504d-bab3-bbba-55a785c9fe5b@intel.com> (raw)
In-Reply-To: <1532429671-1606-2-git-send-email-naga.sureshx.somarowthu@intel.com>
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_ */
>
--
Thanks,
Anatoly
next prev parent reply other threads:[~2018-07-24 11:21 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-06 17:07 [dpdk-dev] [PATCH] add sample functions for packet forwarding Jananee Parthasarathy
2018-07-10 9:53 ` Pattan, Reshma
2018-07-12 8:53 ` [dpdk-dev] [PATCH v2] " Jananee Parthasarathy
2018-07-12 16:00 ` Pattan, Reshma
2018-07-16 16:00 ` [dpdk-dev] [PATCH v3] test: " Jananee Parthasarathy
2018-07-17 8:15 ` Pattan, Reshma
2018-07-17 10:00 ` [dpdk-dev] [PATCH v4] " Jananee Parthasarathy
2018-07-17 10:22 ` Burakov, Anatoly
2018-07-24 10:54 ` [dpdk-dev] [PATCH v5 0/4] add unit tests for bitrate, latency and pdump libraries Naga Suresh Somarowthu
2018-07-24 10:54 ` [dpdk-dev] [PATCH v5 1/4] test: add ring pmd based packet rx/tx for UT Naga Suresh Somarowthu
2018-07-24 11:21 ` Burakov, Anatoly [this message]
2018-07-24 10:54 ` [dpdk-dev] [PATCH v5 2/4] test: add unit tests for bitrate library Naga Suresh Somarowthu
2018-07-24 10:54 ` [dpdk-dev] [PATCH v5 3/4] test: add unit tests for latencystats library Naga Suresh Somarowthu
2018-07-24 10:54 ` [dpdk-dev] [PATCH v5 4/4] test: add unit test for pdump library Naga Suresh Somarowthu
2018-07-24 12:27 ` Burakov, Anatoly
2018-07-25 17:05 ` [dpdk-dev] [PATCH v6 0/4] add unit tests for bitrate, latency and pdump libraries Naga Suresh Somarowthu
2018-07-25 17:05 ` [dpdk-dev] [PATCH v6 1/4] test: add ring pmd based packet rx/tx for UT Naga Suresh Somarowthu
2018-07-26 9:59 ` Burakov, Anatoly
2018-07-25 17:05 ` [dpdk-dev] [PATCH v6 2/4] test: add unit tests for bitrate library Naga Suresh Somarowthu
2018-07-25 17:05 ` [dpdk-dev] [PATCH v6 3/4] test: add unit tests for latencystats library Naga Suresh Somarowthu
2018-07-25 17:06 ` [dpdk-dev] [PATCH v6 4/4] test: add unit test for pdump library Naga Suresh Somarowthu
2018-07-26 10:02 ` Burakov, Anatoly
2018-07-26 12:50 ` [dpdk-dev] [PATCH v7 0/4] add unit tests for bitrate, latency and pdump libraries Naga Suresh Somarowthu
2018-07-26 12:50 ` [dpdk-dev] [PATCH v7 1/4] test: add ring pmd based packet rx/tx for UT Naga Suresh Somarowthu
2018-07-26 16:43 ` Pattan, Reshma
2018-07-27 7:40 ` Burakov, Anatoly
2018-07-26 12:50 ` [dpdk-dev] [PATCH v7 2/4] test: add unit tests for bitrate library Naga Suresh Somarowthu
2018-07-26 12:50 ` [dpdk-dev] [PATCH v7 3/4] test: add unit tests for latencystats library Naga Suresh Somarowthu
2018-07-26 12:50 ` [dpdk-dev] [PATCH v7 4/4] test: add unit test for pdump library Naga Suresh Somarowthu
2018-07-26 16:04 ` [dpdk-dev] [PATCH v7 0/4] add unit tests for bitrate, latency and pdump libraries Pattan, Reshma
2018-07-27 14:26 ` [dpdk-dev] [PATCH v8 " Naga Suresh Somarowthu
2018-07-27 14:26 ` [dpdk-dev] [PATCH v8 1/4] test: add helper functions for tests using ring-PMD Rx/Tx Naga Suresh Somarowthu
2018-07-27 14:26 ` [dpdk-dev] [PATCH v8 2/4] test: add unit tests for bitrate library Naga Suresh Somarowthu
2018-07-27 14:26 ` [dpdk-dev] [PATCH v8 3/4] test: add unit tests for latencystats library Naga Suresh Somarowthu
2018-07-27 14:26 ` [dpdk-dev] [PATCH v8 4/4] test: add unit test for pdump library Naga Suresh Somarowthu
2018-07-31 14:57 ` [dpdk-dev] [PATCH v8 0/4] add unit tests for bitrate, latency and pdump libraries Thomas Monjalon
2018-07-31 16:43 ` Pattan, Reshma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5bb45e92-504d-bab3-bbba-55a785c9fe5b@intel.com \
--to=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=naga.sureshx.somarowthu@intel.com \
--cc=remy.horton@intel.com \
--cc=reshma.pattan@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).