Message-ID: <20260124221439.234122-1-stephen@networkplumber.org>
Date: Sat, 24 Jan 2026 14:14:27 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: netdev@...r.kernel.org
Cc: Stephen Hemminger <stephen@...workplumber.org>
Subject: [PATCH iproute2-next] doc: add AGENTS.md for AI coding assistants

I (and probably others) are experimenting with AI code review
and dumentation tools. It is worthwhile to have a baseline
reference for coding assistants (such as Claude, Copilot,
Cursor, etc.) when working with the iproute2 codebase.

This document is a first draft and covers:
- Coding style based on Linux kernel guidelines with iproute2-specific
  exceptions.
- JSON output requirements using print_XXX helpers with PRINT_ANY
- Command-line argument parsing (strcmp for new code, not matches())
- Kernel compatibility and uapi header update procedures
- Patch submission guidelines including DCO requirements

This helps ensure AI-generated contributions follow project conventions
and reduces review burden from style issues. Changes and revisions
are welcome and expected.

Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
---
 AGENTS.md | 690 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 690 insertions(+)
 create mode 100644 AGENTS.md

diff --git a/AGENTS.md b/AGENTS.md
new file mode 100644
index 00000000..00349192
--- /dev/null
+++ b/AGENTS.md
@@ -0,0 +1,690 @@
+# AGENTS.md - AI Agent Guidelines for iproute2
+
+## Project Overview
+
+iproute2 is a collection of userspace utilities for controlling and monitoring
+networking in the Linux kernel. It includes tools like `ip`, `tc`, `ss`, `bridge`,
+and others. The project is tightly coupled with Linux kernel networking development.
+
+**Important**: iproute2 is userspace code, not kernel code. While the coding style
+is similar to the Linux kernel, some kernel-specific conventions do not apply.
+
+## Repository Information
+
+- **Stable repository**: `git://git.kernel.org/pub/scm/network/iproute2/iproute2.git`
+- **Development repository**: `git://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git`
+- **Mailing list**: `netdev@...r.kernel.org`
+- **Wiki**: https://wiki.linuxfoundation.org/networking/iproute2
+
+## Coding Style
+
+iproute2 follows Linux kernel coding style with some exceptions specific to
+userspace code. Reference: https://www.kernel.org/doc/html/latest/process/coding-style.html
+
+### Indentation
+
+- Use tabs for indentation (tabs are 8 characters)
+- Do not use spaces for indentation (except in continuation lines)
+- Switch and case labels are aligned in the same column:
+
+```c
+switch (suffix) {
+case 'G':
+case 'g':
+	mem <<= 30;
+	break;
+case 'M':
+case 'm':
+	mem <<= 20;
+	break;
+default:
+	break;
+}
+```
+
+### Line Length
+
+- Preferred limit: 100 columns
+- Longer lines are acceptable if breaking them reduces readability
+- Never break user-visible strings (e.g., error messages) - they must be grep-able
+
+### Braces
+
+**Functions**: Opening brace on a new line:
+
+```c
+int function(int x)
+{
+	body of function
+}
+```
+
+**Control structures** (if, switch, for, while, do): Opening brace on same line:
+
+```c
+if (x is true) {
+	we do y
+}
+```
+
+**Single statements**: Do not use braces where a single statement will do:
+
+```c
+if (condition)
+	action();
+```
+
+**Consistency rule**: If one branch needs braces, use braces on all branches:
+
+```c
+if (condition) {
+	do_this();
+	do_that();
+} else {
+	otherwise();
+}
+```
+
+### Spaces
+
+Use a space after these keywords:
+
+	if, switch, case, for, do, while
+
+Do NOT use a space after:
+
+	sizeof, typeof, alignof, __attribute__
+
+Examples:
+```c
+s = sizeof(struct file);      /* correct */
+s = sizeof( struct file );    /* WRONG */
+```
+
+Use one space around binary and ternary operators:
+```c
+=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
+```
+
+No space after unary operators:
+```c
+&  *  +  -  ~  !  sizeof  typeof  alignof  __attribute__
+```
+
+No space around `.` and `->` structure member operators.
+
+### Pointer Declarations
+
+The `*` is adjacent to the variable name, not the type:
+
+```c
+char *linux_banner;                          /* correct */
+char* linux_banner;                          /* WRONG */
+unsigned long long memparse(char *ptr, char **retptr);
+```
+
+### Naming
+
+- Use lowercase with underscores for names: `count_active_users`
+- Do NOT use CamelCase or Hungarian notation
+- Global variables and functions need descriptive names
+- Local variables should be short: `i` for loop counter, `tmp` for temporary
+- Avoid `master/slave` and `blacklist/whitelist` terminology
+
+### File Headers
+
+Every source file must start with an SPDX license identifier:
+
+```c
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * filename.c		Brief description
+ *
+ * Authors:	Name <email@...mple.org>
+ */
+```
+
+### Comments
+
+Multi-line comments use this format:
+
+```c
+/*
+ * This is the preferred style for multi-line
+ * comments in iproute2 source code.
+ *
+ * Description: A column of asterisks on the left side,
+ * with beginning and ending almost-blank lines.
+ */
+```
+
+Comments should explain WHAT the code does, not HOW. Avoid comments inside
+function bodies - if needed, the function may be too complex.
+
+### Variable Declarations
+
+**Note**: Unlike the Linux kernel, iproute2 does NOT require "Christmas tree"
+(reverse Christmas tree / reverse fir tree) style for variable declarations.
+Variables do not need to be ordered by decreasing line length.
+
+Acceptable:
+```c
+int ret;
+struct nlmsghdr *answer;
+const char *filter_dev = NULL;
+__u32 filt_mask = IFLA_STATS_FILTER_BIT(IFLA_STATS_AF_SPEC);
+```
+
+Use one declaration per line to allow for comments on each item.
+
+### Structure Initialization
+
+Use designated initializers for structures:
+
+```c
+struct iplink_req req = {
+	.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+	.n.nlmsg_flags = NLM_F_REQUEST,
+	.i.ifi_family = preferred_family,
+};
+```
+
+### Functions
+
+- Functions should be short and do one thing
+- Limit local variables to 5-10 per function
+- Separate functions with one blank line
+- Use goto for centralized cleanup (see below)
+
+### Centralized Exit with goto
+
+Use goto for cleanup when a function has multiple exit points:
+
+```c
+int fun(int a)
+{
+	int result = 0;
+	char *buffer;
+
+	buffer = malloc(SIZE);
+	if (!buffer)
+		return -ENOMEM;
+
+	if (condition1) {
+		while (loop1) {
+			...
+		}
+		result = 1;
+		goto out_free_buffer;
+	}
+	...
+out_free_buffer:
+	free(buffer);
+	return result;
+}
+```
+
+Use descriptive label names like `out_free_buffer:` not `err1:`.
+
+### Macros
+
+- Macro names defining constants should be CAPITALIZED
+- Enums are preferred over #define for related constants
+- Prefer inline functions over function-like macros
+- Multi-statement macros must use do-while:
+
+```c
+#define MACROFUN(a, b, c)		\
+	do {				\
+		if (a == 5)		\
+			do_this(b, c);	\
+	} while (0)
+```
+
+### Error Messages
+
+Use `fprintf(stderr, ...)` for error messages. This is critical to avoid
+corrupting JSON output on stdout:
+
+```c
+fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n",
+	msg, realval);
+```
+
+Do NOT use `printf()` or `fprintf(stdout, ...)` for error messages.
+
+## Command-Line Argument Parsing
+
+### Legacy Code vs New Code
+
+**Critical distinction for argument parsing:**
+
+- **Legacy code**: Uses `matches()` function which allows abbreviations
+- **New code**: Must use `strcmp()` for exact string comparison only
+
+The `matches()` function has caused problems with abbreviations in the past.
+For all new code, use only full string comparison:
+
+```c
+/* CORRECT - New code should use strcmp() */
+if (strcmp(*argv, "dev") == 0) {
+	NEXT_ARG();
+	/* ... */
+}
+
+/* LEGACY - matches() allows abbreviations, DO NOT use in new code */
+if (matches(*argv, "broadcast") == 0) {  /* matches "b", "br", "bro", etc. */
+	/* ... */
+}
+```
+
+### Argument Processing Macros
+
+Use the standard macros for argument iteration:
+
+```c
+NEXT_ARG();           /* Move to next argument, exit with error if none */
+NEXT_ARG_OK();        /* Check if next argument exists */
+PREV_ARG();           /* Move back one argument */
+```
+
+### Common Patterns
+
+```c
+while (argc > 0) {
+	if (strcmp(*argv, "dev") == 0) {
+		NEXT_ARG();
+		dev = *argv;
+	} else if (strcmp(*argv, "mtu") == 0) {
+		NEXT_ARG();
+		if (get_unsigned(&mtu, *argv, 0))
+			invarg("Invalid \"mtu\" value\n", *argv);
+	} else if (strcmp(*argv, "help") == 0) {
+		usage();
+	} else {
+		fprintf(stderr, "Unknown argument: %s\n", *argv);
+		exit(-1);
+	}
+	argc--; argv++;
+}
+```
+
+## JSON Output Support
+
+**All commands must support JSON output.** Use the helper functions from
+`json_print.h` to ensure consistent output in both regular and JSON modes.
+
+### Key Principles
+
+1. **Avoid special-casing JSON** - Let the library handle the differences
+2. **Use print_XXX helpers** - Do not use `fprintf(fp, ...)` directly for display output
+3. **Error messages to stderr** - Use `fprintf(stderr, ...)` for errors to avoid corrupting JSON
+4. **Pair open/close calls** - Every `open_json_object()` needs `close_json_object()`,
+   every `open_json_array()` needs `close_json_array()`
+
+### Initializing JSON Context
+
+```c
+#include "json_print.h"
+
+/* At the start of output */
+new_json_obj(json);      /* json is a global flag set by -json option */
+
+/* At the end of output */
+delete_json_obj();
+```
+
+### Correct Usage Pattern
+
+**Use `PRINT_ANY` to handle both JSON and text output in one call:**
+
+```c
+/* GOOD - single call handles both modes */
+print_uint(PRINT_ANY, "mtu", "mtu %u ", mtu);
+print_string(PRINT_ANY, "name", "%s ", name);
+
+/* BAD - do not special-case JSON and text separately */
+print_uint(PRINT_JSON, "foo", NULL, bar);
+print_uint(PRINT_FP, NULL, "foo %u", bar);
+```
+
+The correct pattern uses `PRINT_ANY` with both a JSON key and a format string,
+letting the library handle which output mode is active.
+
+### Color Support
+
+Use color variants for these value types to improve readability:
+
+- **Interface names** (e.g., "eth0", "enp0s3")
+- **MAC addresses** (e.g., "1a:0e:d4:cd:70:81")
+- **IPv4 addresses**
+- **IPv6 addresses**
+- **Operational state values**
+
+```c
+print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", ifname);
+print_color_string(PRINT_ANY, COLOR_MAC, "address", "%s ", mac_addr);
+print_color_string(PRINT_ANY, oper_state_color(state), "operstate", "%s ", state_str);
+```
+
+### Output Functions
+
+Use `PRINT_ANY` for output that works in both JSON and text modes:
+
+```c
+/* Simple values */
+print_string(PRINT_ANY, "name", "%s ", name);
+print_uint(PRINT_ANY, "mtu", "mtu %u ", mtu);
+print_u64(PRINT_ANY, "bytes", "%llu", bytes);
+print_bool(PRINT_ANY, "up", "%s", is_up);
+print_on_off(PRINT_ANY, "enabled", "%s ", enabled);
+```
+
+### JSON Objects and Arrays
+
+Objects and arrays must be properly paired:
+
+```c
+/* Objects - MUST pair open/close */
+open_json_object("linkinfo");
+print_string(PRINT_ANY, "kind", "    %s ", kind);
+close_json_object();  /* Required! */
+
+/* Arrays - MUST pair open/close */
+open_json_array(PRINT_ANY, is_json_context() ? "flags" : "<");
+print_string(PRINT_ANY, NULL, flags ? "%s," : "%s", "UP");
+close_json_array(PRINT_ANY, "> ");  /* Required! */
+```
+
+### Conditional Output
+
+For output that must differ between JSON and text modes (use sparingly):
+
+```c
+if (is_json_context()) {
+	print_uint(PRINT_JSON, "operstate_index", NULL, state);
+} else {
+	print_0xhex(PRINT_FP, NULL, "state %#llx", state);
+}
+```
+
+### Output Type Enum
+
+```c
+enum output_type {
+	PRINT_FP = 1,    /* Text output only */
+	PRINT_JSON = 2,  /* JSON output only */
+	PRINT_ANY = 4,   /* Both text and JSON (preferred) */
+};
+```
+
+### Available Print Functions
+
+From `json_print.h`:
+- `print_string()`, `print_uint()`, `print_int()`, `print_u64()`, `print_s64()`
+- `print_bool()`, `print_on_off()`, `print_null()`
+- `print_hex()`, `print_0xhex()`
+- `print_float()`, `print_rate()`, `print_tv()` (timeval)
+- `print_hu()` (unsigned short), `print_hhu()` (unsigned char)
+- `print_luint()` (unsigned long), `print_lluint()` (unsigned long long)
+- `print_nl()` - prints newline in non-JSON context only
+- Color variants: `print_color_string()`, `print_color_uint()`, etc.
+
+### Non-JSON Output Format
+
+The non-JSON (text) output format should be aligned with the command-line
+arguments. The output field names should match or closely correspond to
+the argument names users provide:
+
+```
+$ ip link show dev eth0
+2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP
+    link/ether 00:11:22:33:44:55 brd ff:ff:ff:ff:ff:ff
+```
+
+### Human-Readable vs Raw Values
+
+Commands that output rates, times, or byte counts should:
+
+- **Plaintext output**: Use existing helpers to format into human-readable terms
+- **JSON output**: Print raw numeric values (for script consumption)
+
+JSON output is intended for scripts and programmatic parsing, so raw values
+are more useful than formatted strings.
+
+Example:
+```
+$ tc qdisc show dev enp7s0
+qdisc fq_codel 0: parent :4 limit 10240p flows 1024 quantum 1514 target 5ms
+
+$ tc -j -p qdisc show dev enp7s0
+[ {
+        "kind": "fq_codel",
+        "handle": "0:",
+        "parent": ":4",
+        "options": {
+            "limit": 10240,
+            "flows": 1024,
+            "quantum": 1514,
+            "target": 4999,
+            "interval": 99999,
+            "memory_limit": 33554432,
+            "ecn": true,
+            "drop_batch": 64
+        }
+    } ]
+```
+
+Note how `target` shows as `5ms` in plaintext but `4999` (microseconds) in JSON.
+
+Use helpers like `print_rate()` which automatically handle this distinction.
+
+## Documentation
+
+### No Kernel Docbook Format
+
+iproute2 does **not** use the kernel's docbook documentation format.
+Function documentation should use simple C comments:
+
+```c
+/*
+ * Brief description of what the function does.
+ *
+ * Longer description if needed.
+ * Returns 0 on success, negative on failure.
+ */
+static int my_function(int argc, char **argv)
+```
+
+### Usage Functions
+
+Each command should have a `usage()` function that displays help:
+
+```c
+static void usage(void) __attribute__((noreturn));
+
+static void usage(void)
+{
+	fprintf(stderr,
+		"Usage: ip address {add|change|replace} IFADDR dev IFNAME\n"
+		"       ip address del IFADDR dev IFNAME\n"
+		"       ip address show [ dev IFNAME ]\n"
+		...);
+	exit(-1);
+}
+```
+
+## Submitting Patches
+
+### Patch Format
+
+Patches follow the Linux kernel patch submission guidelines:
+https://www.kernel.org/doc/html/latest/process/submitting-patches.html
+
+### Subject Line
+
+Use the appropriate prefix based on the target tree:
+
+```
+Subject: [PATCH iproute2] component: brief description
+Subject: [PATCH iproute2-next] component: brief description
+```
+
+Examples:
+```
+Subject: [PATCH iproute2-next] ip: fix syntax for rules
+Subject: [PATCH iproute2] tc: fix memory leak in filter parsing
+```
+
+### Commit Message
+
+- First line: brief summary (50 chars or less)
+- Blank line
+- Detailed description wrapped at 72 characters
+- Signed-off-by line
+
+```
+ip: add support for new feature
+
+Detailed explanation of what this patch does and why.
+Reference any relevant kernel commits if this adds support
+for new kernel features.
+
+Signed-off-by: Your Name <your.email@...mple.org>
+```
+
+### Signed-off-by and Developer Certificate of Origin
+
+The `Signed-off-by:` line certifies that you wrote the code or have the right
+to submit it, following the Developer's Certificate of Origin (DCO):
+https://developercertificate.org/
+
+By adding your Signed-off-by, you certify:
+- The contribution was created by you, or
+- It is based on previous work with a compatible license, or
+- It was provided to you by someone who certified the above
+
+Use `git commit -s` to automatically add your Signed-off-by line.
+
+### Sending Patches
+
+Send patches to the netdev mailing list:
+
+```
+git send-email --to=netdev@...r.kernel.org your-patch.patch
+```
+
+## Testing
+
+- Test both JSON and non-JSON output modes
+- Test with various kernel versions (features may not be available on older kernels)
+- Verify error handling with invalid inputs
+- Check for memory leaks with valgrind
+
+## Common Patterns
+
+### Netlink Request Structure
+
+```c
+struct {
+	struct nlmsghdr  n;
+	struct ifaddrmsg ifa;
+	char             buf[256];
+} req = {
+	.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)),
+	.n.nlmsg_flags = NLM_F_REQUEST | flags,
+	.n.nlmsg_type = cmd,
+	.ifa.ifa_family = preferred_family,
+};
+```
+
+### Adding Netlink Attributes
+
+```c
+addattr_l(&req.n, sizeof(req), IFA_LOCAL, &addr.data, addr.bytelen);
+addattr32(&req.n, sizeof(req), IFA_RT_PRIORITY, metric);
+addattr_nest(&req.n, sizeof(req), IFLA_PROP_LIST | NLA_F_NESTED);
+addattr_nest_end(&req.n, proplist);
+```
+
+### Standard Error Helpers
+
+```c
+invarg("invalid value", *argv);     /* Invalid argument value */
+duparg("device", *argv);            /* Duplicate argument */
+duparg2("dev", *argv);              /* Duplicate argument variant */
+missarg("required argument");       /* Missing required argument */
+nodev(devname);                     /* Device not found */
+```
+
+## Kernel Compatibility
+
+iproute2 aims to be compatible across a wide range of kernel versions. A newer
+version of iproute2 will work with older kernels (though new features may not
+be available), and older iproute2 versions work with newer kernels (but cannot
+access new features).
+
+### Sanitized Kernel Headers (uapi)
+
+The `include/uapi/` directory contains sanitized kernel headers that define
+the userspace API for networking. These headers are specific to iproute2 and
+allow the tools to be built with support for features that may not yet be
+present in the build system's kernel headers.
+
+These headers are generated from the kernel source tree using:
+
+```
+make headers_install
+```
+
+**Important rules for uapi updates:**
+
+1. **Separate patches** - Updates to `include/uapi/` must be in a separate patch
+   from the new functionality that uses them
+
+2. **Upstream first** - Changes to uapi headers will only be accepted once the
+   corresponding kernel patch has been merged upstream. Do not submit iproute2
+   patches that depend on unmerged (or potentially rejected) kernel changes
+
+3. **Reference kernel commits** - When updating uapi headers, reference the
+   upstream kernel commit in your patch description
+
+When adding support for new kernel features:
+
+1. Wait for the kernel patch to be merged upstream
+2. Submit a patch updating the relevant headers in `include/uapi/`
+3. Submit a separate patch adding the iproute2 functionality
+4. The code should handle older kernels gracefully - new attributes sent to
+   older kernels may be silently ignored or return an error
+5. Test with both old and new kernel versions when possible
+
+### Runtime Feature Detection
+
+Since iproute2 may run on kernels older than what it was built against:
+
+- Check return values from netlink requests - `EOPNOTSUPP` or similar errors
+  indicate the kernel doesn't support a feature
+- Don't assume features exist - the kernel may silently ignore unknown attributes
+- Provide helpful error messages when features aren't available
+
+## Files and Directories
+
+- `ip/` - ip command and subcommands
+- `tc/` - traffic control utilities
+- `bridge/` - bridge control utilities
+- `misc/` - miscellaneous utilities
+- `lib/` - shared library code
+- `include/` - header files
+- `include/uapi/` - sanitized kernel headers (from `make headers_install`)
+
+## Security Considerations
+
+- Validate all user input
+- Be careful with buffer sizes
+- Check return values from system calls and library functions
+- See SECURITY.md for vulnerability reporting procedures
+
+## License
+
+iproute2 is licensed under GPL-2.0-or-later. Some files may have dual licensing
+(GPL-2.0 OR BSD-2-Clause) - check the SPDX identifier in each file.
-- 
2.51.0

Powered by blists - more mailing lists