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

[Yan, Zhirun]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk at gmail.com>
> Sent: Monday, May 22, 2023 4:26 PM
> To: Yan, Zhirun <zhirun.yan at intel.com>
> Cc: dev at dpdk.org; jerinj at marvell.com; kirankumark at marvell.com;
> ndabilpuram at marvell.com; stephen at networkplumber.org;
> pbhagavatula at marvell.com; Liang, Cunming <cunming.liang at intel.com>; Wang,
> Haiyue <haiyue.wang at 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 at 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 at intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> > Signed-off-by: Zhirun Yan <zhirun.yan at 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 at marvell.com>
> >  M: Kiran Kumar K <kirankumark at marvell.com>
> > +M: Zhirun Yan <zhirun.yan at intel.com>
> 
> Thanks for adding as maintainer.
> Since you are at this change. Could you move up "Nithin Dabilpuram
> <ndabilpuram at 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*