mbox series

[v5,0/2] Add config file support for l3fwd

Message ID 20220204195905.449192-1-sean.morrissey@intel.com (mailing list archive)
Headers
Series Add config file support for l3fwd |

Message

Sean Morrissey Feb. 4, 2022, 7:59 p.m. UTC
  This patchset introduces config file support for l3fwd
and its lookup methods LPM, FIB, and EM, similar to
that of l3fwd-acl. This allows for route rules to be
defined in configuration files and edited there instead
of in each of the lookup methods hardcoded route tables.

V4:
* Fix nondeterministic bug of segfault on termination of
  sample app.
V5:
* Reintroduce hardcoded tables as to not break dts and
  allow for hardcoded tables to be used if no config
  files presented.

Sean Morrissey (2):
  examples/l3fwd: add config file support for LPM/FIB
  examples/l3fwd: add config file support for EM

 doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
 examples/l3fwd/em_default_v4.cfg        |  17 +
 examples/l3fwd/em_default_v6.cfg        |  17 +
 examples/l3fwd/l3fwd.h                  |  41 +++
 examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
 examples/l3fwd/l3fwd_fib.c              |  50 +--
 examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
 examples/l3fwd/l3fwd_route.h            |  41 +++
 examples/l3fwd/lpm_default_v4.cfg       |  17 +
 examples/l3fwd/lpm_default_v6.cfg       |  17 +
 examples/l3fwd/main.c                   |  68 +++-
 11 files changed, 949 insertions(+), 194 deletions(-)
 create mode 100644 examples/l3fwd/em_default_v4.cfg
 create mode 100644 examples/l3fwd/em_default_v6.cfg
 create mode 100644 examples/l3fwd/lpm_default_v4.cfg
 create mode 100644 examples/l3fwd/lpm_default_v6.cfg
  

Comments

Stephen Hemminger Feb. 4, 2022, 10:26 p.m. UTC | #1
On Fri,  4 Feb 2022 19:59:03 +0000
Sean Morrissey <sean.morrissey@intel.com> wrote:

> This patchset introduces config file support for l3fwd
> and its lookup methods LPM, FIB, and EM, similar to
> that of l3fwd-acl. This allows for route rules to be
> defined in configuration files and edited there instead
> of in each of the lookup methods hardcoded route tables.
> 
> V4:
> * Fix nondeterministic bug of segfault on termination of
>   sample app.
> V5:
> * Reintroduce hardcoded tables as to not break dts and
>   allow for hardcoded tables to be used if no config
>   files presented.
> 
> Sean Morrissey (2):
>   examples/l3fwd: add config file support for LPM/FIB
>   examples/l3fwd: add config file support for EM
> 
>  doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
>  examples/l3fwd/em_default_v4.cfg        |  17 +
>  examples/l3fwd/em_default_v6.cfg        |  17 +
>  examples/l3fwd/l3fwd.h                  |  41 +++
>  examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
>  examples/l3fwd/l3fwd_fib.c              |  50 +--
>  examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
>  examples/l3fwd/l3fwd_route.h            |  41 +++
>  examples/l3fwd/lpm_default_v4.cfg       |  17 +
>  examples/l3fwd/lpm_default_v6.cfg       |  17 +
>  examples/l3fwd/main.c                   |  68 +++-
>  11 files changed, 949 insertions(+), 194 deletions(-)
>  create mode 100644 examples/l3fwd/em_default_v4.cfg
>  create mode 100644 examples/l3fwd/em_default_v6.cfg
>  create mode 100644 examples/l3fwd/lpm_default_v4.cfg
>  create mode 100644 examples/l3fwd/lpm_default_v6.cfg
> 

Why not use the DPDK cfgfile library and format?
It is model after standard INI format.
  
Ananyev, Konstantin Feb. 6, 2022, 3:16 p.m. UTC | #2
> > This patchset introduces config file support for l3fwd
> > and its lookup methods LPM, FIB, and EM, similar to
> > that of l3fwd-acl. This allows for route rules to be
> > defined in configuration files and edited there instead
> > of in each of the lookup methods hardcoded route tables.
> >
> > V4:
> > * Fix nondeterministic bug of segfault on termination of
> >   sample app.
> > V5:
> > * Reintroduce hardcoded tables as to not break dts and
> >   allow for hardcoded tables to be used if no config
> >   files presented.
> >
> > Sean Morrissey (2):
> >   examples/l3fwd: add config file support for LPM/FIB
> >   examples/l3fwd: add config file support for EM
> >
> >  doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
> >  examples/l3fwd/em_default_v4.cfg        |  17 +
> >  examples/l3fwd/em_default_v6.cfg        |  17 +
> >  examples/l3fwd/l3fwd.h                  |  41 +++
> >  examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
> >  examples/l3fwd/l3fwd_fib.c              |  50 +--
> >  examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
> >  examples/l3fwd/l3fwd_route.h            |  41 +++
> >  examples/l3fwd/lpm_default_v4.cfg       |  17 +
> >  examples/l3fwd/lpm_default_v6.cfg       |  17 +
> >  examples/l3fwd/main.c                   |  68 +++-
> >  11 files changed, 949 insertions(+), 194 deletions(-)
> >  create mode 100644 examples/l3fwd/em_default_v4.cfg
> >  create mode 100644 examples/l3fwd/em_default_v6.cfg
> >  create mode 100644 examples/l3fwd/lpm_default_v4.cfg
> >  create mode 100644 examples/l3fwd/lpm_default_v6.cfg
> >
> 
> Why not use the DPDK cfgfile library and format?
> It is model after standard INI format.

It is probably some sort of misunderstanding:
This patch doesn't add configuration file for some l3fwd run-time parameters
(number of ports/queues, queue/cpu mappings, etc.).
It allows user to specify he's own routing table instead of hard-coded ones.
For routing table .ini file format is not really suitable.
Instead we follow format similar to what is used in other DPDK apps
(l3fwd-acl, ipsec-secgw, test-acl, test-fib,  test-sad, etc.) for these purposes:
list of route entries, each entry occupies exactly one line.
As an example:
/examples/l3fwd/lpm_default_v4.cfg
#Copy of hard-coded IPv4 FWD table for L3FWD LPM
R198.18.0.0/24 0
R198.18.1.0/24 1
R198.18.2.0/24 2
R198.18.3.0/24 3
....
I suppose it is self-explanatory, intuitive and close enough 
to what user used for with unix-like route config tools.
Konstantin
  
Stephen Hemminger Feb. 8, 2022, 3:04 a.m. UTC | #3
On Sun, 6 Feb 2022 15:16:22 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> > > This patchset introduces config file support for l3fwd
> > > and its lookup methods LPM, FIB, and EM, similar to
> > > that of l3fwd-acl. This allows for route rules to be
> > > defined in configuration files and edited there instead
> > > of in each of the lookup methods hardcoded route tables.
> > >
> > > V4:
> > > * Fix nondeterministic bug of segfault on termination of
> > >   sample app.
> > > V5:
> > > * Reintroduce hardcoded tables as to not break dts and
> > >   allow for hardcoded tables to be used if no config
> > >   files presented.
> > >
> > > Sean Morrissey (2):
> > >   examples/l3fwd: add config file support for LPM/FIB
> > >   examples/l3fwd: add config file support for EM
> > >
> > >  doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
> > >  examples/l3fwd/em_default_v4.cfg        |  17 +
> > >  examples/l3fwd/em_default_v6.cfg        |  17 +
> > >  examples/l3fwd/l3fwd.h                  |  41 +++
> > >  examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
> > >  examples/l3fwd/l3fwd_fib.c              |  50 +--
> > >  examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
> > >  examples/l3fwd/l3fwd_route.h            |  41 +++
> > >  examples/l3fwd/lpm_default_v4.cfg       |  17 +
> > >  examples/l3fwd/lpm_default_v6.cfg       |  17 +
> > >  examples/l3fwd/main.c                   |  68 +++-
> > >  11 files changed, 949 insertions(+), 194 deletions(-)
> > >  create mode 100644 examples/l3fwd/em_default_v4.cfg
> > >  create mode 100644 examples/l3fwd/em_default_v6.cfg
> > >  create mode 100644 examples/l3fwd/lpm_default_v4.cfg
> > >  create mode 100644 examples/l3fwd/lpm_default_v6.cfg
> > >  
> > 
> > Why not use the DPDK cfgfile library and format?
> > It is model after standard INI format.  
> 
> It is probably some sort of misunderstanding:
> This patch doesn't add configuration file for some l3fwd run-time parameters
> (number of ports/queues, queue/cpu mappings, etc.).
> It allows user to specify he's own routing table instead of hard-coded ones.
> For routing table .ini file format is not really suitable.
> Instead we follow format similar to what is used in other DPDK apps
> (l3fwd-acl, ipsec-secgw, test-acl, test-fib,  test-sad, etc.) for these purposes:
> list of route entries, each entry occupies exactly one line.
> As an example:
> /examples/l3fwd/lpm_default_v4.cfg
> #Copy of hard-coded IPv4 FWD table for L3FWD LPM
> R198.18.0.0/24 0
> R198.18.1.0/24 1
> R198.18.2.0/24 2
> R198.18.3.0/24 3
> ....
> I suppose it is self-explanatory, intuitive and close enough 
> to what user used for with unix-like route config tools.
> Konstantin

I was think either, use existing cfgfile and a a section of LPM
so that it could be an example and also have some generic code for handling
prefix entries.

Or have a generic library for reading LPM entries.  L3fwd is supposed
to be as small as possible (it no longer is), and the real work should
be done by libraries to make it easier to build other applications.
  
Ananyev, Konstantin Feb. 8, 2022, 10:44 a.m. UTC | #4
> > > > This patchset introduces config file support for l3fwd
> > > > and its lookup methods LPM, FIB, and EM, similar to
> > > > that of l3fwd-acl. This allows for route rules to be
> > > > defined in configuration files and edited there instead
> > > > of in each of the lookup methods hardcoded route tables.
> > > >
> > > > V4:
> > > > * Fix nondeterministic bug of segfault on termination of
> > > >   sample app.
> > > > V5:
> > > > * Reintroduce hardcoded tables as to not break dts and
> > > >   allow for hardcoded tables to be used if no config
> > > >   files presented.
> > > >
> > > > Sean Morrissey (2):
> > > >   examples/l3fwd: add config file support for LPM/FIB
> > > >   examples/l3fwd: add config file support for EM
> > > >
> > > >  doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
> > > >  examples/l3fwd/em_default_v4.cfg        |  17 +
> > > >  examples/l3fwd/em_default_v6.cfg        |  17 +
> > > >  examples/l3fwd/l3fwd.h                  |  41 +++
> > > >  examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
> > > >  examples/l3fwd/l3fwd_fib.c              |  50 +--
> > > >  examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
> > > >  examples/l3fwd/l3fwd_route.h            |  41 +++
> > > >  examples/l3fwd/lpm_default_v4.cfg       |  17 +
> > > >  examples/l3fwd/lpm_default_v6.cfg       |  17 +
> > > >  examples/l3fwd/main.c                   |  68 +++-
> > > >  11 files changed, 949 insertions(+), 194 deletions(-)
> > > >  create mode 100644 examples/l3fwd/em_default_v4.cfg
> > > >  create mode 100644 examples/l3fwd/em_default_v6.cfg
> > > >  create mode 100644 examples/l3fwd/lpm_default_v4.cfg
> > > >  create mode 100644 examples/l3fwd/lpm_default_v6.cfg
> > > >
> > >
> > > Why not use the DPDK cfgfile library and format?
> > > It is model after standard INI format.
> >
> > It is probably some sort of misunderstanding:
> > This patch doesn't add configuration file for some l3fwd run-time parameters
> > (number of ports/queues, queue/cpu mappings, etc.).
> > It allows user to specify he's own routing table instead of hard-coded ones.
> > For routing table .ini file format is not really suitable.
> > Instead we follow format similar to what is used in other DPDK apps
> > (l3fwd-acl, ipsec-secgw, test-acl, test-fib,  test-sad, etc.) for these purposes:
> > list of route entries, each entry occupies exactly one line.
> > As an example:
> > /examples/l3fwd/lpm_default_v4.cfg
> > #Copy of hard-coded IPv4 FWD table for L3FWD LPM
> > R198.18.0.0/24 0
> > R198.18.1.0/24 1
> > R198.18.2.0/24 2
> > R198.18.3.0/24 3
> > ....
> > I suppose it is self-explanatory, intuitive and close enough
> > to what user used for with unix-like route config tools.
> > Konstantin
> 
> I was think either, use existing cfgfile and a a section of LPM

LPM can have thousands or even millions entries,
it probably wouldn't be nice to pollute all of them in .ini file
together with usual settings.
At least my preference would be to have them in separate file - clean and simple

> so that it could be an example and also have some generic code for handling
> prefix entries.

Didn't get your sentence above about 'generic code for handling prefix entries'.
Could you possibly elaborate.
 
> Or have a generic library for reading LPM entries.  L3fwd is supposed
> to be as small as possible (it no longer is), and the real work should
> be done by libraries to make it easier to build other applications.

I never heard users ask about such thing,
but if there is a demand for that, then I suppose it could be considered.
CC-ing LPM/FIB maintainers to comment.
Though I believe it should be a subject of separate patch and discussion 
(I think many questions will arise - what format should be, how to support 
different types of user-data, to make it generic enough, etc.).
Konstantin
  
Vladimir Medvedkin Feb. 8, 2022, 4:15 p.m. UTC | #5
Hi all,

On 08/02/2022 10:44, Ananyev, Konstantin wrote:
> 
>>>>> This patchset introduces config file support for l3fwd
>>>>> and its lookup methods LPM, FIB, and EM, similar to
>>>>> that of l3fwd-acl. This allows for route rules to be
>>>>> defined in configuration files and edited there instead
>>>>> of in each of the lookup methods hardcoded route tables.
>>>>>
>>>>> V4:
>>>>> * Fix nondeterministic bug of segfault on termination of
>>>>>    sample app.
>>>>> V5:
>>>>> * Reintroduce hardcoded tables as to not break dts and
>>>>>    allow for hardcoded tables to be used if no config
>>>>>    files presented.
>>>>>
>>>>> Sean Morrissey (2):
>>>>>    examples/l3fwd: add config file support for LPM/FIB
>>>>>    examples/l3fwd: add config file support for EM
>>>>>
>>>>>   doc/guides/sample_app_ug/l3_forward.rst |  89 +++--
>>>>>   examples/l3fwd/em_default_v4.cfg        |  17 +
>>>>>   examples/l3fwd/em_default_v6.cfg        |  17 +
>>>>>   examples/l3fwd/l3fwd.h                  |  41 +++
>>>>>   examples/l3fwd/l3fwd_em.c               | 471 +++++++++++++++++-------
>>>>>   examples/l3fwd/l3fwd_fib.c              |  50 +--
>>>>>   examples/l3fwd/l3fwd_lpm.c              | 315 +++++++++++++++-
>>>>>   examples/l3fwd/l3fwd_route.h            |  41 +++
>>>>>   examples/l3fwd/lpm_default_v4.cfg       |  17 +
>>>>>   examples/l3fwd/lpm_default_v6.cfg       |  17 +
>>>>>   examples/l3fwd/main.c                   |  68 +++-
>>>>>   11 files changed, 949 insertions(+), 194 deletions(-)
>>>>>   create mode 100644 examples/l3fwd/em_default_v4.cfg
>>>>>   create mode 100644 examples/l3fwd/em_default_v6.cfg
>>>>>   create mode 100644 examples/l3fwd/lpm_default_v4.cfg
>>>>>   create mode 100644 examples/l3fwd/lpm_default_v6.cfg
>>>>>
>>>>
>>>> Why not use the DPDK cfgfile library and format?
>>>> It is model after standard INI format.
>>>
>>> It is probably some sort of misunderstanding:
>>> This patch doesn't add configuration file for some l3fwd run-time parameters
>>> (number of ports/queues, queue/cpu mappings, etc.).
>>> It allows user to specify he's own routing table instead of hard-coded ones.
>>> For routing table .ini file format is not really suitable.
>>> Instead we follow format similar to what is used in other DPDK apps
>>> (l3fwd-acl, ipsec-secgw, test-acl, test-fib,  test-sad, etc.) for these purposes:
>>> list of route entries, each entry occupies exactly one line.
>>> As an example:
>>> /examples/l3fwd/lpm_default_v4.cfg
>>> #Copy of hard-coded IPv4 FWD table for L3FWD LPM
>>> R198.18.0.0/24 0
>>> R198.18.1.0/24 1
>>> R198.18.2.0/24 2
>>> R198.18.3.0/24 3
>>> ....
>>> I suppose it is self-explanatory, intuitive and close enough
>>> to what user used for with unix-like route config tools.
>>> Konstantin
>>
>> I was think either, use existing cfgfile and a a section of LPM
> 
> LPM can have thousands or even millions entries,
> it probably wouldn't be nice to pollute all of them in .ini file
> together with usual settings.
> At least my preference would be to have them in separate file - clean and simple
> 

Agree with Konstantin

>> so that it could be an example and also have some generic code for handling
>> prefix entries.
> 
> Didn't get your sentence above about 'generic code for handling prefix entries'.
> Could you possibly elaborate.
> 

If it is about parsing prefix string, then it is probably possible to 
use cmdline_parse_ipaddr()?

>> Or have a generic library for reading LPM entries.  L3fwd is supposed
>> to be as small as possible (it no longer is), and the real work should
>> be done by libraries to make it easier to build other applications.
> 
> I never heard users ask about such thing,
> but if there is a demand for that, then I suppose it could be considered.
> CC-ing LPM/FIB maintainers to comment.
> Though I believe it should be a subject of separate patch and discussion
> (I think many questions will arise - what format should be, how to support
> different types of user-data, to make it generic enough, etc.).

Agree, it is very application specific, so it could be really difficult 
to make it generic.

> Konstantin
>
  
Stephen Hemminger Feb. 8, 2022, 5:49 p.m. UTC | #6
On Tue, 8 Feb 2022 16:15:15 +0000
"Medvedkin, Vladimir" <vladimir.medvedkin@intel.com> wrote:

> >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> >> to be as small as possible (it no longer is), and the real work should
> >> be done by libraries to make it easier to build other applications.  
> > 
> > I never heard users ask about such thing,
> > but if there is a demand for that, then I suppose it could be considered.
> > CC-ing LPM/FIB maintainers to comment.
> > Though I believe it should be a subject of separate patch and discussion
> > (I think many questions will arise - what format should be, how to support
> > different types of user-data, to make it generic enough, etc.).  
> 
> Agree, it is very application specific, so it could be really difficult 
> to make it generic.

But several other also have LPM tables, so why not have common code for other applications.

examples/l3fwd-power/main.c
examples/ipsec-secgw/rt.c
examples/ip_fragmentation/main.c
examples/l3fwd/l3fwd_lpm.c
examples/ip_reassembly/main.c
  
Bruce Richardson Feb. 8, 2022, 6:10 p.m. UTC | #7
On Tue, Feb 08, 2022 at 09:49:04AM -0800, Stephen Hemminger wrote:
> On Tue, 8 Feb 2022 16:15:15 +0000
> "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com> wrote:
> 
> > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > >> to be as small as possible (it no longer is), and the real work should
> > >> be done by libraries to make it easier to build other applications.  
> > > 
> > > I never heard users ask about such thing,
> > > but if there is a demand for that, then I suppose it could be considered.
> > > CC-ing LPM/FIB maintainers to comment.
> > > Though I believe it should be a subject of separate patch and discussion
> > > (I think many questions will arise - what format should be, how to support
> > > different types of user-data, to make it generic enough, etc.).  
> > 
> > Agree, it is very application specific, so it could be really difficult 
> > to make it generic.
> 
> But several other also have LPM tables, so why not have common code for other applications.
> 
> examples/l3fwd-power/main.c
> examples/ipsec-secgw/rt.c
> examples/ip_fragmentation/main.c
> examples/l3fwd/l3fwd_lpm.c
> examples/ip_reassembly/main.c

Could we perhaps add a "load" function to the lpm library itself?
  
Ananyev, Konstantin Feb. 9, 2022, noon UTC | #8
> > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > >> to be as small as possible (it no longer is), and the real work should
> > >> be done by libraries to make it easier to build other applications.
> > >
> > > I never heard users ask about such thing,
> > > but if there is a demand for that, then I suppose it could be considered.
> > > CC-ing LPM/FIB maintainers to comment.
> > > Though I believe it should be a subject of separate patch and discussion
> > > (I think many questions will arise - what format should be, how to support
> > > different types of user-data, to make it generic enough, etc.).
> >
> > Agree, it is very application specific, so it could be really difficult
> > to make it generic.
> 
> But several other also have LPM tables, so why not have common code for other applications.
> 
> examples/l3fwd-power/main.c
> examples/ipsec-secgw/rt.c
> examples/ip_fragmentation/main.c
> examples/l3fwd/l3fwd_lpm.c
> examples/ip_reassembly/main.c

Ah yes, that's good point.
All these examples (except ipsec-secgw) started as l3fwd clones,
so all of them have hard-coded LPM (and EM) tables too.
Yes it would be good thing to address that problem too,
and have some common code (and common routes file format) for all of them.
I don't know is that a good idea to introduce parse file function in LPM/FIB library
itself, might be better to  have something like examples/common/lpm_parse*.
Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
My suggestion would be for 22.03 go ahead with current l3fwd patches,
then later we can consider to make it common and update other examples.
Konstantin
  
Bruce Richardson Feb. 9, 2022, 1:54 p.m. UTC | #9
On Wed, Feb 09, 2022 at 12:00:40PM +0000, Ananyev, Konstantin wrote:
> 
> 
> > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > >> to be as small as possible (it no longer is), and the real work should
> > > >> be done by libraries to make it easier to build other applications.
> > > >
> > > > I never heard users ask about such thing,
> > > > but if there is a demand for that, then I suppose it could be considered.
> > > > CC-ing LPM/FIB maintainers to comment.
> > > > Though I believe it should be a subject of separate patch and discussion
> > > > (I think many questions will arise - what format should be, how to support
> > > > different types of user-data, to make it generic enough, etc.).
> > >
> > > Agree, it is very application specific, so it could be really difficult
> > > to make it generic.
> >
> > But several other also have LPM tables, so why not have common code for other applications.
> >
> > examples/l3fwd-power/main.c
> > examples/ipsec-secgw/rt.c
> > examples/ip_fragmentation/main.c
> > examples/l3fwd/l3fwd_lpm.c
> > examples/ip_reassembly/main.c
> 
> Ah yes, that's good point.
> All these examples (except ipsec-secgw) started as l3fwd clones,
> so all of them have hard-coded LPM (and EM) tables too.
> Yes it would be good thing to address that problem too,
> and have some common code (and common routes file format) for all of them.
> I don't know is that a good idea to introduce parse file function in LPM/FIB library
> itself, might be better to  have something like examples/common/lpm_parse*.

I think it really depends on whether you are planning on implementing a
general config file for the application, or whether the file(s) will only
contain the LPM/FIB routing entries. If the plan is reading just the
routing entries from file, then I definitely think that it might be
something that would be generally useful inside the library itself.

If it's a more general config file with other app settings in it, then an
examples-only parse function/file would make more sense.

> Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> My suggestion would be for 22.03 go ahead with current l3fwd patches,
> then later we can consider to make it common and update other examples.
> Konstantin
  
Vladimir Medvedkin Feb. 9, 2022, 4 p.m. UTC | #10
Hi,

On 09/02/2022 13:54, Bruce Richardson wrote:
> On Wed, Feb 09, 2022 at 12:00:40PM +0000, Ananyev, Konstantin wrote:
>>
>>
>>>>>> Or have a generic library for reading LPM entries.  L3fwd is supposed
>>>>>> to be as small as possible (it no longer is), and the real work should
>>>>>> be done by libraries to make it easier to build other applications.
>>>>>
>>>>> I never heard users ask about such thing,
>>>>> but if there is a demand for that, then I suppose it could be considered.
>>>>> CC-ing LPM/FIB maintainers to comment.
>>>>> Though I believe it should be a subject of separate patch and discussion
>>>>> (I think many questions will arise - what format should be, how to support
>>>>> different types of user-data, to make it generic enough, etc.).
>>>>
>>>> Agree, it is very application specific, so it could be really difficult
>>>> to make it generic.
>>>
>>> But several other also have LPM tables, so why not have common code for other applications.
>>>
>>> examples/l3fwd-power/main.c
>>> examples/ipsec-secgw/rt.c
>>> examples/ip_fragmentation/main.c
>>> examples/l3fwd/l3fwd_lpm.c
>>> examples/ip_reassembly/main.c
>>
>> Ah yes, that's good point.
>> All these examples (except ipsec-secgw) started as l3fwd clones,
>> so all of them have hard-coded LPM (and EM) tables too.
>> Yes it would be good thing to address that problem too,
>> and have some common code (and common routes file format) for all of them.
>> I don't know is that a good idea to introduce parse file function in LPM/FIB library
>> itself, might be better to  have something like examples/common/lpm_parse*.
> 
> I think it really depends on whether you are planning on implementing a
> general config file for the application, or whether the file(s) will only
> contain the LPM/FIB routing entries. If the plan is reading just the
> routing entries from file, then I definitely think that it might be
> something that would be generally useful inside the library itself.
> 
> If it's a more general config file with other app settings in it, then an
> examples-only parse function/file would make more sense.
> 

I vote for this code to be in examples where we can control the file 
format with LPM/FIB entries.
I don't think it's a good idea to move this API to the data plane 
library. I think it will only be used in our examples because dpdk users 
will have their own formats for routing information.

>> Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
>> My suggestion would be for 22.03 go ahead with current l3fwd patches,
>> then later we can consider to make it common and update other examples.
>> Konstantin
  
Thomas Monjalon Feb. 22, 2022, 9:59 a.m. UTC | #11
09/02/2022 13:00, Ananyev, Konstantin:
> 
> > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > >> to be as small as possible (it no longer is), and the real work should
> > > >> be done by libraries to make it easier to build other applications.
> > > >
> > > > I never heard users ask about such thing,
> > > > but if there is a demand for that, then I suppose it could be considered.
> > > > CC-ing LPM/FIB maintainers to comment.
> > > > Though I believe it should be a subject of separate patch and discussion
> > > > (I think many questions will arise - what format should be, how to support
> > > > different types of user-data, to make it generic enough, etc.).
> > >
> > > Agree, it is very application specific, so it could be really difficult
> > > to make it generic.
> > 
> > But several other also have LPM tables, so why not have common code for other applications.
> > 
> > examples/l3fwd-power/main.c
> > examples/ipsec-secgw/rt.c
> > examples/ip_fragmentation/main.c
> > examples/l3fwd/l3fwd_lpm.c
> > examples/ip_reassembly/main.c
> 
> Ah yes, that's good point.
> All these examples (except ipsec-secgw) started as l3fwd clones,
> so all of them have hard-coded LPM (and EM) tables too.
> Yes it would be good thing to address that problem too,
> and have some common code (and common routes file format) for all of them.
> I don't know is that a good idea to introduce parse file function in LPM/FIB library
> itself, might be better to  have something like examples/common/lpm_parse*.
> Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> My suggestion would be for 22.03 go ahead with current l3fwd patches,
> then later we can consider to make it common and update other examples.

I don't think this patch is urgent.
I suggest taking time to have common code for all examples
and target a merge in DPDK 22.07.
  
Ananyev, Konstantin Feb. 22, 2022, 10:39 a.m. UTC | #12
> > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > >> to be as small as possible (it no longer is), and the real work should
> > > > >> be done by libraries to make it easier to build other applications.
> > > > >
> > > > > I never heard users ask about such thing,
> > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > (I think many questions will arise - what format should be, how to support
> > > > > different types of user-data, to make it generic enough, etc.).
> > > >
> > > > Agree, it is very application specific, so it could be really difficult
> > > > to make it generic.
> > >
> > > But several other also have LPM tables, so why not have common code for other applications.
> > >
> > > examples/l3fwd-power/main.c
> > > examples/ipsec-secgw/rt.c
> > > examples/ip_fragmentation/main.c
> > > examples/l3fwd/l3fwd_lpm.c
> > > examples/ip_reassembly/main.c
> >
> > Ah yes, that's good point.
> > All these examples (except ipsec-secgw) started as l3fwd clones,
> > so all of them have hard-coded LPM (and EM) tables too.
> > Yes it would be good thing to address that problem too,
> > and have some common code (and common routes file format) for all of them.
> > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > itself, might be better to  have something like examples/common/lpm_parse*.
> > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > then later we can consider to make it common and update other examples.
> 
> I don't think this patch is urgent.
> I suggest taking time to have common code for all examples
> and target a merge in DPDK 22.07.

Well, yes, from one perspective it not really a critical one,
we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
Though l3fwd is one of mostly used examples inside DPDK and
it is quite a pain to patch/rebuild it each time someone needs to run
l3fwd with a different routing table. 
Merging this patch will allow people to use l3fwd for more realistic test
scenarios in a painless manner.
So I believe this patch is really helpful and should be beneficial for the whole community.
Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
Why we can't move one step at a time instead?
That would allow to split and effort in terms of development/testing/upstreaming/etc.
  
Thomas Monjalon Feb. 22, 2022, 1:46 p.m. UTC | #13
22/02/2022 11:39, Ananyev, Konstantin:
> 
> > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > >> be done by libraries to make it easier to build other applications.
> > > > > >
> > > > > > I never heard users ask about such thing,
> > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > different types of user-data, to make it generic enough, etc.).
> > > > >
> > > > > Agree, it is very application specific, so it could be really difficult
> > > > > to make it generic.
> > > >
> > > > But several other also have LPM tables, so why not have common code for other applications.
> > > >
> > > > examples/l3fwd-power/main.c
> > > > examples/ipsec-secgw/rt.c
> > > > examples/ip_fragmentation/main.c
> > > > examples/l3fwd/l3fwd_lpm.c
> > > > examples/ip_reassembly/main.c
> > >
> > > Ah yes, that's good point.
> > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > so all of them have hard-coded LPM (and EM) tables too.
> > > Yes it would be good thing to address that problem too,
> > > and have some common code (and common routes file format) for all of them.
> > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > then later we can consider to make it common and update other examples.
> > 
> > I don't think this patch is urgent.
> > I suggest taking time to have common code for all examples
> > and target a merge in DPDK 22.07.
> 
> Well, yes, from one perspective it not really a critical one,
> we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> Though l3fwd is one of mostly used examples inside DPDK and
> it is quite a pain to patch/rebuild it each time someone needs to run
> l3fwd with a different routing table. 
> Merging this patch will allow people to use l3fwd for more realistic test
> scenarios in a painless manner.
> So I believe this patch is really helpful and should be beneficial for the whole community.
> Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> Why we can't move one step at a time instead?
> That would allow to split and effort in terms of development/testing/upstreaming/etc.

When a feature is merged, there is less incentives to rework.
That's why, when a feature is not urgent,
it is better to wait for the complete work.
  
Ananyev, Konstantin Feb. 22, 2022, 3:13 p.m. UTC | #14
> >
> > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > >
> > > > > > > I never heard users ask about such thing,
> > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > >
> > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > to make it generic.
> > > > >
> > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > >
> > > > > examples/l3fwd-power/main.c
> > > > > examples/ipsec-secgw/rt.c
> > > > > examples/ip_fragmentation/main.c
> > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > examples/ip_reassembly/main.c
> > > >
> > > > Ah yes, that's good point.
> > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > Yes it would be good thing to address that problem too,
> > > > and have some common code (and common routes file format) for all of them.
> > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > then later we can consider to make it common and update other examples.
> > >
> > > I don't think this patch is urgent.
> > > I suggest taking time to have common code for all examples
> > > and target a merge in DPDK 22.07.
> >
> > Well, yes, from one perspective it not really a critical one,
> > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > Though l3fwd is one of mostly used examples inside DPDK and
> > it is quite a pain to patch/rebuild it each time someone needs to run
> > l3fwd with a different routing table.
> > Merging this patch will allow people to use l3fwd for more realistic test
> > scenarios in a painless manner.
> > So I believe this patch is really helpful and should be beneficial for the whole community.
> > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > Why we can't move one step at a time instead?
> > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> 
> When a feature is merged, there is less incentives to rework.
> That's why, when a feature is not urgent,
> it is better to wait for the complete work.

That's true till some extent, though from other side
even without further rework that patch improves situation
from what we have right now.
So I don't see any harm here.
  
Thomas Monjalon Feb. 22, 2022, 4:48 p.m. UTC | #15
22/02/2022 16:13, Ananyev, Konstantin:
> 
> > >
> > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > >
> > > > > > > > I never heard users ask about such thing,
> > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > >
> > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > to make it generic.
> > > > > >
> > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > >
> > > > > > examples/l3fwd-power/main.c
> > > > > > examples/ipsec-secgw/rt.c
> > > > > > examples/ip_fragmentation/main.c
> > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > examples/ip_reassembly/main.c
> > > > >
> > > > > Ah yes, that's good point.
> > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > Yes it would be good thing to address that problem too,
> > > > > and have some common code (and common routes file format) for all of them.
> > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > then later we can consider to make it common and update other examples.
> > > >
> > > > I don't think this patch is urgent.
> > > > I suggest taking time to have common code for all examples
> > > > and target a merge in DPDK 22.07.
> > >
> > > Well, yes, from one perspective it not really a critical one,
> > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > Though l3fwd is one of mostly used examples inside DPDK and
> > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > l3fwd with a different routing table.
> > > Merging this patch will allow people to use l3fwd for more realistic test
> > > scenarios in a painless manner.
> > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > Why we can't move one step at a time instead?
> > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > 
> > When a feature is merged, there is less incentives to rework.
> > That's why, when a feature is not urgent,
> > it is better to wait for the complete work.
> 
> That's true till some extent, though from other side
> even without further rework that patch improves situation
> from what we have right now.
> So I don't see any harm here.

It is adding a lot of code to an example which is already too big.
There are a lot of complain about the size of l3fwd.
That's why I think it makes sense to require this extra code
(not demonstrating anything, but just for testing convenience)
outside of the example.
  
Ananyev, Konstantin Feb. 24, 2022, 11:06 a.m. UTC | #16
> 
> 22/02/2022 16:13, Ananyev, Konstantin:
> >
> > > >
> > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > >
> > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > >
> > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > to make it generic.
> > > > > > >
> > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > >
> > > > > > > examples/l3fwd-power/main.c
> > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > examples/ip_fragmentation/main.c
> > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > examples/ip_reassembly/main.c
> > > > > >
> > > > > > Ah yes, that's good point.
> > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > Yes it would be good thing to address that problem too,
> > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > then later we can consider to make it common and update other examples.
> > > > >
> > > > > I don't think this patch is urgent.
> > > > > I suggest taking time to have common code for all examples
> > > > > and target a merge in DPDK 22.07.
> > > >
> > > > Well, yes, from one perspective it not really a critical one,
> > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > l3fwd with a different routing table.
> > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > scenarios in a painless manner.
> > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > Why we can't move one step at a time instead?
> > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > >
> > > When a feature is merged, there is less incentives to rework.
> > > That's why, when a feature is not urgent,
> > > it is better to wait for the complete work.
> >
> > That's true till some extent, though from other side
> > even without further rework that patch improves situation
> > from what we have right now.
> > So I don't see any harm here.
> 
> It is adding a lot of code to an example which is already too big.
> There are a lot of complain about the size of l3fwd.
> That's why I think it makes sense to require this extra code
> (not demonstrating anything, but just for testing convenience)
> outside of the example.

Ok, so your main concern is l3fwd code size increase, right?
Then would it help if for we'll move file parsing code into a separate file(s)  
(under examples/l3fwd) for now?
Something like examples/l3fwd/(lpm_em)_route_parse.c.
  
Thomas Monjalon Feb. 24, 2022, 1:46 p.m. UTC | #17
24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > > >
> > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > > >
> > > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > > to make it generic.
> > > > > > > >
> > > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > > >
> > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > examples/ip_reassembly/main.c
> > > > > > >
> > > > > > > Ah yes, that's good point.
> > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > > then later we can consider to make it common and update other examples.
> > > > > >
> > > > > > I don't think this patch is urgent.
> > > > > > I suggest taking time to have common code for all examples
> > > > > > and target a merge in DPDK 22.07.
> > > > >
> > > > > Well, yes, from one perspective it not really a critical one,
> > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > > l3fwd with a different routing table.
> > > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > > scenarios in a painless manner.
> > > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > > Why we can't move one step at a time instead?
> > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > > >
> > > > When a feature is merged, there is less incentives to rework.
> > > > That's why, when a feature is not urgent,
> > > > it is better to wait for the complete work.
> > >
> > > That's true till some extent, though from other side
> > > even without further rework that patch improves situation
> > > from what we have right now.
> > > So I don't see any harm here.
> > 
> > It is adding a lot of code to an example which is already too big.
> > There are a lot of complain about the size of l3fwd.
> > That's why I think it makes sense to require this extra code
> > (not demonstrating anything, but just for testing convenience)
> > outside of the example.
> 
> Ok, so your main concern is l3fwd code size increase, right?
> Then would it help if for we'll move file parsing code into a separate file(s)  
> (under examples/l3fwd) for now?
> Something like examples/l3fwd/(lpm_em)_route_parse.c.

Yes it would help to isolate the config file parsing code.
What others think?
  
Bruce Richardson Feb. 24, 2022, 1:58 p.m. UTC | #18
On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote:
> 24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > > > >
> > > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > > > >
> > > > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > > > to make it generic.
> > > > > > > > >
> > > > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > > > >
> > > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > > examples/ip_reassembly/main.c
> > > > > > > >
> > > > > > > > Ah yes, that's good point.
> > > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > > > then later we can consider to make it common and update other examples.
> > > > > > >
> > > > > > > I don't think this patch is urgent.
> > > > > > > I suggest taking time to have common code for all examples
> > > > > > > and target a merge in DPDK 22.07.
> > > > > >
> > > > > > Well, yes, from one perspective it not really a critical one,
> > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > > > l3fwd with a different routing table.
> > > > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > > > scenarios in a painless manner.
> > > > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > > > Why we can't move one step at a time instead?
> > > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > > > >
> > > > > When a feature is merged, there is less incentives to rework.
> > > > > That's why, when a feature is not urgent,
> > > > > it is better to wait for the complete work.
> > > >
> > > > That's true till some extent, though from other side
> > > > even without further rework that patch improves situation
> > > > from what we have right now.
> > > > So I don't see any harm here.
> > > 
> > > It is adding a lot of code to an example which is already too big.
> > > There are a lot of complain about the size of l3fwd.
> > > That's why I think it makes sense to require this extra code
> > > (not demonstrating anything, but just for testing convenience)
> > > outside of the example.
> > 
> > Ok, so your main concern is l3fwd code size increase, right?
> > Then would it help if for we'll move file parsing code into a separate file(s)  
> > (under examples/l3fwd) for now?
> > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> 
> Yes it would help to isolate the config file parsing code.
> What others think?
> 
I still would like config code for loading an LPM table or FIB table to be
put inside the relevant libraries themselves, rather than having it in the
examples themselves (even if shared between them).

/Bruce
  
Honnappa Nagarahalli Feb. 25, 2022, 5:18 a.m. UTC | #19
<snip>

> 
> 24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > > >> Or have a generic library for reading LPM entries.
> > > > > > > > > > >> L3fwd is supposed to be as small as possible (it no
> > > > > > > > > > >> longer is), and the real work should be done by libraries to
> make it easier to build other applications.
> > > > > > > > > > >
> > > > > > > > > > > I never heard users ask about such thing, but if
> > > > > > > > > > > there is a demand for that, then I suppose it could be
> considered.
> > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > Though I believe it should be a subject of separate
> > > > > > > > > > > patch and discussion (I think many questions will
> > > > > > > > > > > arise - what format should be, how to support different types
> of user-data, to make it generic enough, etc.).
> > > > > > > > > >
> > > > > > > > > > Agree, it is very application specific, so it could be
> > > > > > > > > > really difficult to make it generic.
> > > > > > > > >
> > > > > > > > > But several other also have LPM tables, so why not have common
> code for other applications.
> > > > > > > > >
> > > > > > > > > examples/l3fwd-power/main.c examples/ipsec-secgw/rt.c
> > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > examples/l3fwd/l3fwd_lpm.c examples/ip_reassembly/main.c
> > > > > > > >
> > > > > > > > Ah yes, that's good point.
> > > > > > > > All these examples (except ipsec-secgw) started as l3fwd
> > > > > > > > clones, so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > > and have some common code (and common routes file format) for
> all of them.
> > > > > > > > I don't know is that a good idea to introduce parse file
> > > > > > > > function in LPM/FIB library itself, might be better to  have
> something like examples/common/lpm_parse*.
> > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in
> 22.03 timeframe.
> > > > > > > > My suggestion would be for 22.03 go ahead with current
> > > > > > > > l3fwd patches, then later we can consider to make it common and
> update other examples.
> > > > > > >
> > > > > > > I don't think this patch is urgent.
> > > > > > > I suggest taking time to have common code for all examples
> > > > > > > and target a merge in DPDK 22.07.
> > > > > >
> > > > > > Well, yes, from one perspective it not really a critical one,
> > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by
> now.
> > > > > > Though l3fwd is one of mostly used examples inside DPDK and it
> > > > > > is quite a pain to patch/rebuild it each time someone needs to
> > > > > > run l3fwd with a different routing table.
> > > > > > Merging this patch will allow people to use l3fwd for more
> > > > > > realistic test scenarios in a painless manner.
> > > > > > So I believe this patch is really helpful and should be beneficial for the
> whole community.
> > > > > > Looking from that perspective, I don't see why it has to be "all or
> nothing" attitude here.
> > > > > > Why we can't move one step at a time instead?
> > > > > > That would allow to split and effort in terms of
> development/testing/upstreaming/etc.
> > > > >
> > > > > When a feature is merged, there is less incentives to rework.
> > > > > That's why, when a feature is not urgent, it is better to wait
> > > > > for the complete work.
> > > >
> > > > That's true till some extent, though from other side even without
> > > > further rework that patch improves situation from what we have
> > > > right now.
> > > > So I don't see any harm here.
> > >
> > > It is adding a lot of code to an example which is already too big.
> > > There are a lot of complain about the size of l3fwd.
> > > That's why I think it makes sense to require this extra code (not
> > > demonstrating anything, but just for testing convenience) outside of
> > > the example.
> >
> > Ok, so your main concern is l3fwd code size increase, right?
> > Then would it help if for we'll move file parsing code into a separate
> > file(s) (under examples/l3fwd) for now?
> > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> 
> Yes it would help to isolate the config file parsing code.
> What others think?
L3fwd is no more a sample application, suggest moving it to app directory.

>
  
Ananyev, Konstantin Feb. 25, 2022, 10:36 a.m. UTC | #20
> On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote:
> > 24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > > > > >
> > > > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > > > > >
> > > > > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > > > > to make it generic.
> > > > > > > > > >
> > > > > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > > > > >
> > > > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > > > examples/ip_reassembly/main.c
> > > > > > > > >
> > > > > > > > > Ah yes, that's good point.
> > > > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > > > > then later we can consider to make it common and update other examples.
> > > > > > > >
> > > > > > > > I don't think this patch is urgent.
> > > > > > > > I suggest taking time to have common code for all examples
> > > > > > > > and target a merge in DPDK 22.07.
> > > > > > >
> > > > > > > Well, yes, from one perspective it not really a critical one,
> > > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > > > > l3fwd with a different routing table.
> > > > > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > > > > scenarios in a painless manner.
> > > > > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > > > > Why we can't move one step at a time instead?
> > > > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > > > > >
> > > > > > When a feature is merged, there is less incentives to rework.
> > > > > > That's why, when a feature is not urgent,
> > > > > > it is better to wait for the complete work.
> > > > >
> > > > > That's true till some extent, though from other side
> > > > > even without further rework that patch improves situation
> > > > > from what we have right now.
> > > > > So I don't see any harm here.
> > > >
> > > > It is adding a lot of code to an example which is already too big.
> > > > There are a lot of complain about the size of l3fwd.
> > > > That's why I think it makes sense to require this extra code
> > > > (not demonstrating anything, but just for testing convenience)
> > > > outside of the example.
> > >
> > > Ok, so your main concern is l3fwd code size increase, right?
> > > Then would it help if for we'll move file parsing code into a separate file(s)
> > > (under examples/l3fwd) for now?
> > > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> >
> > Yes it would help to isolate the config file parsing code.
> > What others think?
> >
> I still would like config code for loading an LPM table or FIB table to be
> put inside the relevant libraries themselves, rather than having it in the
> examples themselves (even if shared between them).

Honestly, I don't see any good reasons for that:
I presume users of these libraries already have their own routing
config files with their own format requirements and I suppose
these formats differ a lot (depending on use-case).
For DPDK internal purposes (provide sample apps with ability to load routes dynamically)
some common code inside examples/ seems more than enough. 
Konstantin
  
Bruce Richardson Feb. 25, 2022, 10:40 a.m. UTC | #21
On Fri, Feb 25, 2022 at 10:36:29AM +0000, Ananyev, Konstantin wrote:
> 
> > On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote:
> > > 24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > > > > > >
> > > > > > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > > > > > to make it generic.
> > > > > > > > > > >
> > > > > > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > > > > > >
> > > > > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > > > > examples/ip_reassembly/main.c
> > > > > > > > > >
> > > > > > > > > > Ah yes, that's good point.
> > > > > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > > > > > then later we can consider to make it common and update other examples.
> > > > > > > > >
> > > > > > > > > I don't think this patch is urgent.
> > > > > > > > > I suggest taking time to have common code for all examples
> > > > > > > > > and target a merge in DPDK 22.07.
> > > > > > > >
> > > > > > > > Well, yes, from one perspective it not really a critical one,
> > > > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > > > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > > > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > > > > > l3fwd with a different routing table.
> > > > > > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > > > > > scenarios in a painless manner.
> > > > > > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > > > > > Why we can't move one step at a time instead?
> > > > > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > > > > > >
> > > > > > > When a feature is merged, there is less incentives to rework.
> > > > > > > That's why, when a feature is not urgent,
> > > > > > > it is better to wait for the complete work.
> > > > > >
> > > > > > That's true till some extent, though from other side
> > > > > > even without further rework that patch improves situation
> > > > > > from what we have right now.
> > > > > > So I don't see any harm here.
> > > > >
> > > > > It is adding a lot of code to an example which is already too big.
> > > > > There are a lot of complain about the size of l3fwd.
> > > > > That's why I think it makes sense to require this extra code
> > > > > (not demonstrating anything, but just for testing convenience)
> > > > > outside of the example.
> > > >
> > > > Ok, so your main concern is l3fwd code size increase, right?
> > > > Then would it help if for we'll move file parsing code into a separate file(s)
> > > > (under examples/l3fwd) for now?
> > > > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> > >
> > > Yes it would help to isolate the config file parsing code.
> > > What others think?
> > >
> > I still would like config code for loading an LPM table or FIB table to be
> > put inside the relevant libraries themselves, rather than having it in the
> > examples themselves (even if shared between them).
> 
> Honestly, I don't see any good reasons for that:
> I presume users of these libraries already have their own routing
> config files with their own format requirements and I suppose
> these formats differ a lot (depending on use-case).

Yes, I agree that all existing users of the libraries will have this in
place. However, for new users, this may be useful for bootstrapping things,
and it also makes it generally useable for both example apps, and any apps
in the "app" folder, i.e. if l3fwd gets moved there. (Something I agree
with, as it's now more than just example code).
  
Ananyev, Konstantin Feb. 25, 2022, 12:21 p.m. UTC | #22
> > > > > > > > > > > > > >> Or have a generic library for reading LPM entries.  L3fwd is supposed
> > > > > > > > > > > > > >> to be as small as possible (it no longer is), and the real work should
> > > > > > > > > > > > > >> be done by libraries to make it easier to build other applications.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > > > > > but if there is a demand for that, then I suppose it could be considered.
> > > > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > > > > Though I believe it should be a subject of separate patch and discussion
> > > > > > > > > > > > > > (I think many questions will arise - what format should be, how to support
> > > > > > > > > > > > > > different types of user-data, to make it generic enough, etc.).
> > > > > > > > > > > > >
> > > > > > > > > > > > > Agree, it is very application specific, so it could be really difficult
> > > > > > > > > > > > > to make it generic.
> > > > > > > > > > > >
> > > > > > > > > > > > But several other also have LPM tables, so why not have common code for other applications.
> > > > > > > > > > > >
> > > > > > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > > > > > examples/ip_reassembly/main.c
> > > > > > > > > > >
> > > > > > > > > > > Ah yes, that's good point.
> > > > > > > > > > > All these examples (except ipsec-secgw) started as l3fwd clones,
> > > > > > > > > > > so all of them have hard-coded LPM (and EM) tables too.
> > > > > > > > > > > Yes it would be good thing to address that problem too,
> > > > > > > > > > > and have some common code (and common routes file format) for all of them.
> > > > > > > > > > > I don't know is that a good idea to introduce parse file function in LPM/FIB library
> > > > > > > > > > > itself, might be better to  have something like examples/common/lpm_parse*.
> > > > > > > > > > > Anyway, this is an extra effort, and I think no-one has time for it in 22.03 timeframe.
> > > > > > > > > > > My suggestion would be for 22.03 go ahead with current l3fwd patches,
> > > > > > > > > > > then later we can consider to make it common and update other examples.
> > > > > > > > > >
> > > > > > > > > > I don't think this patch is urgent.
> > > > > > > > > > I suggest taking time to have common code for all examples
> > > > > > > > > > and target a merge in DPDK 22.07.
> > > > > > > > >
> > > > > > > > > Well, yes, from one perspective it not really a critical one,
> > > > > > > > > we do live with hard-coded routes inside l3fwd for nearly 10 year by now.
> > > > > > > > > Though l3fwd is one of mostly used examples inside DPDK and
> > > > > > > > > it is quite a pain to patch/rebuild it each time someone needs to run
> > > > > > > > > l3fwd with a different routing table.
> > > > > > > > > Merging this patch will allow people to use l3fwd for more realistic test
> > > > > > > > > scenarios in a painless manner.
> > > > > > > > > So I believe this patch is really helpful and should be beneficial for the whole community.
> > > > > > > > > Looking from that perspective, I don't see why it has to be "all or nothing" attitude here.
> > > > > > > > > Why we can't move one step at a time instead?
> > > > > > > > > That would allow to split and effort in terms of development/testing/upstreaming/etc.
> > > > > > > >
> > > > > > > > When a feature is merged, there is less incentives to rework.
> > > > > > > > That's why, when a feature is not urgent,
> > > > > > > > it is better to wait for the complete work.
> > > > > > >
> > > > > > > That's true till some extent, though from other side
> > > > > > > even without further rework that patch improves situation
> > > > > > > from what we have right now.
> > > > > > > So I don't see any harm here.
> > > > > >
> > > > > > It is adding a lot of code to an example which is already too big.
> > > > > > There are a lot of complain about the size of l3fwd.
> > > > > > That's why I think it makes sense to require this extra code
> > > > > > (not demonstrating anything, but just for testing convenience)
> > > > > > outside of the example.
> > > > >
> > > > > Ok, so your main concern is l3fwd code size increase, right?
> > > > > Then would it help if for we'll move file parsing code into a separate file(s)
> > > > > (under examples/l3fwd) for now?
> > > > > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> > > >
> > > > Yes it would help to isolate the config file parsing code.
> > > > What others think?
> > > >
> > > I still would like config code for loading an LPM table or FIB table to be
> > > put inside the relevant libraries themselves, rather than having it in the
> > > examples themselves (even if shared between them).
> >
> > Honestly, I don't see any good reasons for that:
> > I presume users of these libraries already have their own routing
> > config files with their own format requirements and I suppose
> > these formats differ a lot (depending on use-case).
> 
> Yes, I agree that all existing users of the libraries will have this in
> place. However, for new users, this may be useful for bootstrapping things,
> and it also makes it generally useable for both example apps, and any apps
> in the "app" folder, i.e. if l3fwd gets moved there. (Something I agree
> with, as it's now more than just example code).

Inside dpdk for l3fwd like examples, all we need as data associated
with route is just a port number.
I really doubt that for real-world app that would be all they need -
I expect users would like to associate much more data with each route.
So I don't see how such library function will be useful even for new apps.
From other side, if we will put sample code in examples/l3fwd/*,
users can copy it from there and use it as a starting point for their
specific route config parser.
  
Morten Brørup Feb. 25, 2022, 12:50 p.m. UTC | #23
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 25 February 2022 11.40
> 
> On Fri, Feb 25, 2022 at 10:36:29AM +0000, Ananyev, Konstantin wrote:
> >
> > > On Thu, Feb 24, 2022 at 02:46:24PM +0100, Thomas Monjalon wrote:
> > > > 24/02/2022 12:06, Ananyev, Konstantin:
> > > > > > > > > > > > > >> Or have a generic library for reading LPM
> entries.  L3fwd is supposed
> > > > > > > > > > > > > >> to be as small as possible (it no longer
> is), and the real work should
> > > > > > > > > > > > > >> be done by libraries to make it easier to
> build other applications.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I never heard users ask about such thing,
> > > > > > > > > > > > > > but if there is a demand for that, then I
> suppose it could be considered.
> > > > > > > > > > > > > > CC-ing LPM/FIB maintainers to comment.
> > > > > > > > > > > > > > Though I believe it should be a subject of
> separate patch and discussion
> > > > > > > > > > > > > > (I think many questions will arise - what
> format should be, how to support
> > > > > > > > > > > > > > different types of user-data, to make it
> generic enough, etc.).
> > > > > > > > > > > > >
> > > > > > > > > > > > > Agree, it is very application specific, so it
> could be really difficult
> > > > > > > > > > > > > to make it generic.
> > > > > > > > > > > >
> > > > > > > > > > > > But several other also have LPM tables, so why
> not have common code for other applications.
> > > > > > > > > > > >
> > > > > > > > > > > > examples/l3fwd-power/main.c
> > > > > > > > > > > > examples/ipsec-secgw/rt.c
> > > > > > > > > > > > examples/ip_fragmentation/main.c
> > > > > > > > > > > > examples/l3fwd/l3fwd_lpm.c
> > > > > > > > > > > > examples/ip_reassembly/main.c
> > > > > > > > > > >
> > > > > > > > > > > Ah yes, that's good point.
> > > > > > > > > > > All these examples (except ipsec-secgw) started as
> l3fwd clones,
> > > > > > > > > > > so all of them have hard-coded LPM (and EM) tables
> too.
> > > > > > > > > > > Yes it would be good thing to address that problem
> too,
> > > > > > > > > > > and have some common code (and common routes file
> format) for all of them.
> > > > > > > > > > > I don't know is that a good idea to introduce parse
> file function in LPM/FIB library
> > > > > > > > > > > itself, might be better to  have something like
> examples/common/lpm_parse*.
> > > > > > > > > > > Anyway, this is an extra effort, and I think no-one
> has time for it in 22.03 timeframe.
> > > > > > > > > > > My suggestion would be for 22.03 go ahead with
> current l3fwd patches,
> > > > > > > > > > > then later we can consider to make it common and
> update other examples.
> > > > > > > > > >
> > > > > > > > > > I don't think this patch is urgent.
> > > > > > > > > > I suggest taking time to have common code for all
> examples
> > > > > > > > > > and target a merge in DPDK 22.07.
> > > > > > > > >
> > > > > > > > > Well, yes, from one perspective it not really a
> critical one,
> > > > > > > > > we do live with hard-coded routes inside l3fwd for
> nearly 10 year by now.
> > > > > > > > > Though l3fwd is one of mostly used examples inside DPDK
> and
> > > > > > > > > it is quite a pain to patch/rebuild it each time
> someone needs to run
> > > > > > > > > l3fwd with a different routing table.
> > > > > > > > > Merging this patch will allow people to use l3fwd for
> more realistic test
> > > > > > > > > scenarios in a painless manner.
> > > > > > > > > So I believe this patch is really helpful and should be
> beneficial for the whole community.
> > > > > > > > > Looking from that perspective, I don't see why it has
> to be "all or nothing" attitude here.
> > > > > > > > > Why we can't move one step at a time instead?
> > > > > > > > > That would allow to split and effort in terms of
> development/testing/upstreaming/etc.
> > > > > > > >
> > > > > > > > When a feature is merged, there is less incentives to
> rework.
> > > > > > > > That's why, when a feature is not urgent,
> > > > > > > > it is better to wait for the complete work.
> > > > > > >
> > > > > > > That's true till some extent, though from other side
> > > > > > > even without further rework that patch improves situation
> > > > > > > from what we have right now.
> > > > > > > So I don't see any harm here.
> > > > > >
> > > > > > It is adding a lot of code to an example which is already too
> big.
> > > > > > There are a lot of complain about the size of l3fwd.
> > > > > > That's why I think it makes sense to require this extra code
> > > > > > (not demonstrating anything, but just for testing
> convenience)
> > > > > > outside of the example.
> > > > >
> > > > > Ok, so your main concern is l3fwd code size increase, right?
> > > > > Then would it help if for we'll move file parsing code into a
> separate file(s)
> > > > > (under examples/l3fwd) for now?
> > > > > Something like examples/l3fwd/(lpm_em)_route_parse.c.
> > > >
> > > > Yes it would help to isolate the config file parsing code.
> > > > What others think?
> > > >
> > > I still would like config code for loading an LPM table or FIB
> table to be
> > > put inside the relevant libraries themselves, rather than having it
> in the
> > > examples themselves (even if shared between them).
> >
> > Honestly, I don't see any good reasons for that:
> > I presume users of these libraries already have their own routing
> > config files with their own format requirements and I suppose
> > these formats differ a lot (depending on use-case).

I can confirm this assumption.

Our appliances use our own configuration file handling, which supports full, partial and diff configurations. Furthermore, it uses a schema with type definitions for simple syntax checking, as well as a programmable validator to check system-wide configuration validity and integrity.

> 
> Yes, I agree that all existing users of the libraries will have this in
> place. However, for new users, this may be useful for bootstrapping
> things,
> and it also makes it generally useable for both example apps, and any
> apps
> in the "app" folder, i.e. if l3fwd gets moved there. (Something I agree
> with, as it's now more than just example code).
> 

Don't put any config file parsers or similar in the fast path libraries. It is unwanted bloat!

If you want to standardize on a DPDK specific config file format for the DPDK provided examples and applications, which I do agree with the benefits of having, then provide a separate library to interact with the underlying fast path libraries.