Re: [PATCH v3 0/7] flow_parser: add shared parser library

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 16 Jan 2026 00:01:06 +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, among others, the following APIs:
> - rte_flow_parser_parse(): Parse command strings into structured output
> - 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 shared library, ensuring a single
> maintained parser implementation.
> 
> Testing and Demo
> -------
> - Functional tests in dpdk-test
> - Example application: examples/flow_parsing
> 
> Changes
> -------
> 
> 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 (7):
>   cmdline: include stddef.h for offsetof
>   flow_parser: add shared parser library
>   app/testpmd: use shared flow parser library
>   examples/flow_parsing: add flow parser demo
>   dpdk-test: add flow parser library functional tests
>   mailmap: update a new contributor email
>   dts: fix invalid f-string syntax in testpmd API
> 
>  .mailmap                                      |     2 +-
>  MAINTAINERS                                   |     6 +-
>  app/test-pmd/cmd_flex_item.c                  |    41 +-
>  app/test-pmd/cmdline.c                        |   254 +-
>  app/test-pmd/config.c                         |   112 +-
>  app/test-pmd/flow_parser.c                    |   406 +
>  app/test-pmd/flow_parser.h                    |     8 +
>  app/test-pmd/flow_parser_cli.c                |   149 +
>  app/test-pmd/meson.build                      |     5 +-
>  app/test-pmd/testpmd.c                        |     4 +
>  app/test-pmd/testpmd.h                        |   126 +-
>  app/test/meson.build                          |     1 +
>  app/test/test_flow_parser.c                   |   698 +
>  doc/guides/prog_guide/flow_parser_lib.rst     |   140 +
>  doc/guides/prog_guide/index.rst               |     1 +
>  doc/guides/rel_notes/release_26_03.rst        |     9 +
>  dts/api/testpmd/__init__.py                   |     8 +-
>  examples/flow_parsing/main.c                  |   288 +
>  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                       |    60 +
>  lib/flow_parser/meson.build                   |     6 +
>  .../flow_parser/rte_flow_parser.c             | 11320 ++++++++--------
>  lib/flow_parser/rte_flow_parser.h             |  1306 ++
>  lib/meson.build                               |     2 +
>  27 files changed, 9300 insertions(+), 5773 deletions(-)
>  create mode 100644 app/test-pmd/flow_parser.c
>  create mode 100644 app/test-pmd/flow_parser.h
>  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
> 

Overall looks good, some AI comments.


---

## DPDK Patch Review: flow_parser library v3 (7 patches)

### Overview
This series introduces `librte_flow_parser` as an experimental library exposing the testpmd flow CLI parser as a reusable component, along with supporting changes.

---

## Patch 1/7: `cmdline: include stddef.h for offsetof`

### Commit Message ✅
| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 41 characters |
| Lowercase after colon | ✅ Pass | "include stddef.h..." |
| Correct prefix | ✅ Pass | `cmdline:` is correct for lib/cmdline |
| Imperative mood | ✅ Pass | "include" is imperative |
| No trailing period | ✅ Pass | |
| Body ≤75 chars | ✅ Pass | |
| Signed-off-by | ✅ Pass | Present with valid name/email |

### Code ✅
- Simple, correct fix
- No issues identified

**Verdict: PASS** - Ready to merge

---

## Patch 2/7: `flow_parser: add shared parser library`

### Commit Message

| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 38 characters |
| Lowercase after colon | ✅ Pass | |
| Correct prefix | ✅ Pass | `flow_parser:` for new lib |
| Body ≤75 chars | ✅ Pass | |
| Signed-off-by | ✅ Pass | |

**⚠️ Warning (typo in body):** Line 212 has "Lightweigth" → should be "Lightweight"

### License/Copyright ✅

| File | SPDX | Copyright | Blank line |
|------|------|-----------|------------|
| rte_flow_parser.c | ✅ BSD-3-Clause | ✅ "Copyright 2026 Dyna-NIC" | ✅ |
| rte_flow_parser.h | ✅ BSD-3-Clause | ✅ "Copyright(c) 2026 Dyna-NIC" | ✅ |
| meson.build | ✅ BSD-3-Clause | N/A (scripts don't require) | ✅ |

### API Tags ✅

- All new APIs correctly marked `__rte_experimental`
- `__rte_experimental` appears alone on line preceding return type (verified in header)
- Proper `RTE_EXPORT_EXPERIMENTAL_SYMBOL` usage in .c file

### Code Style

| Criteria | Status | Notes |
|----------|--------|-------|
| Lines ≤100 chars | ✅ Pass | |
| rte_ prefix on symbols | ✅ Pass | All external symbols use `rte_flow_parser_*` |
| Header guards | ✅ Pass | `RTE_FLOW_PARSER_H` |
| Include order | ✅ Pass | System, then DPDK, then local |

### Documentation ✅

- ✅ Programmer's guide added (`doc/guides/prog_guide/flow_parser_lib.rst`)
- ✅ Release notes updated (`doc/guides/rel_notes/release_26_03.rst`)
- ✅ MAINTAINERS updated
- ✅ Doxygen comments for public APIs

### Structure Concerns

**⚠️ Warning:** New ethdev APIs added in this patch:
- `rte_eth_rss_type_info_get()`
- `rte_eth_rss_type_from_str()`
- `rte_eth_rss_type_to_str()`
- New typedefs: `rte_port_id_t`, `rte_queue_id_t`

These ethdev additions should ideally be in a separate patch for easier review and bisectability.

**⚠️ Warning:** The new ethdev APIs (`rte_eth_rss_type_*`) are marked experimental but should have testpmd hooks and tests per DPDK guidelines.

### Meson Build ✅

```python
# SPDX-License-Identifier: BSD-3-Clause

sources = files('rte_flow_parser.c')
headers = files('rte_flow_parser.h')

deps += ['cmdline', 'ethdev']
```
- 4-space indentation ✅
- Short lists on single line ✅

**Verdict: PASS with warnings**

---

## Patch 3/7: `app/testpmd: integrate flow parser library`

### Commit Message ✅
| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 43 characters |
| Correct prefix | ✅ Pass | `app/testpmd:` |
| Signed-off-by | ✅ Pass | |

This is a large refactoring patch that moves code from testpmd to the new library. The changes look appropriate for the integration.

**Verdict: PASS**

---

## Patch 4/7: `examples/flow_parsing: add flow parser demo`

### Commit Message ✅
| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 42 characters |
| Correct prefix | ✅ Pass | `examples/flow_parsing:` |
| Signed-off-by | ✅ Pass | |

### License ✅
- SPDX-License-Identifier: BSD-3-Clause on line 1
- Copyright on line 2
- Blank line before code

### Code ✅
- Good example demonstrating the API
- Proper include order
- Comments explain the purpose

**Verdict: PASS**

---

## Patch 5/7: Copyright attribution fix

This patch updates copyright attribution in rte_flow_parser.c to include original authors (6WIND, Mellanox) since the code derives from testpmd's cmdline_flow.c.

**Verdict: PASS** - Good practice to maintain attribution

---

## Patch 6/7: `mailmap: update contributor email`

### Commit Message

| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 29 characters |
| Correct prefix | ✅ Pass | `mailmap:` |
| Signed-off-by | ✅ Pass | |

**❌ Error:** Commit body is empty. Per AGENTS.md guidelines, the body should describe the change.

**Recommendation:** Add body text like:
```
Update email address for Lukas Sismis from cesnet.cz to dyna-nic.com.
```

**Verdict: NEEDS WORK** - Add commit body

---

## Patch 7/7: `dts: fix invalid f-string syntax in testpmd API`

### Commit Message ✅

| Criteria | Status | Notes |
|----------|--------|-------|
| Subject ≤60 chars | ✅ Pass | 50 characters |
| Correct prefix | ✅ Pass | `dts:` |
| Fixes tag | ✅ Pass | Present with 12-char SHA |
| Cc: stable@dpdk.org | ✅ Pass | Present for bug fix |
| Tag order | ✅ Pass | Fixes, Cc, blank line, Signed-off-by |
| Signed-off-by | ✅ Pass | |

### Code ✅
The fix changes `""` to `''` inside f-strings, which is correct. Python f-strings require the inner quotes to differ from outer quotes.

**Verdict: PASS**

---

## Summary

| Patch | Status | Issues |
|-------|--------|--------|
| 1/7 cmdline | ✅ PASS | None |
| 2/7 flow_parser | ⚠️ PASS | Typo "Lightweigth", ethdev APIs should be separate patch |
| 3/7 testpmd integration | ✅ PASS | None |
| 4/7 example | ✅ PASS | None |
| 5/7 copyright | ✅ PASS | None |
| 6/7 mailmap | ❌ NEEDS WORK | Missing commit body |
| 7/7 dts fix | ✅ PASS | None |

### Required Fixes
1. **Patch 6/7:** Add commit message body describing the email update

### Recommended Improvements
1. **Patch 2/7:** Fix typo "Lightweigth" → "Lightweight" in commit body
2. **Patch 2/7:** Consider splitting ethdev API additions (`rte_eth_rss_type_*`, typedefs) into a separate prerequisite patch
3. **Patch 2/7:** Ensure new ethdev APIs have corresponding tests