Re: [PATCH v5 0/2] Add config file support for l3fwd

["Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>]

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
> 

-- 
Regards,
Vladimir