[v1] graph: remove the useless duplicate name check

Message ID 20220307064318.1002855-1-haiyue.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v1] graph: remove the useless duplicate name check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed

Commit Message

Wang, Haiyue March 7, 2022, 6:43 a.m. UTC
  The node clone API parameter 'name' is the new node's postfix name, not
the final node name, so it makes no sense to check it. And the new name
will be checked duplicate when calling API '__rte_node_register'.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 lib/graph/node.c | 4 ----
 1 file changed, 4 deletions(-)
  

Comments

Jerin Jacob March 7, 2022, 7:44 a.m. UTC | #1
On Mon, Mar 7, 2022 at 12:47 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> The node clone API parameter 'name' is the new node's postfix name, not
> the final node name, so it makes no sense to check it. And the new name
> will be checked duplicate when calling API '__rte_node_register'.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Could you check the test failure with this change?
Reproducer:

echo "graph_autotest" | ./build/app/test/dpdk-test -c 0x30 --no-huge

> ---
>  lib/graph/node.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/lib/graph/node.c b/lib/graph/node.c
> index 79230035a2..ae6eadb260 100644
> --- a/lib/graph/node.c
> +++ b/lib/graph/node.c
> @@ -150,10 +150,6 @@ node_clone(struct node *node, const char *name)
>                 goto fail;
>         }
>
> -       /* Check for duplicate name */
> -       if (node_has_duplicate_entry(name))
> -               goto fail;
> -
>         reg = calloc(1, sizeof(*reg) + (sizeof(char *) * node->nb_edges));
>         if (reg == NULL) {
>                 rte_errno = ENOMEM;
> --
> 2.35.1
>
  
Wang, Haiyue March 7, 2022, 7:59 a.m. UTC | #2
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, March 7, 2022 15:45
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dpdk-dev <dev@dpdk.org>; Jerin Jacob <jerinj@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
> Nithin Dabilpuram <ndabilpuram@marvell.com>
> Subject: Re: [PATCH v1] graph: remove the useless duplicate name check
> 
> On Mon, Mar 7, 2022 at 12:47 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > The node clone API parameter 'name' is the new node's postfix name, not
> > the final node name, so it makes no sense to check it. And the new name
> > will be checked duplicate when calling API '__rte_node_register'.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> Could you check the test failure with this change?
> Reproducer:
> 
> echo "graph_autotest" | ./build/app/test/dpdk-test -c 0x30 --no-huge

I see, so the API not want "test_node00-test_node00".
LGTM, if so, I will drop this patch. ;-)

> 
> > ---
> >  lib/graph/node.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/lib/graph/node.c b/lib/graph/node.c
> > index 79230035a2..ae6eadb260 100644
> > --- a/lib/graph/node.c
> > +++ b/lib/graph/node.c
> > @@ -150,10 +150,6 @@ node_clone(struct node *node, const char *name)
> >                 goto fail;
> >         }
> >
> > -       /* Check for duplicate name */
> > -       if (node_has_duplicate_entry(name))
> > -               goto fail;
> > -
> >         reg = calloc(1, sizeof(*reg) + (sizeof(char *) * node->nb_edges));
> >         if (reg == NULL) {
> >                 rte_errno = ENOMEM;
> > --
> > 2.35.1
> >
  
Wang, Haiyue March 7, 2022, 8:05 a.m. UTC | #3
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, March 7, 2022 15:45
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dpdk-dev <dev@dpdk.org>; Jerin Jacob <jerinj@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
> Nithin Dabilpuram <ndabilpuram@marvell.com>
> Subject: Re: [PATCH v1] graph: remove the useless duplicate name check
> 
> On Mon, Mar 7, 2022 at 12:47 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > The node clone API parameter 'name' is the new node's postfix name, not
> > the final node name, so it makes no sense to check it. And the new name
> > will be checked duplicate when calling API '__rte_node_register'.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> 
> Could you check the test failure with this change?
> Reproducer:
> 
> echo "graph_autotest" | ./build/app/test/dpdk-test -c 0x30 --no-huge
> 

I update the new case, how about this one ?



index 35e1a95b89..7deaae99af 100644
--- a/app/test/test_graph.c
+++ b/app/test/test_graph.c
@@ -544,7 +544,7 @@ static int
 test_node_clone(void)
 {
        test_main_t *tm = &test_main;
-       uint32_t node_id, dummy_id;
+       uint32_t node_id, dummy_id, dummy_id1;
        int i;

        node_id = rte_node_from_name("test_node00");
@@ -552,7 +552,13 @@ test_node_clone(void)

        /* Clone with same name, should fail */
        dummy_id = rte_node_clone(node_id, "test_node00");
-       if (!rte_node_is_invalid(dummy_id)) {
+       if (rte_node_is_invalid(dummy_id)) {
+               printf("Got valid id when clone with same name, Expecting fail\n");
+               return -1;
+       }
+
+       dummy_id1 = rte_node_clone(node_id, "test_node00");
+       if (!rte_node_is_invalid(dummy_id1)) {
                printf("Got valid id when clone with same name, Expecting fail\n");
                return -1;
        }

> > ---
> >  lib/graph/node.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/lib/graph/node.c b/lib/graph/node.c
> > index 79230035a2..ae6eadb260 100644
> > --- a/lib/graph/node.c
> > +++ b/lib/graph/node.c
> > @@ -150,10 +150,6 @@ node_clone(struct node *node, const char *name)
> >                 goto fail;
> >         }
> >
> > -       /* Check for duplicate name */
> > -       if (node_has_duplicate_entry(name))
> > -               goto fail;
> > -
> >         reg = calloc(1, sizeof(*reg) + (sizeof(char *) * node->nb_edges));
> >         if (reg == NULL) {
> >                 rte_errno = ENOMEM;
> > --
> > 2.35.1
> >
  

Patch

diff --git a/lib/graph/node.c b/lib/graph/node.c
index 79230035a2..ae6eadb260 100644
--- a/lib/graph/node.c
+++ b/lib/graph/node.c
@@ -150,10 +150,6 @@  node_clone(struct node *node, const char *name)
 		goto fail;
 	}
 
-	/* Check for duplicate name */
-	if (node_has_duplicate_entry(name))
-		goto fail;
-
 	reg = calloc(1, sizeof(*reg) + (sizeof(char *) * node->nb_edges));
 	if (reg == NULL) {
 		rte_errno = ENOMEM;