diff mbox series

[v2] eventdev/eth_tx: fix runtime parameter test

Message ID 20230502055711.1133134-1-s.v.naga.harish.k@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [v2] eventdev/eth_tx: fix runtime parameter test |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Functional fail Functional Testing issues

Commit Message

Naga Harish K, S V May 2, 2023, 5:57 a.m. UTC 
  TX adapter capability check logic is simplified.
The UT has been updated to skip the test, if the API
to set runtime parameters is not supported.

Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get parameters")

Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
 app/test/test_event_eth_tx_adapter.c    | 11 ++++++---
 lib/eventdev/rte_event_eth_tx_adapter.c | 33 +++++--------------------
 2 files changed, 14 insertions(+), 30 deletions(-)

Comments

Jerin Jacob May 17, 2023, 5:19 a.m. UTC | #1 
On Tue, May 2, 2023 at 11:27 AM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> TX adapter capability check logic is simplified.
> The UT has been updated to skip the test, if the API
> to set runtime parameters is not supported.
>
> Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get parameters")
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
>  app/test/test_event_eth_tx_adapter.c    | 11 ++++++---
>  lib/eventdev/rte_event_eth_tx_adapter.c | 33 +++++--------------------
>  2 files changed, 14 insertions(+), 30 deletions(-)


Please fix
Is it candidate for Cc: stable@dpdk.org backport?
        eventdev/eth_tx: fix runtime parameter test

Invalid patch(es) found - checked 1 patch
check-git-log failed

### [PATCH] eventdev/eth_tx: fix runtime parameter test

WARNING:BAD_FIXES_TAG: Please use correct Fixes: style 'Fixes: <12
chars of sha1> ("<title line>")' - ie: 'Fixes: 1d176c7add08
("eventdev/eth_tx: support runtime set/get parameters")'
#10:
Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get parameters")

total: 0 errors, 1 warnings, 90 lines checked

0/1 valid patch
checkpatch failed

>
> diff --git a/app/test/test_event_eth_tx_adapter.c b/app/test/test_event_eth_tx_adapter.c
> index 4e1d821bf9..616c972ac0 100644
> --- a/app/test/test_event_eth_tx_adapter.c
> +++ b/app/test/test_event_eth_tx_adapter.c
> @@ -800,13 +800,17 @@ tx_adapter_queue_start_stop(void)
>  static int
>  tx_adapter_set_get_params(void)
>  {
> -       int err;
> +       int err, rc;
>         struct rte_event_eth_tx_adapter_runtime_params in_params;
>         struct rte_event_eth_tx_adapter_runtime_params out_params;
>
>         err = rte_event_eth_tx_adapter_queue_add(TEST_INST_ID,
>                                                  TEST_ETHDEV_ID,
>                                                  0);
> +       if (err == -ENOTSUP) {
> +               rc = TEST_SKIPPED;
> +               goto skip;
> +       }
>         TEST_ASSERT(err == 0, "Expected 0 got %d", err);
>
>         err = rte_event_eth_tx_adapter_runtime_params_init(&in_params);
> @@ -916,13 +920,14 @@ tx_adapter_set_get_params(void)
>         TEST_ASSERT(in_params.flush_threshold == out_params.flush_threshold,
>                     "Expected %u got %u",
>                     in_params.flush_threshold, out_params.flush_threshold);
> -
> +       rc = TEST_SUCCESS;
> +skip:
>         err = rte_event_eth_tx_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 int
> diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
> index 131e11e01d..360d5caf6a 100644
> --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> @@ -1310,36 +1310,15 @@ rte_event_eth_tx_adapter_runtime_params_init(
>  }
>
>  static int
> -txa_caps_check(uint8_t id, struct txa_service_data *txa)
> +txa_caps_check(struct txa_service_data *txa)
>  {
> -       uint32_t caps = 0;
> -       struct rte_eth_dev *eth_dev = NULL;
> -       struct txa_service_ethdev *tdi;
> -       int i;
> -
>         if (!txa->dev_count)
>                 return -EINVAL;
>
> -       /* The eth_dev used is always the same type.
> -        * Hence first valid eth_dev is taken.
> -        */
> -       for (i = 0; i < txa->dev_count; i++) {
> -               tdi = &txa->txa_ethdev[i];
> -               if (tdi->nb_queues) {
> -                       eth_dev = tdi->dev;
> -                       break;
> -               }
> -       }
> -       if (eth_dev == NULL)
> -               return -EINVAL;
> -
> -       if (txa_dev_caps_get(id))
> -               txa_dev_caps_get(id)(txa_evdev(id), eth_dev, &caps);
> -
> -       if (caps & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)
> -               return -ENOTSUP;
> +       if (txa->service_id != TXA_INVALID_SERVICE_ID)
> +               return 0;
>
> -       return 0;
> +       return -ENOTSUP;
>  }
>
>  int
> @@ -1361,7 +1340,7 @@ rte_event_eth_tx_adapter_runtime_params_set(uint8_t id,
>         if (txa == NULL)
>                 return -EINVAL;
>
> -       ret = txa_caps_check(id, txa);
> +       ret = txa_caps_check(txa);
>         if (ret)
>                 return ret;
>
> @@ -1392,7 +1371,7 @@ rte_event_eth_tx_adapter_runtime_params_get(uint8_t id,
>         if (txa == NULL)
>                 return -EINVAL;
>
> -       ret = txa_caps_check(id, txa);
> +       ret = txa_caps_check(txa);
>         if (ret)
>                 return ret;
>
> --
> 2.23.0
>
Naga Harish K, S V May 17, 2023, 5:39 a.m. UTC | #2 
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, May 17, 2023 10:49 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2] eventdev/eth_tx: fix runtime parameter test
> 
> On Tue, May 2, 2023 at 11:27 AM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > TX adapter capability check logic is simplified.
> > The UT has been updated to skip the test, if the API to set runtime
> > parameters is not supported.
> >
> > Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get
> > parameters")
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
> >  app/test/test_event_eth_tx_adapter.c    | 11 ++++++---
> >  lib/eventdev/rte_event_eth_tx_adapter.c | 33
> > +++++--------------------
> >  2 files changed, 14 insertions(+), 30 deletions(-)
> 
> 
> Please fix
> Is it candidate for Cc: stable@dpdk.org backport?
>         eventdev/eth_tx: fix runtime parameter test
> 

This patch (feature) is added in DPDK 23.03 release.
So, thinking this fix may need not be backported.
Please let me know if it still needs to be backported.

> Invalid patch(es) found - checked 1 patch check-git-log failed
> 
> ### [PATCH] eventdev/eth_tx: fix runtime parameter test
> 
> WARNING:BAD_FIXES_TAG: Please use correct Fixes: style 'Fixes: <12 chars
> of sha1> ("<title line>")' - ie: 'Fixes: 1d176c7add08
> ("eventdev/eth_tx: support runtime set/get parameters")'
> #10:
> Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get
> parameters")
> 
> total: 0 errors, 1 warnings, 90 lines checked
> 
> 0/1 valid patch
> checkpatch failed
> 
> >
> > diff --git a/app/test/test_event_eth_tx_adapter.c
> > b/app/test/test_event_eth_tx_adapter.c
> > index 4e1d821bf9..616c972ac0 100644
> > --- a/app/test/test_event_eth_tx_adapter.c
> > +++ b/app/test/test_event_eth_tx_adapter.c
> > @@ -800,13 +800,17 @@ tx_adapter_queue_start_stop(void)  static int
> >  tx_adapter_set_get_params(void)
> >  {
> > -       int err;
> > +       int err, rc;
> >         struct rte_event_eth_tx_adapter_runtime_params in_params;
> >         struct rte_event_eth_tx_adapter_runtime_params out_params;
> >
> >         err = rte_event_eth_tx_adapter_queue_add(TEST_INST_ID,
> >                                                  TEST_ETHDEV_ID,
> >                                                  0);
> > +       if (err == -ENOTSUP) {
> > +               rc = TEST_SKIPPED;
> > +               goto skip;
> > +       }
> >         TEST_ASSERT(err == 0, "Expected 0 got %d", err);
> >
> >         err =
> > rte_event_eth_tx_adapter_runtime_params_init(&in_params);
> > @@ -916,13 +920,14 @@ tx_adapter_set_get_params(void)
> >         TEST_ASSERT(in_params.flush_threshold ==
> out_params.flush_threshold,
> >                     "Expected %u got %u",
> >                     in_params.flush_threshold,
> > out_params.flush_threshold);
> > -
> > +       rc = TEST_SUCCESS;
> > +skip:
> >         err = rte_event_eth_tx_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 int
> > diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
> > b/lib/eventdev/rte_event_eth_tx_adapter.c
> > index 131e11e01d..360d5caf6a 100644
> > --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> > @@ -1310,36 +1310,15 @@
> rte_event_eth_tx_adapter_runtime_params_init(
> >  }
> >
> >  static int
> > -txa_caps_check(uint8_t id, struct txa_service_data *txa)
> > +txa_caps_check(struct txa_service_data *txa)
> >  {
> > -       uint32_t caps = 0;
> > -       struct rte_eth_dev *eth_dev = NULL;
> > -       struct txa_service_ethdev *tdi;
> > -       int i;
> > -
> >         if (!txa->dev_count)
> >                 return -EINVAL;
> >
> > -       /* The eth_dev used is always the same type.
> > -        * Hence first valid eth_dev is taken.
> > -        */
> > -       for (i = 0; i < txa->dev_count; i++) {
> > -               tdi = &txa->txa_ethdev[i];
> > -               if (tdi->nb_queues) {
> > -                       eth_dev = tdi->dev;
> > -                       break;
> > -               }
> > -       }
> > -       if (eth_dev == NULL)
> > -               return -EINVAL;
> > -
> > -       if (txa_dev_caps_get(id))
> > -               txa_dev_caps_get(id)(txa_evdev(id), eth_dev, &caps);
> > -
> > -       if (caps & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)
> > -               return -ENOTSUP;
> > +       if (txa->service_id != TXA_INVALID_SERVICE_ID)
> > +               return 0;
> >
> > -       return 0;
> > +       return -ENOTSUP;
> >  }
> >
> >  int
> > @@ -1361,7 +1340,7 @@
> rte_event_eth_tx_adapter_runtime_params_set(uint8_t id,
> >         if (txa == NULL)
> >                 return -EINVAL;
> >
> > -       ret = txa_caps_check(id, txa);
> > +       ret = txa_caps_check(txa);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1392,7 +1371,7 @@
> rte_event_eth_tx_adapter_runtime_params_get(uint8_t id,
> >         if (txa == NULL)
> >                 return -EINVAL;
> >
> > -       ret = txa_caps_check(id, txa);
> > +       ret = txa_caps_check(txa);
> >         if (ret)
> >                 return ret;
> >
> > --
> > 2.23.0
> >
Jerin Jacob May 17, 2023, 8:15 a.m. UTC | #3 
On Wed, May 17, 2023 at 11:09 AM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, May 17, 2023 10:49 AM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; dev@dpdk.org; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2] eventdev/eth_tx: fix runtime parameter test
> >
> > On Tue, May 2, 2023 at 11:27 AM Naga Harish K S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > > TX adapter capability check logic is simplified.
> > > The UT has been updated to skip the test, if the API to set runtime
> > > parameters is not supported.
> > >
> > > Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get
> > > parameters")
> > >
> > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > > ---
> > >  app/test/test_event_eth_tx_adapter.c    | 11 ++++++---
> > >  lib/eventdev/rte_event_eth_tx_adapter.c | 33
> > > +++++--------------------
> > >  2 files changed, 14 insertions(+), 30 deletions(-)
> >
> >
> > Please fix
> > Is it candidate for Cc: stable@dpdk.org backport?
> >         eventdev/eth_tx: fix runtime parameter test
> >
>
> This patch (feature) is added in DPDK 23.03 release.
> So, thinking this fix may need not be backported.
> Please let me know if it still needs to be backported.

If there is stable free for 23.03 then we need to do this.
From patch author PoV(as per policy), we can mark as Cc:
stable@dpdk.org and stable tree maintainer will do rest.

The only case where an author does need to add Cc: stable@dpdk.org is
when, it is fixing something in the current release where a bug is
introduced in this current release.
Naga Harish K, S V May 17, 2023, 10:37 a.m. UTC | #4 
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, May 17, 2023 1:46 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2] eventdev/eth_tx: fix runtime parameter test
> 
> On Wed, May 17, 2023 at 11:09 AM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, May 17, 2023 10:49 AM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2] eventdev/eth_tx: fix runtime parameter test
> > >
> > > On Tue, May 2, 2023 at 11:27 AM Naga Harish K S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > > TX adapter capability check logic is simplified.
> > > > The UT has been updated to skip the test, if the API to set
> > > > runtime parameters is not supported.
> > > >
> > > > Fixes: 1d176c7add08581 ("eventdev/eth_tx: support runtime set/get
> > > > parameters")
> > > >
> > > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > > > ---
> > > >  app/test/test_event_eth_tx_adapter.c    | 11 ++++++---
> > > >  lib/eventdev/rte_event_eth_tx_adapter.c | 33
> > > > +++++--------------------
> > > >  2 files changed, 14 insertions(+), 30 deletions(-)
> > >
> > >
> > > Please fix
> > > Is it candidate for Cc: stable@dpdk.org backport?
> > >         eventdev/eth_tx: fix runtime parameter test
> > >
> >
> > This patch (feature) is added in DPDK 23.03 release.
> > So, thinking this fix may need not be backported.
> > Please let me know if it still needs to be backported.
> 
> If there is stable free for 23.03 then we need to do this.
> From patch author PoV(as per policy), we can mark as Cc:
> stable@dpdk.org and stable tree maintainer will do rest.
> 
> The only case where an author does need to add Cc: stable@dpdk.org is
> when, it is fixing something in the current release where a bug is introduced
> in this current release.

All review comments are addressed in V3 version of the patch.
diff mbox series

Patch

diff --git a/app/test/test_event_eth_tx_adapter.c b/app/test/test_event_eth_tx_adapter.c
index 4e1d821bf9..616c972ac0 100644
--- a/app/test/test_event_eth_tx_adapter.c
+++ b/app/test/test_event_eth_tx_adapter.c
@@ -800,13 +800,17 @@  tx_adapter_queue_start_stop(void)
 static int
 tx_adapter_set_get_params(void)
 {
-	int err;
+	int err, rc;
 	struct rte_event_eth_tx_adapter_runtime_params in_params;
 	struct rte_event_eth_tx_adapter_runtime_params out_params;
 
 	err = rte_event_eth_tx_adapter_queue_add(TEST_INST_ID,
 						 TEST_ETHDEV_ID,
 						 0);
+	if (err == -ENOTSUP) {
+		rc = TEST_SKIPPED;
+		goto skip;
+	}
 	TEST_ASSERT(err == 0, "Expected 0 got %d", err);
 
 	err = rte_event_eth_tx_adapter_runtime_params_init(&in_params);
@@ -916,13 +920,14 @@  tx_adapter_set_get_params(void)
 	TEST_ASSERT(in_params.flush_threshold == out_params.flush_threshold,
 		    "Expected %u got %u",
 		    in_params.flush_threshold, out_params.flush_threshold);
-
+	rc = TEST_SUCCESS;
+skip:
 	err = rte_event_eth_tx_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 int
diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
index 131e11e01d..360d5caf6a 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -1310,36 +1310,15 @@  rte_event_eth_tx_adapter_runtime_params_init(
 }
 
 static int
-txa_caps_check(uint8_t id, struct txa_service_data *txa)
+txa_caps_check(struct txa_service_data *txa)
 {
-	uint32_t caps = 0;
-	struct rte_eth_dev *eth_dev = NULL;
-	struct txa_service_ethdev *tdi;
-	int i;
-
 	if (!txa->dev_count)
 		return -EINVAL;
 
-	/* The eth_dev used is always the same type.
-	 * Hence first valid eth_dev is taken.
-	 */
-	for (i = 0; i < txa->dev_count; i++) {
-		tdi = &txa->txa_ethdev[i];
-		if (tdi->nb_queues) {
-			eth_dev = tdi->dev;
-			break;
-		}
-	}
-	if (eth_dev == NULL)
-		return -EINVAL;
-
-	if (txa_dev_caps_get(id))
-		txa_dev_caps_get(id)(txa_evdev(id), eth_dev, &caps);
-
-	if (caps & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)
-		return -ENOTSUP;
+	if (txa->service_id != TXA_INVALID_SERVICE_ID)
+		return 0;
 
-	return 0;
+	return -ENOTSUP;
 }
 
 int
@@ -1361,7 +1340,7 @@  rte_event_eth_tx_adapter_runtime_params_set(uint8_t id,
 	if (txa == NULL)
 		return -EINVAL;
 
-	ret = txa_caps_check(id, txa);
+	ret = txa_caps_check(txa);
 	if (ret)
 		return ret;
 
@@ -1392,7 +1371,7 @@  rte_event_eth_tx_adapter_runtime_params_get(uint8_t id,
 	if (txa == NULL)
 		return -EINVAL;
 
-	ret = txa_caps_check(id, txa);
+	ret = txa_caps_check(txa);
 	if (ret)
 		return ret;