From: Mahipal Challa <mchalla@marvell.com>
To: Gavin Hu <Gavin.Hu@arm.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
"Narayana Prasad Raju Athreya" <pathreya@marvell.com>,
Subrahmanyam Nilla <snilla@marvell.com>,
Venkateshwarlu Nalla <venkatn@marvell.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v3 6/6] raw/octeontx2_ep: add driver self test
Date: Tue, 7 Jan 2020 04:14:57 +0000 [thread overview]
Message-ID: <MWHPR18MB137605FF123D7B3791EDA04FBE3F0@MWHPR18MB1376.namprd18.prod.outlook.com> (raw)
In-Reply-To: <VI1PR08MB53764A29A027DA9CE37872258F3F0@VI1PR08MB5376.eurprd08.prod.outlook.com>
Hi Gavin,
Please see response inline
> -----Original Message-----
> From: Gavin Hu <Gavin.Hu@arm.com>
> Sent: Tuesday, January 7, 2020 8:13 AM
> To: Mahipal Challa <mchalla@marvell.com>; dev@dpdk.org
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju
> Athreya <pathreya@marvell.com>; Subrahmanyam Nilla
> <snilla@marvell.com>; Venkateshwarlu Nalla <venkatn@marvell.com>; nd
> <nd@arm.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v3 6/6] raw/octeontx2_ep: add driver
> self test
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Mahipal,
>
> > -----Original Message-----
> > From: Mahipal Challa <mchalla@marvell.com>
> > Sent: Monday, January 6, 2020 8:07 PM
> > To: dev@dpdk.org
> > Cc: jerinj@marvell.com; pathreya@marvell.com; snilla@marvell.com;
> > venkatn@marvell.com; Gavin Hu <Gavin.Hu@arm.com>
> > Subject: [dpdk-dev] [PATCH v3 6/6] raw/octeontx2_ep: add driver self
> > test
> >
> > Add rawdev's selftest feature in SDP VF driver, which verifies the EP
> > mode functionality test.
> >
> > Signed-off-by: Mahipal Challa <mchalla@marvell.com>
> > ---
> > doc/guides/rawdevs/octeontx2_ep.rst | 13 +++
> > drivers/raw/octeontx2_ep/Makefile | 1 +
> > drivers/raw/octeontx2_ep/meson.build | 1 +
> > drivers/raw/octeontx2_ep/otx2_ep_rawdev.c | 1 +
> > drivers/raw/octeontx2_ep/otx2_ep_rawdev.h | 2 +
> > drivers/raw/octeontx2_ep/otx2_ep_test.c | 166
> > ++++++++++++++++++++++++++++++
> > 6 files changed, 184 insertions(+)
> >
> > diff --git a/doc/guides/rawdevs/octeontx2_ep.rst
> > b/doc/guides/rawdevs/octeontx2_ep.rst
> > index 39a7c29..bbcf530 100644
> > --- a/doc/guides/rawdevs/octeontx2_ep.rst
> > +++ b/doc/guides/rawdevs/octeontx2_ep.rst
> > @@ -74,3 +74,16 @@ Performing Data Transfer To perform data transfer
> > using SDP VF EP rawdev devices use standard
> > ``rte_rawdev_enqueue_buffers()`` and ``rte_rawdev_dequeue_buffers()``
> > APIs.
> >
> > +Self test
> > +---------
> > +
> > +On EAL initialization, SDP VF devices will be probed and populated
> > +into the raw devices. The rawdev ID of the device can be obtained
> > +using
> > +
> > +* Invoke ``rte_rawdev_get_dev_id("SDPEP:x")`` from the test
> > +application
> > + where x is the VF device's bus id specified in
> > +"bus:device.func"(BDF)
> > + format. Use this index for further rawdev function calls.
> > +
> > +* The driver's selftest rawdev API can be used to verify the SDP EP
> > +mode
> > + functional tests which can send/receive the raw data packets
> > +to/from the
> > + EP device.
> > diff --git a/drivers/raw/octeontx2_ep/Makefile
> > b/drivers/raw/octeontx2_ep/Makefile
> > index 02853fb..44fdf89 100644
> > --- a/drivers/raw/octeontx2_ep/Makefile
> > +++ b/drivers/raw/octeontx2_ep/Makefile
> > @@ -37,6 +37,7 @@ LIBABIVER := 1
> > #
> > SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX2_EP_RAWDEV) +=
> otx2_ep_rawdev.c
> > SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX2_EP_RAWDEV) +=
> otx2_ep_enqdeq.c
> > +SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX2_EP_RAWDEV) +=
> > otx2_ep_test.c
> > SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX2_EP_RAWDEV) +=
> otx2_ep_vf.c
> >
> >
> > diff --git a/drivers/raw/octeontx2_ep/meson.build
> > b/drivers/raw/octeontx2_ep/meson.build
> > index 99e6c6d..0e6338f 100644
> > --- a/drivers/raw/octeontx2_ep/meson.build
> > +++ b/drivers/raw/octeontx2_ep/meson.build
> > @@ -5,4 +5,5 @@
> > deps += ['bus_pci', 'common_octeontx2', 'rawdev'] sources =
> > files('otx2_ep_rawdev.c',
> > 'otx2_ep_enqdeq.c',
> > + 'otx2_ep_test.c',
> > 'otx2_ep_vf.c')
> > diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
> > b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
> > index 7158b97..0778603 100644
> > --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
> > +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
> > @@ -253,6 +253,7 @@
> > .dev_close = sdp_rawdev_close,
> > .enqueue_bufs = sdp_rawdev_enqueue,
> > .dequeue_bufs = sdp_rawdev_dequeue,
> > + .dev_selftest = sdp_rawdev_selftest,
> > };
> >
> > static int
> > diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.h
> > b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.h
> > index a77cbab..dab2fb7 100644
> > --- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.h
> > +++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.h
> > @@ -494,4 +494,6 @@ int sdp_rawdev_enqueue(struct rte_rawdev *dev,
> > struct rte_rawdev_buf **buffers, int sdp_rawdev_dequeue(struct
> > rte_rawdev *dev, struct rte_rawdev_buf **buffers,
> > unsigned int count, rte_rawdev_obj_t context);
> >
> > +int sdp_rawdev_selftest(uint16_t dev_id);
> > +
> > #endif /* _OTX2_EP_RAWDEV_H_ */
> > diff --git a/drivers/raw/octeontx2_ep/otx2_ep_test.c
> > b/drivers/raw/octeontx2_ep/otx2_ep_test.c
> > new file mode 100644
> > index 0000000..fc913a6
> > --- /dev/null
> > +++ b/drivers/raw/octeontx2_ep/otx2_ep_test.c
> > @@ -0,0 +1,166 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(C) 2019 Marvell International Ltd.
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +
> > +#include <rte_common.h>
> > +#include <rte_eal.h>
> > +#include <rte_lcore.h>
> > +#include <rte_mempool.h>
> > +
> > +#include <rte_rawdev.h>
> > +#include <rte_rawdev_pmd.h>
> > +
> > +#include "otx2_common.h"
> > +#include "otx2_ep_rawdev.h"
> > +
> > +#define SDP_IOQ_NUM_BUFS (4 * 1024)
> > +#define SDP_IOQ_BUF_SIZE (2 * 1024)
> > +
> > +#define SDP_TEST_PKT_FSZ (0)
> > +#define SDP_TEST_PKT_SIZE (1024)
> > +
> > +static int
> > +sdp_validate_data(struct sdp_droq_pkt *oq_pkt, uint8_t *iq_pkt,
> > + uint32_t pkt_len)
> > +{
> > + if (!oq_pkt)
> > + return -EINVAL;
> > +
> > + if (pkt_len != oq_pkt->len) {
> > + otx2_err("Invalid packet length");
> > + return -EINVAL;
> > + }
> > +
> > + if (memcmp(oq_pkt->data, iq_pkt, pkt_len) != 0) {
> > + otx2_err("Data validation failed");
> > + return -EINVAL;
> > + }
> > + otx2_sdp_dbg("Data validation successful");
> > +
> > + return 0;
> > +}
> > +
> > +static void
> > +sdp_ioq_buffer_fill(uint8_t *addr, uint32_t len) {
> > + uint32_t idx;
> > +
> > + memset(addr, 0, len);
> > +
> > + for (idx = 0; idx < len; idx++)
> > + addr[idx] = idx;
> > +}
> > +
> > +static struct rte_mempool*
> > +sdp_ioq_mempool_create(void)
> > +{
> > + struct rte_mempool *mpool;
> > +
> > + mpool = rte_mempool_create("ioqbuf_pool",
> > + SDP_IOQ_NUM_BUFS /*num elt*/,
> > + SDP_IOQ_BUF_SIZE /*elt size*/,
> > + 0 /*cache_size*/,
> > + 0 /*private_data_size*/,
> > + NULL /*mp_init*/,
> > + NULL /*mp_init arg*/,
> > + NULL /*obj_init*/,
> > + NULL /*obj_init arg*/,
> > + rte_socket_id() /*socket id*/,
> > + (MEMPOOL_F_SP_PUT |
> > MEMPOOL_F_SC_GET));
> > +
> > + return mpool;
> > +}
> > +
> > +
> > +int
> > +sdp_rawdev_selftest(uint16_t dev_id)
> > +{
> > + struct sdp_rawdev_info app_info = {0};
> > + struct rte_rawdev_info dev_info = {0};
> > +
> > + struct rte_rawdev_buf *d_buf[1];
> > + struct sdp_droq_pkt oq_pkt;
> > + struct sdp_soft_instr si;
> > + struct sdp_device sdpvf;
> > +
> > + uint32_t buf_size;
> > + int ret = 0;
> > + void *buf;
> > +
> > + otx2_info("SDP RAWDEV Self Test: Started");
> > +
> > + memset(&oq_pkt, 0x00, sizeof(oq_pkt));
> > + d_buf[0] = (struct rte_rawdev_buf *)&oq_pkt;
> > +
> > + struct rte_mempool *ioq_mpool = sdp_ioq_mempool_create();
> > + if (!ioq_mpool) {
> > + otx2_err("IOQ mpool creation failed");
> > + return -ENOMEM;
> > + }
> > +
> > + app_info.enqdeq_mpool = ioq_mpool;
> > + app_info.app_conf = NULL; /* Use default conf */
> > +
> > + dev_info.dev_private = &app_info;
> > +
> > + ret = rte_rawdev_configure(dev_id, &dev_info);
> > + if (ret) {
> > + otx2_err("Unable to configure SDP_VF %d", dev_id);
>
> The ioq_mpool was still not freed in the failure path?
[Mahipal]: Will handle it in v4, but I feel there is no harm with this as this is a test API for a minimal test application which exits end of this test, anyway will handle it.
> I see similar possible leakage in the other patches.
[Mahipal]: Assuming you are talking about the comment you gave in v2, Please see the response to the same. We have handled all the err handling, it is part of the patch " [v3,3/6] raw/octeontx2_ep: add device uninitialization".
If you still find anymore please feel free to point me.
>
> > + return -ENOMEM;
> > + }
> > + otx2_info("SDP VF rawdev[%d] configured successfully", dev_id);
> > +
> > + memset(&si, 0x00, sizeof(si));
> > + memset(&sdpvf, 0x00, sizeof(sdpvf));
> > +
> > + buf_size = SDP_TEST_PKT_SIZE;
> > +
> > + si.q_no = 0;
> > + si.reqtype = SDP_REQTYPE_NORESP;
> > + si.rptr = NULL;
> > +
> > + si.ih.fsz = SDP_TEST_PKT_FSZ;
> > + si.ih.tlen = buf_size;
> > + si.ih.gather = 0;
> > +
> > + /* Enqueue raw pkt data */
> > + rte_mempool_get(ioq_mpool, &buf);
> > + if (!buf) {
> > + otx2_err("Buffer allocation failed");
> > + return -ENOMEM;
> > + }
> > +
> > + sdp_ioq_buffer_fill(buf, buf_size);
> > + si.dptr = (uint8_t *)buf;
> > +
> > + rte_rawdev_enqueue_buffers(dev_id, NULL, 1, &si);
> > + usleep(10000);
> > +
> > + /* Dequeue raw pkt data */
> > + ret = 0;
> > + while (ret < 1) {
> > + ret = rte_rawdev_dequeue_buffers(dev_id, &d_buf[0], 1,
> > &si);
> > + rte_pause();
> > + }
> > +
> > + /* Validate the dequeued raw pkt data */
> > + if (sdp_validate_data((struct sdp_droq_pkt *)d_buf[0],
> > + buf, buf_size) != 0) {
> > + otx2_err("Data invalid");
> > + return -EINVAL;
> > + }
> > +
> > + rte_mempool_put(ioq_mpool, ((struct sdp_droq_pkt *)d_buf[0])-
> > >data);
> > + if (ioq_mpool)
> > + rte_mempool_free(ioq_mpool);
> > +
> > + otx2_info("SDP RAWDEV Self Test: Successful");
> > +
> > + return 0;
> > +}
> > +
> > --
> > 1.8.3.1
Thanks,
Mahipal
prev parent reply other threads:[~2020-01-07 4:15 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-06 12:07 [dpdk-dev] [PATCH v3 0/6] OCTEON TX2 End Point Driver Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 1/6] raw/octeontx2_ep: add build infra and device probe Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 2/6] raw/octeontx2_ep: add device configuration Mahipal Challa
2020-01-07 6:01 ` Gavin Hu
2020-01-07 6:46 ` Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 3/6] raw/octeontx2_ep: add device uninitialization Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 4/6] raw/octeontx2_ep: add enqueue operation Mahipal Challa
2020-01-07 5:55 ` Gavin Hu
2020-01-07 7:03 ` Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 5/6] raw/octeontx2_ep: add dequeue operation Mahipal Challa
2020-01-07 5:42 ` Gavin Hu
2020-01-07 7:18 ` Mahipal Challa
2020-01-06 12:07 ` [dpdk-dev] [PATCH v3 6/6] raw/octeontx2_ep: add driver self test Mahipal Challa
2020-01-07 2:42 ` Gavin Hu
2020-01-07 4:14 ` Mahipal Challa [this message]
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=MWHPR18MB137605FF123D7B3791EDA04FBE3F0@MWHPR18MB1376.namprd18.prod.outlook.com \
--to=mchalla@marvell.com \
--cc=Gavin.Hu@arm.com \
--cc=dev@dpdk.org \
--cc=jerinj@marvell.com \
--cc=nd@arm.com \
--cc=pathreya@marvell.com \
--cc=snilla@marvell.com \
--cc=venkatn@marvell.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).