mbox series

[v11,00/16] graph enhancement for multi-core dispatch

Message ID 20230608151844.1823783-1-zhirun.yan@intel.com (mailing list archive)
Headers
Series graph enhancement for multi-core dispatch |

Message

Yan, Zhirun June 8, 2023, 3:18 p.m. UTC
  V11:
Update comments and fix to add experimental flags for rte_graph_model_is_valid() in patch 04.
Update added symbols in alphabetical order in version.map with patch 04,05,06,08,10.
Update commit message in patch 16.

V10:
Add rte_graph_worker_model_no_check_get() for fast path, extract rte_graph_model_is_valid()
in patch 04.
Change RTE_ASSERT to return in patch 06.
Change to treat not defined RTE_GRAPH_MODEL_SELECT as runtime pick in patch 13.
Move stats into dispatch union in patch 14.
Change example to align with RTE_GRAPH_MODEL_SELECT scheme in patch 16.
Squash patch 17(doc) into patch 13(prog_guide), 16(example guide).


V9:
Fix CI build issues for doc building(move TAILQ next pointer out of union) in patch 09,10.
Fix graph model check in rte_graph_worker_model_set() in patch 04.
Fix typo in doc.

V8:
No performance dorp for original l3fwd-graph and graph_perf_autotest.

Update graph model set/get functions and add graph_model_is_valid() in patch 04.
Update doc for new scheme usage(choose model in runtime or compile time).
Update dispatch schedule struct into union.
Change enum rte_graph_worker_model to macro define in rte_graph_worker_common.h.
Add model clone in graph_clone() in patch 08.
Remove unnecessary inline for slow path func graph_src_node_avail() in patch 06.


V7:
Revert rte_rdtsc_precise() in fastpath to fix performance issues in patch 03.
Introduce new scheme for model choosing. Use RTE_GRAPH_MODEL_SELECT to choose in
  compile-time in patch 13, 15.(must have rte_graph_worker_model_set() to help
  other config func to do model specific things like alloc wq, collect stats)
Extract the common func clone_name() into graph_private.h for graph/node clone in
  patch 07.(new patch)
Use rte_graph->model in rte_graph_worker_model_set() instead of RTE_PER_LCORE_*.
Add test case for all new APIs in patch 16(new patch).
Remove *_END line in enum rte_graph_worker_model in patch 04.
Add model check for graph lcore binding.
Rename workqueue as graph_mcore_dispatch_wq_node in patch 09.
Change all new model files/APIs with prefix _mcore_dispatch_.
Change description of new API, comments of func/structure to explicitly mention for
  mcore dispatch model only. Add Doxygen comments.
Update l3fwd-graph with new scheme, Update doc.
Update MAINTAINERS.
Fix typo and format issues.

V6:
Change rte_rdtsc() to rte_rdtsc_precise().
Add union in rte_graph_param to configure models.
Remove memset in fastpath, add RTE_ASSERT for cloned graph.
Update copyright in patch 02.
Update l3fwd-graph node affinity, start from rx core successively.

V5:
Fix CI build issues about dynamically update doc.

V4:
Fix CI build issues about undefined reference of sched apis.
Remove inline for model setting.

V3:
Fix CI build issues about TLS and typo.

V2:
Use git mv to keep git history.
Use TLS for per-thread local storage.
Change model name to mcore dispatch.
Change API with specific mode name.
Split big patch.
Fix CI issues.
Rebase l3fwd-graph example.
Update doc and maintainers files.


Currently, rte_graph supports RTC (Run-To-Completion) model within each
of a single core.
RTC is one of the typical model of packet processing. Others like
Pipeline or Hybrid are lack of support.

The patch set introduces a 'multicore dispatch' model selection which
is a self-reacting scheme according to the core affinity.
The new model enables a cross-core dispatching mechanism which employs a
scheduling work-queue to dispatch streams to other worker cores which
being associated with the destination node. When core flavor of the
destination node is a default 'current', the stream can be continue
executed as normal.

Example:
3-node graph targets 3-core budget

RTC:
Graph: node-0 -> node-1 -> node-2 @Core0.

+ - - - - - - - - - - - - - - - - - - - - - +
'                Core #0/1/2                '
'                                           '
' +--------+     +---------+     +--------+ '
' | Node-0 | --> | Node-1  | --> | Node-2 | '
' +--------+     +---------+     +--------+ '
'                                           '
+ - - - - - - - - - - - - - - - - - - - - - +

Dispatch:

Graph topo: node-0 -> Core1; node-1 -> node-2; node-2 -> node-3.
Config graph: node-0 @Core0; node-1/3 @Core1; node-2 @Core2.

.. code-block:: diff

    + - - - - - -+     +- - - - - - - - - - - - - +     + - - - - - -+
    '  Core #0   '     '          Core #1         '     '  Core #2   '
    '            '     '                          '     '            '
    ' +--------+ '     ' +--------+    +--------+ '     ' +--------+ '
    ' | Node-0 | - - - ->| Node-1 |    | Node-3 |<- - - - | Node-2 | '
    ' +--------+ '     ' +--------+    +--------+ '     ' +--------+ '
    '            '     '     |                    '     '      ^     '
    + - - - - - -+     +- - -|- - - - - - - - - - +     + - - -|- - -+
                             |                                 |
                             + - - - - - - - - - - - - - - - - +


The patch set has been break down as below:

1. Split graph worker into common and default model part.
2. Inline graph node processing to make it reusable.
3. Add set/get APIs to choose worker model.
4. Introduce core affinity API to set the node run on specific worker core.
  (only use in new model)
5. Introduce graph affinity API to bind one graph with specific worker
  core.
6. Introduce graph clone API.
7. Introduce stream moving with scheduler work-queue in patch 8~12.
8. Add stats for new models.
9. Abstract default graph config process and integrate new model into
  example/l3fwd-graph. Add new parameters for model choosing.

We could run with new worker model by this:
./dpdk-l3fwd-graph -l 8,9,10,11 -n 4 -- -p 0x1 --config="(0,0,9)" -P
--model="dispatch"

References:
https://static.sched.com/hosted_files/dpdkuserspace22/a6/graph%20introduce%20remote%20dispatch%20for%20mult-core%20scaling.pdf


Zhirun Yan (16):
  graph: rename rte_graph_work as common
  graph: split graph worker into common and default model
  graph: move node process into inline function
  graph: add get/set graph worker model APIs
  graph: introduce graph node core affinity API
  graph: introduce graph bind unbind API
  graph: move node clone name func into private as common
  graph: introduce graph clone API for other worker core
  graph: add structure for stream moving between cores
  graph: introduce stream moving cross cores
  graph: enable create and destroy graph scheduling workqueue
  graph: introduce graph walk by cross-core dispatch
  graph: enable graph multicore dispatch scheduler model
  graph: add stats for mcore dispatch model
  test/graph: add functional tests for mcore dispatch model
  examples/l3fwd-graph: introduce mcore dispatch worker model

 MAINTAINERS                                   |   3 +-
 app/test/test_graph.c                         | 130 ++++
 doc/guides/prog_guide/graph_lib.rst           |  71 ++-
 doc/guides/sample_app_ug/l3_forward_graph.rst |  16 +
 examples/l3fwd-graph/main.c                   | 230 +++++--
 lib/graph/graph.c                             | 161 +++++
 lib/graph/graph_debug.c                       |   6 +
 lib/graph/graph_populate.c                    |   1 +
 lib/graph/graph_private.h                     |  90 +++
 lib/graph/graph_stats.c                       |  76 ++-
 lib/graph/meson.build                         |   4 +-
 lib/graph/node.c                              |  27 +-
 lib/graph/rte_graph.h                         |  65 ++
 lib/graph/rte_graph_model_mcore_dispatch.c    | 191 ++++++
 lib/graph/rte_graph_model_mcore_dispatch.h    | 134 ++++
 lib/graph/rte_graph_model_rtc.h               |  46 ++
 lib/graph/rte_graph_worker.c                  |  39 ++
 lib/graph/rte_graph_worker.h                  | 503 +--------------
 lib/graph/rte_graph_worker_common.h           | 598 ++++++++++++++++++
 lib/graph/version.map                         |  11 +
 20 files changed, 1834 insertions(+), 568 deletions(-)
 create mode 100644 lib/graph/rte_graph_model_mcore_dispatch.c
 create mode 100644 lib/graph/rte_graph_model_mcore_dispatch.h
 create mode 100644 lib/graph/rte_graph_model_rtc.h
 create mode 100644 lib/graph/rte_graph_worker.c
 create mode 100644 lib/graph/rte_graph_worker_common.h
  

Comments

Jerin Jacob June 8, 2023, 3:30 p.m. UTC | #1
On Thu, Jun 8, 2023 at 8:55 PM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> V11:
> Update comments and fix to add experimental flags for rte_graph_model_is_valid() in patch 04.
> Update added symbols in alphabetical order in version.map with patch 04,05,06,08,10.
> Update commit message in patch 16.

+ @Thomas Monjalon  This patch series looks good from my PoV to merge
this in rc1. If you don't find any issue, please consider merging in
this rc1.


>
> V10:
> Add rte_graph_worker_model_no_check_get() for fast path, extract rte_graph_model_is_valid()
> in patch 04.
> Change RTE_ASSERT to return in patch 06.
> Change to treat not defined RTE_GRAPH_MODEL_SELECT as runtime pick in patch 13.
> Move stats into dispatch union in patch 14.
> Change example to align with RTE_GRAPH_MODEL_SELECT scheme in patch 16.
> Squash patch 17(doc) into patch 13(prog_guide), 16(example guide).
>
>
> V9:
> Fix CI build issues for doc building(move TAILQ next pointer out of union) in patch 09,10.
> Fix graph model check in rte_graph_worker_model_set() in patch 04.
> Fix typo in doc.
>
> V8:
> No performance dorp for original l3fwd-graph and graph_perf_autotest.
>
> Update graph model set/get functions and add graph_model_is_valid() in patch 04.
> Update doc for new scheme usage(choose model in runtime or compile time).
> Update dispatch schedule struct into union.
> Change enum rte_graph_worker_model to macro define in rte_graph_worker_common.h.
> Add model clone in graph_clone() in patch 08.
> Remove unnecessary inline for slow path func graph_src_node_avail() in patch 06.
>
>
> V7:
> Revert rte_rdtsc_precise() in fastpath to fix performance issues in patch 03.
> Introduce new scheme for model choosing. Use RTE_GRAPH_MODEL_SELECT to choose in
>   compile-time in patch 13, 15.(must have rte_graph_worker_model_set() to help
>   other config func to do model specific things like alloc wq, collect stats)
> Extract the common func clone_name() into graph_private.h for graph/node clone in
>   patch 07.(new patch)
> Use rte_graph->model in rte_graph_worker_model_set() instead of RTE_PER_LCORE_*.
> Add test case for all new APIs in patch 16(new patch).
> Remove *_END line in enum rte_graph_worker_model in patch 04.
> Add model check for graph lcore binding.
> Rename workqueue as graph_mcore_dispatch_wq_node in patch 09.
> Change all new model files/APIs with prefix _mcore_dispatch_.
> Change description of new API, comments of func/structure to explicitly mention for
>   mcore dispatch model only. Add Doxygen comments.
> Update l3fwd-graph with new scheme, Update doc.
> Update MAINTAINERS.
> Fix typo and format issues.
>
> V6:
> Change rte_rdtsc() to rte_rdtsc_precise().
> Add union in rte_graph_param to configure models.
> Remove memset in fastpath, add RTE_ASSERT for cloned graph.
> Update copyright in patch 02.
> Update l3fwd-graph node affinity, start from rx core successively.
>
> V5:
> Fix CI build issues about dynamically update doc.
>
> V4:
> Fix CI build issues about undefined reference of sched apis.
> Remove inline for model setting.
>
> V3:
> Fix CI build issues about TLS and typo.
>
> V2:
> Use git mv to keep git history.
> Use TLS for per-thread local storage.
> Change model name to mcore dispatch.
> Change API with specific mode name.
> Split big patch.
> Fix CI issues.
> Rebase l3fwd-graph example.
> Update doc and maintainers files.
>
>
> Currently, rte_graph supports RTC (Run-To-Completion) model within each
> of a single core.
> RTC is one of the typical model of packet processing. Others like
> Pipeline or Hybrid are lack of support.
>
> The patch set introduces a 'multicore dispatch' model selection which
> is a self-reacting scheme according to the core affinity.
> The new model enables a cross-core dispatching mechanism which employs a
> scheduling work-queue to dispatch streams to other worker cores which
> being associated with the destination node. When core flavor of the
> destination node is a default 'current', the stream can be continue
> executed as normal.
>
> Example:
> 3-node graph targets 3-core budget
>
> RTC:
> Graph: node-0 -> node-1 -> node-2 @Core0.
>
> + - - - - - - - - - - - - - - - - - - - - - +
> '                Core #0/1/2                '
> '                                           '
> ' +--------+     +---------+     +--------+ '
> ' | Node-0 | --> | Node-1  | --> | Node-2 | '
> ' +--------+     +---------+     +--------+ '
> '                                           '
> + - - - - - - - - - - - - - - - - - - - - - +
>
> Dispatch:
>
> Graph topo: node-0 -> Core1; node-1 -> node-2; node-2 -> node-3.
> Config graph: node-0 @Core0; node-1/3 @Core1; node-2 @Core2.
>
> .. code-block:: diff
>
>     + - - - - - -+     +- - - - - - - - - - - - - +     + - - - - - -+
>     '  Core #0   '     '          Core #1         '     '  Core #2   '
>     '            '     '                          '     '            '
>     ' +--------+ '     ' +--------+    +--------+ '     ' +--------+ '
>     ' | Node-0 | - - - ->| Node-1 |    | Node-3 |<- - - - | Node-2 | '
>     ' +--------+ '     ' +--------+    +--------+ '     ' +--------+ '
>     '            '     '     |                    '     '      ^     '
>     + - - - - - -+     +- - -|- - - - - - - - - - +     + - - -|- - -+
>                              |                                 |
>                              + - - - - - - - - - - - - - - - - +
>
>
> The patch set has been break down as below:
>
> 1. Split graph worker into common and default model part.
> 2. Inline graph node processing to make it reusable.
> 3. Add set/get APIs to choose worker model.
> 4. Introduce core affinity API to set the node run on specific worker core.
>   (only use in new model)
> 5. Introduce graph affinity API to bind one graph with specific worker
>   core.
> 6. Introduce graph clone API.
> 7. Introduce stream moving with scheduler work-queue in patch 8~12.
> 8. Add stats for new models.
> 9. Abstract default graph config process and integrate new model into
>   example/l3fwd-graph. Add new parameters for model choosing.
>
> We could run with new worker model by this:
> ./dpdk-l3fwd-graph -l 8,9,10,11 -n 4 -- -p 0x1 --config="(0,0,9)" -P
> --model="dispatch"
>
> References:
> https://static.sched.com/hosted_files/dpdkuserspace22/a6/graph%20introduce%20remote%20dispatch%20for%20mult-core%20scaling.pdf
>
>
> Zhirun Yan (16):
>   graph: rename rte_graph_work as common
>   graph: split graph worker into common and default model
>   graph: move node process into inline function
>   graph: add get/set graph worker model APIs
>   graph: introduce graph node core affinity API
>   graph: introduce graph bind unbind API
>   graph: move node clone name func into private as common
>   graph: introduce graph clone API for other worker core
>   graph: add structure for stream moving between cores
>   graph: introduce stream moving cross cores
>   graph: enable create and destroy graph scheduling workqueue
>   graph: introduce graph walk by cross-core dispatch
>   graph: enable graph multicore dispatch scheduler model
>   graph: add stats for mcore dispatch model
>   test/graph: add functional tests for mcore dispatch model
>   examples/l3fwd-graph: introduce mcore dispatch worker model
>
>  MAINTAINERS                                   |   3 +-
>  app/test/test_graph.c                         | 130 ++++
>  doc/guides/prog_guide/graph_lib.rst           |  71 ++-
>  doc/guides/sample_app_ug/l3_forward_graph.rst |  16 +
>  examples/l3fwd-graph/main.c                   | 230 +++++--
>  lib/graph/graph.c                             | 161 +++++
>  lib/graph/graph_debug.c                       |   6 +
>  lib/graph/graph_populate.c                    |   1 +
>  lib/graph/graph_private.h                     |  90 +++
>  lib/graph/graph_stats.c                       |  76 ++-
>  lib/graph/meson.build                         |   4 +-
>  lib/graph/node.c                              |  27 +-
>  lib/graph/rte_graph.h                         |  65 ++
>  lib/graph/rte_graph_model_mcore_dispatch.c    | 191 ++++++
>  lib/graph/rte_graph_model_mcore_dispatch.h    | 134 ++++
>  lib/graph/rte_graph_model_rtc.h               |  46 ++
>  lib/graph/rte_graph_worker.c                  |  39 ++
>  lib/graph/rte_graph_worker.h                  | 503 +--------------
>  lib/graph/rte_graph_worker_common.h           | 598 ++++++++++++++++++
>  lib/graph/version.map                         |  11 +
>  20 files changed, 1834 insertions(+), 568 deletions(-)
>  create mode 100644 lib/graph/rte_graph_model_mcore_dispatch.c
>  create mode 100644 lib/graph/rte_graph_model_mcore_dispatch.h
>  create mode 100644 lib/graph/rte_graph_model_rtc.h
>  create mode 100644 lib/graph/rte_graph_worker.c
>  create mode 100644 lib/graph/rte_graph_worker_common.h
>
> --
> 2.37.2
>
  
David Marchand June 9, 2023, 1:39 p.m. UTC | #2
On Thu, Jun 8, 2023 at 5:31 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> On Thu, Jun 8, 2023 at 8:55 PM Zhirun Yan <zhirun.yan@intel.com> wrote:
> >
> > V11:
> > Update comments and fix to add experimental flags for rte_graph_model_is_valid() in patch 04.
> > Update added symbols in alphabetical order in version.map with patch 04,05,06,08,10.
> > Update commit message in patch 16.
>
> + @Thomas Monjalon  This patch series looks good from my PoV to merge
> this in rc1. If you don't find any issue, please consider merging in
> this rc1.

Compilation is broken at patch 1 because of the file rename...
I hope I won't find anything else broken.
  
David Marchand June 9, 2023, 2:36 p.m. UTC | #3
On Fri, Jun 9, 2023 at 3:39 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Thu, Jun 8, 2023 at 5:31 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > On Thu, Jun 8, 2023 at 8:55 PM Zhirun Yan <zhirun.yan@intel.com> wrote:
> > >
> > > V11:
> > > Update comments and fix to add experimental flags for rte_graph_model_is_valid() in patch 04.
> > > Update added symbols in alphabetical order in version.map with patch 04,05,06,08,10.
> > > Update commit message in patch 16.
> >
> > + @Thomas Monjalon  This patch series looks good from my PoV to merge
> > this in rc1. If you don't find any issue, please consider merging in
> > this rc1.
>
> Compilation is broken at patch 1 because of the file rename...
> I hope I won't find anything else broken.

Afaics, header exports are incorrect. I did not look further.

About headers exports: if a public header (declared in headers meson
var) includes sub_headerA,B,C, those sub headers must be listed either
in headers or indirect_headers meson variables.
Otherwise those sub headers won't be distributed for external
applications consumption.

For checking, I recommend running:
git rebase -i origin/main -x 'DPDK_ABI_REF_VERSION=v23.03
DPDK_BUILD_TEST_EXAMPLES=all DPDK_BUILD_TEST_DIR=$HOME/builds/main
./devtools/test-meson-builds.sh'


If we don't get fixes soon, this series will have to wait rc2 (or next release).
  
Yan, Zhirun June 9, 2023, 3:47 p.m. UTC | #4
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, June 9, 2023 10:37 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>
> Cc: Yan, Zhirun <zhirun.yan@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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>;
> mattias.ronnblom <mattias.ronnblom@ericsson.com>
> Subject: Re: [PATCH v11 00/16] graph enhancement for multi-core dispatch
> 
> On Fri, Jun 9, 2023 at 3:39 PM David Marchand <david.marchand@redhat.com>
> wrote:
> >
> > On Thu, Jun 8, 2023 at 5:31 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Thu, Jun 8, 2023 at 8:55 PM Zhirun Yan <zhirun.yan@intel.com> wrote:
> > > >
> > > > V11:
> > > > Update comments and fix to add experimental flags for
> rte_graph_model_is_valid() in patch 04.
> > > > Update added symbols in alphabetical order in version.map with patch
> 04,05,06,08,10.
> > > > Update commit message in patch 16.
> > >
> > > + @Thomas Monjalon  This patch series looks good from my PoV to
> > > + merge
> > > this in rc1. If you don't find any issue, please consider merging in
> > > this rc1.
> >
> > Compilation is broken at patch 1 because of the file rename...
> > I hope I won't find anything else broken.
> 
> Afaics, header exports are incorrect. I did not look further.
> 
> About headers exports: if a public header (declared in headers meson
> var) includes sub_headerA,B,C, those sub headers must be listed either in
> headers or indirect_headers meson variables.
> Otherwise those sub headers won't be distributed for external applications
> consumption.


The changed public header is used by lib/node/*, l3fwd-graph and graph-test. 

Actually the first patch rename the public header, and then 2nd patch change it back.
It caused the broken at 1.

Patch 01 and 02 are split as 2 patches because I want to retain more git history and make
git log clean.
I think it could be fixed by changing the used public header to the new name in patch 01,
and change them back in patch 02. Is that a good way?

> 
> For checking, I recommend running:
> git rebase -i origin/main -x 'DPDK_ABI_REF_VERSION=v23.03
> DPDK_BUILD_TEST_EXAMPLES=all DPDK_BUILD_TEST_DIR=$HOME/builds/main
> ./devtools/test-meson-builds.sh'
> 
> 
> If we don't get fixes soon, this series will have to wait rc2 (or next release).
> 
> 
> --
> David Marchand
  
David Marchand June 12, 2023, 2:55 p.m. UTC | #5
On Fri, Jun 9, 2023 at 5:48 PM Yan, Zhirun <zhirun.yan@intel.com> wrote:
> > > Compilation is broken at patch 1 because of the file rename...
> > > I hope I won't find anything else broken.
> >
> > Afaics, header exports are incorrect. I did not look further.
> >
> > About headers exports: if a public header (declared in headers meson
> > var) includes sub_headerA,B,C, those sub headers must be listed either in
> > headers or indirect_headers meson variables.
> > Otherwise those sub headers won't be distributed for external applications
> > consumption.
>
>
> The changed public header is used by lib/node/*, l3fwd-graph and graph-test.
>
> Actually the first patch rename the public header, and then 2nd patch change it back.
> It caused the broken at 1.
>
> Patch 01 and 02 are split as 2 patches because I want to retain more git history and make
> git log clean.
> I think it could be fixed by changing the used public header to the new name in patch 01,
> and change them back in patch 02. Is that a good way?

Another option was to keep a (almost empty) rte_graph_worker.h header
in patch 1 that includes the new _common header.
Then update in patch 2, etc...


> >
> > For checking, I recommend running:
> > git rebase -i origin/main -x 'DPDK_ABI_REF_VERSION=v23.03
> > DPDK_BUILD_TEST_EXAMPLES=all DPDK_BUILD_TEST_DIR=$HOME/builds/main
> > ./devtools/test-meson-builds.sh'

However, the v12 revision that got posted does not fix the other point
I reported.

If you split a public header, *all* sub headers must be exported as
public headers too.
I wrote some new test for the CI, and put the v12 series on top of it.
As expected, inclusion of the graph header is broken out of dpdk.
See the graph example compilation:
https://github.com/david-marchand/dpdk/actions/runs/5244846217/jobs/9471420118#step:18:12092
  
Yan, Zhirun June 13, 2023, 8:06 a.m. UTC | #6
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, June 12, 2023 10:56 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: Jerin Jacob <jerinjacobk@gmail.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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>;
> mattias.ronnblom <mattias.ronnblom@ericsson.com>
> Subject: Re: [PATCH v11 00/16] graph enhancement for multi-core dispatch
> 
> On Fri, Jun 9, 2023 at 5:48 PM Yan, Zhirun <zhirun.yan@intel.com> wrote:
> > > > Compilation is broken at patch 1 because of the file rename...
> > > > I hope I won't find anything else broken.
> > >
> > > Afaics, header exports are incorrect. I did not look further.
> > >
> > > About headers exports: if a public header (declared in headers meson
> > > var) includes sub_headerA,B,C, those sub headers must be listed
> > > either in headers or indirect_headers meson variables.
> > > Otherwise those sub headers won't be distributed for external
> > > applications consumption.
> >
> >
> > The changed public header is used by lib/node/*, l3fwd-graph and graph-test.
> >
> > Actually the first patch rename the public header, and then 2nd patch change it
> back.
> > It caused the broken at 1.
> >
> > Patch 01 and 02 are split as 2 patches because I want to retain more
> > git history and make git log clean.
> > I think it could be fixed by changing the used public header to the
> > new name in patch 01, and change them back in patch 02. Is that a good way?
> 
> Another option was to keep a (almost empty) rte_graph_worker.h header in
> patch 1 that includes the new _common header.
> Then update in patch 2, etc...
> 
Thanks. 
Actually, it will cause the patch too big, the git log will have much del/add, it is also not friendly for review. So it may be more clear to rename it to fix.
> 
> > >
> > > For checking, I recommend running:
> > > git rebase -i origin/main -x 'DPDK_ABI_REF_VERSION=v23.03
> > > DPDK_BUILD_TEST_EXAMPLES=all
> DPDK_BUILD_TEST_DIR=$HOME/builds/main
> > > ./devtools/test-meson-builds.sh'
> 
> However, the v12 revision that got posted does not fix the other point I reported.
> 
> If you split a public header, *all* sub headers must be exported as public headers
> too.
> I wrote some new test for the CI, and put the v12 series on top of it.
> As expected, inclusion of the graph header is broken out of dpdk.
> See the graph example compilation:
> https://github.com/david-
> marchand/dpdk/actions/runs/5244846217/jobs/9471420118#step:18:12092
> 

Got it.  All sub header must be exported as public. There are some static functions in sub header.
I will list all sub header to export. Please see the next version. Thanks.
> 
> --
> David Marchand