public inbox for dev@dpdk.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox