Re: [RFC] doc: add AGENTS.md for AI-powered code review tools

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 9 Jan 2026 09:46:00 +0000
Bruce Richardson <bruce.richardson@intel.com> 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 <stephen@networkplumber.org>
> > ---  
> 
> 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: original_author@example.com
> > +  ```  
> 
> 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: stable@dpdk.org  
> 
> 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.