All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Lukas Sismis <sismis@dyna-nic.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v10 0/6] flow_parser: add shared parser library
Date: Mon, 2 Feb 2026 10:37:54 -0800	[thread overview]
Message-ID: <20260202103754.3d996282@phoenix.local> (raw)
In-Reply-To: <20260202113659.24052-1-sismis@dyna-nic.com>

On Mon,  2 Feb 2026 12:36:52 +0100
Lukas Sismis <sismis@dyna-nic.com> wrote:

> This series extracts the testpmd flow CLI parser into a reusable library,
> enabling external applications to parse rte_flow rules using testpmd syntax.
> 
> Motivation
> ----------
> External applications like Suricata IDS [1] need to express hardware filtering
> rules in a consistent, human-readable format. Rather than inventing custom
> syntax, reusing testpmd's well-tested flow grammar provides immediate
> compatibility with existing documentation and user knowledge.
> 
> Note: This library provides only one way to create rte_flow structures.
> Applications can also construct rte_flow_attr, rte_flow_item[], and
> rte_flow_action[] directly in C code.
> 
> Design
> ------
> The library (librte_flow_parser) exposes the following APIs:
> - rte_flow_parser_parse_attr_str(): Parse attributes only
> - rte_flow_parser_parse_pattern_str(): Parse patterns only
> - rte_flow_parser_parse_actions_str(): Parse actions only
> 
> Testpmd is updated to use the library, ensuring a single
> maintained parser implementation.
> 
> Testing and Demo
> -------
> - Functional tests in dpdk-test
> - Example application: examples/flow_parsing
> 
> Changes
> -------
> v10:
> - rebased to avoid Github Actions CI build failure
> - merge conflict solved in rel_notes/release_26_03.rst
> - release notes shortened
> 
> v9:
> - removed extra new line from the flow parser docs file
> 
> v8:
> - rte_port/queue_id_t typedefs removal to be included in a separate patch series
> - move of accidental changes of rte_flow parser library from the testpmd commit
> - DynaNIC copyright name update
> 
> v7:
> - Fixed implicit integer comparison (while (left) -> while (left != 0))
> - NULL checks fixed
> - arpa header removed for Windows compatibility
> - minor comments from the last review addressed
> 
> v6:
> - Inconsistent Experimental API Version adjusted
> - Fixes Tag added to MSVC build commit
> - Non-Standard Header Guards updated
> - Implicit Pointer Comparison and Return Type issues addressed in many places
> - commit message in patch 6 updated
> 
> v5:
> - removed/replaced (f)printf code from the library 
> - reverted back to exporting the internal/private API as it is needed by
>   testpmd and cannot be easily split further.
> - adjusted length of certain lines
> - marking port/queue id typedef as experimental
> - updated release rel_notes
> - copyeright adjustments
> 
> v4:
> - ethdev changes in separate commit
> - library's public API only exposes attribute, pattern and action parsing,
>   while the full command parsing is kept internal for testpmd usage only.
> - Addressed Stephen's comments from V3
> - dpdk-test now have tests focused on public and internal library functions
> 
> v3:
> - Add more functional tests
> - More concise MAINTAINERS updates
> - Updated license headers
> - A thing to note: When playing with flow commands, I figured, some may use
>   non-flow commands, such as raw decap/encap, policy meter and others.
>   Flow parser library itself now supports `set` command to set e.g. the decap/
>   encap parameters, as the flow syntax only supports defining the index of the
>   encap/decap configs. The library, however, does not support e.g. `create`
>   command to create policy meters, as that is just an ID and it can be created
>   separately using rte_meter APIs.
> 
> [1] https://github.com/OISF/suricata/pull/13950
> 
> Lukas Sismis (6):
>   cmdline: include stddef.h for MSVC compatibility
>   ethdev: add RSS type helper APIs
>   flow_parser: add shared parser library
>   app/testpmd: use shared flow parser library
>   examples/flow_parsing: add flow parser demo
>   test: add flow parser library functional tests
> 
>  MAINTAINERS                                   |     6 +-
>  app/test-pmd/cmd_flex_item.c                  |    41 +-
>  app/test-pmd/cmdline.c                        |   267 +-
>  app/test-pmd/config.c                         |   112 +-
>  app/test-pmd/flow_parser.c                    |   409 +
>  app/test-pmd/flow_parser_cli.c                |   153 +
>  app/test-pmd/meson.build                      |     5 +-
>  app/test-pmd/testpmd.c                        |     4 +
>  app/test-pmd/testpmd.h                        |   122 +-
>  app/test/meson.build                          |     1 +
>  app/test/test_ethdev_api.c                    |    51 +
>  app/test/test_flow_parser.c                   |   923 ++
>  doc/api/doxy-api-index.md                     |     1 +
>  doc/api/doxy-api.conf.in                      |     1 +
>  doc/guides/prog_guide/flow_parser_lib.rst     |   110 +
>  doc/guides/prog_guide/index.rst               |     1 +
>  doc/guides/rel_notes/release_26_03.rst        |     9 +
>  examples/flow_parsing/main.c                  |   292 +
>  examples/flow_parsing/meson.build             |    11 +
>  examples/meson.build                          |     1 +
>  lib/cmdline/cmdline_parse.h                   |     2 +
>  lib/ethdev/rte_ethdev.c                       |   107 +
>  lib/ethdev/rte_ethdev.h                       |    55 +
>  lib/flow_parser/meson.build                   |     7 +
>  .../flow_parser/rte_flow_parser.c             | 11420 ++++++++--------
>  lib/flow_parser/rte_flow_parser.h             |   134 +
>  lib/flow_parser/rte_flow_parser_private.h     |  1247 ++
>  lib/meson.build                               |     2 +
>  28 files changed, 9671 insertions(+), 5823 deletions(-)
>  create mode 100644 app/test-pmd/flow_parser.c
>  create mode 100644 app/test-pmd/flow_parser_cli.c
>  create mode 100644 app/test/test_flow_parser.c
>  create mode 100644 doc/guides/prog_guide/flow_parser_lib.rst
>  create mode 100644 examples/flow_parsing/main.c
>  create mode 100644 examples/flow_parsing/meson.build
>  create mode 100644 lib/flow_parser/meson.build
>  rename app/test-pmd/cmdline_flow.c => lib/flow_parser/rte_flow_parser.c (55%)
>  create mode 100644 lib/flow_parser/rte_flow_parser.h
>  create mode 100644 lib/flow_parser/rte_flow_parser_private.h
> 

Queued to next-net.

PS: you made AI very happy...
Review Complete
I've thoroughly reviewed the 6-patch flow parser v10 series against AGENTS.md guidelines.
Bottom line: This is an excellent patchset - APPROVED with no errors or warnings.
The series introduces a new librte_flow_parser library that extracts testpmd's flow command parser into a reusable component. All patches follow DPDK conventions meticulously:

✅ Proper commit message formatting and tags
✅ All new APIs marked __rte_experimental with RTE_EXPORT_* macros
✅ Comprehensive testing (unit tests + functional tests + example app)
✅ Complete documentation (prog guide + API docs + release notes)
✅ Correct SPDX licensing on all files
✅ MAINTAINERS file updated appropriately

This demonstrates exemplary DPDK development practices.

  parent reply	other threads:[~2026-02-02 18:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-02 11:36 [PATCH v10 0/6] flow_parser: add shared parser library Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 1/6] cmdline: include stddef.h for MSVC compatibility Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 2/6] ethdev: add RSS type helper APIs Lukas Sismis
2026-02-17 14:40   ` Stephen Hemminger
2026-02-02 11:36 ` [PATCH v10 4/6] app/testpmd: use shared flow parser library Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 5/6] examples/flow_parsing: add flow parser demo Lukas Sismis
2026-02-02 11:36 ` [PATCH v10 6/6] test: add flow parser library functional tests Lukas Sismis
2026-02-02 18:37 ` Stephen Hemminger [this message]
     [not found] ` <20260202113659.24052-4-sismis@dyna-nic.com>
2026-02-02 20:03   ` [PATCH v10 3/6] flow_parser: add shared parser library Stephen Hemminger
2026-02-03  8:34     ` Lukáš Šišmiš
2026-02-04 14:53       ` Stephen Hemminger
2026-02-06 14:01         ` Thomas Monjalon
2026-02-06 15:40           ` Lukáš Šišmiš
2026-02-13  0:43             ` Stephen Hemminger
2026-02-13  7:46               ` Lukáš Šišmiš
2026-02-13 19:16                 ` Stephen Hemminger
2026-02-14  4:35                 ` Stephen Hemminger
2026-02-10 14:44   ` Stephen Hemminger
2026-02-10 14:45 ` [PATCH v10 0/6] " Stephen Hemminger
2026-02-12  8:56   ` Lukáš Šišmiš

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260202103754.3d996282@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=sismis@dyna-nic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.