diff mbox series

[v6,01/15] graph: rename rte_graph_work as common

Message ID 20230509060347.1237884-2-zhirun.yan@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series graph enhancement for multi-core dispatch |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Yan, Zhirun May 9, 2023, 6:03 a.m. UTC 
  Rename rte_graph_work.h to rte_graph_work_common.h for supporting
multiple graph worker model.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Signed-off-by: Cunming Liang <cunming.liang@intel.com>
Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
---
 MAINTAINERS                                                 | 1 +
 lib/graph/graph_pcap.c                                      | 2 +-
 lib/graph/graph_private.h                                   | 2 +-
 lib/graph/meson.build                                       | 2 +-
 lib/graph/{rte_graph_worker.h => rte_graph_worker_common.h} | 6 +++---
 5 files changed, 7 insertions(+), 6 deletions(-)
 rename lib/graph/{rte_graph_worker.h => rte_graph_worker_common.h} (99%)

Comments

Jerin Jacob May 22, 2023, 8:25 a.m. UTC | #1 
On Tue, May 9, 2023 at 11:34 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> Rename rte_graph_work.h to rte_graph_work_common.h for supporting
> multiple graph worker model.


I have requested to check the performance with dpdk-test and l3fwd
graph in last series.
Have you checked the performance? In my testing, there is regression.
Please check the performance with dpdk-test and l3fwd graph, there
should not be any regression in RTC mode.

There is around -300% regression arm64 and x86.
Command to mesure:
./build/app/test/dpdk-test -c 0xf00000 -- graph_perf_autotest

There is around ~-2% regression in l3fwd-graph. I dont think, there
should not be any reason for regression as it is model are separate
header file.
Please check the common header file in fastpath and fix the regression
to accept this series.

./build/examples/dpdk-l3fwd-graph -a 0002:02:00.0 -c 0xc00000  -- -p
0x1 --config="(0, 0, 23)" -P (edited)
Old
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+
|Node                           |calls          |objs
|realloc_count  |objs/call      |objs/sec(10E6) |cycles/call|
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+
|ip4_lookup                     |7282757        |1864385584     |1
         |256.000        |38.704896      |1770.0000  |
|ip4_rewrite                    |7282758        |1864385840     |1
         |256.000        |38.704896      |1431.0000  |
|ethdev_tx-0                    |7282758        |1864385840     |1
         |256.000        |38.704896      |922.0000   |
|ethdev_rx-0-0                  |14882133       |1864386096     |2
         |256.000        |38.704896      |2015.0000  |
|pkt_cls                        |7282760        |1864386352     |1
         |256.000        |38.704896      |392.0000   |
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+


New
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+
|Node                           |calls          |objs
|realloc_count  |objs/call      |objs/sec(10E6) |cycles/call|
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+
|ip4_lookup                     |3002135        |768546560      |2
         |256.000        |38.402048      |1770.0000  |
|ip4_rewrite                    |3002136        |768546816      |1
         |256.000        |38.402048      |1425.0000  |
|ethdev_tx-0                    |3002137        |768547072      |2
         |256.000        |38.402048      |949.0000   |
|ethdev_rx-0-0                  |3002138        |768547328      |2
         |256.000        |38.402048      |1966.0000  |
|pkt_cls                        |3002138        |768547328      |1
         |256.000        |38.402048      |408.0000   |
+-------------------------------+---------------+---------------+---------------+---------------+---------------+-----------+

NAK for this series till the performance issues fixed.



>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> ---

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8df23e5099..cc11328242 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1714,6 +1714,7 @@ F: doc/guides/prog_guide/bpf_lib.rst
>  Graph - EXPERIMENTAL
>  M: Jerin Jacob <jerinj@marvell.com>
>  M: Kiran Kumar K <kirankumark@marvell.com>
> +M: Zhirun Yan <zhirun.yan@intel.com>

Thanks for adding as maintainer.
Since you are at this change. Could you move up "Nithin Dabilpuram
<ndabilpuram@marvell.com>" two lines below and group all together?

>  F: lib/graph/
>  F: doc/guides/prog_guide/graph_lib.rst
>  F: app/test/test_graph*
Yan, Zhirun May 23, 2023, 8:13 a.m. UTC | #2 
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, May 22, 2023 4:26 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; stephen@networkplumber.org;
> pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>
> Subject: Re: [PATCH v6 01/15] graph: rename rte_graph_work as common
> 
> On Tue, May 9, 2023 at 11:34 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
> >
> > Rename rte_graph_work.h to rte_graph_work_common.h for supporting
> > multiple graph worker model.
> 
> 
> I have requested to check the performance with dpdk-test and l3fwd graph in
> last series.
> Have you checked the performance? In my testing, there is regression.
> Please check the performance with dpdk-test and l3fwd graph, there should not
> be any regression in RTC mode.
> 
> There is around -300% regression arm64 and x86.
> Command to mesure:
> ./build/app/test/dpdk-test -c 0xf00000 -- graph_perf_autotest
> 
> There is around ~-2% regression in l3fwd-graph. I dont think, there should not be
> any reason for regression as it is model are separate header file.
> Please check the common header file in fastpath and fix the regression to accept
> this series.
> 
> ./build/examples/dpdk-l3fwd-graph -a 0002:02:00.0 -c 0xc00000  -- -p
> 0x1 --config="(0, 0, 23)" -P (edited)
> Old
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> |Node                           |calls          |objs
> |realloc_count  |objs/call      |objs/sec(10E6) |cycles/call|
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> |ip4_lookup                     |7282757        |1864385584     |1
>          |256.000        |38.704896      |1770.0000  |
> |ip4_rewrite                    |7282758        |1864385840     |1
>          |256.000        |38.704896      |1431.0000  |
> |ethdev_tx-0                    |7282758        |1864385840     |1
>          |256.000        |38.704896      |922.0000   |
> |ethdev_rx-0-0                  |14882133       |1864386096     |2
>          |256.000        |38.704896      |2015.0000  |
> |pkt_cls                        |7282760        |1864386352     |1
>          |256.000        |38.704896      |392.0000   |
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> 
> 
> New
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> |Node                           |calls          |objs
> |realloc_count  |objs/call      |objs/sec(10E6) |cycles/call|
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> |ip4_lookup                     |3002135        |768546560      |2
>          |256.000        |38.402048      |1770.0000  |
> |ip4_rewrite                    |3002136        |768546816      |1
>          |256.000        |38.402048      |1425.0000  |
> |ethdev_tx-0                    |3002137        |768547072      |2
>          |256.000        |38.402048      |949.0000   |
> |ethdev_rx-0-0                  |3002138        |768547328      |2
>          |256.000        |38.402048      |1966.0000  |
> |pkt_cls                        |3002138        |768547328      |1
>          |256.000        |38.402048      |408.0000   |
> +-------------------------------+---------------+---------------+---------------+--------------
> -+---------------+-----------+
> 
> NAK for this series till the performance issues fixed.
> 

The root cause is come from V5->V6, change rte_rdtsc() to rte_rdtsc_precise() in node process in patch 03.

rte_rdtsc_precise() is heavy than rte_rdtsc(). And the graph walk will call __rte_node_process() for each node.

I will revert this change.


> 
> 
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > ---
> 
> > diff --git a/MAINTAINERS b/MAINTAINERS index 8df23e5099..cc11328242
> > 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1714,6 +1714,7 @@ F: doc/guides/prog_guide/bpf_lib.rst  Graph -
> > EXPERIMENTAL
> >  M: Jerin Jacob <jerinj@marvell.com>
> >  M: Kiran Kumar K <kirankumark@marvell.com>
> > +M: Zhirun Yan <zhirun.yan@intel.com>
> 
> Thanks for adding as maintainer.
> Since you are at this change. Could you move up "Nithin Dabilpuram
> <ndabilpuram@marvell.com>" two lines below and group all together?


Yes, I will do in next version.
> 
> >  F: lib/graph/
> >  F: doc/guides/prog_guide/graph_lib.rst
> >  F: app/test/test_graph*
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8df23e5099..cc11328242 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1714,6 +1714,7 @@  F: doc/guides/prog_guide/bpf_lib.rst
 Graph - EXPERIMENTAL
 M: Jerin Jacob <jerinj@marvell.com>
 M: Kiran Kumar K <kirankumark@marvell.com>
+M: Zhirun Yan <zhirun.yan@intel.com>
 F: lib/graph/
 F: doc/guides/prog_guide/graph_lib.rst
 F: app/test/test_graph*
diff --git a/lib/graph/graph_pcap.c b/lib/graph/graph_pcap.c
index 6c43330029..8a220370fa 100644
--- a/lib/graph/graph_pcap.c
+++ b/lib/graph/graph_pcap.c
@@ -10,7 +10,7 @@ 
 #include <rte_mbuf.h>
 #include <rte_pcapng.h>
 
-#include "rte_graph_worker.h"
+#include "rte_graph_worker_common.h"
 
 #include "graph_pcap_private.h"
 
diff --git a/lib/graph/graph_private.h b/lib/graph/graph_private.h
index eacdef45f0..307e5f70bc 100644
--- a/lib/graph/graph_private.h
+++ b/lib/graph/graph_private.h
@@ -13,7 +13,7 @@ 
 #include <rte_spinlock.h>
 
 #include "rte_graph.h"
-#include "rte_graph_worker.h"
+#include "rte_graph_worker_common.h"
 
 extern int rte_graph_logtype;
 
diff --git a/lib/graph/meson.build b/lib/graph/meson.build
index 3526d1b5d4..4e2b612ad3 100644
--- a/lib/graph/meson.build
+++ b/lib/graph/meson.build
@@ -16,6 +16,6 @@  sources = files(
         'graph_populate.c',
         'graph_pcap.c',
 )
-headers = files('rte_graph.h', 'rte_graph_worker.h')
+headers = files('rte_graph.h', 'rte_graph_worker_common.h')
 
 deps += ['eal', 'pcapng']
diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker_common.h
similarity index 99%
rename from lib/graph/rte_graph_worker.h
rename to lib/graph/rte_graph_worker_common.h
index 438595b15c..0bad2938f3 100644
--- a/lib/graph/rte_graph_worker.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -2,8 +2,8 @@ 
  * Copyright(C) 2020 Marvell International Ltd.
  */
 
-#ifndef _RTE_GRAPH_WORKER_H_
-#define _RTE_GRAPH_WORKER_H_
+#ifndef _RTE_GRAPH_WORKER_COMMON_H_
+#define _RTE_GRAPH_WORKER_COMMON_H_
 
 /**
  * @file rte_graph_worker.h
@@ -518,4 +518,4 @@  rte_node_next_stream_move(struct rte_graph *graph, struct rte_node *src,
 }
 #endif
 
-#endif /* _RTE_GRAPH_WORKER_H_ */
+#endif /* _RTE_GRAPH_WORKER_COIMMON_H_ */