On Fri, 9 Jan 2026 09:46:00 +0000 Bruce Richardson <[email protected]> wrote:
> On Thu, Jan 08, 2026 at 05:41:04PM -0800, Stephen Hemminger wrote: > > Add a structured reference document that enables AI code review tools > > to validate DPDK contributions against project standards. This document > > consolidates requirements from multiple sources into a machine-readable > > format optimized for automated validation workflows. > > > > The AGENTS.md file synthesizes guidelines from: > > - DPDK Contributing Code documentation (patches.rst) > > - DPDK Coding Style guidelines (coding_style.rst) > > - Linux kernel patch submission process (submitting-patches.rst) > > - SPDX License Identifier specification (spdx.org) > > > > Key sections include: > > - SPDX license identifier requirements > > - Commit message format and tag ordering > > - C coding style rules and naming conventions > > - Patch validation checklists with severity levels > > - Meson build file style requirements > > > > The document provides actionable checklists and concrete examples to > > support integration with CI/CD pipelines and automated review systems. > > Severity levels (error/warning/info) help tools prioritize feedback > > appropriately. > > > > This supports the broader goal of maintaining code quality and > > consistency as the project scales, while reducing manual review burden > > for maintainers on mechanical style issues. > > > > References: > > - https://doc.dpdk.org/guides/contributing/patches.html > > - https://doc.dpdk.org/guides/contributing/coding_style.html > > - https://www.kernel.org/doc/html/latest/process/submitting-patches.html > > - https://spdx.org/licenses/ > > > > Signed-off-by: Stephen Hemminger <[email protected]> > > --- > > Good to see this being added. > > Feedback inline below. > > Thanks, > /Bruce > > > AGENTS.md | 410 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 410 insertions(+) > > create mode 100644 AGENTS.md > > > > diff --git a/AGENTS.md b/AGENTS.md > > new file mode 100644 > > index 0000000000..cfaaa81af3 > > --- /dev/null > > +++ b/AGENTS.md > > @@ -0,0 +1,410 @@ > > +# AGENTS.md - DPDK Code Review Guidelines for AI Tools > > + > > +This document provides guidelines for AI-powered code review tools when > > reviewing contributions to the Data Plane Development Kit (DPDK). It is > > derived from the official DPDK contributor guidelines. > > + > > +## Overview > > + > > +DPDK follows a development process modeled on the Linux Kernel. All > > patches are reviewed publicly on the mailing list before being merged. AI > > review tools should verify compliance with the standards outlined below. > > + > > +--- > > + > > +## Source License Requirements > > + > > +### SPDX License Identifiers > > + > > +- **Every file must begin with an SPDX license identifier** on the first > > line (or second line for `#!` scripts) > > +- Core libraries and drivers use `BSD-3-Clause` > > +- Kernel components use `GPL-2.0` > > +- Dual-licensed code uses: `(BSD-3-Clause OR GPL-2.0)` > > + > > +```c > > +/* Correct */ > > +/* SPDX-License-Identifier: BSD-3-Clause */ > > + > > +/* Incorrect - no boilerplate license text should follow */ > > +``` > > + > > +- A blank line must follow the license header before any other content > > + > > Not strictly true, the blank line follows the copyright line immediately > after the SPDX header. The copyright line should be included in > descriptions and examples above. I think that was something Linux kernel wanted. > > > +--- > > + > > +## Commit Message Requirements > > + > > +### Subject Line (First Line) > > + > > +- **Must capture the area and impact** of the change > > +- **~50 characters** recommended length > > In practice I think we allow up to 60 chars. So 50 chars recommended, 60 > char max. > > > +- **Lowercase** except for acronyms > > +- **Prefixed with component name** (check `git log` for existing > > components) > > +- Use **imperative mood** (instructions to the codebase) > > +- **No trailing period** (causes double periods in patch filenames) > > + > > +``` > > +# Good examples > > +ixgbe: fix offload config option name > > Should be "net/ixgbe". > Since you have a driver example below, I'd suggest picking an example for a > library e.g. hash, to include as well. I suspect AI got this from old examples. > > > +config: increase max queues per port > > +net/mlx5: add support for flow counters > > + > > +# Bad examples > > +Fixed the offload config option. # past tense, has period > > +IXGBE: Fix Offload Config # uppercase > > +``` > > + > > +### Commit Body > > + > > +- Describe the issue being fixed or feature being added > > +- Provide enough context for reviewers to understand the purpose > > +- Wrap text at **72 characters** > > +- **Must end with** `Signed-off-by:` line (real name, not alias) > > +- When fixing regressions, include: > > + ``` > > + Fixes: abcdefgh1234 ("original commit subject") > > + Cc: [email protected] > > + ``` > > I know we *should* CC original authors, but in practice I don't think we > do. So many original authors have moved on, I suspect that this guidance > just causes too many email bounces to be useful. I would update this to > Fixes and CC stable for all fixes. Let's not burden the contributor with > determining if a patch needs backport or not - I assume the stable tree > maintainers have tooling to pull out only relevant fixes for their trees. Would prefer that get_maintainer script used instead. > > + > > +### Required Tags > > + > > +``` > > +# For Coverity issues: > > +Coverity issue: 12345 > > + > > +# For Bugzilla issues: > > +Bugzilla ID: 12345 > > + > > +# For stable release backport candidates: > > +Cc: [email protected] > > I'd make this an always-add for bugfixes, unless it causes us other issues. Should be checkpatch rule? > > + > > +# For patch dependencies: > > +Depends-on: series-NNNNN ("Title of the series") > > +``` > > + > > +### Tag Order > > + > > +``` > > +Coverity issue: > > +Bugzilla ID: > > +Fixes: > > +Cc: > > + > > +Reported-by: > > +Suggested-by: > > +Signed-off-by: > > +Acked-by: > > +Reviewed-by: > > +Tested-by: > > +``` > > + > > +Note: Empty line between the first group and `Reported-by:` > > + > > +--- > > + > > +## C Coding Style > > + > > +### General Formatting > > + > > +- **Line length**: Recommended ≤80 characters, acceptable up to 100 > > I'd drop the 80 reference. If we can use 100 chars, let's do so, rather > than having lines split. > > > +- **Tab width**: 8 characters (hard tabs for indentation, spaces for > > alignment) > > +- **No trailing whitespace** on lines or at end of files > > +- Code style must be consistent within each file > > + > > +### Comments > > + > > +```c > > +/* Most single-line comments look like this. */ > > + > > +/* > > + * VERY important single-line comments look like this. > > + */ > > + > > +/* > > + * Multi-line comments look like this. Make them real sentences. Fill > > + * them so they look like real paragraphs. > > + */ > > +``` > > + > > +### Header File Organization > > + > > +Include order (each group separated by blank line): > > +1. System/libc includes > > +2. DPDK EAL includes > > +3. DPDK misc library includes > > +4. Application-specific includes > > + > > +```c > > +#include <stdio.h> > > +#include <stdlib.h> > > + > > +#include <rte_eal.h> > > + > > +#include <rte_ring.h> > > +#include <rte_mempool.h> > > + > > +#include "application.h" > > +``` > > + > > +### Header Guards > > + > > +```c > > +#ifndef _FILE_H_ > > +#define _FILE_H_ > > + > > +/* Code */ > > + > > +#endif /* _FILE_H_ */ > > +``` > > + > > +### Naming Conventions > > + > > +- **All external symbols** must have `RTE_` or `rte_` prefix > > +- **Macros**: ALL_UPPERCASE > > +- **Functions**: lowercase with underscores only (no CamelCase) > > +- **Variables**: lowercase with underscores only > > +- **Enum values**: ALL_UPPERCASE with `RTE_<ENUM>_` prefix > > +- **Struct types**: prefer `struct name` over typedefs > > + > > We can strengthen and clarify this, I think: for new definitions, typedefs > only for function pointer types. Should also add exception for base type code. + > > +### Structure Layout > > + > > +- Order members by: use, then size (largest to smallest), then > > alphabetically > > +- New additions to existing structures go at the end (ABI compatibility) > > +- Align member names with spaces > > > Nice to have, ignored in practice AFAIK. Maybe not worth including. > > > +- Avoid `bool` in structures (unclear size, wastes space) > > Disagree on this one. Unless it's a datapath struct that must be small, > using bool is a lot clearer and easier to manage than bit fields. I think this was inherited from kernel style. Lots of bool flags when bitfield could be used. > > > + > > +```c > > +struct foo { > > + struct foo *next; /* List of active foo. */ > > + struct mumble amumble; /* Comment for mumble. */ > > + int bar; /* Try to align the comments. */ > > +}; > > +``` > > + > > +--- > > + > > +## Code Quality Requirements > > + > > +### Compilation > > + > > +- Each commit must compile independently (for `git bisect`) > > +- No forward dependencies within a patchset > > +- Test with multiple targets, compilers, and options > > +- Use `devtools/test-meson-builds.sh` > > + > > +### Testing > > + > > +- Add tests to `app/test` unit test framework > > +- New API functions must be used in `/app` test directory > > +- New device APIs require at least one driver implementation > > + > > +### Documentation > > + > > +- Add Doxygen comments for public APIs > > +- Update release notes in `doc/guides/rel_notes/` for important changes > > +- Code and documentation must be updated atomically in same patch > > + > > +### ABI Compatibility > > + > > +- New external functions must be exported properly > > +- Follow ABI policy and versioning guidelines > > +- Enable ABI checks with `DPDK_ABI_REF_VERSION` environment variable > > + > > +--- > > + > > +## Patch Validation Checklist > > + > > +AI review tools should verify: > > + > > +### Commit Message > > +- [ ] Subject line ~50 chars, lowercase (except acronyms) > > 60 chars > > > +- [ ] Component prefix present and valid > > +- [ ] Imperative mood used > > +- [ ] No trailing period on subject > > +- [ ] Body wrapped at 72 characters > > +- [ ] `Signed-off-by:` present with real name > > +- [ ] `Fixes:` tag present for bug fixes with 12-char SHA > > +- [ ] Tags in correct order > > + > > +### License > > +- [ ] SPDX identifier on first line (or second for scripts) > > +- [ ] Appropriate license for file type > > +- [ ] Blank line after license header > > + > > +### Code Style > > +- [ ] Lines ≤100 characters (prefer ≤80) > > +- [ ] Hard tabs for indentation > > +- [ ] No trailing whitespace > > +- [ ] Proper include order > > +- [ ] Header guards present > > +- [ ] `rte_`/`RTE_` prefix on external symbols > > +- [ ] No prohibited terminology > > +- [ ] Proper brace style > > +- [ ] Function return type on own line > > +- [ ] NULL comparisons use `== NULL` > - [ ] Integer comparisons use `== 0` > > > + > > +### Structure > > +- [ ] Each commit compiles independently > > +- [ ] Code and docs updated together > > +- [ ] Tests added/updated as needed > > +- [ ] Release notes updated for significant changes > > + > > +--- > > + > > +## Meson Build Files > > + > > +### Style Requirements > > + > > +- 4-space indentation (no tabs) > > +- Line continuations double-indented > > +- Lists alphabetically ordered > > +- Short lists (≤3 items): single line, no trailing comma > > +- Long lists: one item per line, trailing comma on last item > > + > > +```python > > +# Short list > > +sources = files('file1.c', 'file2.c') > > + > > +# Long list > > +headers = files( > > + 'header1.h', > > + 'header2.h', > > + 'header3.h', > > +) > > +``` > > + > > +--- > > + > > +## Python Code > > + > > +- Must comply with PEP8 > > +- Line length acceptable up to 100 characters > > +- Use `pep8` tool for validation > > + > > I think we are switching to using "black" as the python formatting tool of > choice. > > > +--- > > + > > +## Review Process Notes > > + > > +### For AI Review Tools > > + > > +When providing feedback: > > +- Reference specific line numbers > > +- Cite the relevant guideline section > > +- Suggest concrete fixes > > +- Prioritize: errors > warnings > style suggestions > > +- Flag potential ABI breaks > > +- Check for missing documentation updates > > +- Verify test coverage for new functionality > > + > > +### Severity Levels > > + > > +**Error** (must fix): > > +- Missing SPDX license > > +- Missing Signed-off-by > > +- Compilation failures > > +- ABI breaks without proper versioning > > + > > +**Warning** (should fix): > > +- Style violations > > +- Missing Fixes tag for bug fixes > > +- Documentation gaps > > +- Missing tests > > + > > +**Info** (consider): > > +- Minor style preferences > > +- Optimization suggestions > > +- Alternative approaches > > -- > > 2.51.0 > > I will try and make a prompt to get a better result. Often good stuff happens with better prompt rather than manual edits.
