[v3] eventdev/eth_rx: fix invalid memory access
Checks
Commit Message
From: Pavan Nikhilesh <pbhagavatula@marvell.com>
Rx adapter internal service structures are only initialized
when non-internal port is added to the adapter.
Check if Rx adapter service is initialized to determine if
runtime set/get parameters are supported.
Fixes: 3716f52186af ("eventdev/eth_rx: support runtime set/get parameters")
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
v3 Changes:
----------
- Teardown queue when test is not supported.
v2 Changes:
----------
- Remove unused variables.
- Fix spell check.
app/test/test_event_eth_rx_adapter.c | 10 ++++++++--
lib/eventdev/rte_event_eth_rx_adapter.c | 24 ++++--------------------
2 files changed, 12 insertions(+), 22 deletions(-)
--
2.25.1
Comments
Acked-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Friday, April 7, 2023 12:18 PM
> To: jerinj@marvell.com; Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: [PATCH v3] eventdev/eth_rx: fix invalid memory access
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Rx adapter internal service structures are only initialized when non-internal
> port is added to the adapter.
> Check if Rx adapter service is initialized to determine if runtime set/get
> parameters are supported.
>
> Fixes: 3716f52186af ("eventdev/eth_rx: support runtime set/get
> parameters")
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> v3 Changes:
> ----------
> - Teardown queue when test is not supported.
>
> v2 Changes:
> ----------
> - Remove unused variables.
> - Fix spell check.
>
> app/test/test_event_eth_rx_adapter.c | 10 ++++++++--
> lib/eventdev/rte_event_eth_rx_adapter.c | 24 ++++--------------------
> 2 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/app/test/test_event_eth_rx_adapter.c
> b/app/test/test_event_eth_rx_adapter.c
> index cfe6052d7e..52d146f97c 100644
> --- a/app/test/test_event_eth_rx_adapter.c
> +++ b/app/test/test_event_eth_rx_adapter.c
> @@ -1201,7 +1201,7 @@ adapter_intrq_instance_get(void) static int
> adapter_get_set_params(void)
> {
> - int err;
> + int err, rc;
> struct rte_event_eth_rx_adapter_runtime_params in_params;
> struct rte_event_eth_rx_adapter_runtime_params out_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_config = {0};
> @@ -1230,6 +1230,10 @@ adapter_get_set_params(void)
> /* Case 1: Get the default value of mbufs processed by Rx adapter */
> err =
> rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID,
> &out_params);
> + if (err == -ENOTSUP) {
> + rc = TEST_SKIPPED;
> + goto skip;
> + }
> TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>
> /* Case 2: Set max_nb_rx = 32 (=BATCH_SEIZE) */ @@ -1302,11
> +1306,13 @@ adapter_get_set_params(void)
> "Expected %u got %u",
> in_params.max_nb_rx, out_params.max_nb_rx);
>
> + rc = TEST_SUCCESS;
> +skip:
> err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
> TEST_ETHDEV_ID, 0);
> TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>
> - return TEST_SUCCESS;
> + return rc;
> }
>
> static struct unit_test_suite event_eth_rx_tests = { diff --git
> a/lib/eventdev/rte_event_eth_rx_adapter.c
> b/lib/eventdev/rte_event_eth_rx_adapter.c
> index c1cd124002..a8a5564b65 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -3466,30 +3466,14 @@
> rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id, static int
> rxa_caps_check(struct event_eth_rx_adapter *rxa) {
> - uint16_t eth_dev_id;
> - uint32_t caps = 0;
> - int ret;
> -
> if (!rxa->nb_queues)
> return -EINVAL;
>
> - /* The eth_dev used is always of same type.
> - * Hence eth_dev_id is taken from first entry of poll array.
> - */
> - eth_dev_id = rxa->eth_rx_poll[0].eth_dev_id;
> - ret = rte_event_eth_rx_adapter_caps_get(rxa->eventdev_id,
> - eth_dev_id,
> - &caps);
> - if (ret) {
> - RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %"
> PRIu8
> - "eth port %" PRIu16, rxa->eventdev_id, eth_dev_id);
> - return ret;
> - }
> -
> - if (caps & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)
> - return -ENOTSUP;
> + /* Check if there is at least one non-internal ethernet port. */
> + if (rxa->service_inited)
> + return 0;
>
> - return 0;
> + return -ENOTSUP;
> }
>
> int
> --
> 2.25.1
On Mon, Apr 17, 2023 at 12:00 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Acked-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Applied to dpdk-next-net-eventdev/for-main. Thanks
>
> > -----Original Message-----
> > From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> > Sent: Friday, April 7, 2023 12:18 PM
> > To: jerinj@marvell.com; Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Subject: [PATCH v3] eventdev/eth_rx: fix invalid memory access
> >
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Rx adapter internal service structures are only initialized when non-internal
> > port is added to the adapter.
> > Check if Rx adapter service is initialized to determine if runtime set/get
> > parameters are supported.
> >
> > Fixes: 3716f52186af ("eventdev/eth_rx: support runtime set/get
> > parameters")
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> > v3 Changes:
> > ----------
> > - Teardown queue when test is not supported.
> >
> > v2 Changes:
> > ----------
> > - Remove unused variables.
> > - Fix spell check.
> >
> > app/test/test_event_eth_rx_adapter.c | 10 ++++++++--
> > lib/eventdev/rte_event_eth_rx_adapter.c | 24 ++++--------------------
> > 2 files changed, 12 insertions(+), 22 deletions(-)
> >
> > diff --git a/app/test/test_event_eth_rx_adapter.c
> > b/app/test/test_event_eth_rx_adapter.c
> > index cfe6052d7e..52d146f97c 100644
> > --- a/app/test/test_event_eth_rx_adapter.c
> > +++ b/app/test/test_event_eth_rx_adapter.c
> > @@ -1201,7 +1201,7 @@ adapter_intrq_instance_get(void) static int
> > adapter_get_set_params(void)
> > {
> > - int err;
> > + int err, rc;
> > struct rte_event_eth_rx_adapter_runtime_params in_params;
> > struct rte_event_eth_rx_adapter_runtime_params out_params;
> > struct rte_event_eth_rx_adapter_queue_conf queue_config = {0};
> > @@ -1230,6 +1230,10 @@ adapter_get_set_params(void)
> > /* Case 1: Get the default value of mbufs processed by Rx adapter */
> > err =
> > rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID,
> > &out_params);
> > + if (err == -ENOTSUP) {
> > + rc = TEST_SKIPPED;
> > + goto skip;
> > + }
> > TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >
> > /* Case 2: Set max_nb_rx = 32 (=BATCH_SEIZE) */ @@ -1302,11
> > +1306,13 @@ adapter_get_set_params(void)
> > "Expected %u got %u",
> > in_params.max_nb_rx, out_params.max_nb_rx);
> >
> > + rc = TEST_SUCCESS;
> > +skip:
> > err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
> > TEST_ETHDEV_ID, 0);
> > TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >
> > - return TEST_SUCCESS;
> > + return rc;
> > }
> >
> > static struct unit_test_suite event_eth_rx_tests = { diff --git
> > a/lib/eventdev/rte_event_eth_rx_adapter.c
> > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index c1cd124002..a8a5564b65 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -3466,30 +3466,14 @@
> > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id, static int
> > rxa_caps_check(struct event_eth_rx_adapter *rxa) {
> > - uint16_t eth_dev_id;
> > - uint32_t caps = 0;
> > - int ret;
> > -
> > if (!rxa->nb_queues)
> > return -EINVAL;
> >
> > - /* The eth_dev used is always of same type.
> > - * Hence eth_dev_id is taken from first entry of poll array.
> > - */
> > - eth_dev_id = rxa->eth_rx_poll[0].eth_dev_id;
> > - ret = rte_event_eth_rx_adapter_caps_get(rxa->eventdev_id,
> > - eth_dev_id,
> > - &caps);
> > - if (ret) {
> > - RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %"
> > PRIu8
> > - "eth port %" PRIu16, rxa->eventdev_id, eth_dev_id);
> > - return ret;
> > - }
> > -
> > - if (caps & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)
> > - return -ENOTSUP;
> > + /* Check if there is at least one non-internal ethernet port. */
> > + if (rxa->service_inited)
> > + return 0;
> >
> > - return 0;
> > + return -ENOTSUP;
> > }
> >
> > int
> > --
> > 2.25.1
>
@@ -1201,7 +1201,7 @@ adapter_intrq_instance_get(void)
static int
adapter_get_set_params(void)
{
- int err;
+ int err, rc;
struct rte_event_eth_rx_adapter_runtime_params in_params;
struct rte_event_eth_rx_adapter_runtime_params out_params;
struct rte_event_eth_rx_adapter_queue_conf queue_config = {0};
@@ -1230,6 +1230,10 @@ adapter_get_set_params(void)
/* Case 1: Get the default value of mbufs processed by Rx adapter */
err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID,
&out_params);
+ if (err == -ENOTSUP) {
+ rc = TEST_SKIPPED;
+ goto skip;
+ }
TEST_ASSERT(err == 0, "Expected 0 got %d", err);
/* Case 2: Set max_nb_rx = 32 (=BATCH_SEIZE) */
@@ -1302,11 +1306,13 @@ adapter_get_set_params(void)
"Expected %u got %u",
in_params.max_nb_rx, out_params.max_nb_rx);
+ rc = TEST_SUCCESS;
+skip:
err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
TEST_ETHDEV_ID, 0);
TEST_ASSERT(err == 0, "Expected 0 got %d", err);
- return TEST_SUCCESS;
+ return rc;
}
static struct unit_test_suite event_eth_rx_tests = {
@@ -3466,30 +3466,14 @@ rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
static int
rxa_caps_check(struct event_eth_rx_adapter *rxa)
{
- uint16_t eth_dev_id;
- uint32_t caps = 0;
- int ret;
-
if (!rxa->nb_queues)
return -EINVAL;
- /* The eth_dev used is always of same type.
- * Hence eth_dev_id is taken from first entry of poll array.
- */
- eth_dev_id = rxa->eth_rx_poll[0].eth_dev_id;
- ret = rte_event_eth_rx_adapter_caps_get(rxa->eventdev_id,
- eth_dev_id,
- &caps);
- if (ret) {
- RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
- "eth port %" PRIu16, rxa->eventdev_id, eth_dev_id);
- return ret;
- }
-
- if (caps & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)
- return -ENOTSUP;
+ /* Check if there is at least one non-internal ethernet port. */
+ if (rxa->service_inited)
+ return 0;
- return 0;
+ return -ENOTSUP;
}
int