{"owner":"DPDK","repo":"dpdk","hasSkills":true,"totalSkillsCount":4,"totalTokensCount":13026,"categories":["root-instruction","mcp-config","marketplace","plugin-manifest"],"hasMcp":true,"mcpConfig":{"mcpServers":{"dpdk":{"command":"npx","args":["-y","@modelcontextprotocol/server-dpdk"]}}},"found":["AGENTS.md",".cursor/mcp.json",".claude-plugin/marketplace.json",".claude-plugin/plugin.json"],"skills":{"AGENTS.md":"\n\n\n# AGENTS.md - DPDK Code Review Guidelines for AI Tools\n\n## CRITICAL INSTRUCTION - READ FIRST\n\nThis document has two categories of review rules with different confidence thresholds,\nplus rules on evidence and scope that apply to both.\n\n### 1. Correctness Bugs -- HIGHEST PRIORITY (report at >=50% confidence)\n\n**Always report potential correctness bugs.** These are the most valuable findings.\nWhen in doubt, report them -- subject to the evidence rule below.\nUncertainty about code you have read is fine:\na possible use-after-free or resource leak is worth mentioning even if you are not certain.\nUncertainty about code you have not read is not a finding at all.\n\nCorrectness bugs include:\n- Use-after-free (accessing memory after `free`/`rte_free`)\n- Resource leaks on error paths (memory, file descriptors, locks)\n- Double-free or double-close\n- NULL pointer dereference\n- Buffer overflows or out-of-bounds access\n- Uninitialized variable use in a reachable code path\n- Race conditions (unsynchronized shared state)\n- `volatile` used instead of atomic operations for inter-thread shared variables\n- Missing error checks on functions that can fail\n- Error paths that skip cleanup (goto labels, missing free/close)\n- Incorrect error propagation (wrong return value, lost errno)\n- Logic errors in conditionals (wrong operator, inverted test)\n- Integer overflow/truncation in size calculations\n- Missing bounds checks on user-supplied sizes or indices\n- Statistics accumulation using `=` instead of `+=`\n- Integer multiply without widening cast losing upper bits (16x16, 32x32, etc.)\n- Unbounded descriptor chain traversal on guest/API-supplied data\n- `1 << n` on 64-bit bitmask (must use `1ULL << n` or `RTE_BIT64()`)\n- Left shift of narrow unsigned (`uint8_t`/`uint16_t`) used as 64-bit value (sign extension via implicit `int` promotion)\n- Variable assigned then overwritten before being read (dead store)\n- Same variable used as loop counter in nested loops\n- `rte_mbuf_raw_free_bulk()` called on mbufs that may originate from different mempools (Tx burst, ring dequeue)\n- MTU confused with frame length (MTU is L3 payload; frame length = MTU + L2 overhead)\n- Using `dev_conf.rxmode.mtu` after configure instead of `dev->data->mtu`\n- Hardcoded Ethernet overhead instead of per-device calculation\n- MTU set without enabling `RTE_ETH_RX_OFFLOAD_SCATTER` when frame size exceeds mbuf data room\n- `mtu_set` callback rejects valid MTU when scatter Rx is already enabled\n- Rx queue setup silently drops oversized packets instead of enabling scatter or returning an error\n- Rx function selection ignores `scattered_rx` flag or MTU-vs-mbuf-size check\n\n**Do NOT self-censor correctness bugs.**\nIf you identify a code path where a resource could leak or memory could be used after free,\nreport it -- subject to the evidence rule below.\nDo not talk yourself out of a bug you can see.\nDo not talk yourself into one you cannot.\n\n### Evidence rule (applies to every finding)\n\nEvery finding must rest on code you have actually read.\n\"Read\" means a line in the patch, or a file you opened with a tool.\n\nThis document is used in two ways and the rule is the same in both.\n\n**Diff only** -- CI, `review-patch.py`, and any run without tool access.\nAll you have read is the patch: added and removed lines, plus a few lines of context.\nYou cannot see the rest of the file, other callers, or any other file in the tree,\nso any claim that depends on them cannot be made.\n\n**With tool access** -- CLI or agentic use, where you can clone the tree,\napply the series, grep, and read whole files.\nRead them.\nCheck the surrounding function, the callers, and the headers before you report.\nTool access does not lower the bar; it widens what you are able to read,\nand therefore what you are expected to have read before filing a finding.\n\nIn either mode:\n\n- Report a finding only if you can name the line that causes it and quote it verbatim.\n For anything the patch introduces or changes, that line must be an added or changed line.\n A note about surrounding code may rest on a line you read in the tree,\n and is Info only -- see \"Pre-existing code\" below.\n- The line you quote must itself show the defect you claim.\n If you claim a missing barrier before a read and the line you quote is a write,\n the finding is wrong.\n Delete it rather than reword the claim to fit the quote.\n- Never quote, paraphrase, or reconstruct code you have not read.\n If you are writing a code block with line numbers you did not read, you are inventing it.\n Delete the finding.\n- If a claim depends on code you have not read -- other callers, other files,\n the rest of the function -- either go read it or omit the claim.\n Do not file it with a caveat, do not call it a risk,\n and do not ask the author to verify it on your behalf.\n- \"presumably\", \"appears to be\", \"if this is\", \"potentially\", \"could be\" used to hedge\n *what the code says* mean you lack the evidence.\n Get it or omit the finding.\n A conditional about *runtime behaviour*, with the line in hand, is not a hedge\n and is what a correctness finding looks like:\n \"this leaks if `rte_malloc()` returns NULL\" is a finding, \"this appears to allocate\n somewhere\" is not.\n The test: would removing the qualifier require reading code you have not read?\n\nThe >=50% bar for correctness bugs applies to your judgement about code you have read.\nIt is not permission to speculate about code you have not.\n\n### Pre-existing code (applies to every finding)\n\nThe unit of review is the patch.\nCode that appears only as diff context, or that you read in the tree but the patch does not touch,\nis not what this author is being asked to fix.\nA finding about such code must never be an Error or a Warning\nand must never withhold a Reviewed-by.\n\nReport it as Info, and say plainly that it is pre-existing and not introduced by this patch.\nThis is worth doing: it puts the observation in front of the people working in that code\nso it can be picked up in a separate patch later.\n\nThe evidence rule still applies in full.\nAn Info note about surrounding code needs the same proof as any other finding:\nyou must have read the code and be able to quote the line.\n\"Pre-existing\" is a lower severity, not a lower standard of evidence.\n\n### Hardening suggestions (applies to every finding)\n\nValidating a value supplied by a cooperating peer -- a PF driver, firmware,\nor the device's own descriptors -- is a hardening suggestion, not a bug.\nReport it as Info.\n\nIt becomes a Warning or an Error only when the value crosses a trust boundary,\nor when it drives index, length, or state arithmetic:\n\n- Error: a buffer count from the PF used as a ring index or loop bound\n without being checked against `nb_desc`\n- Info: clamping a residual length field that a well-behaved PF would never\n get wrong, where an out-of-range value cannot escape the local computation\n\n\"A misbehaving firmware could send garbage\" is true of every field in every\nmailbox message. On its own it is not a severity.\n\n### 2. Style, Process, and Formatting -- suppress false positives\n\n**NEVER list a style/process item under \"Errors\" or \"Warnings\" if you conclude it is correct.**\n\nBefore outputting any style, formatting, or process error/warning, verify it is actually wrong.\nIf your analysis concludes with phrases like \"there's no issue here\", \"which is fine\",\n\"appears correct\", \"is acceptable\", or \"this is actually correct\" --\nthen DO NOT INCLUDE IT IN YOUR OUTPUT AT ALL.\nDelete it.\nOmit it entirely.\n\nThis suppression rule applies to: naming conventions, code style, and process compliance.\nIt does NOT apply to correctness bugs listed above.\n(SPDX/copyright format and commit message formatting are handled by checkpatch\nand are excluded from AI review entirely.)\n\n---\n\nThis document provides guidelines for AI-powered code review tools\nwhen reviewing contributions to the Data Plane Development Kit (DPDK).\nIt is derived from the official DPDK contributor guidelines and validation scripts.\n\n## Overview\n\nDPDK follows a development process modeled on the Linux Kernel.\nAll patches are reviewed publicly on the mailing list before being merged.\nAI review tools should verify compliance with the standards outlined below.\n\n## Review Philosophy\n\n**Correctness bugs are the primary goal of AI review.** Style and formatting checks are secondary.\nA review that catches a use-after-free but misses a style nit is far more valuable\nthan one that catches every style issue but misses the bug.\n\n**BEFORE OUTPUTTING YOUR REVIEW**: Re-read each item.\n- For correctness bugs: keep them.\n If you have reasonable doubt that a code path is safe, report it.\n- For style/process items: if ANY item contains phrases like \"is fine\", \"no issue\",\n \"appears correct\", \"is acceptable\", \"actually correct\" -- DELETE THAT ITEM.\n Do not include it.\n\n### Correctness review guidelines\n- Trace error paths: for every function that allocates a resource or acquires a lock,\n verify that ALL error paths after that point release it\n- Check every `goto error` and early `return`: does it clean up everything allocated so far?\n- Look for use-after-free: after `free(p)`, is `p` accessed again?\n- Check that error codes are propagated, not silently dropped\n- Report at >=50% confidence about code you have read\n- Never substitute a confidence caveat for evidence:\n if the reasoning depends on code you have not read, go read it or drop the finding\n- It is better to report a potential bug that turns out to be safe than to miss a real bug,\n provided you can point to the line that causes it\n\n### Style and process review guidelines\n- Only comment on style/process issues when you have HIGH CONFIDENCE (>80%) that an issue exists\n- Be concise: one sentence per comment when possible\n- Focus on actionable feedback, not observations\n- When reviewing text, only comment on clarity issues if the text is genuinely confusing\n or could lead to errors.\n- Do NOT comment on copyright years, SPDX format, or copyright holders -\n not subject to AI review\n- Do NOT report an issue then contradict yourself -\n if something is acceptable, do not mention it at all\n- Do NOT include items in Errors/Warnings that you then say are \"acceptable\" or \"correct\"\n- Do NOT mention things that are correct or \"not an issue\" - only report actual problems\n- Do NOT speculate about contributor circumstances (employment, company policies, etc.)\n- Before adding any style item to your review, ask: \"Is this actually wrong?\"\n If no, omit it entirely.\n- NEVER write \"(Correction: ...)\" - if you need to correct yourself, simply omit the item entirely\n- Do NOT add vague suggestions like \"should be verified\" or \"should be checked\" -\n either it's wrong or don't mention it\n- Do NOT flag something as an Error then say \"which is correct\" in the same item\n- Do NOT say \"no issue here\" or \"this is actually correct\" -\n if there's no issue, do not include it in your review\n- Do NOT analyze cross-patch dependencies or compilation order -\n you cannot reliably determine this from patch review\n- Do NOT claim a patch \"would cause compilation failure\" based on symbols used\n in other patches in the series\n- Review each patch individually for its own correctness;\n assume the patch author ordered them correctly\n- When reviewing a patch series, OMIT patches that have no issues.\n Do not include a patch in your output just to say \"no issues found\"\n or to summarize what the patch does.\n Only include patches where you have actual findings to report.\n- Do NOT end the review with a summary section listing every patch in the series.\n A clean patch must not appear anywhere in the output, including in a summary or status table.\n\n## Priority Areas (Review These)\n\n### Security & Safety\n- Unsafe code blocks without justification\n- Command injection risks (shell commands, user input)\n- Path traversal vulnerabilities\n- Credential exposure or hard coded secrets\n- Missing input validation on external data\n- Improper error handling that could leak sensitive info\n\n### Correctness Issues\n- Logic errors that could cause panics or incorrect behavior\n- Buffer overflows\n- Race conditions\n- **`volatile` for inter-thread synchronization**:\n `volatile` does not provide atomicity or memory ordering between threads.\n Use `rte_atomic_load_explicit()`/`rte_atomic_store_explicit()` with appropriate `rte_memory_order_*` instead.\n See the Shared Variable Access section under Forbidden Tokens for details.\n- Resource leaks (files, connections, memory)\n- Off-by-one errors or boundary conditions\n- Incorrect error propagation\n- **Use-after-free** (any access to memory after it has been freed)\n- **Error path resource leaks**:\n For every allocation or fd open,\n trace each error path (`goto`, early `return`, conditional) to verify the resource is released.\n Common patterns to check:\n - `malloc`/`rte_malloc` followed by a failure that does `return -1` instead of `goto cleanup`\n - `open()`/`socket()` fd not closed on a later error\n - Lock acquired but not released on an error branch\n - Partially initialized structure where early fields are allocated\n but later allocation fails without freeing the early ones\n- **Double-free / double-close**:\n resource freed in both a normal path and an error path,\n or fd closed but not set to -1 allowing a second close\n- **Missing error checks**:\n functions that can fail (malloc, open, ioctl, etc.) whose return value is not checked\n- **Do NOT flag unchecked return values from functions\n that always succeed on Linux.** Some POSIX functions are documented to always return zero on Linux\n when called with valid arguments.\n Flagging unchecked returns from these is a false positive.\n The list includes:\n - `pthread_mutex_init()`, `pthread_cond_init()`\n - `pthread_cond_signal()`, `pthread_cond_broadcast()`, `pthread_cond_wait()`\n - `pthread_condattr_init()`, `pthread_condattr_destroy()`\n - `pthread_attr_init()`, `pthread_attr_destroy()`\n- Changes to API without release notes\n- Changes to ABI on non-LTS release\n- Usage of deprecated API when replacements exist\n- Overly defensive code that adds unnecessary checks\n- Unnecessary comments that just restate what the code already shows (remove them)\n- **Process-shared synchronization errors** (pthread mutexes in shared memory without `PTHREAD_PROCESS_SHARED`)\n- **Statistics accumulation using `=` instead of `+=`**:\n When accumulating statistics (counters, byte totals, packet counts),\n using `=` overwrites the running total with only the latest value.\n This silently produces wrong results.\n ```c\n /* BAD - overwrites instead of accumulating */\n stats->rx_packets = nb_rx;\n stats->rx_bytes = total_bytes;\n\n /* GOOD - accumulates over time */\n stats->rx_packets += nb_rx;\n stats->rx_bytes += total_bytes;\n ```\n Note: `=` is correct for gauge-type values (e.g., queue depth, link status)\n and for initial assignment.\n Only flag when the context is clearly incremental accumulation\n (loop bodies, per-burst counters, callback tallies).\n- **Integer multiply without widening cast**:\n When multiplying integers to produce a result wider than the operands (sizes, offsets, byte counts),\n the multiplication is performed at the operand width\n and the upper bits are silently lost before the assignment.\n This applies to any narrowing scenario: 16x16 assigned to a 32-bit variable,\n 32x32 assigned to a 64-bit variable, etc.\n ```c\n /* BAD - 32x32 overflows before widening to 64 */\n uint64_t total_size = num_entries * entry_size; /* both are uint32_t */\n size_t offset = ring->idx * ring->desc_size; /* 32x32 -> truncated */\n\n /* BAD - 16x16 overflows before widening to 32 */\n uint32_t byte_count = pkt_len * nb_segs; /* both are uint16_t */\n\n /* GOOD - widen before multiply */\n uint64_t total_size = (uint64_t)num_entries * entry_size;\n size_t offset = (size_t)ring->idx * ring->desc_size;\n uint32_t byte_count = (uint32_t)pkt_len * nb_segs;\n ```\n- **Unbounded descriptor chain traversal**:\n When walking a chain of descriptors (virtio, DMA, NIC Rx/Tx rings) where the chain length\n or next-index comes from guest memory or an untrusted API caller,\n the traversal MUST have a bounds check or loop counter to prevent infinite loops\n or out-of-bounds access from malicious/corrupt data.\n ```c\n /* BAD - guest controls desc[idx].next with no bound */\n while (desc[idx].flags & VRING_DESC_F_NEXT) {\n idx = desc[idx].next; /* guest-supplied, unbounded */\n process(desc[idx]);\n }\n\n /* GOOD - cap iterations to descriptor ring size */\n for (i = 0; i < ring_size; i++) {\n if (!(desc[idx].flags & VRING_DESC_F_NEXT))\n break;\n idx = desc[idx].next;\n if (idx >= ring_size) /* bounds check */\n return -EINVAL;\n process(desc[idx]);\n }\n ```\n This applies to any chain/linked-list traversal where indices\n or pointers originate from untrusted input (guest VMs, user-space callers, network packets).\n- **Bitmask shift using `1` instead of `1ULL` on 64-bit masks**:\n The literal `1` is `int` (32 bits).\n Shifting it by 32 or more is undefined behavior; shifting it by less than 32\n but assigning to a `uint64_t` silently zeroes the upper 32 bits.\n Use `1ULL << n`, `UINT64_C(1) << n`, or the DPDK `RTE_BIT64(n)` macro.\n ```c\n /* BAD - 1 is int, UB if n >= 32, wrong if result used as uint64_t */\n uint64_t mask = 1 << bit_pos;\n if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) /* bit 15 OK, bit 32+ UB */\n\n /* GOOD */\n uint64_t mask = UINT64_C(1) << bit_pos;\n uint64_t mask = 1ULL << bit_pos;\n uint64_t mask = RTE_BIT64(bit_pos); /* preferred in DPDK */\n if (features & RTE_BIT64(VIRTIO_NET_F_MRG_RXBUF))\n ```\n Note: `1U << n` is acceptable when the mask is known to be 32-bit\n (e.g., `uint32_t` register fields with `n < 32`).\n Only flag when the result is stored in, compared against,\n or returned as a 64-bit type, or when `n` could be >= 32.\n- **Left shift of narrow unsigned type sign-extends to 64-bit**:\n When a `uint8_t` or `uint16_t` value is left-shifted,\n C integer promotion converts it to `int` (signed 32-bit) before the shift.\n If the result has bit 31 set, implicit conversion to `uint64_t`, `size_t`,\n or use in pointer arithmetic sign-extends the upper 32 bits to all-1s,\n producing a wrong address or value.\n This is Coverity SIGN_EXTENSION.\n The fix is to cast the narrow operand to an unsigned type\n at least as wide as the target before shifting.\n ```c\n /* BAD - uint16_t promotes to signed int, bit 31 may set,\n * then sign-extends when converted to 64-bit for pointer math */\n uint16_t idx = get_index();\n void *addr = base + (idx << wqebb_shift); /* SIGN_EXTENSION */\n uint64_t off = (uint64_t)(idx << shift); /* too late: shift already in int */\n\n /* BAD - uint8_t shift with result used as size_t */\n uint8_t page_order = get_order();\n size_t size = page_order << PAGE_SHIFT; /* promotes to int first */\n\n /* GOOD - cast before shift */\n void *addr = base + ((uint64_t)idx << wqebb_shift);\n uint64_t off = (uint64_t)idx << shift;\n size_t size = (size_t)page_order << PAGE_SHIFT;\n\n /* GOOD - intermediate unsigned variable */\n uint32_t offset = (uint32_t)idx << wqebb_shift; /* OK if result fits 32 bits */\n ```\n Note: This is distinct from the `1 << n` pattern (where the literal `1` is the problem)\n and from the integer-multiply pattern (where the operation is `*` not `<<`).\n The mechanism is the same C integer promotion rule,\n but the code patterns and Coverity checker names differ.\n Only flag when the shift result is used in a context wider than 32 bits (64-bit assignment,\n pointer arithmetic, function argument expecting `uint64_t`/`size_t`).\n A shift whose result is stored in a `uint32_t` or narrower variable is not affected.\n- **Variable overwrite before read (dead store)**:\n A variable is assigned a value that is unconditionally overwritten before it is ever read.\n This usually indicates a logic error (wrong variable name, missing `if`, copy-paste mistake)\n or at minimum is dead code.\n ```c\n /* BAD - first assignment is never read */\n ret = validate_input(cfg);\n ret = apply_config(cfg); /* overwrites without checking first ret */\n if (ret != 0)\n return ret;\n\n /* GOOD - check each return value */\n ret = validate_input(cfg);\n if (ret != 0)\n return ret;\n ret = apply_config(cfg);\n if (ret != 0)\n return ret;\n ```\n Do NOT flag cases where the initial value is intentionally a default that may\n or may not be overwritten (e.g., `int ret = 0;` followed by a conditional assignment).\n Only flag unconditional overwrites where the first value can never be observed.\n- **Shared loop counter in nested loops**:\n Using the same variable as the loop counter in both an outer\n and inner loop causes the outer loop to malfunction because the inner loop modifies its counter.\n ```c\n /* BAD - inner loop clobbers outer loop counter */\n int i;\n for (i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (i = 0; i < nb_descs; i++) /* BUG: reuses i */\n init_desc(i);\n }\n\n /* GOOD - distinct loop counters */\n for (int i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (int j = 0; j < nb_descs; j++)\n init_desc(j);\n }\n ```\n- **`rte_mbuf_raw_free_bulk()` on mixed-pool mbuf arrays**:\n Tx burst functions and ring/queue dequeue paths receive mbufs\n that may originate from different mempools (applications are free to send mbufs from any pool).\n `rte_mbuf_raw_free_bulk()` takes an explicit mempool parameter\n and calls `rte_mempool_put_bulk()` directly --\n ALL mbufs in the array must come from that single pool.\n If mbufs come from different pools, they are returned to the wrong pool,\n corrupting pool accounting and causing hard-to-debug failures.\n Note: `rte_pktmbuf_free_bulk()` is safe for mixed pools --\n it batches mbufs by pool internally and flushes whenever the pool changes.\n ```c\n /* BAD - assumes all mbufs are from the same pool */\n /* (in tx_burst completion or ring dequeue error path) */\n rte_mbuf_raw_free_bulk(mp, mbufs, nb_mbufs);\n\n /* GOOD - rte_pktmbuf_free_bulk handles mixed pools correctly */\n rte_pktmbuf_free_bulk(mbufs, nb_mbufs);\n\n /* GOOD - free individually (each mbuf returned to its own pool) */\n for (i = 0; i < nb_mbufs; i++)\n rte_pktmbuf_free(mbufs[i]);\n ```\n This applies to any path that frees mbufs submitted by the application:\n Tx completion, Tx error cleanup, and ring/queue drain paths.\n `rte_mbuf_raw_free_bulk()` is an optimization for the fast-free case (`RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE`)\n where the application guarantees all mbufs come from a single pool with refcnt=1.\n- **MTU confused with Ethernet frame length**:\n Maximum Transmission Unit (MTU) is the maximum L3 payload size\n (e.g., 1500 bytes for standard Ethernet).\n The maximum Ethernet *frame length* includes L2 overhead:\n Ethernet header (14 bytes) + optional VLAN tags (4 bytes each) + CRC (4 bytes).\n The overhead varies per device depending on supported encapsulations (VLAN, QinQ, etc.).\n Confusing MTU with frame length produces off-by-14-to-22-byte errors in packet size limits,\n buffer sizing, and scattered Rx decisions.\n\n **VLAN tag accounting:** The outer VLAN tag is L2 overhead\n and does NOT count toward MTU (matching Linux and FreeBSD).\n A 1522-byte single-tagged frame is valid at MTU 1500.\n However, in QinQ the inner (customer) tag DOES consume MTU --\n it is part of the customer frame.\n So QinQ with MTU 1500 allows only 1496 bytes of L3 payload\n unless the port MTU is raised to 1504.\n\n **Using `rxmode.mtu` after configure:** After `rte_eth_dev_configure()` completes,\n the canonical MTU is stored in `dev->data->mtu`.\n The `dev->data->dev_conf.rxmode.mtu` field is the user's *request*\n and must not be read after configure --\n it becomes stale if `rte_eth_dev_set_mtu()` is called later.\n Both configure and set_mtu write to `dev->data->mtu`; PMDs should always read from there.\n\n **Overhead calculation:** Do not hardcode a single overhead constant.\n Use the device's own overhead calculation\n (typically available via `dev_info.max_rx_pktlen - dev_info.max_mtu`\n or an internal `eth_overhead` field).\n Different devices support different encapsulations, so the overhead is not a universal constant.\n\n **Scattered Rx decision:** PMDs compare maximum frame length (MTU + per-device overhead)\n against Rx buffer size to decide whether scattered Rx is needed.\n Comparing raw MTU against buffer size is wrong --\n it underestimates the actual frame size by the overhead.\n ```c\n /* BAD - MTU used where frame length is needed */\n if (dev->data->mtu > rxq->buf_size)\n enable_scattered_rx();\n\n /* BAD - hardcoded overhead, wrong for QinQ-capable devices */\n #define ETHER_OVERHEAD 18 /* may be 22 or 26 for VLAN/QinQ */\n max_frame = mtu + ETHER_OVERHEAD;\n\n /* BAD - reading rxmode.mtu after configure (stale if set_mtu called) */\n static int\n mydrv_rx_queue_setup(...) {\n mtu = dev->data->dev_conf.rxmode.mtu; /* WRONG - may be stale */\n ...\n }\n\n /* GOOD - use dev->data->mtu, the canonical post-configure value */\n static int\n mydrv_rx_queue_setup(...) {\n uint16_t mtu = dev->data->mtu;\n ...\n }\n\n /* GOOD - use per-device overhead for frame length calculation */\n uint32_t frame_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;\n uint32_t max_frame_len = dev->data->mtu + frame_overhead;\n if (max_frame_len > rxq->buf_size)\n enable_scattered_rx();\n\n /* GOOD - device-specific overhead constant derived from capabilities */\n static uint32_t\n mydrv_eth_overhead(struct rte_eth_dev *dev) {\n uint32_t overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_VLAN)\n overhead += RTE_VLAN_HLEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_QINQ)\n overhead += RTE_VLAN_HLEN;\n return overhead;\n }\n ```\n Note: In `rte_eth_dev_configure()` itself, reading `rxmode.mtu` is correct --\n that is where the user's request is consumed and written to `dev->data->mtu`.\n Only flag reads of `rxmode.mtu` *outside* configure (queue setup, start, link update, MTU set, etc.).\n- **Missing scatter Rx for large MTU**:\n When the configured MTU produces a frame size (MTU + Ethernet overhead) larger\n than the mbuf data buffer size (`rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`),\n the PMD MUST either enable scatter Rx (multi-segment receive) or reject the configuration.\n Silently accepting the MTU and then truncating or dropping oversized packets is a correctness bug.\n ```c\n /* BAD - accepts MTU but will truncate packets that don't fit */\n static int\n mydrv_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)\n {\n /* No check against mbuf size or scatter capability */\n dev->data->mtu = mtu;\n return 0;\n }\n\n /* BAD - rejects valid MTU even though scatter is enabled */\n if (frame_size > mbuf_data_size)\n return -EINVAL; /* wrong: should allow if scatter is on */\n\n /* GOOD - check scatter and mbuf size */\n if (!dev->data->scattered_rx &&\n frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)\n return -EINVAL;\n\n /* GOOD - auto-enable scatter when needed */\n if (frame_size > mbuf_data_size) {\n if (!(dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SCATTER))\n return -EINVAL;\n dev->data->dev_conf.rxmode.offloads |=\n RTE_ETH_RX_OFFLOAD_SCATTER;\n dev->data->scattered_rx = 1;\n }\n ```\n Key relationships:\n - `dev_info.max_rx_pktlen`: maximum frame the hardware can receive\n - `dev_info.max_mtu`: maximum MTU = `max_rx_pktlen` - overhead\n - `dev_info.min_rx_bufsize`: minimum Rx buffer the HW requires\n - `dev_info.max_rx_bufsize`: maximum single-descriptor buffer size\n - `mbuf data size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`\n - When scatter is off: frame length must fit in a single mbuf\n - When scatter is on: frame length can span multiple mbufs;\n the PMD selects a scattered Rx function\n\n This pattern should be checked in three places:\n 1. `dev_configure()` -- validate MTU against mbuf size / scatter\n 2. `rx_queue_setup()` -- select scattered vs non-scattered Rx path\n 3. `mtu_set()` -- runtime MTU change must re-validate\n- **Rx queue function selection ignoring scatter**:\n When a PMD has separate fast-path Rx functions for scalar (single-segment)\n and scattered (multi-segment) modes,\n it must select the scattered variant whenever `dev->data->scattered_rx` is set OR\n when the configured frame length exceeds the single mbuf data size.\n Failing to do so causes the scalar Rx function to silently drop or corrupt multi-segment packets.\n ```c\n /* BAD - only checks offload flag, ignores actual need */\n if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER)\n rx_func = mydrv_recv_scattered;\n else\n rx_func = mydrv_recv_single; /* will drop oversized pkts */\n\n /* GOOD - check both the flag and the size */\n mbuf_size = rte_pktmbuf_data_room_size(rxq->mp) -\n RTE_PKTMBUF_HEADROOM;\n max_pkt = dev->data->mtu + overhead;\n if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||\n max_pkt > mbuf_size) {\n dev->data->scattered_rx = 1;\n rx_func = mydrv_recv_scattered;\n } else {\n rx_func = mydrv_recv_single;\n }\n ```\n\n### Cryptographic and Security Code\n\nApplies only when the patch touches crypto PMDs (`drivers/crypto/`),\nthe security or ipsec libraries,\nor code handling key material, IVs, or authentication digests.\nStays silent on all other patches.\n\n- **Non-constant-time digest/tag comparison** (Error):\n When verifying an authentication tag, digest, MAC, or ICV --\n comparing a computed value against an attacker-supplied one to decide accept/reject --\n plain `memcmp()` leaks timing information.\n Use `rte_memeq_timingsafe()` instead.\n Return semantics differ: `memcmp()` returns 0 on equal,\n `rte_memeq_timingsafe()` returns true on equal.\n So `if (memcmp(tag, digest, len))` becomes `if (!rte_memeq_timingsafe(tag, digest, len))`.\n Do NOT flag `memcmp()` on non-secret data:\n algorithm IDs, key lengths, capability/feature structs, lookup keys.\n Only flag comparisons that gate acceptance of attacker-influenced data.\n\n- **Sensitive material not zeroed before free** (Error):\n Keys, expanded key schedules, HMAC ipad/opad, and session secrets must be wiped,\n not merely freed -- a plain free leaves secrets in the heap.\n - `rte_malloc`/`rte_zmalloc`'d secret: use `rte_free_sensitive()` instead of `rte_free()`\n - local or stack secret going out of scope: `rte_memzero_explicit()` before return;\n plain `memset()` may be optimized away\n Flag only where the patch itself allocates, copies, or frees the secret.\n Do NOT flag buffers that never held secrets (descriptors, `dev_info`),\n pre-existing free paths the patch does not touch,\n or `memset()` the compiler cannot elide because the pointer escapes to `free()`.\n `rte_memzero_explicit()` followed by a plain `rte_free()` is correct\n and is not a missing `rte_free_sensitive()`.\n\n- **Non-cryptographic RNG for keys/IVs** (Error):\n `rte_rand()` is seeded from system entropy (`getentropy()`, with RDSEED and TSC fallbacks),\n but the generator itself is an LFSR and is linear:\n an observer who sees enough output can recover the state and predict every later value.\n Good seeding is not sufficient for key, IV, or nonce generation.\n Use a cryptographic source instead:\n the device's own entropy source or DRBG,\n or `getrandom(2)` / `/dev/urandom` in the control path.\n `rand()` and `random()` are weaker still and must not appear in crypto or security code.\n Do NOT flag `rte_rand()` used for non-secret purposes:\n hash seeds (`lib/ipsec/ipsec_sad.c` uses it for `hash_func_init_val`),\n jitter, backoff, load balancing, or test data.\n\nDo NOT flag:\n- IV/nonce reuse -- a runtime property, not determinable from a patch.\n- `memcmp()` on lengths, algorithm selectors, or non-secret config.\n\n### Architecture & Patterns\n- Code that violates existing patterns in the code base\n- Missing error handling\n- Code that is not safe against signals\n\n### New Library API Design\n\nWhen a patch adds a new library under `lib/`, review API design in addition to correctness and style.\n\n**API boundary.** A library should be a compiler, not a framework.\nThe model is `rte_acl`: create a context, feed input, get structured output,\ncaller decides what to do with it.\nNo callbacks needed.\nIf the library requires callers to implement a callback table to function,\nthe boundary is wrong -- the library is asking the caller to be its backend.\n\n**Callback structs** (Warning / Error).\nAny function-pointer struct in an installed header is an ABI break waiting to happen.\nAdding or reordering a member breaks all consumers.\n- Prefer a single callback parameter over an ops table.\n- \\>5 callbacks: **Warning** -- likely needs redesign.\n- \\>20 callbacks: **Error** -- this is an app plugin API, not a library.\n- All callbacks must have Doxygen (contract, return values, ownership).\n- Void-returning callbacks for failable operations swallow errors -- flag as **Error**.\n- Callbacks serving app-specific needs (e.g. `verbose_level_get`)\n indicate wrong code was extracted into the library.\n\n**Extensible structures.** Prefer TLV / tagged-array patterns over enum + union,\nfollowing `rte_flow_item` and `rte_flow_action` as the model.\nType tag + pointer to type-specific data allows adding types without ABI breaks.\nFlag as **Warning**:\n- Large enums (100+) consumers must switch on.\n- Unions that grow with every new feature.\n- Ask: \"What changes when a feature is added next release?\"\n If \"add an enum value and union arm\" -- should be TLV.\n\n**Installed headers.** If it's in `headers` or `indirect_headers` in meson.build, it's public API.\nDon't call it \"private.\"\nIf truly internal, don't install it.\n\n**Global state.** Prefer handle-based API (`create`/`destroy`) over singletons.\n`rte_acl` allows multiple independent classifier instances; new libraries should do the same.\n\n**Output ownership.** Prefer caller-allocated or library-allocated- caller-freed\nover internal static buffers.\nIf static buffers are used, document lifetime\nand ensure Doxygen examples don't show stale-pointer usage.\n\n---\n\n## C Coding Style\n\n### General Formatting\n\n- **Tab width**:\n 8 characters (hard tabs for indentation, spaces for alignment)\n- **No trailing whitespace** on lines or at end of files\n- Files must end with a new line\n- Code style should be consistent within each file\n\n### Comments\n\n```c\n/* Most single-line comments look like this. */\n\n/*\n * VERY important single-line comments look like this.\n */\n\n/*\n * Multi-line comments look like this. Make them real sentences. Fill\n * them so they look like real paragraphs.\n */\n```\n\n### Header File Organization\n\nInclude order (each group separated by blank line):\n1. System/libc includes\n2. DPDK EAL includes\n3. DPDK misc library includes\n4. Application-specific includes\n\n```c\n#include \n#include \n\n#include \n\n#include \n#include \n\n#include \"application.h\"\n```\n\n### Header Guards\n\n```c\n#ifndef _FILE_H_\n#define _FILE_H_\n\n/* Code */\n\n#endif /* _FILE_H_ */\n```\n\n### Naming Conventions\n\n- **All external symbols** must have `RTE_` or `rte_` prefix\n- **Macros**:\n ALL_UPPERCASE with `RTE_` prefix\n- **Functions**:\n lowercase with underscores only (no CamelCase)\n- **Variables**:\n lowercase with underscores only\n- **Enum values**:\n ALL_UPPERCASE with `RTE__` prefix\n\n**Exception**: Driver base directories (`drivers/*/base/`) may use different naming conventions\nwhen sharing code across platforms or with upstream vendor code.\n\n#### Symbol Naming for Static Linking\n\nDrivers and libraries must not expose global variables that could clash\nwhen statically linked with other DPDK components or applications.\nUse consistent and unique prefixes for all exported symbols to avoid namespace collisions.\n\n**Good practice**: Use a driver-specific or library-specific prefix for all global variables:\n\n```c\n/* Good - virtio driver uses consistent \"virtio_\" prefix */\nconst struct virtio_ops virtio_legacy_ops = {\n\t.read = virtio_legacy_read,\n\t.write = virtio_legacy_write,\n\t.configure = virtio_legacy_configure,\n};\n\nconst struct virtio_ops virtio_modern_ops = {\n\t.read = virtio_modern_read,\n\t.write = virtio_modern_write,\n\t.configure = virtio_modern_configure,\n};\n\n/* Good - mlx5 driver uses consistent \"mlx5_\" prefix */\nstruct mlx5_flow_driver_ops mlx5_flow_dv_ops;\n```\n\n**Bad practice**: Generic names that may clash:\n\n```c\n/* Bad - \"ops\" is too generic, will clash with other drivers */\nconst struct virtio_ops ops = { ... };\n\n/* Bad - \"legacy_ops\" could clash with other legacy implementations */\nconst struct virtio_ops legacy_ops = { ... };\n\n/* Bad - \"driver_config\" is not unique */\nstruct driver_config config;\n```\n\n**Guidelines**:\n- Prefix all global variables with the driver or library name (e.g., `virtio_`, `mlx5_`, `ixgbe_`)\n- Prefix all global functions similarly unless they use the `rte_` namespace\n- Internal static variables do not require prefixes as they have file scope\n- Consider using the `RTE_` or `rte_` prefix only for symbols that are part of the public DPDK API\n\n#### Prohibited Terminology\n\nDo not use non-inclusive naming including:\n- `master/slave` -> Use: primary/secondary, controller/worker, leader/follower\n- `blacklist/whitelist` -> Use: denylist/allowlist, blocklist/passlist\n- `cripple` -> Use: impacted, degraded, restricted, immobilized\n- `tribe` -> Use: team, squad\n- `sanity check` -> Use: coherence check, test, verification\n\n\n### Comparisons and Boolean Logic\n\nDPDK style requires explicit comparison against `NULL`, `0`, or `'\\0'` rather than\ntruthiness on pointers, integers, and characters: write `if (p == NULL)` not `if (!p)`,\nand `if (a != 0)` not `if (a)`.\nDirect truthiness is acceptable only on actual `bool` types.\n`likely()` and `unlikely()` wrappers do not change whether a comparison is explicit\n(`if (likely(p != NULL))` is still explicit).\n\nMechanical rewrites for these are handled by coccinelle in `devtools/cocci/`;\nAI review need only catch cases cocci can't see (e.g. macro-expanded conditions).\n\n### Boolean Usage\n\nPrefer `bool` (from ``) over `int` for variables, parameters,\nand return values that are purely true/false.\nUsing `bool` makes intent explicit, enables compiler diagnostics for misuse, and is self-documenting.\n\n```c\n/* Bad - int used as boolean flag */\nint verbose = 0;\nint is_enabled = 1;\n\nint\ncheck_valid(struct item *item)\n{\n\tif (item->flags & ITEM_VALID)\n\t\treturn 1;\n\treturn 0;\n}\n\n/* Good - bool communicates intent */\nbool verbose = false;\nbool is_enabled = true;\n\nbool\ncheck_valid(struct item *item)\n{\n\treturn item->flags & ITEM_VALID;\n}\n```\n\n**Guidelines:**\n- Use `bool` for variables that only hold true/false values\n- Use `bool` return type for predicate functions (functions that answer a yes/no question,\n often named `is_*`, `has_*`, `can_*`)\n- Use `true`/`false` rather than `1`/`0` for boolean assignments\n- Boolean variables and parameters should not use explicit comparison:\n `if (verbose)` is correct, not `if (verbose == true)`\n- `int` is still appropriate when a value can be negative, is an error code,\n or carries more than two states\n\n**Structure fields:**\n- `bool` occupies 1 byte.\n In packed or cache-critical structures, consider using a bitfield or flags word instead\n- For configuration structures and non-hot-path data, `bool` is preferred over `int` for flag fields\n\n```c\n/* Bad - int flags waste space and obscure intent */\nstruct port_config {\n\tint promiscuous; /* 0 or 1 */\n\tint link_up; /* 0 or 1 */\n\tint autoneg; /* 0 or 1 */\n\tuint16_t mtu;\n};\n\n/* Good - bool for flag fields */\nstruct port_config {\n\tbool promiscuous;\n\tbool link_up;\n\tbool autoneg;\n\tuint16_t mtu;\n};\n\n/* Also good - bitfield for cache-critical structures */\nstruct fast_path_config {\n\tuint32_t flags; /* bitmask of CONFIG_F_* */\n\t/* ... hot-path fields ... */\n};\n```\n\n**Do NOT flag:**\n- `int` return type for functions that return error codes (0 for success, negative for error) --\n these are NOT boolean\n- `int` used for tri-state or multi-state values\n- `int` flags in existing code where changing the type would be a large, unrelated refactor\n- Bitfield or flags-word approaches in performance-critical structures\n\n### Indentation and Braces\n\n```c\n/* Control statements - no braces for single statements */\nif (val != NULL)\n\tval = realloc(val, newsize);\n\n/* Braces on same line as else */\nif (test)\n\tstmt;\nelse if (bar) {\n\tstmt;\n\tstmt;\n} else\n\tstmt;\n\n/* Switch statements - don't indent case */\nswitch (ch) {\ncase 'a':\n\taflag = 1;\n\t/* FALLTHROUGH */\ncase 'b':\n\tbflag = 1;\n\tbreak;\ndefault:\n\tusage();\n}\n\n/* Long conditions - double indent continuation */\nif (really_long_variable_name_1 == really_long_variable_name_2 &&\n\t\treally_long_variable_name_3 == really_long_variable_name_4)\n\tstmt;\n```\n\n### Variable Declarations\n\n- Prefer declaring variables inside the basic block where they are used\n- Variables may be declared either at the start of the block, or at point of first use (C99 style)\n- Both declaration styles are acceptable; consistency within a function is preferred\n- Initialize variables only when a meaningful value exists at declaration time\n- Use C99 designated initializers for structures\n\n```c\n/* Good - declaration at start of block */\nint ret;\nret = some_function();\n\n/* Also good - declaration at point of use (C99 style) */\nfor (int i = 0; i < count; i++)\n\tprocess(i);\n\n/* Good - declaration in inner block where variable is used */\nif (condition) {\n\tint local_val = compute();\n\tuse(local_val);\n}\n\n/* Bad - unnecessary initialization defeats compiler warnings */\nint ret = 0;\nret = some_function(); /* Compiler won't warn if assignment removed */\n```\n\n### Function Format\n\n- Return type on its own line\n- Opening brace on its own line\n- Place an empty line between declarations and statements\n\n```c\nstatic char *\nfunction(int a1, int b1)\n{\n\tchar *p;\n\n\tp = do_something(a1, b1);\n\treturn p;\n}\n```\n\n---\n\n## Unnecessary Code Patterns\n\nThe following patterns add unnecessary code, hide bugs, or reduce performance.\nAvoid them.\nSeveral closely related patterns are caught by coccinelle scripts in `devtools/cocci/`\n(unnecessary `void *` casts, NULL checks before `free()`/`rte_free()`,\n`memset()` before `free()`, zero-length array members, dead variable initialization);\nthose are not duplicated here.\n\n### Appropriate Use of rte_malloc()\n\n`rte_malloc()` allocates from hugepage memory.\nUse it only when required:\n\n- Memory that will be accessed by DMA (NIC descriptors, packet buffers)\n- Memory shared between primary and secondary DPDK processes\n- Memory requiring specific NUMA node placement\n\nFor general allocations, use standard `malloc()` which is faster\nand does not consume limited hugepage resources.\n\nQueue-related buffers (descriptor rings, Rx/Tx queue control structures)\nshould use `rte_zmalloc_socket()` rather\nthan plain `rte_malloc()`: zero-initialization avoids stale descriptor bugs,\nthe `_socket` variant ensures NUMA-local allocation,\nand hugepage backing makes the memory visible to secondary processes.\n\n```c\n/* Bad - rte_malloc for ordinary data structure */\nstruct config *cfg = rte_malloc(NULL, sizeof(*cfg), 0);\n\n/* Good - standard malloc for control structures */\nstruct config *cfg = malloc(sizeof(*cfg));\n\n/* Good - rte_zmalloc_socket for descriptor rings / queue structures */\nstruct my_rx_desc *ring = rte_zmalloc_socket(\"rx_ring\",\n n * sizeof(*ring), RTE_CACHE_LINE_SIZE, socket_id);\n```\n\n### Appropriate Use of rte_memcpy()\n\n`rte_memcpy()` is optimized for bulk data transfer in the fast path.\nFor general use, standard `memcpy()` is preferred because:\n\n- Modern compilers optimize `memcpy()` effectively\n- `memcpy()` includes bounds checking with `_FORTIFY_SOURCE`\n- `memcpy()` handles small fixed-size copies efficiently\n\n```c\n/* Bad - rte_memcpy in control path */\nrte_memcpy(&config, &default_config, sizeof(config));\n\n/* Good - standard memcpy for control path */\nmemcpy(&config, &default_config, sizeof(config));\n\n/* Good - rte_memcpy for packet data in fast path */\nrte_memcpy(rte_pktmbuf_mtod(m, void *), payload, len);\n```\n\n### Non-const Function Pointer Arrays\n\nArrays of function pointers (ops tables, dispatch tables, callback arrays) should be declared `const`\nwhen their contents are fixed at compile time.\nA non-`const` function pointer array can be overwritten by bugs or exploits,\nand prevents the compiler from placing the table in read-only memory.\n\n```c\n/* Bad - mutable when it doesn't need to be */\nstatic rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n\n/* Good - immutable dispatch table */\nstatic const rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n```\n\n**Exceptions** (do NOT flag):\n- Arrays modified at runtime for CPU feature detection or capability probing (e.g.,\n selecting a burst function based on `rte_cpu_get_flag_enabled()`)\n- Arrays containing mutable state (e.g., entries that are linked into lists)\n- Arrays populated dynamically via registration API\n- `dev_ops` or similar structures assigned per-device at init time\n\nOnly flag when the array is fully initialized at declaration with constant values\nand never modified thereafter.\n\n---\n\n## Forbidden Tokens\n\n### Functions\n\nForbidden function additions (`rte_panic()`, `rte_exit()`, `perror()`, `printf()`,\n`fprintf()`, `getenv()` in `lib/` and `drivers/`, with appropriate per-function\nexceptions for EAL, `examples/`, and `app/test/`) are caught by `devtools/checkpatches.sh`.\n\n### Atomics and Memory Barriers\n\nSubstitutions from the deprecated atomic API to the C11 DPDK wrappers\n(`__atomic_*`/`__ATOMIC_*` -> `rte_atomic_*_explicit()`/`rte_memory_order_*`,\n`__sync_*` -> `rte_atomic_*`, `rte_smp_{r,w,}mb()` -> `rte_atomic_thread_fence()`,\n`rte_atomic{16,32,64}_*()` -> `rte_atomic_*_explicit()`)\nare handled by coccinelle scripts in `devtools/cocci/`.\nAI review only needs to flag conceptual misuse, covered in the sections below.\n\n#### Shared Variable Access: volatile vs Atomics\n\nVariables shared between threads or between a thread and a signal handler **must** use atomic operations.\nThe C `volatile` keyword is NOT a substitute for atomics\n-- it prevents compiler optimization of accesses but provides no atomicity guarantees\nand no memory ordering between threads.\nOn some architectures, `volatile` reads and writes may tear on unaligned or multi-word values.\n\nDPDK provides C11 atomic wrappers that are portable across all supported compilers and architectures.\nAlways use these for shared state.\n\n**Reading shared variables:**\n\n```c\n/* BAD - volatile provides no atomicity or ordering guarantee */\nvolatile int stop_flag;\nif (stop_flag) /* data race, compiler/CPU can reorder */\n return;\n\n/* BAD - direct access to shared variable without atomic */\nif (shared->running) /* undefined behavior if another thread writes */\n process();\n\n/* GOOD - DPDK C11 atomic wrapper */\nif (rte_atomic_load_explicit(&shared->stop_flag, rte_memory_order_acquire))\n return;\n\n/* GOOD - relaxed is fine for statistics or polling a flag where\n * you don't need to synchronize other memory accesses */\ncount = rte_atomic_load_explicit(&shared->count, rte_memory_order_relaxed);\n```\n\n**Writing shared variables:**\n\n```c\n/* BAD - volatile write */\nvolatile int *flag = &shared->ready;\n*flag = 1;\n\n/* GOOD - atomic store with appropriate ordering */\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n```\n\n**Read-modify-write operations:**\n\n```c\n/* BAD - not atomic even with volatile */\nvolatile uint64_t *counter = &stats->packets;\n*counter += nb_rx; /* TOCTOU: load, add, store is 3 operations */\n\n/* GOOD - atomic add */\nrte_atomic_fetch_add_explicit(&stats->packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Memory Ordering Guide\n\nUse the weakest ordering that is correct.\nStronger ordering constrains hardware and compiler optimization unnecessarily.\n\n| DPDK Ordering | When to Use |\n|---------------|-------------|\n| `rte_memory_order_relaxed` | Statistics counters, polling flags where no other data depends on the value. Most common for simple counters. |\n| `rte_memory_order_acquire` | **Load** side of a flag/pointer that guards access to other shared data. Ensures subsequent reads see data published by the releasing thread. |\n| `rte_memory_order_release` | **Store** side of a flag/pointer that publishes shared data. Ensures all prior writes are visible to a thread that does an acquire load. |\n| `rte_memory_order_acq_rel` | Read-modify-write operations (e.g., `fetch_add`) that both consume and publish shared state in one operation. |\n| `rte_memory_order_seq_cst` | Rarely needed. Only when multiple independent atomic variables must be observed in a globally consistent total order. Avoid unless required. |\n\n**Common pattern -- producer/consumer flag:**\n\n```c\n/* Producer thread: fill buffer, then signal ready */\nfill_buffer(buf, data, len);\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n\n/* Consumer thread: wait for flag, then read buffer */\nwhile (!rte_atomic_load_explicit(&shared->ready, rte_memory_order_acquire))\n rte_pause();\nprocess_buffer(buf, len); /* guaranteed to see producer's writes */\n```\n\n**Common pattern -- statistics counter (no ordering needed):**\n\n```c\nrte_atomic_fetch_add_explicit(&port_stats->rx_packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Standalone Fences\n\nPrefer ordering on the atomic operation itself (acquire load, release store) over standalone fences.\nStandalone fences (`rte_atomic_thread_fence()`) are a blunt instrument\nthat orders ALL memory accesses around the fence, ",".cursor/mcp.json":"# Model Context Protocol (MCP) Configuration\n\nPath: `.cursor/mcp.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.cursor/mcp.json)",".claude-plugin/marketplace.json":"# Claude Plugin Marketplace Catalog\n\nPath: `.claude-plugin/marketplace.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/marketplace.json)",".claude-plugin/plugin.json":"# claude-plugin Plugin Manifest\n\nPath: `.claude-plugin/plugin.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/plugin.json)"},"files":{"AGENTS.md":"\n\n\n# AGENTS.md - DPDK Code Review Guidelines for AI Tools\n\n## CRITICAL INSTRUCTION - READ FIRST\n\nThis document has two categories of review rules with different confidence thresholds,\nplus rules on evidence and scope that apply to both.\n\n### 1. Correctness Bugs -- HIGHEST PRIORITY (report at >=50% confidence)\n\n**Always report potential correctness bugs.** These are the most valuable findings.\nWhen in doubt, report them -- subject to the evidence rule below.\nUncertainty about code you have read is fine:\na possible use-after-free or resource leak is worth mentioning even if you are not certain.\nUncertainty about code you have not read is not a finding at all.\n\nCorrectness bugs include:\n- Use-after-free (accessing memory after `free`/`rte_free`)\n- Resource leaks on error paths (memory, file descriptors, locks)\n- Double-free or double-close\n- NULL pointer dereference\n- Buffer overflows or out-of-bounds access\n- Uninitialized variable use in a reachable code path\n- Race conditions (unsynchronized shared state)\n- `volatile` used instead of atomic operations for inter-thread shared variables\n- Missing error checks on functions that can fail\n- Error paths that skip cleanup (goto labels, missing free/close)\n- Incorrect error propagation (wrong return value, lost errno)\n- Logic errors in conditionals (wrong operator, inverted test)\n- Integer overflow/truncation in size calculations\n- Missing bounds checks on user-supplied sizes or indices\n- Statistics accumulation using `=` instead of `+=`\n- Integer multiply without widening cast losing upper bits (16x16, 32x32, etc.)\n- Unbounded descriptor chain traversal on guest/API-supplied data\n- `1 << n` on 64-bit bitmask (must use `1ULL << n` or `RTE_BIT64()`)\n- Left shift of narrow unsigned (`uint8_t`/`uint16_t`) used as 64-bit value (sign extension via implicit `int` promotion)\n- Variable assigned then overwritten before being read (dead store)\n- Same variable used as loop counter in nested loops\n- `rte_mbuf_raw_free_bulk()` called on mbufs that may originate from different mempools (Tx burst, ring dequeue)\n- MTU confused with frame length (MTU is L3 payload; frame length = MTU + L2 overhead)\n- Using `dev_conf.rxmode.mtu` after configure instead of `dev->data->mtu`\n- Hardcoded Ethernet overhead instead of per-device calculation\n- MTU set without enabling `RTE_ETH_RX_OFFLOAD_SCATTER` when frame size exceeds mbuf data room\n- `mtu_set` callback rejects valid MTU when scatter Rx is already enabled\n- Rx queue setup silently drops oversized packets instead of enabling scatter or returning an error\n- Rx function selection ignores `scattered_rx` flag or MTU-vs-mbuf-size check\n\n**Do NOT self-censor correctness bugs.**\nIf you identify a code path where a resource could leak or memory could be used after free,\nreport it -- subject to the evidence rule below.\nDo not talk yourself out of a bug you can see.\nDo not talk yourself into one you cannot.\n\n### Evidence rule (applies to every finding)\n\nEvery finding must rest on code you have actually read.\n\"Read\" means a line in the patch, or a file you opened with a tool.\n\nThis document is used in two ways and the rule is the same in both.\n\n**Diff only** -- CI, `review-patch.py`, and any run without tool access.\nAll you have read is the patch: added and removed lines, plus a few lines of context.\nYou cannot see the rest of the file, other callers, or any other file in the tree,\nso any claim that depends on them cannot be made.\n\n**With tool access** -- CLI or agentic use, where you can clone the tree,\napply the series, grep, and read whole files.\nRead them.\nCheck the surrounding function, the callers, and the headers before you report.\nTool access does not lower the bar; it widens what you are able to read,\nand therefore what you are expected to have read before filing a finding.\n\nIn either mode:\n\n- Report a finding only if you can name the line that causes it and quote it verbatim.\n For anything the patch introduces or changes, that line must be an added or changed line.\n A note about surrounding code may rest on a line you read in the tree,\n and is Info only -- see \"Pre-existing code\" below.\n- The line you quote must itself show the defect you claim.\n If you claim a missing barrier before a read and the line you quote is a write,\n the finding is wrong.\n Delete it rather than reword the claim to fit the quote.\n- Never quote, paraphrase, or reconstruct code you have not read.\n If you are writing a code block with line numbers you did not read, you are inventing it.\n Delete the finding.\n- If a claim depends on code you have not read -- other callers, other files,\n the rest of the function -- either go read it or omit the claim.\n Do not file it with a caveat, do not call it a risk,\n and do not ask the author to verify it on your behalf.\n- \"presumably\", \"appears to be\", \"if this is\", \"potentially\", \"could be\" used to hedge\n *what the code says* mean you lack the evidence.\n Get it or omit the finding.\n A conditional about *runtime behaviour*, with the line in hand, is not a hedge\n and is what a correctness finding looks like:\n \"this leaks if `rte_malloc()` returns NULL\" is a finding, \"this appears to allocate\n somewhere\" is not.\n The test: would removing the qualifier require reading code you have not read?\n\nThe >=50% bar for correctness bugs applies to your judgement about code you have read.\nIt is not permission to speculate about code you have not.\n\n### Pre-existing code (applies to every finding)\n\nThe unit of review is the patch.\nCode that appears only as diff context, or that you read in the tree but the patch does not touch,\nis not what this author is being asked to fix.\nA finding about such code must never be an Error or a Warning\nand must never withhold a Reviewed-by.\n\nReport it as Info, and say plainly that it is pre-existing and not introduced by this patch.\nThis is worth doing: it puts the observation in front of the people working in that code\nso it can be picked up in a separate patch later.\n\nThe evidence rule still applies in full.\nAn Info note about surrounding code needs the same proof as any other finding:\nyou must have read the code and be able to quote the line.\n\"Pre-existing\" is a lower severity, not a lower standard of evidence.\n\n### Hardening suggestions (applies to every finding)\n\nValidating a value supplied by a cooperating peer -- a PF driver, firmware,\nor the device's own descriptors -- is a hardening suggestion, not a bug.\nReport it as Info.\n\nIt becomes a Warning or an Error only when the value crosses a trust boundary,\nor when it drives index, length, or state arithmetic:\n\n- Error: a buffer count from the PF used as a ring index or loop bound\n without being checked against `nb_desc`\n- Info: clamping a residual length field that a well-behaved PF would never\n get wrong, where an out-of-range value cannot escape the local computation\n\n\"A misbehaving firmware could send garbage\" is true of every field in every\nmailbox message. On its own it is not a severity.\n\n### 2. Style, Process, and Formatting -- suppress false positives\n\n**NEVER list a style/process item under \"Errors\" or \"Warnings\" if you conclude it is correct.**\n\nBefore outputting any style, formatting, or process error/warning, verify it is actually wrong.\nIf your analysis concludes with phrases like \"there's no issue here\", \"which is fine\",\n\"appears correct\", \"is acceptable\", or \"this is actually correct\" --\nthen DO NOT INCLUDE IT IN YOUR OUTPUT AT ALL.\nDelete it.\nOmit it entirely.\n\nThis suppression rule applies to: naming conventions, code style, and process compliance.\nIt does NOT apply to correctness bugs listed above.\n(SPDX/copyright format and commit message formatting are handled by checkpatch\nand are excluded from AI review entirely.)\n\n---\n\nThis document provides guidelines for AI-powered code review tools\nwhen reviewing contributions to the Data Plane Development Kit (DPDK).\nIt is derived from the official DPDK contributor guidelines and validation scripts.\n\n## Overview\n\nDPDK follows a development process modeled on the Linux Kernel.\nAll patches are reviewed publicly on the mailing list before being merged.\nAI review tools should verify compliance with the standards outlined below.\n\n## Review Philosophy\n\n**Correctness bugs are the primary goal of AI review.** Style and formatting checks are secondary.\nA review that catches a use-after-free but misses a style nit is far more valuable\nthan one that catches every style issue but misses the bug.\n\n**BEFORE OUTPUTTING YOUR REVIEW**: Re-read each item.\n- For correctness bugs: keep them.\n If you have reasonable doubt that a code path is safe, report it.\n- For style/process items: if ANY item contains phrases like \"is fine\", \"no issue\",\n \"appears correct\", \"is acceptable\", \"actually correct\" -- DELETE THAT ITEM.\n Do not include it.\n\n### Correctness review guidelines\n- Trace error paths: for every function that allocates a resource or acquires a lock,\n verify that ALL error paths after that point release it\n- Check every `goto error` and early `return`: does it clean up everything allocated so far?\n- Look for use-after-free: after `free(p)`, is `p` accessed again?\n- Check that error codes are propagated, not silently dropped\n- Report at >=50% confidence about code you have read\n- Never substitute a confidence caveat for evidence:\n if the reasoning depends on code you have not read, go read it or drop the finding\n- It is better to report a potential bug that turns out to be safe than to miss a real bug,\n provided you can point to the line that causes it\n\n### Style and process review guidelines\n- Only comment on style/process issues when you have HIGH CONFIDENCE (>80%) that an issue exists\n- Be concise: one sentence per comment when possible\n- Focus on actionable feedback, not observations\n- When reviewing text, only comment on clarity issues if the text is genuinely confusing\n or could lead to errors.\n- Do NOT comment on copyright years, SPDX format, or copyright holders -\n not subject to AI review\n- Do NOT report an issue then contradict yourself -\n if something is acceptable, do not mention it at all\n- Do NOT include items in Errors/Warnings that you then say are \"acceptable\" or \"correct\"\n- Do NOT mention things that are correct or \"not an issue\" - only report actual problems\n- Do NOT speculate about contributor circumstances (employment, company policies, etc.)\n- Before adding any style item to your review, ask: \"Is this actually wrong?\"\n If no, omit it entirely.\n- NEVER write \"(Correction: ...)\" - if you need to correct yourself, simply omit the item entirely\n- Do NOT add vague suggestions like \"should be verified\" or \"should be checked\" -\n either it's wrong or don't mention it\n- Do NOT flag something as an Error then say \"which is correct\" in the same item\n- Do NOT say \"no issue here\" or \"this is actually correct\" -\n if there's no issue, do not include it in your review\n- Do NOT analyze cross-patch dependencies or compilation order -\n you cannot reliably determine this from patch review\n- Do NOT claim a patch \"would cause compilation failure\" based on symbols used\n in other patches in the series\n- Review each patch individually for its own correctness;\n assume the patch author ordered them correctly\n- When reviewing a patch series, OMIT patches that have no issues.\n Do not include a patch in your output just to say \"no issues found\"\n or to summarize what the patch does.\n Only include patches where you have actual findings to report.\n- Do NOT end the review with a summary section listing every patch in the series.\n A clean patch must not appear anywhere in the output, including in a summary or status table.\n\n## Priority Areas (Review These)\n\n### Security & Safety\n- Unsafe code blocks without justification\n- Command injection risks (shell commands, user input)\n- Path traversal vulnerabilities\n- Credential exposure or hard coded secrets\n- Missing input validation on external data\n- Improper error handling that could leak sensitive info\n\n### Correctness Issues\n- Logic errors that could cause panics or incorrect behavior\n- Buffer overflows\n- Race conditions\n- **`volatile` for inter-thread synchronization**:\n `volatile` does not provide atomicity or memory ordering between threads.\n Use `rte_atomic_load_explicit()`/`rte_atomic_store_explicit()` with appropriate `rte_memory_order_*` instead.\n See the Shared Variable Access section under Forbidden Tokens for details.\n- Resource leaks (files, connections, memory)\n- Off-by-one errors or boundary conditions\n- Incorrect error propagation\n- **Use-after-free** (any access to memory after it has been freed)\n- **Error path resource leaks**:\n For every allocation or fd open,\n trace each error path (`goto`, early `return`, conditional) to verify the resource is released.\n Common patterns to check:\n - `malloc`/`rte_malloc` followed by a failure that does `return -1` instead of `goto cleanup`\n - `open()`/`socket()` fd not closed on a later error\n - Lock acquired but not released on an error branch\n - Partially initialized structure where early fields are allocated\n but later allocation fails without freeing the early ones\n- **Double-free / double-close**:\n resource freed in both a normal path and an error path,\n or fd closed but not set to -1 allowing a second close\n- **Missing error checks**:\n functions that can fail (malloc, open, ioctl, etc.) whose return value is not checked\n- **Do NOT flag unchecked return values from functions\n that always succeed on Linux.** Some POSIX functions are documented to always return zero on Linux\n when called with valid arguments.\n Flagging unchecked returns from these is a false positive.\n The list includes:\n - `pthread_mutex_init()`, `pthread_cond_init()`\n - `pthread_cond_signal()`, `pthread_cond_broadcast()`, `pthread_cond_wait()`\n - `pthread_condattr_init()`, `pthread_condattr_destroy()`\n - `pthread_attr_init()`, `pthread_attr_destroy()`\n- Changes to API without release notes\n- Changes to ABI on non-LTS release\n- Usage of deprecated API when replacements exist\n- Overly defensive code that adds unnecessary checks\n- Unnecessary comments that just restate what the code already shows (remove them)\n- **Process-shared synchronization errors** (pthread mutexes in shared memory without `PTHREAD_PROCESS_SHARED`)\n- **Statistics accumulation using `=` instead of `+=`**:\n When accumulating statistics (counters, byte totals, packet counts),\n using `=` overwrites the running total with only the latest value.\n This silently produces wrong results.\n ```c\n /* BAD - overwrites instead of accumulating */\n stats->rx_packets = nb_rx;\n stats->rx_bytes = total_bytes;\n\n /* GOOD - accumulates over time */\n stats->rx_packets += nb_rx;\n stats->rx_bytes += total_bytes;\n ```\n Note: `=` is correct for gauge-type values (e.g., queue depth, link status)\n and for initial assignment.\n Only flag when the context is clearly incremental accumulation\n (loop bodies, per-burst counters, callback tallies).\n- **Integer multiply without widening cast**:\n When multiplying integers to produce a result wider than the operands (sizes, offsets, byte counts),\n the multiplication is performed at the operand width\n and the upper bits are silently lost before the assignment.\n This applies to any narrowing scenario: 16x16 assigned to a 32-bit variable,\n 32x32 assigned to a 64-bit variable, etc.\n ```c\n /* BAD - 32x32 overflows before widening to 64 */\n uint64_t total_size = num_entries * entry_size; /* both are uint32_t */\n size_t offset = ring->idx * ring->desc_size; /* 32x32 -> truncated */\n\n /* BAD - 16x16 overflows before widening to 32 */\n uint32_t byte_count = pkt_len * nb_segs; /* both are uint16_t */\n\n /* GOOD - widen before multiply */\n uint64_t total_size = (uint64_t)num_entries * entry_size;\n size_t offset = (size_t)ring->idx * ring->desc_size;\n uint32_t byte_count = (uint32_t)pkt_len * nb_segs;\n ```\n- **Unbounded descriptor chain traversal**:\n When walking a chain of descriptors (virtio, DMA, NIC Rx/Tx rings) where the chain length\n or next-index comes from guest memory or an untrusted API caller,\n the traversal MUST have a bounds check or loop counter to prevent infinite loops\n or out-of-bounds access from malicious/corrupt data.\n ```c\n /* BAD - guest controls desc[idx].next with no bound */\n while (desc[idx].flags & VRING_DESC_F_NEXT) {\n idx = desc[idx].next; /* guest-supplied, unbounded */\n process(desc[idx]);\n }\n\n /* GOOD - cap iterations to descriptor ring size */\n for (i = 0; i < ring_size; i++) {\n if (!(desc[idx].flags & VRING_DESC_F_NEXT))\n break;\n idx = desc[idx].next;\n if (idx >= ring_size) /* bounds check */\n return -EINVAL;\n process(desc[idx]);\n }\n ```\n This applies to any chain/linked-list traversal where indices\n or pointers originate from untrusted input (guest VMs, user-space callers, network packets).\n- **Bitmask shift using `1` instead of `1ULL` on 64-bit masks**:\n The literal `1` is `int` (32 bits).\n Shifting it by 32 or more is undefined behavior; shifting it by less than 32\n but assigning to a `uint64_t` silently zeroes the upper 32 bits.\n Use `1ULL << n`, `UINT64_C(1) << n`, or the DPDK `RTE_BIT64(n)` macro.\n ```c\n /* BAD - 1 is int, UB if n >= 32, wrong if result used as uint64_t */\n uint64_t mask = 1 << bit_pos;\n if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) /* bit 15 OK, bit 32+ UB */\n\n /* GOOD */\n uint64_t mask = UINT64_C(1) << bit_pos;\n uint64_t mask = 1ULL << bit_pos;\n uint64_t mask = RTE_BIT64(bit_pos); /* preferred in DPDK */\n if (features & RTE_BIT64(VIRTIO_NET_F_MRG_RXBUF))\n ```\n Note: `1U << n` is acceptable when the mask is known to be 32-bit\n (e.g., `uint32_t` register fields with `n < 32`).\n Only flag when the result is stored in, compared against,\n or returned as a 64-bit type, or when `n` could be >= 32.\n- **Left shift of narrow unsigned type sign-extends to 64-bit**:\n When a `uint8_t` or `uint16_t` value is left-shifted,\n C integer promotion converts it to `int` (signed 32-bit) before the shift.\n If the result has bit 31 set, implicit conversion to `uint64_t`, `size_t`,\n or use in pointer arithmetic sign-extends the upper 32 bits to all-1s,\n producing a wrong address or value.\n This is Coverity SIGN_EXTENSION.\n The fix is to cast the narrow operand to an unsigned type\n at least as wide as the target before shifting.\n ```c\n /* BAD - uint16_t promotes to signed int, bit 31 may set,\n * then sign-extends when converted to 64-bit for pointer math */\n uint16_t idx = get_index();\n void *addr = base + (idx << wqebb_shift); /* SIGN_EXTENSION */\n uint64_t off = (uint64_t)(idx << shift); /* too late: shift already in int */\n\n /* BAD - uint8_t shift with result used as size_t */\n uint8_t page_order = get_order();\n size_t size = page_order << PAGE_SHIFT; /* promotes to int first */\n\n /* GOOD - cast before shift */\n void *addr = base + ((uint64_t)idx << wqebb_shift);\n uint64_t off = (uint64_t)idx << shift;\n size_t size = (size_t)page_order << PAGE_SHIFT;\n\n /* GOOD - intermediate unsigned variable */\n uint32_t offset = (uint32_t)idx << wqebb_shift; /* OK if result fits 32 bits */\n ```\n Note: This is distinct from the `1 << n` pattern (where the literal `1` is the problem)\n and from the integer-multiply pattern (where the operation is `*` not `<<`).\n The mechanism is the same C integer promotion rule,\n but the code patterns and Coverity checker names differ.\n Only flag when the shift result is used in a context wider than 32 bits (64-bit assignment,\n pointer arithmetic, function argument expecting `uint64_t`/`size_t`).\n A shift whose result is stored in a `uint32_t` or narrower variable is not affected.\n- **Variable overwrite before read (dead store)**:\n A variable is assigned a value that is unconditionally overwritten before it is ever read.\n This usually indicates a logic error (wrong variable name, missing `if`, copy-paste mistake)\n or at minimum is dead code.\n ```c\n /* BAD - first assignment is never read */\n ret = validate_input(cfg);\n ret = apply_config(cfg); /* overwrites without checking first ret */\n if (ret != 0)\n return ret;\n\n /* GOOD - check each return value */\n ret = validate_input(cfg);\n if (ret != 0)\n return ret;\n ret = apply_config(cfg);\n if (ret != 0)\n return ret;\n ```\n Do NOT flag cases where the initial value is intentionally a default that may\n or may not be overwritten (e.g., `int ret = 0;` followed by a conditional assignment).\n Only flag unconditional overwrites where the first value can never be observed.\n- **Shared loop counter in nested loops**:\n Using the same variable as the loop counter in both an outer\n and inner loop causes the outer loop to malfunction because the inner loop modifies its counter.\n ```c\n /* BAD - inner loop clobbers outer loop counter */\n int i;\n for (i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (i = 0; i < nb_descs; i++) /* BUG: reuses i */\n init_desc(i);\n }\n\n /* GOOD - distinct loop counters */\n for (int i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (int j = 0; j < nb_descs; j++)\n init_desc(j);\n }\n ```\n- **`rte_mbuf_raw_free_bulk()` on mixed-pool mbuf arrays**:\n Tx burst functions and ring/queue dequeue paths receive mbufs\n that may originate from different mempools (applications are free to send mbufs from any pool).\n `rte_mbuf_raw_free_bulk()` takes an explicit mempool parameter\n and calls `rte_mempool_put_bulk()` directly --\n ALL mbufs in the array must come from that single pool.\n If mbufs come from different pools, they are returned to the wrong pool,\n corrupting pool accounting and causing hard-to-debug failures.\n Note: `rte_pktmbuf_free_bulk()` is safe for mixed pools --\n it batches mbufs by pool internally and flushes whenever the pool changes.\n ```c\n /* BAD - assumes all mbufs are from the same pool */\n /* (in tx_burst completion or ring dequeue error path) */\n rte_mbuf_raw_free_bulk(mp, mbufs, nb_mbufs);\n\n /* GOOD - rte_pktmbuf_free_bulk handles mixed pools correctly */\n rte_pktmbuf_free_bulk(mbufs, nb_mbufs);\n\n /* GOOD - free individually (each mbuf returned to its own pool) */\n for (i = 0; i < nb_mbufs; i++)\n rte_pktmbuf_free(mbufs[i]);\n ```\n This applies to any path that frees mbufs submitted by the application:\n Tx completion, Tx error cleanup, and ring/queue drain paths.\n `rte_mbuf_raw_free_bulk()` is an optimization for the fast-free case (`RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE`)\n where the application guarantees all mbufs come from a single pool with refcnt=1.\n- **MTU confused with Ethernet frame length**:\n Maximum Transmission Unit (MTU) is the maximum L3 payload size\n (e.g., 1500 bytes for standard Ethernet).\n The maximum Ethernet *frame length* includes L2 overhead:\n Ethernet header (14 bytes) + optional VLAN tags (4 bytes each) + CRC (4 bytes).\n The overhead varies per device depending on supported encapsulations (VLAN, QinQ, etc.).\n Confusing MTU with frame length produces off-by-14-to-22-byte errors in packet size limits,\n buffer sizing, and scattered Rx decisions.\n\n **VLAN tag accounting:** The outer VLAN tag is L2 overhead\n and does NOT count toward MTU (matching Linux and FreeBSD).\n A 1522-byte single-tagged frame is valid at MTU 1500.\n However, in QinQ the inner (customer) tag DOES consume MTU --\n it is part of the customer frame.\n So QinQ with MTU 1500 allows only 1496 bytes of L3 payload\n unless the port MTU is raised to 1504.\n\n **Using `rxmode.mtu` after configure:** After `rte_eth_dev_configure()` completes,\n the canonical MTU is stored in `dev->data->mtu`.\n The `dev->data->dev_conf.rxmode.mtu` field is the user's *request*\n and must not be read after configure --\n it becomes stale if `rte_eth_dev_set_mtu()` is called later.\n Both configure and set_mtu write to `dev->data->mtu`; PMDs should always read from there.\n\n **Overhead calculation:** Do not hardcode a single overhead constant.\n Use the device's own overhead calculation\n (typically available via `dev_info.max_rx_pktlen - dev_info.max_mtu`\n or an internal `eth_overhead` field).\n Different devices support different encapsulations, so the overhead is not a universal constant.\n\n **Scattered Rx decision:** PMDs compare maximum frame length (MTU + per-device overhead)\n against Rx buffer size to decide whether scattered Rx is needed.\n Comparing raw MTU against buffer size is wrong --\n it underestimates the actual frame size by the overhead.\n ```c\n /* BAD - MTU used where frame length is needed */\n if (dev->data->mtu > rxq->buf_size)\n enable_scattered_rx();\n\n /* BAD - hardcoded overhead, wrong for QinQ-capable devices */\n #define ETHER_OVERHEAD 18 /* may be 22 or 26 for VLAN/QinQ */\n max_frame = mtu + ETHER_OVERHEAD;\n\n /* BAD - reading rxmode.mtu after configure (stale if set_mtu called) */\n static int\n mydrv_rx_queue_setup(...) {\n mtu = dev->data->dev_conf.rxmode.mtu; /* WRONG - may be stale */\n ...\n }\n\n /* GOOD - use dev->data->mtu, the canonical post-configure value */\n static int\n mydrv_rx_queue_setup(...) {\n uint16_t mtu = dev->data->mtu;\n ...\n }\n\n /* GOOD - use per-device overhead for frame length calculation */\n uint32_t frame_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;\n uint32_t max_frame_len = dev->data->mtu + frame_overhead;\n if (max_frame_len > rxq->buf_size)\n enable_scattered_rx();\n\n /* GOOD - device-specific overhead constant derived from capabilities */\n static uint32_t\n mydrv_eth_overhead(struct rte_eth_dev *dev) {\n uint32_t overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_VLAN)\n overhead += RTE_VLAN_HLEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_QINQ)\n overhead += RTE_VLAN_HLEN;\n return overhead;\n }\n ```\n Note: In `rte_eth_dev_configure()` itself, reading `rxmode.mtu` is correct --\n that is where the user's request is consumed and written to `dev->data->mtu`.\n Only flag reads of `rxmode.mtu` *outside* configure (queue setup, start, link update, MTU set, etc.).\n- **Missing scatter Rx for large MTU**:\n When the configured MTU produces a frame size (MTU + Ethernet overhead) larger\n than the mbuf data buffer size (`rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`),\n the PMD MUST either enable scatter Rx (multi-segment receive) or reject the configuration.\n Silently accepting the MTU and then truncating or dropping oversized packets is a correctness bug.\n ```c\n /* BAD - accepts MTU but will truncate packets that don't fit */\n static int\n mydrv_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)\n {\n /* No check against mbuf size or scatter capability */\n dev->data->mtu = mtu;\n return 0;\n }\n\n /* BAD - rejects valid MTU even though scatter is enabled */\n if (frame_size > mbuf_data_size)\n return -EINVAL; /* wrong: should allow if scatter is on */\n\n /* GOOD - check scatter and mbuf size */\n if (!dev->data->scattered_rx &&\n frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)\n return -EINVAL;\n\n /* GOOD - auto-enable scatter when needed */\n if (frame_size > mbuf_data_size) {\n if (!(dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SCATTER))\n return -EINVAL;\n dev->data->dev_conf.rxmode.offloads |=\n RTE_ETH_RX_OFFLOAD_SCATTER;\n dev->data->scattered_rx = 1;\n }\n ```\n Key relationships:\n - `dev_info.max_rx_pktlen`: maximum frame the hardware can receive\n - `dev_info.max_mtu`: maximum MTU = `max_rx_pktlen` - overhead\n - `dev_info.min_rx_bufsize`: minimum Rx buffer the HW requires\n - `dev_info.max_rx_bufsize`: maximum single-descriptor buffer size\n - `mbuf data size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`\n - When scatter is off: frame length must fit in a single mbuf\n - When scatter is on: frame length can span multiple mbufs;\n the PMD selects a scattered Rx function\n\n This pattern should be checked in three places:\n 1. `dev_configure()` -- validate MTU against mbuf size / scatter\n 2. `rx_queue_setup()` -- select scattered vs non-scattered Rx path\n 3. `mtu_set()` -- runtime MTU change must re-validate\n- **Rx queue function selection ignoring scatter**:\n When a PMD has separate fast-path Rx functions for scalar (single-segment)\n and scattered (multi-segment) modes,\n it must select the scattered variant whenever `dev->data->scattered_rx` is set OR\n when the configured frame length exceeds the single mbuf data size.\n Failing to do so causes the scalar Rx function to silently drop or corrupt multi-segment packets.\n ```c\n /* BAD - only checks offload flag, ignores actual need */\n if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER)\n rx_func = mydrv_recv_scattered;\n else\n rx_func = mydrv_recv_single; /* will drop oversized pkts */\n\n /* GOOD - check both the flag and the size */\n mbuf_size = rte_pktmbuf_data_room_size(rxq->mp) -\n RTE_PKTMBUF_HEADROOM;\n max_pkt = dev->data->mtu + overhead;\n if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||\n max_pkt > mbuf_size) {\n dev->data->scattered_rx = 1;\n rx_func = mydrv_recv_scattered;\n } else {\n rx_func = mydrv_recv_single;\n }\n ```\n\n### Cryptographic and Security Code\n\nApplies only when the patch touches crypto PMDs (`drivers/crypto/`),\nthe security or ipsec libraries,\nor code handling key material, IVs, or authentication digests.\nStays silent on all other patches.\n\n- **Non-constant-time digest/tag comparison** (Error):\n When verifying an authentication tag, digest, MAC, or ICV --\n comparing a computed value against an attacker-supplied one to decide accept/reject --\n plain `memcmp()` leaks timing information.\n Use `rte_memeq_timingsafe()` instead.\n Return semantics differ: `memcmp()` returns 0 on equal,\n `rte_memeq_timingsafe()` returns true on equal.\n So `if (memcmp(tag, digest, len))` becomes `if (!rte_memeq_timingsafe(tag, digest, len))`.\n Do NOT flag `memcmp()` on non-secret data:\n algorithm IDs, key lengths, capability/feature structs, lookup keys.\n Only flag comparisons that gate acceptance of attacker-influenced data.\n\n- **Sensitive material not zeroed before free** (Error):\n Keys, expanded key schedules, HMAC ipad/opad, and session secrets must be wiped,\n not merely freed -- a plain free leaves secrets in the heap.\n - `rte_malloc`/`rte_zmalloc`'d secret: use `rte_free_sensitive()` instead of `rte_free()`\n - local or stack secret going out of scope: `rte_memzero_explicit()` before return;\n plain `memset()` may be optimized away\n Flag only where the patch itself allocates, copies, or frees the secret.\n Do NOT flag buffers that never held secrets (descriptors, `dev_info`),\n pre-existing free paths the patch does not touch,\n or `memset()` the compiler cannot elide because the pointer escapes to `free()`.\n `rte_memzero_explicit()` followed by a plain `rte_free()` is correct\n and is not a missing `rte_free_sensitive()`.\n\n- **Non-cryptographic RNG for keys/IVs** (Error):\n `rte_rand()` is seeded from system entropy (`getentropy()`, with RDSEED and TSC fallbacks),\n but the generator itself is an LFSR and is linear:\n an observer who sees enough output can recover the state and predict every later value.\n Good seeding is not sufficient for key, IV, or nonce generation.\n Use a cryptographic source instead:\n the device's own entropy source or DRBG,\n or `getrandom(2)` / `/dev/urandom` in the control path.\n `rand()` and `random()` are weaker still and must not appear in crypto or security code.\n Do NOT flag `rte_rand()` used for non-secret purposes:\n hash seeds (`lib/ipsec/ipsec_sad.c` uses it for `hash_func_init_val`),\n jitter, backoff, load balancing, or test data.\n\nDo NOT flag:\n- IV/nonce reuse -- a runtime property, not determinable from a patch.\n- `memcmp()` on lengths, algorithm selectors, or non-secret config.\n\n### Architecture & Patterns\n- Code that violates existing patterns in the code base\n- Missing error handling\n- Code that is not safe against signals\n\n### New Library API Design\n\nWhen a patch adds a new library under `lib/`, review API design in addition to correctness and style.\n\n**API boundary.** A library should be a compiler, not a framework.\nThe model is `rte_acl`: create a context, feed input, get structured output,\ncaller decides what to do with it.\nNo callbacks needed.\nIf the library requires callers to implement a callback table to function,\nthe boundary is wrong -- the library is asking the caller to be its backend.\n\n**Callback structs** (Warning / Error).\nAny function-pointer struct in an installed header is an ABI break waiting to happen.\nAdding or reordering a member breaks all consumers.\n- Prefer a single callback parameter over an ops table.\n- \\>5 callbacks: **Warning** -- likely needs redesign.\n- \\>20 callbacks: **Error** -- this is an app plugin API, not a library.\n- All callbacks must have Doxygen (contract, return values, ownership).\n- Void-returning callbacks for failable operations swallow errors -- flag as **Error**.\n- Callbacks serving app-specific needs (e.g. `verbose_level_get`)\n indicate wrong code was extracted into the library.\n\n**Extensible structures.** Prefer TLV / tagged-array patterns over enum + union,\nfollowing `rte_flow_item` and `rte_flow_action` as the model.\nType tag + pointer to type-specific data allows adding types without ABI breaks.\nFlag as **Warning**:\n- Large enums (100+) consumers must switch on.\n- Unions that grow with every new feature.\n- Ask: \"What changes when a feature is added next release?\"\n If \"add an enum value and union arm\" -- should be TLV.\n\n**Installed headers.** If it's in `headers` or `indirect_headers` in meson.build, it's public API.\nDon't call it \"private.\"\nIf truly internal, don't install it.\n\n**Global state.** Prefer handle-based API (`create`/`destroy`) over singletons.\n`rte_acl` allows multiple independent classifier instances; new libraries should do the same.\n\n**Output ownership.** Prefer caller-allocated or library-allocated- caller-freed\nover internal static buffers.\nIf static buffers are used, document lifetime\nand ensure Doxygen examples don't show stale-pointer usage.\n\n---\n\n## C Coding Style\n\n### General Formatting\n\n- **Tab width**:\n 8 characters (hard tabs for indentation, spaces for alignment)\n- **No trailing whitespace** on lines or at end of files\n- Files must end with a new line\n- Code style should be consistent within each file\n\n### Comments\n\n```c\n/* Most single-line comments look like this. */\n\n/*\n * VERY important single-line comments look like this.\n */\n\n/*\n * Multi-line comments look like this. Make them real sentences. Fill\n * them so they look like real paragraphs.\n */\n```\n\n### Header File Organization\n\nInclude order (each group separated by blank line):\n1. System/libc includes\n2. DPDK EAL includes\n3. DPDK misc library includes\n4. Application-specific includes\n\n```c\n#include \n#include \n\n#include \n\n#include \n#include \n\n#include \"application.h\"\n```\n\n### Header Guards\n\n```c\n#ifndef _FILE_H_\n#define _FILE_H_\n\n/* Code */\n\n#endif /* _FILE_H_ */\n```\n\n### Naming Conventions\n\n- **All external symbols** must have `RTE_` or `rte_` prefix\n- **Macros**:\n ALL_UPPERCASE with `RTE_` prefix\n- **Functions**:\n lowercase with underscores only (no CamelCase)\n- **Variables**:\n lowercase with underscores only\n- **Enum values**:\n ALL_UPPERCASE with `RTE__` prefix\n\n**Exception**: Driver base directories (`drivers/*/base/`) may use different naming conventions\nwhen sharing code across platforms or with upstream vendor code.\n\n#### Symbol Naming for Static Linking\n\nDrivers and libraries must not expose global variables that could clash\nwhen statically linked with other DPDK components or applications.\nUse consistent and unique prefixes for all exported symbols to avoid namespace collisions.\n\n**Good practice**: Use a driver-specific or library-specific prefix for all global variables:\n\n```c\n/* Good - virtio driver uses consistent \"virtio_\" prefix */\nconst struct virtio_ops virtio_legacy_ops = {\n\t.read = virtio_legacy_read,\n\t.write = virtio_legacy_write,\n\t.configure = virtio_legacy_configure,\n};\n\nconst struct virtio_ops virtio_modern_ops = {\n\t.read = virtio_modern_read,\n\t.write = virtio_modern_write,\n\t.configure = virtio_modern_configure,\n};\n\n/* Good - mlx5 driver uses consistent \"mlx5_\" prefix */\nstruct mlx5_flow_driver_ops mlx5_flow_dv_ops;\n```\n\n**Bad practice**: Generic names that may clash:\n\n```c\n/* Bad - \"ops\" is too generic, will clash with other drivers */\nconst struct virtio_ops ops = { ... };\n\n/* Bad - \"legacy_ops\" could clash with other legacy implementations */\nconst struct virtio_ops legacy_ops = { ... };\n\n/* Bad - \"driver_config\" is not unique */\nstruct driver_config config;\n```\n\n**Guidelines**:\n- Prefix all global variables with the driver or library name (e.g., `virtio_`, `mlx5_`, `ixgbe_`)\n- Prefix all global functions similarly unless they use the `rte_` namespace\n- Internal static variables do not require prefixes as they have file scope\n- Consider using the `RTE_` or `rte_` prefix only for symbols that are part of the public DPDK API\n\n#### Prohibited Terminology\n\nDo not use non-inclusive naming including:\n- `master/slave` -> Use: primary/secondary, controller/worker, leader/follower\n- `blacklist/whitelist` -> Use: denylist/allowlist, blocklist/passlist\n- `cripple` -> Use: impacted, degraded, restricted, immobilized\n- `tribe` -> Use: team, squad\n- `sanity check` -> Use: coherence check, test, verification\n\n\n### Comparisons and Boolean Logic\n\nDPDK style requires explicit comparison against `NULL`, `0`, or `'\\0'` rather than\ntruthiness on pointers, integers, and characters: write `if (p == NULL)` not `if (!p)`,\nand `if (a != 0)` not `if (a)`.\nDirect truthiness is acceptable only on actual `bool` types.\n`likely()` and `unlikely()` wrappers do not change whether a comparison is explicit\n(`if (likely(p != NULL))` is still explicit).\n\nMechanical rewrites for these are handled by coccinelle in `devtools/cocci/`;\nAI review need only catch cases cocci can't see (e.g. macro-expanded conditions).\n\n### Boolean Usage\n\nPrefer `bool` (from ``) over `int` for variables, parameters,\nand return values that are purely true/false.\nUsing `bool` makes intent explicit, enables compiler diagnostics for misuse, and is self-documenting.\n\n```c\n/* Bad - int used as boolean flag */\nint verbose = 0;\nint is_enabled = 1;\n\nint\ncheck_valid(struct item *item)\n{\n\tif (item->flags & ITEM_VALID)\n\t\treturn 1;\n\treturn 0;\n}\n\n/* Good - bool communicates intent */\nbool verbose = false;\nbool is_enabled = true;\n\nbool\ncheck_valid(struct item *item)\n{\n\treturn item->flags & ITEM_VALID;\n}\n```\n\n**Guidelines:**\n- Use `bool` for variables that only hold true/false values\n- Use `bool` return type for predicate functions (functions that answer a yes/no question,\n often named `is_*`, `has_*`, `can_*`)\n- Use `true`/`false` rather than `1`/`0` for boolean assignments\n- Boolean variables and parameters should not use explicit comparison:\n `if (verbose)` is correct, not `if (verbose == true)`\n- `int` is still appropriate when a value can be negative, is an error code,\n or carries more than two states\n\n**Structure fields:**\n- `bool` occupies 1 byte.\n In packed or cache-critical structures, consider using a bitfield or flags word instead\n- For configuration structures and non-hot-path data, `bool` is preferred over `int` for flag fields\n\n```c\n/* Bad - int flags waste space and obscure intent */\nstruct port_config {\n\tint promiscuous; /* 0 or 1 */\n\tint link_up; /* 0 or 1 */\n\tint autoneg; /* 0 or 1 */\n\tuint16_t mtu;\n};\n\n/* Good - bool for flag fields */\nstruct port_config {\n\tbool promiscuous;\n\tbool link_up;\n\tbool autoneg;\n\tuint16_t mtu;\n};\n\n/* Also good - bitfield for cache-critical structures */\nstruct fast_path_config {\n\tuint32_t flags; /* bitmask of CONFIG_F_* */\n\t/* ... hot-path fields ... */\n};\n```\n\n**Do NOT flag:**\n- `int` return type for functions that return error codes (0 for success, negative for error) --\n these are NOT boolean\n- `int` used for tri-state or multi-state values\n- `int` flags in existing code where changing the type would be a large, unrelated refactor\n- Bitfield or flags-word approaches in performance-critical structures\n\n### Indentation and Braces\n\n```c\n/* Control statements - no braces for single statements */\nif (val != NULL)\n\tval = realloc(val, newsize);\n\n/* Braces on same line as else */\nif (test)\n\tstmt;\nelse if (bar) {\n\tstmt;\n\tstmt;\n} else\n\tstmt;\n\n/* Switch statements - don't indent case */\nswitch (ch) {\ncase 'a':\n\taflag = 1;\n\t/* FALLTHROUGH */\ncase 'b':\n\tbflag = 1;\n\tbreak;\ndefault:\n\tusage();\n}\n\n/* Long conditions - double indent continuation */\nif (really_long_variable_name_1 == really_long_variable_name_2 &&\n\t\treally_long_variable_name_3 == really_long_variable_name_4)\n\tstmt;\n```\n\n### Variable Declarations\n\n- Prefer declaring variables inside the basic block where they are used\n- Variables may be declared either at the start of the block, or at point of first use (C99 style)\n- Both declaration styles are acceptable; consistency within a function is preferred\n- Initialize variables only when a meaningful value exists at declaration time\n- Use C99 designated initializers for structures\n\n```c\n/* Good - declaration at start of block */\nint ret;\nret = some_function();\n\n/* Also good - declaration at point of use (C99 style) */\nfor (int i = 0; i < count; i++)\n\tprocess(i);\n\n/* Good - declaration in inner block where variable is used */\nif (condition) {\n\tint local_val = compute();\n\tuse(local_val);\n}\n\n/* Bad - unnecessary initialization defeats compiler warnings */\nint ret = 0;\nret = some_function(); /* Compiler won't warn if assignment removed */\n```\n\n### Function Format\n\n- Return type on its own line\n- Opening brace on its own line\n- Place an empty line between declarations and statements\n\n```c\nstatic char *\nfunction(int a1, int b1)\n{\n\tchar *p;\n\n\tp = do_something(a1, b1);\n\treturn p;\n}\n```\n\n---\n\n## Unnecessary Code Patterns\n\nThe following patterns add unnecessary code, hide bugs, or reduce performance.\nAvoid them.\nSeveral closely related patterns are caught by coccinelle scripts in `devtools/cocci/`\n(unnecessary `void *` casts, NULL checks before `free()`/`rte_free()`,\n`memset()` before `free()`, zero-length array members, dead variable initialization);\nthose are not duplicated here.\n\n### Appropriate Use of rte_malloc()\n\n`rte_malloc()` allocates from hugepage memory.\nUse it only when required:\n\n- Memory that will be accessed by DMA (NIC descriptors, packet buffers)\n- Memory shared between primary and secondary DPDK processes\n- Memory requiring specific NUMA node placement\n\nFor general allocations, use standard `malloc()` which is faster\nand does not consume limited hugepage resources.\n\nQueue-related buffers (descriptor rings, Rx/Tx queue control structures)\nshould use `rte_zmalloc_socket()` rather\nthan plain `rte_malloc()`: zero-initialization avoids stale descriptor bugs,\nthe `_socket` variant ensures NUMA-local allocation,\nand hugepage backing makes the memory visible to secondary processes.\n\n```c\n/* Bad - rte_malloc for ordinary data structure */\nstruct config *cfg = rte_malloc(NULL, sizeof(*cfg), 0);\n\n/* Good - standard malloc for control structures */\nstruct config *cfg = malloc(sizeof(*cfg));\n\n/* Good - rte_zmalloc_socket for descriptor rings / queue structures */\nstruct my_rx_desc *ring = rte_zmalloc_socket(\"rx_ring\",\n n * sizeof(*ring), RTE_CACHE_LINE_SIZE, socket_id);\n```\n\n### Appropriate Use of rte_memcpy()\n\n`rte_memcpy()` is optimized for bulk data transfer in the fast path.\nFor general use, standard `memcpy()` is preferred because:\n\n- Modern compilers optimize `memcpy()` effectively\n- `memcpy()` includes bounds checking with `_FORTIFY_SOURCE`\n- `memcpy()` handles small fixed-size copies efficiently\n\n```c\n/* Bad - rte_memcpy in control path */\nrte_memcpy(&config, &default_config, sizeof(config));\n\n/* Good - standard memcpy for control path */\nmemcpy(&config, &default_config, sizeof(config));\n\n/* Good - rte_memcpy for packet data in fast path */\nrte_memcpy(rte_pktmbuf_mtod(m, void *), payload, len);\n```\n\n### Non-const Function Pointer Arrays\n\nArrays of function pointers (ops tables, dispatch tables, callback arrays) should be declared `const`\nwhen their contents are fixed at compile time.\nA non-`const` function pointer array can be overwritten by bugs or exploits,\nand prevents the compiler from placing the table in read-only memory.\n\n```c\n/* Bad - mutable when it doesn't need to be */\nstatic rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n\n/* Good - immutable dispatch table */\nstatic const rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n```\n\n**Exceptions** (do NOT flag):\n- Arrays modified at runtime for CPU feature detection or capability probing (e.g.,\n selecting a burst function based on `rte_cpu_get_flag_enabled()`)\n- Arrays containing mutable state (e.g., entries that are linked into lists)\n- Arrays populated dynamically via registration API\n- `dev_ops` or similar structures assigned per-device at init time\n\nOnly flag when the array is fully initialized at declaration with constant values\nand never modified thereafter.\n\n---\n\n## Forbidden Tokens\n\n### Functions\n\nForbidden function additions (`rte_panic()`, `rte_exit()`, `perror()`, `printf()`,\n`fprintf()`, `getenv()` in `lib/` and `drivers/`, with appropriate per-function\nexceptions for EAL, `examples/`, and `app/test/`) are caught by `devtools/checkpatches.sh`.\n\n### Atomics and Memory Barriers\n\nSubstitutions from the deprecated atomic API to the C11 DPDK wrappers\n(`__atomic_*`/`__ATOMIC_*` -> `rte_atomic_*_explicit()`/`rte_memory_order_*`,\n`__sync_*` -> `rte_atomic_*`, `rte_smp_{r,w,}mb()` -> `rte_atomic_thread_fence()`,\n`rte_atomic{16,32,64}_*()` -> `rte_atomic_*_explicit()`)\nare handled by coccinelle scripts in `devtools/cocci/`.\nAI review only needs to flag conceptual misuse, covered in the sections below.\n\n#### Shared Variable Access: volatile vs Atomics\n\nVariables shared between threads or between a thread and a signal handler **must** use atomic operations.\nThe C `volatile` keyword is NOT a substitute for atomics\n-- it prevents compiler optimization of accesses but provides no atomicity guarantees\nand no memory ordering between threads.\nOn some architectures, `volatile` reads and writes may tear on unaligned or multi-word values.\n\nDPDK provides C11 atomic wrappers that are portable across all supported compilers and architectures.\nAlways use these for shared state.\n\n**Reading shared variables:**\n\n```c\n/* BAD - volatile provides no atomicity or ordering guarantee */\nvolatile int stop_flag;\nif (stop_flag) /* data race, compiler/CPU can reorder */\n return;\n\n/* BAD - direct access to shared variable without atomic */\nif (shared->running) /* undefined behavior if another thread writes */\n process();\n\n/* GOOD - DPDK C11 atomic wrapper */\nif (rte_atomic_load_explicit(&shared->stop_flag, rte_memory_order_acquire))\n return;\n\n/* GOOD - relaxed is fine for statistics or polling a flag where\n * you don't need to synchronize other memory accesses */\ncount = rte_atomic_load_explicit(&shared->count, rte_memory_order_relaxed);\n```\n\n**Writing shared variables:**\n\n```c\n/* BAD - volatile write */\nvolatile int *flag = &shared->ready;\n*flag = 1;\n\n/* GOOD - atomic store with appropriate ordering */\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n```\n\n**Read-modify-write operations:**\n\n```c\n/* BAD - not atomic even with volatile */\nvolatile uint64_t *counter = &stats->packets;\n*counter += nb_rx; /* TOCTOU: load, add, store is 3 operations */\n\n/* GOOD - atomic add */\nrte_atomic_fetch_add_explicit(&stats->packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Memory Ordering Guide\n\nUse the weakest ordering that is correct.\nStronger ordering constrains hardware and compiler optimization unnecessarily.\n\n| DPDK Ordering | When to Use |\n|---------------|-------------|\n| `rte_memory_order_relaxed` | Statistics counters, polling flags where no other data depends on the value. Most common for simple counters. |\n| `rte_memory_order_acquire` | **Load** side of a flag/pointer that guards access to other shared data. Ensures subsequent reads see data published by the releasing thread. |\n| `rte_memory_order_release` | **Store** side of a flag/pointer that publishes shared data. Ensures all prior writes are visible to a thread that does an acquire load. |\n| `rte_memory_order_acq_rel` | Read-modify-write operations (e.g., `fetch_add`) that both consume and publish shared state in one operation. |\n| `rte_memory_order_seq_cst` | Rarely needed. Only when multiple independent atomic variables must be observed in a globally consistent total order. Avoid unless required. |\n\n**Common pattern -- producer/consumer flag:**\n\n```c\n/* Producer thread: fill buffer, then signal ready */\nfill_buffer(buf, data, len);\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n\n/* Consumer thread: wait for flag, then read buffer */\nwhile (!rte_atomic_load_explicit(&shared->ready, rte_memory_order_acquire))\n rte_pause();\nprocess_buffer(buf, len); /* guaranteed to see producer's writes */\n```\n\n**Common pattern -- statistics counter (no ordering needed):**\n\n```c\nrte_atomic_fetch_add_explicit(&port_stats->rx_packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Standalone Fences\n\nPrefer ordering on the atomic operation itself (acquire load, release store) over standalone fences.\nStandalone fences (`rte_atomic_thread_fence()`) are a blunt instrument\nthat orders ALL memory accesses around the fence, ",".cursor/mcp.json":"# Model Context Protocol (MCP) Configuration\n\nPath: `.cursor/mcp.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.cursor/mcp.json)",".claude-plugin/marketplace.json":"# Claude Plugin Marketplace Catalog\n\nPath: `.claude-plugin/marketplace.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/marketplace.json)",".claude-plugin/plugin.json":"# claude-plugin Plugin Manifest\n\nPath: `.claude-plugin/plugin.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/plugin.json)"},"items":[{"name":"AGENTS.md","path":"AGENTS.md","rawUrl":"https://raw.githubusercontent.com/DPDK/dpdk/HEAD/AGENTS.md","title":"AI Agent Protocol & Instructions","category":"root-instruction","format":"markdown","content":"\n\n\n# AGENTS.md - DPDK Code Review Guidelines for AI Tools\n\n## CRITICAL INSTRUCTION - READ FIRST\n\nThis document has two categories of review rules with different confidence thresholds,\nplus rules on evidence and scope that apply to both.\n\n### 1. Correctness Bugs -- HIGHEST PRIORITY (report at >=50% confidence)\n\n**Always report potential correctness bugs.** These are the most valuable findings.\nWhen in doubt, report them -- subject to the evidence rule below.\nUncertainty about code you have read is fine:\na possible use-after-free or resource leak is worth mentioning even if you are not certain.\nUncertainty about code you have not read is not a finding at all.\n\nCorrectness bugs include:\n- Use-after-free (accessing memory after `free`/`rte_free`)\n- Resource leaks on error paths (memory, file descriptors, locks)\n- Double-free or double-close\n- NULL pointer dereference\n- Buffer overflows or out-of-bounds access\n- Uninitialized variable use in a reachable code path\n- Race conditions (unsynchronized shared state)\n- `volatile` used instead of atomic operations for inter-thread shared variables\n- Missing error checks on functions that can fail\n- Error paths that skip cleanup (goto labels, missing free/close)\n- Incorrect error propagation (wrong return value, lost errno)\n- Logic errors in conditionals (wrong operator, inverted test)\n- Integer overflow/truncation in size calculations\n- Missing bounds checks on user-supplied sizes or indices\n- Statistics accumulation using `=` instead of `+=`\n- Integer multiply without widening cast losing upper bits (16x16, 32x32, etc.)\n- Unbounded descriptor chain traversal on guest/API-supplied data\n- `1 << n` on 64-bit bitmask (must use `1ULL << n` or `RTE_BIT64()`)\n- Left shift of narrow unsigned (`uint8_t`/`uint16_t`) used as 64-bit value (sign extension via implicit `int` promotion)\n- Variable assigned then overwritten before being read (dead store)\n- Same variable used as loop counter in nested loops\n- `rte_mbuf_raw_free_bulk()` called on mbufs that may originate from different mempools (Tx burst, ring dequeue)\n- MTU confused with frame length (MTU is L3 payload; frame length = MTU + L2 overhead)\n- Using `dev_conf.rxmode.mtu` after configure instead of `dev->data->mtu`\n- Hardcoded Ethernet overhead instead of per-device calculation\n- MTU set without enabling `RTE_ETH_RX_OFFLOAD_SCATTER` when frame size exceeds mbuf data room\n- `mtu_set` callback rejects valid MTU when scatter Rx is already enabled\n- Rx queue setup silently drops oversized packets instead of enabling scatter or returning an error\n- Rx function selection ignores `scattered_rx` flag or MTU-vs-mbuf-size check\n\n**Do NOT self-censor correctness bugs.**\nIf you identify a code path where a resource could leak or memory could be used after free,\nreport it -- subject to the evidence rule below.\nDo not talk yourself out of a bug you can see.\nDo not talk yourself into one you cannot.\n\n### Evidence rule (applies to every finding)\n\nEvery finding must rest on code you have actually read.\n\"Read\" means a line in the patch, or a file you opened with a tool.\n\nThis document is used in two ways and the rule is the same in both.\n\n**Diff only** -- CI, `review-patch.py`, and any run without tool access.\nAll you have read is the patch: added and removed lines, plus a few lines of context.\nYou cannot see the rest of the file, other callers, or any other file in the tree,\nso any claim that depends on them cannot be made.\n\n**With tool access** -- CLI or agentic use, where you can clone the tree,\napply the series, grep, and read whole files.\nRead them.\nCheck the surrounding function, the callers, and the headers before you report.\nTool access does not lower the bar; it widens what you are able to read,\nand therefore what you are expected to have read before filing a finding.\n\nIn either mode:\n\n- Report a finding only if you can name the line that causes it and quote it verbatim.\n For anything the patch introduces or changes, that line must be an added or changed line.\n A note about surrounding code may rest on a line you read in the tree,\n and is Info only -- see \"Pre-existing code\" below.\n- The line you quote must itself show the defect you claim.\n If you claim a missing barrier before a read and the line you quote is a write,\n the finding is wrong.\n Delete it rather than reword the claim to fit the quote.\n- Never quote, paraphrase, or reconstruct code you have not read.\n If you are writing a code block with line numbers you did not read, you are inventing it.\n Delete the finding.\n- If a claim depends on code you have not read -- other callers, other files,\n the rest of the function -- either go read it or omit the claim.\n Do not file it with a caveat, do not call it a risk,\n and do not ask the author to verify it on your behalf.\n- \"presumably\", \"appears to be\", \"if this is\", \"potentially\", \"could be\" used to hedge\n *what the code says* mean you lack the evidence.\n Get it or omit the finding.\n A conditional about *runtime behaviour*, with the line in hand, is not a hedge\n and is what a correctness finding looks like:\n \"this leaks if `rte_malloc()` returns NULL\" is a finding, \"this appears to allocate\n somewhere\" is not.\n The test: would removing the qualifier require reading code you have not read?\n\nThe >=50% bar for correctness bugs applies to your judgement about code you have read.\nIt is not permission to speculate about code you have not.\n\n### Pre-existing code (applies to every finding)\n\nThe unit of review is the patch.\nCode that appears only as diff context, or that you read in the tree but the patch does not touch,\nis not what this author is being asked to fix.\nA finding about such code must never be an Error or a Warning\nand must never withhold a Reviewed-by.\n\nReport it as Info, and say plainly that it is pre-existing and not introduced by this patch.\nThis is worth doing: it puts the observation in front of the people working in that code\nso it can be picked up in a separate patch later.\n\nThe evidence rule still applies in full.\nAn Info note about surrounding code needs the same proof as any other finding:\nyou must have read the code and be able to quote the line.\n\"Pre-existing\" is a lower severity, not a lower standard of evidence.\n\n### Hardening suggestions (applies to every finding)\n\nValidating a value supplied by a cooperating peer -- a PF driver, firmware,\nor the device's own descriptors -- is a hardening suggestion, not a bug.\nReport it as Info.\n\nIt becomes a Warning or an Error only when the value crosses a trust boundary,\nor when it drives index, length, or state arithmetic:\n\n- Error: a buffer count from the PF used as a ring index or loop bound\n without being checked against `nb_desc`\n- Info: clamping a residual length field that a well-behaved PF would never\n get wrong, where an out-of-range value cannot escape the local computation\n\n\"A misbehaving firmware could send garbage\" is true of every field in every\nmailbox message. On its own it is not a severity.\n\n### 2. Style, Process, and Formatting -- suppress false positives\n\n**NEVER list a style/process item under \"Errors\" or \"Warnings\" if you conclude it is correct.**\n\nBefore outputting any style, formatting, or process error/warning, verify it is actually wrong.\nIf your analysis concludes with phrases like \"there's no issue here\", \"which is fine\",\n\"appears correct\", \"is acceptable\", or \"this is actually correct\" --\nthen DO NOT INCLUDE IT IN YOUR OUTPUT AT ALL.\nDelete it.\nOmit it entirely.\n\nThis suppression rule applies to: naming conventions, code style, and process compliance.\nIt does NOT apply to correctness bugs listed above.\n(SPDX/copyright format and commit message formatting are handled by checkpatch\nand are excluded from AI review entirely.)\n\n---\n\nThis document provides guidelines for AI-powered code review tools\nwhen reviewing contributions to the Data Plane Development Kit (DPDK).\nIt is derived from the official DPDK contributor guidelines and validation scripts.\n\n## Overview\n\nDPDK follows a development process modeled on the Linux Kernel.\nAll patches are reviewed publicly on the mailing list before being merged.\nAI review tools should verify compliance with the standards outlined below.\n\n## Review Philosophy\n\n**Correctness bugs are the primary goal of AI review.** Style and formatting checks are secondary.\nA review that catches a use-after-free but misses a style nit is far more valuable\nthan one that catches every style issue but misses the bug.\n\n**BEFORE OUTPUTTING YOUR REVIEW**: Re-read each item.\n- For correctness bugs: keep them.\n If you have reasonable doubt that a code path is safe, report it.\n- For style/process items: if ANY item contains phrases like \"is fine\", \"no issue\",\n \"appears correct\", \"is acceptable\", \"actually correct\" -- DELETE THAT ITEM.\n Do not include it.\n\n### Correctness review guidelines\n- Trace error paths: for every function that allocates a resource or acquires a lock,\n verify that ALL error paths after that point release it\n- Check every `goto error` and early `return`: does it clean up everything allocated so far?\n- Look for use-after-free: after `free(p)`, is `p` accessed again?\n- Check that error codes are propagated, not silently dropped\n- Report at >=50% confidence about code you have read\n- Never substitute a confidence caveat for evidence:\n if the reasoning depends on code you have not read, go read it or drop the finding\n- It is better to report a potential bug that turns out to be safe than to miss a real bug,\n provided you can point to the line that causes it\n\n### Style and process review guidelines\n- Only comment on style/process issues when you have HIGH CONFIDENCE (>80%) that an issue exists\n- Be concise: one sentence per comment when possible\n- Focus on actionable feedback, not observations\n- When reviewing text, only comment on clarity issues if the text is genuinely confusing\n or could lead to errors.\n- Do NOT comment on copyright years, SPDX format, or copyright holders -\n not subject to AI review\n- Do NOT report an issue then contradict yourself -\n if something is acceptable, do not mention it at all\n- Do NOT include items in Errors/Warnings that you then say are \"acceptable\" or \"correct\"\n- Do NOT mention things that are correct or \"not an issue\" - only report actual problems\n- Do NOT speculate about contributor circumstances (employment, company policies, etc.)\n- Before adding any style item to your review, ask: \"Is this actually wrong?\"\n If no, omit it entirely.\n- NEVER write \"(Correction: ...)\" - if you need to correct yourself, simply omit the item entirely\n- Do NOT add vague suggestions like \"should be verified\" or \"should be checked\" -\n either it's wrong or don't mention it\n- Do NOT flag something as an Error then say \"which is correct\" in the same item\n- Do NOT say \"no issue here\" or \"this is actually correct\" -\n if there's no issue, do not include it in your review\n- Do NOT analyze cross-patch dependencies or compilation order -\n you cannot reliably determine this from patch review\n- Do NOT claim a patch \"would cause compilation failure\" based on symbols used\n in other patches in the series\n- Review each patch individually for its own correctness;\n assume the patch author ordered them correctly\n- When reviewing a patch series, OMIT patches that have no issues.\n Do not include a patch in your output just to say \"no issues found\"\n or to summarize what the patch does.\n Only include patches where you have actual findings to report.\n- Do NOT end the review with a summary section listing every patch in the series.\n A clean patch must not appear anywhere in the output, including in a summary or status table.\n\n## Priority Areas (Review These)\n\n### Security & Safety\n- Unsafe code blocks without justification\n- Command injection risks (shell commands, user input)\n- Path traversal vulnerabilities\n- Credential exposure or hard coded secrets\n- Missing input validation on external data\n- Improper error handling that could leak sensitive info\n\n### Correctness Issues\n- Logic errors that could cause panics or incorrect behavior\n- Buffer overflows\n- Race conditions\n- **`volatile` for inter-thread synchronization**:\n `volatile` does not provide atomicity or memory ordering between threads.\n Use `rte_atomic_load_explicit()`/`rte_atomic_store_explicit()` with appropriate `rte_memory_order_*` instead.\n See the Shared Variable Access section under Forbidden Tokens for details.\n- Resource leaks (files, connections, memory)\n- Off-by-one errors or boundary conditions\n- Incorrect error propagation\n- **Use-after-free** (any access to memory after it has been freed)\n- **Error path resource leaks**:\n For every allocation or fd open,\n trace each error path (`goto`, early `return`, conditional) to verify the resource is released.\n Common patterns to check:\n - `malloc`/`rte_malloc` followed by a failure that does `return -1` instead of `goto cleanup`\n - `open()`/`socket()` fd not closed on a later error\n - Lock acquired but not released on an error branch\n - Partially initialized structure where early fields are allocated\n but later allocation fails without freeing the early ones\n- **Double-free / double-close**:\n resource freed in both a normal path and an error path,\n or fd closed but not set to -1 allowing a second close\n- **Missing error checks**:\n functions that can fail (malloc, open, ioctl, etc.) whose return value is not checked\n- **Do NOT flag unchecked return values from functions\n that always succeed on Linux.** Some POSIX functions are documented to always return zero on Linux\n when called with valid arguments.\n Flagging unchecked returns from these is a false positive.\n The list includes:\n - `pthread_mutex_init()`, `pthread_cond_init()`\n - `pthread_cond_signal()`, `pthread_cond_broadcast()`, `pthread_cond_wait()`\n - `pthread_condattr_init()`, `pthread_condattr_destroy()`\n - `pthread_attr_init()`, `pthread_attr_destroy()`\n- Changes to API without release notes\n- Changes to ABI on non-LTS release\n- Usage of deprecated API when replacements exist\n- Overly defensive code that adds unnecessary checks\n- Unnecessary comments that just restate what the code already shows (remove them)\n- **Process-shared synchronization errors** (pthread mutexes in shared memory without `PTHREAD_PROCESS_SHARED`)\n- **Statistics accumulation using `=` instead of `+=`**:\n When accumulating statistics (counters, byte totals, packet counts),\n using `=` overwrites the running total with only the latest value.\n This silently produces wrong results.\n ```c\n /* BAD - overwrites instead of accumulating */\n stats->rx_packets = nb_rx;\n stats->rx_bytes = total_bytes;\n\n /* GOOD - accumulates over time */\n stats->rx_packets += nb_rx;\n stats->rx_bytes += total_bytes;\n ```\n Note: `=` is correct for gauge-type values (e.g., queue depth, link status)\n and for initial assignment.\n Only flag when the context is clearly incremental accumulation\n (loop bodies, per-burst counters, callback tallies).\n- **Integer multiply without widening cast**:\n When multiplying integers to produce a result wider than the operands (sizes, offsets, byte counts),\n the multiplication is performed at the operand width\n and the upper bits are silently lost before the assignment.\n This applies to any narrowing scenario: 16x16 assigned to a 32-bit variable,\n 32x32 assigned to a 64-bit variable, etc.\n ```c\n /* BAD - 32x32 overflows before widening to 64 */\n uint64_t total_size = num_entries * entry_size; /* both are uint32_t */\n size_t offset = ring->idx * ring->desc_size; /* 32x32 -> truncated */\n\n /* BAD - 16x16 overflows before widening to 32 */\n uint32_t byte_count = pkt_len * nb_segs; /* both are uint16_t */\n\n /* GOOD - widen before multiply */\n uint64_t total_size = (uint64_t)num_entries * entry_size;\n size_t offset = (size_t)ring->idx * ring->desc_size;\n uint32_t byte_count = (uint32_t)pkt_len * nb_segs;\n ```\n- **Unbounded descriptor chain traversal**:\n When walking a chain of descriptors (virtio, DMA, NIC Rx/Tx rings) where the chain length\n or next-index comes from guest memory or an untrusted API caller,\n the traversal MUST have a bounds check or loop counter to prevent infinite loops\n or out-of-bounds access from malicious/corrupt data.\n ```c\n /* BAD - guest controls desc[idx].next with no bound */\n while (desc[idx].flags & VRING_DESC_F_NEXT) {\n idx = desc[idx].next; /* guest-supplied, unbounded */\n process(desc[idx]);\n }\n\n /* GOOD - cap iterations to descriptor ring size */\n for (i = 0; i < ring_size; i++) {\n if (!(desc[idx].flags & VRING_DESC_F_NEXT))\n break;\n idx = desc[idx].next;\n if (idx >= ring_size) /* bounds check */\n return -EINVAL;\n process(desc[idx]);\n }\n ```\n This applies to any chain/linked-list traversal where indices\n or pointers originate from untrusted input (guest VMs, user-space callers, network packets).\n- **Bitmask shift using `1` instead of `1ULL` on 64-bit masks**:\n The literal `1` is `int` (32 bits).\n Shifting it by 32 or more is undefined behavior; shifting it by less than 32\n but assigning to a `uint64_t` silently zeroes the upper 32 bits.\n Use `1ULL << n`, `UINT64_C(1) << n`, or the DPDK `RTE_BIT64(n)` macro.\n ```c\n /* BAD - 1 is int, UB if n >= 32, wrong if result used as uint64_t */\n uint64_t mask = 1 << bit_pos;\n if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) /* bit 15 OK, bit 32+ UB */\n\n /* GOOD */\n uint64_t mask = UINT64_C(1) << bit_pos;\n uint64_t mask = 1ULL << bit_pos;\n uint64_t mask = RTE_BIT64(bit_pos); /* preferred in DPDK */\n if (features & RTE_BIT64(VIRTIO_NET_F_MRG_RXBUF))\n ```\n Note: `1U << n` is acceptable when the mask is known to be 32-bit\n (e.g., `uint32_t` register fields with `n < 32`).\n Only flag when the result is stored in, compared against,\n or returned as a 64-bit type, or when `n` could be >= 32.\n- **Left shift of narrow unsigned type sign-extends to 64-bit**:\n When a `uint8_t` or `uint16_t` value is left-shifted,\n C integer promotion converts it to `int` (signed 32-bit) before the shift.\n If the result has bit 31 set, implicit conversion to `uint64_t`, `size_t`,\n or use in pointer arithmetic sign-extends the upper 32 bits to all-1s,\n producing a wrong address or value.\n This is Coverity SIGN_EXTENSION.\n The fix is to cast the narrow operand to an unsigned type\n at least as wide as the target before shifting.\n ```c\n /* BAD - uint16_t promotes to signed int, bit 31 may set,\n * then sign-extends when converted to 64-bit for pointer math */\n uint16_t idx = get_index();\n void *addr = base + (idx << wqebb_shift); /* SIGN_EXTENSION */\n uint64_t off = (uint64_t)(idx << shift); /* too late: shift already in int */\n\n /* BAD - uint8_t shift with result used as size_t */\n uint8_t page_order = get_order();\n size_t size = page_order << PAGE_SHIFT; /* promotes to int first */\n\n /* GOOD - cast before shift */\n void *addr = base + ((uint64_t)idx << wqebb_shift);\n uint64_t off = (uint64_t)idx << shift;\n size_t size = (size_t)page_order << PAGE_SHIFT;\n\n /* GOOD - intermediate unsigned variable */\n uint32_t offset = (uint32_t)idx << wqebb_shift; /* OK if result fits 32 bits */\n ```\n Note: This is distinct from the `1 << n` pattern (where the literal `1` is the problem)\n and from the integer-multiply pattern (where the operation is `*` not `<<`).\n The mechanism is the same C integer promotion rule,\n but the code patterns and Coverity checker names differ.\n Only flag when the shift result is used in a context wider than 32 bits (64-bit assignment,\n pointer arithmetic, function argument expecting `uint64_t`/`size_t`).\n A shift whose result is stored in a `uint32_t` or narrower variable is not affected.\n- **Variable overwrite before read (dead store)**:\n A variable is assigned a value that is unconditionally overwritten before it is ever read.\n This usually indicates a logic error (wrong variable name, missing `if`, copy-paste mistake)\n or at minimum is dead code.\n ```c\n /* BAD - first assignment is never read */\n ret = validate_input(cfg);\n ret = apply_config(cfg); /* overwrites without checking first ret */\n if (ret != 0)\n return ret;\n\n /* GOOD - check each return value */\n ret = validate_input(cfg);\n if (ret != 0)\n return ret;\n ret = apply_config(cfg);\n if (ret != 0)\n return ret;\n ```\n Do NOT flag cases where the initial value is intentionally a default that may\n or may not be overwritten (e.g., `int ret = 0;` followed by a conditional assignment).\n Only flag unconditional overwrites where the first value can never be observed.\n- **Shared loop counter in nested loops**:\n Using the same variable as the loop counter in both an outer\n and inner loop causes the outer loop to malfunction because the inner loop modifies its counter.\n ```c\n /* BAD - inner loop clobbers outer loop counter */\n int i;\n for (i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (i = 0; i < nb_descs; i++) /* BUG: reuses i */\n init_desc(i);\n }\n\n /* GOOD - distinct loop counters */\n for (int i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (int j = 0; j < nb_descs; j++)\n init_desc(j);\n }\n ```\n- **`rte_mbuf_raw_free_bulk()` on mixed-pool mbuf arrays**:\n Tx burst functions and ring/queue dequeue paths receive mbufs\n that may originate from different mempools (applications are free to send mbufs from any pool).\n `rte_mbuf_raw_free_bulk()` takes an explicit mempool parameter\n and calls `rte_mempool_put_bulk()` directly --\n ALL mbufs in the array must come from that single pool.\n If mbufs come from different pools, they are returned to the wrong pool,\n corrupting pool accounting and causing hard-to-debug failures.\n Note: `rte_pktmbuf_free_bulk()` is safe for mixed pools --\n it batches mbufs by pool internally and flushes whenever the pool changes.\n ```c\n /* BAD - assumes all mbufs are from the same pool */\n /* (in tx_burst completion or ring dequeue error path) */\n rte_mbuf_raw_free_bulk(mp, mbufs, nb_mbufs);\n\n /* GOOD - rte_pktmbuf_free_bulk handles mixed pools correctly */\n rte_pktmbuf_free_bulk(mbufs, nb_mbufs);\n\n /* GOOD - free individually (each mbuf returned to its own pool) */\n for (i = 0; i < nb_mbufs; i++)\n rte_pktmbuf_free(mbufs[i]);\n ```\n This applies to any path that frees mbufs submitted by the application:\n Tx completion, Tx error cleanup, and ring/queue drain paths.\n `rte_mbuf_raw_free_bulk()` is an optimization for the fast-free case (`RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE`)\n where the application guarantees all mbufs come from a single pool with refcnt=1.\n- **MTU confused with Ethernet frame length**:\n Maximum Transmission Unit (MTU) is the maximum L3 payload size\n (e.g., 1500 bytes for standard Ethernet).\n The maximum Ethernet *frame length* includes L2 overhead:\n Ethernet header (14 bytes) + optional VLAN tags (4 bytes each) + CRC (4 bytes).\n The overhead varies per device depending on supported encapsulations (VLAN, QinQ, etc.).\n Confusing MTU with frame length produces off-by-14-to-22-byte errors in packet size limits,\n buffer sizing, and scattered Rx decisions.\n\n **VLAN tag accounting:** The outer VLAN tag is L2 overhead\n and does NOT count toward MTU (matching Linux and FreeBSD).\n A 1522-byte single-tagged frame is valid at MTU 1500.\n However, in QinQ the inner (customer) tag DOES consume MTU --\n it is part of the customer frame.\n So QinQ with MTU 1500 allows only 1496 bytes of L3 payload\n unless the port MTU is raised to 1504.\n\n **Using `rxmode.mtu` after configure:** After `rte_eth_dev_configure()` completes,\n the canonical MTU is stored in `dev->data->mtu`.\n The `dev->data->dev_conf.rxmode.mtu` field is the user's *request*\n and must not be read after configure --\n it becomes stale if `rte_eth_dev_set_mtu()` is called later.\n Both configure and set_mtu write to `dev->data->mtu`; PMDs should always read from there.\n\n **Overhead calculation:** Do not hardcode a single overhead constant.\n Use the device's own overhead calculation\n (typically available via `dev_info.max_rx_pktlen - dev_info.max_mtu`\n or an internal `eth_overhead` field).\n Different devices support different encapsulations, so the overhead is not a universal constant.\n\n **Scattered Rx decision:** PMDs compare maximum frame length (MTU + per-device overhead)\n against Rx buffer size to decide whether scattered Rx is needed.\n Comparing raw MTU against buffer size is wrong --\n it underestimates the actual frame size by the overhead.\n ```c\n /* BAD - MTU used where frame length is needed */\n if (dev->data->mtu > rxq->buf_size)\n enable_scattered_rx();\n\n /* BAD - hardcoded overhead, wrong for QinQ-capable devices */\n #define ETHER_OVERHEAD 18 /* may be 22 or 26 for VLAN/QinQ */\n max_frame = mtu + ETHER_OVERHEAD;\n\n /* BAD - reading rxmode.mtu after configure (stale if set_mtu called) */\n static int\n mydrv_rx_queue_setup(...) {\n mtu = dev->data->dev_conf.rxmode.mtu; /* WRONG - may be stale */\n ...\n }\n\n /* GOOD - use dev->data->mtu, the canonical post-configure value */\n static int\n mydrv_rx_queue_setup(...) {\n uint16_t mtu = dev->data->mtu;\n ...\n }\n\n /* GOOD - use per-device overhead for frame length calculation */\n uint32_t frame_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;\n uint32_t max_frame_len = dev->data->mtu + frame_overhead;\n if (max_frame_len > rxq->buf_size)\n enable_scattered_rx();\n\n /* GOOD - device-specific overhead constant derived from capabilities */\n static uint32_t\n mydrv_eth_overhead(struct rte_eth_dev *dev) {\n uint32_t overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_VLAN)\n overhead += RTE_VLAN_HLEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_QINQ)\n overhead += RTE_VLAN_HLEN;\n return overhead;\n }\n ```\n Note: In `rte_eth_dev_configure()` itself, reading `rxmode.mtu` is correct --\n that is where the user's request is consumed and written to `dev->data->mtu`.\n Only flag reads of `rxmode.mtu` *outside* configure (queue setup, start, link update, MTU set, etc.).\n- **Missing scatter Rx for large MTU**:\n When the configured MTU produces a frame size (MTU + Ethernet overhead) larger\n than the mbuf data buffer size (`rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`),\n the PMD MUST either enable scatter Rx (multi-segment receive) or reject the configuration.\n Silently accepting the MTU and then truncating or dropping oversized packets is a correctness bug.\n ```c\n /* BAD - accepts MTU but will truncate packets that don't fit */\n static int\n mydrv_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)\n {\n /* No check against mbuf size or scatter capability */\n dev->data->mtu = mtu;\n return 0;\n }\n\n /* BAD - rejects valid MTU even though scatter is enabled */\n if (frame_size > mbuf_data_size)\n return -EINVAL; /* wrong: should allow if scatter is on */\n\n /* GOOD - check scatter and mbuf size */\n if (!dev->data->scattered_rx &&\n frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)\n return -EINVAL;\n\n /* GOOD - auto-enable scatter when needed */\n if (frame_size > mbuf_data_size) {\n if (!(dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SCATTER))\n return -EINVAL;\n dev->data->dev_conf.rxmode.offloads |=\n RTE_ETH_RX_OFFLOAD_SCATTER;\n dev->data->scattered_rx = 1;\n }\n ```\n Key relationships:\n - `dev_info.max_rx_pktlen`: maximum frame the hardware can receive\n - `dev_info.max_mtu`: maximum MTU = `max_rx_pktlen` - overhead\n - `dev_info.min_rx_bufsize`: minimum Rx buffer the HW requires\n - `dev_info.max_rx_bufsize`: maximum single-descriptor buffer size\n - `mbuf data size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`\n - When scatter is off: frame length must fit in a single mbuf\n - When scatter is on: frame length can span multiple mbufs;\n the PMD selects a scattered Rx function\n\n This pattern should be checked in three places:\n 1. `dev_configure()` -- validate MTU against mbuf size / scatter\n 2. `rx_queue_setup()` -- select scattered vs non-scattered Rx path\n 3. `mtu_set()` -- runtime MTU change must re-validate\n- **Rx queue function selection ignoring scatter**:\n When a PMD has separate fast-path Rx functions for scalar (single-segment)\n and scattered (multi-segment) modes,\n it must select the scattered variant whenever `dev->data->scattered_rx` is set OR\n when the configured frame length exceeds the single mbuf data size.\n Failing to do so causes the scalar Rx function to silently drop or corrupt multi-segment packets.\n ```c\n /* BAD - only checks offload flag, ignores actual need */\n if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER)\n rx_func = mydrv_recv_scattered;\n else\n rx_func = mydrv_recv_single; /* will drop oversized pkts */\n\n /* GOOD - check both the flag and the size */\n mbuf_size = rte_pktmbuf_data_room_size(rxq->mp) -\n RTE_PKTMBUF_HEADROOM;\n max_pkt = dev->data->mtu + overhead;\n if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||\n max_pkt > mbuf_size) {\n dev->data->scattered_rx = 1;\n rx_func = mydrv_recv_scattered;\n } else {\n rx_func = mydrv_recv_single;\n }\n ```\n\n### Cryptographic and Security Code\n\nApplies only when the patch touches crypto PMDs (`drivers/crypto/`),\nthe security or ipsec libraries,\nor code handling key material, IVs, or authentication digests.\nStays silent on all other patches.\n\n- **Non-constant-time digest/tag comparison** (Error):\n When verifying an authentication tag, digest, MAC, or ICV --\n comparing a computed value against an attacker-supplied one to decide accept/reject --\n plain `memcmp()` leaks timing information.\n Use `rte_memeq_timingsafe()` instead.\n Return semantics differ: `memcmp()` returns 0 on equal,\n `rte_memeq_timingsafe()` returns true on equal.\n So `if (memcmp(tag, digest, len))` becomes `if (!rte_memeq_timingsafe(tag, digest, len))`.\n Do NOT flag `memcmp()` on non-secret data:\n algorithm IDs, key lengths, capability/feature structs, lookup keys.\n Only flag comparisons that gate acceptance of attacker-influenced data.\n\n- **Sensitive material not zeroed before free** (Error):\n Keys, expanded key schedules, HMAC ipad/opad, and session secrets must be wiped,\n not merely freed -- a plain free leaves secrets in the heap.\n - `rte_malloc`/`rte_zmalloc`'d secret: use `rte_free_sensitive()` instead of `rte_free()`\n - local or stack secret going out of scope: `rte_memzero_explicit()` before return;\n plain `memset()` may be optimized away\n Flag only where the patch itself allocates, copies, or frees the secret.\n Do NOT flag buffers that never held secrets (descriptors, `dev_info`),\n pre-existing free paths the patch does not touch,\n or `memset()` the compiler cannot elide because the pointer escapes to `free()`.\n `rte_memzero_explicit()` followed by a plain `rte_free()` is correct\n and is not a missing `rte_free_sensitive()`.\n\n- **Non-cryptographic RNG for keys/IVs** (Error):\n `rte_rand()` is seeded from system entropy (`getentropy()`, with RDSEED and TSC fallbacks),\n but the generator itself is an LFSR and is linear:\n an observer who sees enough output can recover the state and predict every later value.\n Good seeding is not sufficient for key, IV, or nonce generation.\n Use a cryptographic source instead:\n the device's own entropy source or DRBG,\n or `getrandom(2)` / `/dev/urandom` in the control path.\n `rand()` and `random()` are weaker still and must not appear in crypto or security code.\n Do NOT flag `rte_rand()` used for non-secret purposes:\n hash seeds (`lib/ipsec/ipsec_sad.c` uses it for `hash_func_init_val`),\n jitter, backoff, load balancing, or test data.\n\nDo NOT flag:\n- IV/nonce reuse -- a runtime property, not determinable from a patch.\n- `memcmp()` on lengths, algorithm selectors, or non-secret config.\n\n### Architecture & Patterns\n- Code that violates existing patterns in the code base\n- Missing error handling\n- Code that is not safe against signals\n\n### New Library API Design\n\nWhen a patch adds a new library under `lib/`, review API design in addition to correctness and style.\n\n**API boundary.** A library should be a compiler, not a framework.\nThe model is `rte_acl`: create a context, feed input, get structured output,\ncaller decides what to do with it.\nNo callbacks needed.\nIf the library requires callers to implement a callback table to function,\nthe boundary is wrong -- the library is asking the caller to be its backend.\n\n**Callback structs** (Warning / Error).\nAny function-pointer struct in an installed header is an ABI break waiting to happen.\nAdding or reordering a member breaks all consumers.\n- Prefer a single callback parameter over an ops table.\n- \\>5 callbacks: **Warning** -- likely needs redesign.\n- \\>20 callbacks: **Error** -- this is an app plugin API, not a library.\n- All callbacks must have Doxygen (contract, return values, ownership).\n- Void-returning callbacks for failable operations swallow errors -- flag as **Error**.\n- Callbacks serving app-specific needs (e.g. `verbose_level_get`)\n indicate wrong code was extracted into the library.\n\n**Extensible structures.** Prefer TLV / tagged-array patterns over enum + union,\nfollowing `rte_flow_item` and `rte_flow_action` as the model.\nType tag + pointer to type-specific data allows adding types without ABI breaks.\nFlag as **Warning**:\n- Large enums (100+) consumers must switch on.\n- Unions that grow with every new feature.\n- Ask: \"What changes when a feature is added next release?\"\n If \"add an enum value and union arm\" -- should be TLV.\n\n**Installed headers.** If it's in `headers` or `indirect_headers` in meson.build, it's public API.\nDon't call it \"private.\"\nIf truly internal, don't install it.\n\n**Global state.** Prefer handle-based API (`create`/`destroy`) over singletons.\n`rte_acl` allows multiple independent classifier instances; new libraries should do the same.\n\n**Output ownership.** Prefer caller-allocated or library-allocated- caller-freed\nover internal static buffers.\nIf static buffers are used, document lifetime\nand ensure Doxygen examples don't show stale-pointer usage.\n\n---\n\n## C Coding Style\n\n### General Formatting\n\n- **Tab width**:\n 8 characters (hard tabs for indentation, spaces for alignment)\n- **No trailing whitespace** on lines or at end of files\n- Files must end with a new line\n- Code style should be consistent within each file\n\n### Comments\n\n```c\n/* Most single-line comments look like this. */\n\n/*\n * VERY important single-line comments look like this.\n */\n\n/*\n * Multi-line comments look like this. Make them real sentences. Fill\n * them so they look like real paragraphs.\n */\n```\n\n### Header File Organization\n\nInclude order (each group separated by blank line):\n1. System/libc includes\n2. DPDK EAL includes\n3. DPDK misc library includes\n4. Application-specific includes\n\n```c\n#include \n#include \n\n#include \n\n#include \n#include \n\n#include \"application.h\"\n```\n\n### Header Guards\n\n```c\n#ifndef _FILE_H_\n#define _FILE_H_\n\n/* Code */\n\n#endif /* _FILE_H_ */\n```\n\n### Naming Conventions\n\n- **All external symbols** must have `RTE_` or `rte_` prefix\n- **Macros**:\n ALL_UPPERCASE with `RTE_` prefix\n- **Functions**:\n lowercase with underscores only (no CamelCase)\n- **Variables**:\n lowercase with underscores only\n- **Enum values**:\n ALL_UPPERCASE with `RTE__` prefix\n\n**Exception**: Driver base directories (`drivers/*/base/`) may use different naming conventions\nwhen sharing code across platforms or with upstream vendor code.\n\n#### Symbol Naming for Static Linking\n\nDrivers and libraries must not expose global variables that could clash\nwhen statically linked with other DPDK components or applications.\nUse consistent and unique prefixes for all exported symbols to avoid namespace collisions.\n\n**Good practice**: Use a driver-specific or library-specific prefix for all global variables:\n\n```c\n/* Good - virtio driver uses consistent \"virtio_\" prefix */\nconst struct virtio_ops virtio_legacy_ops = {\n\t.read = virtio_legacy_read,\n\t.write = virtio_legacy_write,\n\t.configure = virtio_legacy_configure,\n};\n\nconst struct virtio_ops virtio_modern_ops = {\n\t.read = virtio_modern_read,\n\t.write = virtio_modern_write,\n\t.configure = virtio_modern_configure,\n};\n\n/* Good - mlx5 driver uses consistent \"mlx5_\" prefix */\nstruct mlx5_flow_driver_ops mlx5_flow_dv_ops;\n```\n\n**Bad practice**: Generic names that may clash:\n\n```c\n/* Bad - \"ops\" is too generic, will clash with other drivers */\nconst struct virtio_ops ops = { ... };\n\n/* Bad - \"legacy_ops\" could clash with other legacy implementations */\nconst struct virtio_ops legacy_ops = { ... };\n\n/* Bad - \"driver_config\" is not unique */\nstruct driver_config config;\n```\n\n**Guidelines**:\n- Prefix all global variables with the driver or library name (e.g., `virtio_`, `mlx5_`, `ixgbe_`)\n- Prefix all global functions similarly unless they use the `rte_` namespace\n- Internal static variables do not require prefixes as they have file scope\n- Consider using the `RTE_` or `rte_` prefix only for symbols that are part of the public DPDK API\n\n#### Prohibited Terminology\n\nDo not use non-inclusive naming including:\n- `master/slave` -> Use: primary/secondary, controller/worker, leader/follower\n- `blacklist/whitelist` -> Use: denylist/allowlist, blocklist/passlist\n- `cripple` -> Use: impacted, degraded, restricted, immobilized\n- `tribe` -> Use: team, squad\n- `sanity check` -> Use: coherence check, test, verification\n\n\n### Comparisons and Boolean Logic\n\nDPDK style requires explicit comparison against `NULL`, `0`, or `'\\0'` rather than\ntruthiness on pointers, integers, and characters: write `if (p == NULL)` not `if (!p)`,\nand `if (a != 0)` not `if (a)`.\nDirect truthiness is acceptable only on actual `bool` types.\n`likely()` and `unlikely()` wrappers do not change whether a comparison is explicit\n(`if (likely(p != NULL))` is still explicit).\n\nMechanical rewrites for these are handled by coccinelle in `devtools/cocci/`;\nAI review need only catch cases cocci can't see (e.g. macro-expanded conditions).\n\n### Boolean Usage\n\nPrefer `bool` (from ``) over `int` for variables, parameters,\nand return values that are purely true/false.\nUsing `bool` makes intent explicit, enables compiler diagnostics for misuse, and is self-documenting.\n\n```c\n/* Bad - int used as boolean flag */\nint verbose = 0;\nint is_enabled = 1;\n\nint\ncheck_valid(struct item *item)\n{\n\tif (item->flags & ITEM_VALID)\n\t\treturn 1;\n\treturn 0;\n}\n\n/* Good - bool communicates intent */\nbool verbose = false;\nbool is_enabled = true;\n\nbool\ncheck_valid(struct item *item)\n{\n\treturn item->flags & ITEM_VALID;\n}\n```\n\n**Guidelines:**\n- Use `bool` for variables that only hold true/false values\n- Use `bool` return type for predicate functions (functions that answer a yes/no question,\n often named `is_*`, `has_*`, `can_*`)\n- Use `true`/`false` rather than `1`/`0` for boolean assignments\n- Boolean variables and parameters should not use explicit comparison:\n `if (verbose)` is correct, not `if (verbose == true)`\n- `int` is still appropriate when a value can be negative, is an error code,\n or carries more than two states\n\n**Structure fields:**\n- `bool` occupies 1 byte.\n In packed or cache-critical structures, consider using a bitfield or flags word instead\n- For configuration structures and non-hot-path data, `bool` is preferred over `int` for flag fields\n\n```c\n/* Bad - int flags waste space and obscure intent */\nstruct port_config {\n\tint promiscuous; /* 0 or 1 */\n\tint link_up; /* 0 or 1 */\n\tint autoneg; /* 0 or 1 */\n\tuint16_t mtu;\n};\n\n/* Good - bool for flag fields */\nstruct port_config {\n\tbool promiscuous;\n\tbool link_up;\n\tbool autoneg;\n\tuint16_t mtu;\n};\n\n/* Also good - bitfield for cache-critical structures */\nstruct fast_path_config {\n\tuint32_t flags; /* bitmask of CONFIG_F_* */\n\t/* ... hot-path fields ... */\n};\n```\n\n**Do NOT flag:**\n- `int` return type for functions that return error codes (0 for success, negative for error) --\n these are NOT boolean\n- `int` used for tri-state or multi-state values\n- `int` flags in existing code where changing the type would be a large, unrelated refactor\n- Bitfield or flags-word approaches in performance-critical structures\n\n### Indentation and Braces\n\n```c\n/* Control statements - no braces for single statements */\nif (val != NULL)\n\tval = realloc(val, newsize);\n\n/* Braces on same line as else */\nif (test)\n\tstmt;\nelse if (bar) {\n\tstmt;\n\tstmt;\n} else\n\tstmt;\n\n/* Switch statements - don't indent case */\nswitch (ch) {\ncase 'a':\n\taflag = 1;\n\t/* FALLTHROUGH */\ncase 'b':\n\tbflag = 1;\n\tbreak;\ndefault:\n\tusage();\n}\n\n/* Long conditions - double indent continuation */\nif (really_long_variable_name_1 == really_long_variable_name_2 &&\n\t\treally_long_variable_name_3 == really_long_variable_name_4)\n\tstmt;\n```\n\n### Variable Declarations\n\n- Prefer declaring variables inside the basic block where they are used\n- Variables may be declared either at the start of the block, or at point of first use (C99 style)\n- Both declaration styles are acceptable; consistency within a function is preferred\n- Initialize variables only when a meaningful value exists at declaration time\n- Use C99 designated initializers for structures\n\n```c\n/* Good - declaration at start of block */\nint ret;\nret = some_function();\n\n/* Also good - declaration at point of use (C99 style) */\nfor (int i = 0; i < count; i++)\n\tprocess(i);\n\n/* Good - declaration in inner block where variable is used */\nif (condition) {\n\tint local_val = compute();\n\tuse(local_val);\n}\n\n/* Bad - unnecessary initialization defeats compiler warnings */\nint ret = 0;\nret = some_function(); /* Compiler won't warn if assignment removed */\n```\n\n### Function Format\n\n- Return type on its own line\n- Opening brace on its own line\n- Place an empty line between declarations and statements\n\n```c\nstatic char *\nfunction(int a1, int b1)\n{\n\tchar *p;\n\n\tp = do_something(a1, b1);\n\treturn p;\n}\n```\n\n---\n\n## Unnecessary Code Patterns\n\nThe following patterns add unnecessary code, hide bugs, or reduce performance.\nAvoid them.\nSeveral closely related patterns are caught by coccinelle scripts in `devtools/cocci/`\n(unnecessary `void *` casts, NULL checks before `free()`/`rte_free()`,\n`memset()` before `free()`, zero-length array members, dead variable initialization);\nthose are not duplicated here.\n\n### Appropriate Use of rte_malloc()\n\n`rte_malloc()` allocates from hugepage memory.\nUse it only when required:\n\n- Memory that will be accessed by DMA (NIC descriptors, packet buffers)\n- Memory shared between primary and secondary DPDK processes\n- Memory requiring specific NUMA node placement\n\nFor general allocations, use standard `malloc()` which is faster\nand does not consume limited hugepage resources.\n\nQueue-related buffers (descriptor rings, Rx/Tx queue control structures)\nshould use `rte_zmalloc_socket()` rather\nthan plain `rte_malloc()`: zero-initialization avoids stale descriptor bugs,\nthe `_socket` variant ensures NUMA-local allocation,\nand hugepage backing makes the memory visible to secondary processes.\n\n```c\n/* Bad - rte_malloc for ordinary data structure */\nstruct config *cfg = rte_malloc(NULL, sizeof(*cfg), 0);\n\n/* Good - standard malloc for control structures */\nstruct config *cfg = malloc(sizeof(*cfg));\n\n/* Good - rte_zmalloc_socket for descriptor rings / queue structures */\nstruct my_rx_desc *ring = rte_zmalloc_socket(\"rx_ring\",\n n * sizeof(*ring), RTE_CACHE_LINE_SIZE, socket_id);\n```\n\n### Appropriate Use of rte_memcpy()\n\n`rte_memcpy()` is optimized for bulk data transfer in the fast path.\nFor general use, standard `memcpy()` is preferred because:\n\n- Modern compilers optimize `memcpy()` effectively\n- `memcpy()` includes bounds checking with `_FORTIFY_SOURCE`\n- `memcpy()` handles small fixed-size copies efficiently\n\n```c\n/* Bad - rte_memcpy in control path */\nrte_memcpy(&config, &default_config, sizeof(config));\n\n/* Good - standard memcpy for control path */\nmemcpy(&config, &default_config, sizeof(config));\n\n/* Good - rte_memcpy for packet data in fast path */\nrte_memcpy(rte_pktmbuf_mtod(m, void *), payload, len);\n```\n\n### Non-const Function Pointer Arrays\n\nArrays of function pointers (ops tables, dispatch tables, callback arrays) should be declared `const`\nwhen their contents are fixed at compile time.\nA non-`const` function pointer array can be overwritten by bugs or exploits,\nand prevents the compiler from placing the table in read-only memory.\n\n```c\n/* Bad - mutable when it doesn't need to be */\nstatic rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n\n/* Good - immutable dispatch table */\nstatic const rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n```\n\n**Exceptions** (do NOT flag):\n- Arrays modified at runtime for CPU feature detection or capability probing (e.g.,\n selecting a burst function based on `rte_cpu_get_flag_enabled()`)\n- Arrays containing mutable state (e.g., entries that are linked into lists)\n- Arrays populated dynamically via registration API\n- `dev_ops` or similar structures assigned per-device at init time\n\nOnly flag when the array is fully initialized at declaration with constant values\nand never modified thereafter.\n\n---\n\n## Forbidden Tokens\n\n### Functions\n\nForbidden function additions (`rte_panic()`, `rte_exit()`, `perror()`, `printf()`,\n`fprintf()`, `getenv()` in `lib/` and `drivers/`, with appropriate per-function\nexceptions for EAL, `examples/`, and `app/test/`) are caught by `devtools/checkpatches.sh`.\n\n### Atomics and Memory Barriers\n\nSubstitutions from the deprecated atomic API to the C11 DPDK wrappers\n(`__atomic_*`/`__ATOMIC_*` -> `rte_atomic_*_explicit()`/`rte_memory_order_*`,\n`__sync_*` -> `rte_atomic_*`, `rte_smp_{r,w,}mb()` -> `rte_atomic_thread_fence()`,\n`rte_atomic{16,32,64}_*()` -> `rte_atomic_*_explicit()`)\nare handled by coccinelle scripts in `devtools/cocci/`.\nAI review only needs to flag conceptual misuse, covered in the sections below.\n\n#### Shared Variable Access: volatile vs Atomics\n\nVariables shared between threads or between a thread and a signal handler **must** use atomic operations.\nThe C `volatile` keyword is NOT a substitute for atomics\n-- it prevents compiler optimization of accesses but provides no atomicity guarantees\nand no memory ordering between threads.\nOn some architectures, `volatile` reads and writes may tear on unaligned or multi-word values.\n\nDPDK provides C11 atomic wrappers that are portable across all supported compilers and architectures.\nAlways use these for shared state.\n\n**Reading shared variables:**\n\n```c\n/* BAD - volatile provides no atomicity or ordering guarantee */\nvolatile int stop_flag;\nif (stop_flag) /* data race, compiler/CPU can reorder */\n return;\n\n/* BAD - direct access to shared variable without atomic */\nif (shared->running) /* undefined behavior if another thread writes */\n process();\n\n/* GOOD - DPDK C11 atomic wrapper */\nif (rte_atomic_load_explicit(&shared->stop_flag, rte_memory_order_acquire))\n return;\n\n/* GOOD - relaxed is fine for statistics or polling a flag where\n * you don't need to synchronize other memory accesses */\ncount = rte_atomic_load_explicit(&shared->count, rte_memory_order_relaxed);\n```\n\n**Writing shared variables:**\n\n```c\n/* BAD - volatile write */\nvolatile int *flag = &shared->ready;\n*flag = 1;\n\n/* GOOD - atomic store with appropriate ordering */\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n```\n\n**Read-modify-write operations:**\n\n```c\n/* BAD - not atomic even with volatile */\nvolatile uint64_t *counter = &stats->packets;\n*counter += nb_rx; /* TOCTOU: load, add, store is 3 operations */\n\n/* GOOD - atomic add */\nrte_atomic_fetch_add_explicit(&stats->packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Memory Ordering Guide\n\nUse the weakest ordering that is correct.\nStronger ordering constrains hardware and compiler optimization unnecessarily.\n\n| DPDK Ordering | When to Use |\n|---------------|-------------|\n| `rte_memory_order_relaxed` | Statistics counters, polling flags where no other data depends on the value. Most common for simple counters. |\n| `rte_memory_order_acquire` | **Load** side of a flag/pointer that guards access to other shared data. Ensures subsequent reads see data published by the releasing thread. |\n| `rte_memory_order_release` | **Store** side of a flag/pointer that publishes shared data. Ensures all prior writes are visible to a thread that does an acquire load. |\n| `rte_memory_order_acq_rel` | Read-modify-write operations (e.g., `fetch_add`) that both consume and publish shared state in one operation. |\n| `rte_memory_order_seq_cst` | Rarely needed. Only when multiple independent atomic variables must be observed in a globally consistent total order. Avoid unless required. |\n\n**Common pattern -- producer/consumer flag:**\n\n```c\n/* Producer thread: fill buffer, then signal ready */\nfill_buffer(buf, data, len);\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n\n/* Consumer thread: wait for flag, then read buffer */\nwhile (!rte_atomic_load_explicit(&shared->ready, rte_memory_order_acquire))\n rte_pause();\nprocess_buffer(buf, len); /* guaranteed to see producer's writes */\n```\n\n**Common pattern -- statistics counter (no ordering needed):**\n\n```c\nrte_atomic_fetch_add_explicit(&port_stats->rx_packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Standalone Fences\n\nPrefer ordering on the atomic operation itself (acquire load, release store) over standalone fences.\nStandalone fences (`rte_atomic_thread_fence()`) are a blunt instrument\nthat orders ALL memory accesses around the fence, ","isInternal":false,"tokens":12126,"sizeBytes":50000},{"name":"marketplace.json","path":".claude-plugin/marketplace.json","rawUrl":"https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/marketplace.json","title":"Claude Plugin Marketplace Catalog","category":"marketplace","format":"json","content":"# Claude Plugin Marketplace Catalog\n\nPath: `.claude-plugin/marketplace.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/marketplace.json)","tokens":300,"sizeBytes":188},{"name":"plugin.json","path":".claude-plugin/plugin.json","rawUrl":"https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/plugin.json","title":"claude-plugin Plugin Manifest","category":"plugin-manifest","format":"json","content":"# claude-plugin Plugin Manifest\n\nPath: `.claude-plugin/plugin.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/plugin.json)","tokens":300,"sizeBytes":174},{"name":"mcp.json","path":".cursor/mcp.json","rawUrl":"https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.cursor/mcp.json","title":"Model Context Protocol (MCP) Configuration","category":"mcp-config","format":"json","content":"# Model Context Protocol (MCP) Configuration\n\nPath: `.cursor/mcp.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.cursor/mcp.json)","tokens":300,"sizeBytes":167}],"systemPromptSnippet":"\n\n\n\n\n# AGENTS.md - DPDK Code Review Guidelines for AI Tools\n\n## CRITICAL INSTRUCTION - READ FIRST\n\nThis document has two categories of review rules with different confidence thresholds,\nplus rules on evidence and scope that apply to both.\n\n### 1. Correctness Bugs -- HIGHEST PRIORITY (report at >=50% confidence)\n\n**Always report potential correctness bugs.** These are the most valuable findings.\nWhen in doubt, report them -- subject to the evidence rule below.\nUncertainty about code you have read is fine:\na possible use-after-free or resource leak is worth mentioning even if you are not certain.\nUncertainty about code you have not read is not a finding at all.\n\nCorrectness bugs include:\n- Use-after-free (accessing memory after `free`/`rte_free`)\n- Resource leaks on error paths (memory, file descriptors, locks)\n- Double-free or double-close\n- NULL pointer dereference\n- Buffer overflows or out-of-bounds access\n- Uninitialized variable use in a reachable code path\n- Race conditions (unsynchronized shared state)\n- `volatile` used instead of atomic operations for inter-thread shared variables\n- Missing error checks on functions that can fail\n- Error paths that skip cleanup (goto labels, missing free/close)\n- Incorrect error propagation (wrong return value, lost errno)\n- Logic errors in conditionals (wrong operator, inverted test)\n- Integer overflow/truncation in size calculations\n- Missing bounds checks on user-supplied sizes or indices\n- Statistics accumulation using `=` instead of `+=`\n- Integer multiply without widening cast losing upper bits (16x16, 32x32, etc.)\n- Unbounded descriptor chain traversal on guest/API-supplied data\n- `1 << n` on 64-bit bitmask (must use `1ULL << n` or `RTE_BIT64()`)\n- Left shift of narrow unsigned (`uint8_t`/`uint16_t`) used as 64-bit value (sign extension via implicit `int` promotion)\n- Variable assigned then overwritten before being read (dead store)\n- Same variable used as loop counter in nested loops\n- `rte_mbuf_raw_free_bulk()` called on mbufs that may originate from different mempools (Tx burst, ring dequeue)\n- MTU confused with frame length (MTU is L3 payload; frame length = MTU + L2 overhead)\n- Using `dev_conf.rxmode.mtu` after configure instead of `dev->data->mtu`\n- Hardcoded Ethernet overhead instead of per-device calculation\n- MTU set without enabling `RTE_ETH_RX_OFFLOAD_SCATTER` when frame size exceeds mbuf data room\n- `mtu_set` callback rejects valid MTU when scatter Rx is already enabled\n- Rx queue setup silently drops oversized packets instead of enabling scatter or returning an error\n- Rx function selection ignores `scattered_rx` flag or MTU-vs-mbuf-size check\n\n**Do NOT self-censor correctness bugs.**\nIf you identify a code path where a resource could leak or memory could be used after free,\nreport it -- subject to the evidence rule below.\nDo not talk yourself out of a bug you can see.\nDo not talk yourself into one you cannot.\n\n### Evidence rule (applies to every finding)\n\nEvery finding must rest on code you have actually read.\n\"Read\" means a line in the patch, or a file you opened with a tool.\n\nThis document is used in two ways and the rule is the same in both.\n\n**Diff only** -- CI, `review-patch.py`, and any run without tool access.\nAll you have read is the patch: added and removed lines, plus a few lines of context.\nYou cannot see the rest of the file, other callers, or any other file in the tree,\nso any claim that depends on them cannot be made.\n\n**With tool access** -- CLI or agentic use, where you can clone the tree,\napply the series, grep, and read whole files.\nRead them.\nCheck the surrounding function, the callers, and the headers before you report.\nTool access does not lower the bar; it widens what you are able to read,\nand therefore what you are expected to have read before filing a finding.\n\nIn either mode:\n\n- Report a finding only if you can name the line that causes it and quote it verbatim.\n For anything the patch introduces or changes, that line must be an added or changed line.\n A note about surrounding code may rest on a line you read in the tree,\n and is Info only -- see \"Pre-existing code\" below.\n- The line you quote must itself show the defect you claim.\n If you claim a missing barrier before a read and the line you quote is a write,\n the finding is wrong.\n Delete it rather than reword the claim to fit the quote.\n- Never quote, paraphrase, or reconstruct code you have not read.\n If you are writing a code block with line numbers you did not read, you are inventing it.\n Delete the finding.\n- If a claim depends on code you have not read -- other callers, other files,\n the rest of the function -- either go read it or omit the claim.\n Do not file it with a caveat, do not call it a risk,\n and do not ask the author to verify it on your behalf.\n- \"presumably\", \"appears to be\", \"if this is\", \"potentially\", \"could be\" used to hedge\n *what the code says* mean you lack the evidence.\n Get it or omit the finding.\n A conditional about *runtime behaviour*, with the line in hand, is not a hedge\n and is what a correctness finding looks like:\n \"this leaks if `rte_malloc()` returns NULL\" is a finding, \"this appears to allocate\n somewhere\" is not.\n The test: would removing the qualifier require reading code you have not read?\n\nThe >=50% bar for correctness bugs applies to your judgement about code you have read.\nIt is not permission to speculate about code you have not.\n\n### Pre-existing code (applies to every finding)\n\nThe unit of review is the patch.\nCode that appears only as diff context, or that you read in the tree but the patch does not touch,\nis not what this author is being asked to fix.\nA finding about such code must never be an Error or a Warning\nand must never withhold a Reviewed-by.\n\nReport it as Info, and say plainly that it is pre-existing and not introduced by this patch.\nThis is worth doing: it puts the observation in front of the people working in that code\nso it can be picked up in a separate patch later.\n\nThe evidence rule still applies in full.\nAn Info note about surrounding code needs the same proof as any other finding:\nyou must have read the code and be able to quote the line.\n\"Pre-existing\" is a lower severity, not a lower standard of evidence.\n\n### Hardening suggestions (applies to every finding)\n\nValidating a value supplied by a cooperating peer -- a PF driver, firmware,\nor the device's own descriptors -- is a hardening suggestion, not a bug.\nReport it as Info.\n\nIt becomes a Warning or an Error only when the value crosses a trust boundary,\nor when it drives index, length, or state arithmetic:\n\n- Error: a buffer count from the PF used as a ring index or loop bound\n without being checked against `nb_desc`\n- Info: clamping a residual length field that a well-behaved PF would never\n get wrong, where an out-of-range value cannot escape the local computation\n\n\"A misbehaving firmware could send garbage\" is true of every field in every\nmailbox message. On its own it is not a severity.\n\n### 2. Style, Process, and Formatting -- suppress false positives\n\n**NEVER list a style/process item under \"Errors\" or \"Warnings\" if you conclude it is correct.**\n\nBefore outputting any style, formatting, or process error/warning, verify it is actually wrong.\nIf your analysis concludes with phrases like \"there's no issue here\", \"which is fine\",\n\"appears correct\", \"is acceptable\", or \"this is actually correct\" --\nthen DO NOT INCLUDE IT IN YOUR OUTPUT AT ALL.\nDelete it.\nOmit it entirely.\n\nThis suppression rule applies to: naming conventions, code style, and process compliance.\nIt does NOT apply to correctness bugs listed above.\n(SPDX/copyright format and commit message formatting are handled by checkpatch\nand are excluded from AI review entirely.)\n\n---\n\nThis document provides guidelines for AI-powered code review tools\nwhen reviewing contributions to the Data Plane Development Kit (DPDK).\nIt is derived from the official DPDK contributor guidelines and validation scripts.\n\n## Overview\n\nDPDK follows a development process modeled on the Linux Kernel.\nAll patches are reviewed publicly on the mailing list before being merged.\nAI review tools should verify compliance with the standards outlined below.\n\n## Review Philosophy\n\n**Correctness bugs are the primary goal of AI review.** Style and formatting checks are secondary.\nA review that catches a use-after-free but misses a style nit is far more valuable\nthan one that catches every style issue but misses the bug.\n\n**BEFORE OUTPUTTING YOUR REVIEW**: Re-read each item.\n- For correctness bugs: keep them.\n If you have reasonable doubt that a code path is safe, report it.\n- For style/process items: if ANY item contains phrases like \"is fine\", \"no issue\",\n \"appears correct\", \"is acceptable\", \"actually correct\" -- DELETE THAT ITEM.\n Do not include it.\n\n### Correctness review guidelines\n- Trace error paths: for every function that allocates a resource or acquires a lock,\n verify that ALL error paths after that point release it\n- Check every `goto error` and early `return`: does it clean up everything allocated so far?\n- Look for use-after-free: after `free(p)`, is `p` accessed again?\n- Check that error codes are propagated, not silently dropped\n- Report at >=50% confidence about code you have read\n- Never substitute a confidence caveat for evidence:\n if the reasoning depends on code you have not read, go read it or drop the finding\n- It is better to report a potential bug that turns out to be safe than to miss a real bug,\n provided you can point to the line that causes it\n\n### Style and process review guidelines\n- Only comment on style/process issues when you have HIGH CONFIDENCE (>80%) that an issue exists\n- Be concise: one sentence per comment when possible\n- Focus on actionable feedback, not observations\n- When reviewing text, only comment on clarity issues if the text is genuinely confusing\n or could lead to errors.\n- Do NOT comment on copyright years, SPDX format, or copyright holders -\n not subject to AI review\n- Do NOT report an issue then contradict yourself -\n if something is acceptable, do not mention it at all\n- Do NOT include items in Errors/Warnings that you then say are \"acceptable\" or \"correct\"\n- Do NOT mention things that are correct or \"not an issue\" - only report actual problems\n- Do NOT speculate about contributor circumstances (employment, company policies, etc.)\n- Before adding any style item to your review, ask: \"Is this actually wrong?\"\n If no, omit it entirely.\n- NEVER write \"(Correction: ...)\" - if you need to correct yourself, simply omit the item entirely\n- Do NOT add vague suggestions like \"should be verified\" or \"should be checked\" -\n either it's wrong or don't mention it\n- Do NOT flag something as an Error then say \"which is correct\" in the same item\n- Do NOT say \"no issue here\" or \"this is actually correct\" -\n if there's no issue, do not include it in your review\n- Do NOT analyze cross-patch dependencies or compilation order -\n you cannot reliably determine this from patch review\n- Do NOT claim a patch \"would cause compilation failure\" based on symbols used\n in other patches in the series\n- Review each patch individually for its own correctness;\n assume the patch author ordered them correctly\n- When reviewing a patch series, OMIT patches that have no issues.\n Do not include a patch in your output just to say \"no issues found\"\n or to summarize what the patch does.\n Only include patches where you have actual findings to report.\n- Do NOT end the review with a summary section listing every patch in the series.\n A clean patch must not appear anywhere in the output, including in a summary or status table.\n\n## Priority Areas (Review These)\n\n### Security & Safety\n- Unsafe code blocks without justification\n- Command injection risks (shell commands, user input)\n- Path traversal vulnerabilities\n- Credential exposure or hard coded secrets\n- Missing input validation on external data\n- Improper error handling that could leak sensitive info\n\n### Correctness Issues\n- Logic errors that could cause panics or incorrect behavior\n- Buffer overflows\n- Race conditions\n- **`volatile` for inter-thread synchronization**:\n `volatile` does not provide atomicity or memory ordering between threads.\n Use `rte_atomic_load_explicit()`/`rte_atomic_store_explicit()` with appropriate `rte_memory_order_*` instead.\n See the Shared Variable Access section under Forbidden Tokens for details.\n- Resource leaks (files, connections, memory)\n- Off-by-one errors or boundary conditions\n- Incorrect error propagation\n- **Use-after-free** (any access to memory after it has been freed)\n- **Error path resource leaks**:\n For every allocation or fd open,\n trace each error path (`goto`, early `return`, conditional) to verify the resource is released.\n Common patterns to check:\n - `malloc`/`rte_malloc` followed by a failure that does `return -1` instead of `goto cleanup`\n - `open()`/`socket()` fd not closed on a later error\n - Lock acquired but not released on an error branch\n - Partially initialized structure where early fields are allocated\n but later allocation fails without freeing the early ones\n- **Double-free / double-close**:\n resource freed in both a normal path and an error path,\n or fd closed but not set to -1 allowing a second close\n- **Missing error checks**:\n functions that can fail (malloc, open, ioctl, etc.) whose return value is not checked\n- **Do NOT flag unchecked return values from functions\n that always succeed on Linux.** Some POSIX functions are documented to always return zero on Linux\n when called with valid arguments.\n Flagging unchecked returns from these is a false positive.\n The list includes:\n - `pthread_mutex_init()`, `pthread_cond_init()`\n - `pthread_cond_signal()`, `pthread_cond_broadcast()`, `pthread_cond_wait()`\n - `pthread_condattr_init()`, `pthread_condattr_destroy()`\n - `pthread_attr_init()`, `pthread_attr_destroy()`\n- Changes to API without release notes\n- Changes to ABI on non-LTS release\n- Usage of deprecated API when replacements exist\n- Overly defensive code that adds unnecessary checks\n- Unnecessary comments that just restate what the code already shows (remove them)\n- **Process-shared synchronization errors** (pthread mutexes in shared memory without `PTHREAD_PROCESS_SHARED`)\n- **Statistics accumulation using `=` instead of `+=`**:\n When accumulating statistics (counters, byte totals, packet counts),\n using `=` overwrites the running total with only the latest value.\n This silently produces wrong results.\n ```c\n /* BAD - overwrites instead of accumulating */\n stats->rx_packets = nb_rx;\n stats->rx_bytes = total_bytes;\n\n /* GOOD - accumulates over time */\n stats->rx_packets += nb_rx;\n stats->rx_bytes += total_bytes;\n ```\n Note: `=` is correct for gauge-type values (e.g., queue depth, link status)\n and for initial assignment.\n Only flag when the context is clearly incremental accumulation\n (loop bodies, per-burst counters, callback tallies).\n- **Integer multiply without widening cast**:\n When multiplying integers to produce a result wider than the operands (sizes, offsets, byte counts),\n the multiplication is performed at the operand width\n and the upper bits are silently lost before the assignment.\n This applies to any narrowing scenario: 16x16 assigned to a 32-bit variable,\n 32x32 assigned to a 64-bit variable, etc.\n ```c\n /* BAD - 32x32 overflows before widening to 64 */\n uint64_t total_size = num_entries * entry_size; /* both are uint32_t */\n size_t offset = ring->idx * ring->desc_size; /* 32x32 -> truncated */\n\n /* BAD - 16x16 overflows before widening to 32 */\n uint32_t byte_count = pkt_len * nb_segs; /* both are uint16_t */\n\n /* GOOD - widen before multiply */\n uint64_t total_size = (uint64_t)num_entries * entry_size;\n size_t offset = (size_t)ring->idx * ring->desc_size;\n uint32_t byte_count = (uint32_t)pkt_len * nb_segs;\n ```\n- **Unbounded descriptor chain traversal**:\n When walking a chain of descriptors (virtio, DMA, NIC Rx/Tx rings) where the chain length\n or next-index comes from guest memory or an untrusted API caller,\n the traversal MUST have a bounds check or loop counter to prevent infinite loops\n or out-of-bounds access from malicious/corrupt data.\n ```c\n /* BAD - guest controls desc[idx].next with no bound */\n while (desc[idx].flags & VRING_DESC_F_NEXT) {\n idx = desc[idx].next; /* guest-supplied, unbounded */\n process(desc[idx]);\n }\n\n /* GOOD - cap iterations to descriptor ring size */\n for (i = 0; i < ring_size; i++) {\n if (!(desc[idx].flags & VRING_DESC_F_NEXT))\n break;\n idx = desc[idx].next;\n if (idx >= ring_size) /* bounds check */\n return -EINVAL;\n process(desc[idx]);\n }\n ```\n This applies to any chain/linked-list traversal where indices\n or pointers originate from untrusted input (guest VMs, user-space callers, network packets).\n- **Bitmask shift using `1` instead of `1ULL` on 64-bit masks**:\n The literal `1` is `int` (32 bits).\n Shifting it by 32 or more is undefined behavior; shifting it by less than 32\n but assigning to a `uint64_t` silently zeroes the upper 32 bits.\n Use `1ULL << n`, `UINT64_C(1) << n`, or the DPDK `RTE_BIT64(n)` macro.\n ```c\n /* BAD - 1 is int, UB if n >= 32, wrong if result used as uint64_t */\n uint64_t mask = 1 << bit_pos;\n if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) /* bit 15 OK, bit 32+ UB */\n\n /* GOOD */\n uint64_t mask = UINT64_C(1) << bit_pos;\n uint64_t mask = 1ULL << bit_pos;\n uint64_t mask = RTE_BIT64(bit_pos); /* preferred in DPDK */\n if (features & RTE_BIT64(VIRTIO_NET_F_MRG_RXBUF))\n ```\n Note: `1U << n` is acceptable when the mask is known to be 32-bit\n (e.g., `uint32_t` register fields with `n < 32`).\n Only flag when the result is stored in, compared against,\n or returned as a 64-bit type, or when `n` could be >= 32.\n- **Left shift of narrow unsigned type sign-extends to 64-bit**:\n When a `uint8_t` or `uint16_t` value is left-shifted,\n C integer promotion converts it to `int` (signed 32-bit) before the shift.\n If the result has bit 31 set, implicit conversion to `uint64_t`, `size_t`,\n or use in pointer arithmetic sign-extends the upper 32 bits to all-1s,\n producing a wrong address or value.\n This is Coverity SIGN_EXTENSION.\n The fix is to cast the narrow operand to an unsigned type\n at least as wide as the target before shifting.\n ```c\n /* BAD - uint16_t promotes to signed int, bit 31 may set,\n * then sign-extends when converted to 64-bit for pointer math */\n uint16_t idx = get_index();\n void *addr = base + (idx << wqebb_shift); /* SIGN_EXTENSION */\n uint64_t off = (uint64_t)(idx << shift); /* too late: shift already in int */\n\n /* BAD - uint8_t shift with result used as size_t */\n uint8_t page_order = get_order();\n size_t size = page_order << PAGE_SHIFT; /* promotes to int first */\n\n /* GOOD - cast before shift */\n void *addr = base + ((uint64_t)idx << wqebb_shift);\n uint64_t off = (uint64_t)idx << shift;\n size_t size = (size_t)page_order << PAGE_SHIFT;\n\n /* GOOD - intermediate unsigned variable */\n uint32_t offset = (uint32_t)idx << wqebb_shift; /* OK if result fits 32 bits */\n ```\n Note: This is distinct from the `1 << n` pattern (where the literal `1` is the problem)\n and from the integer-multiply pattern (where the operation is `*` not `<<`).\n The mechanism is the same C integer promotion rule,\n but the code patterns and Coverity checker names differ.\n Only flag when the shift result is used in a context wider than 32 bits (64-bit assignment,\n pointer arithmetic, function argument expecting `uint64_t`/`size_t`).\n A shift whose result is stored in a `uint32_t` or narrower variable is not affected.\n- **Variable overwrite before read (dead store)**:\n A variable is assigned a value that is unconditionally overwritten before it is ever read.\n This usually indicates a logic error (wrong variable name, missing `if`, copy-paste mistake)\n or at minimum is dead code.\n ```c\n /* BAD - first assignment is never read */\n ret = validate_input(cfg);\n ret = apply_config(cfg); /* overwrites without checking first ret */\n if (ret != 0)\n return ret;\n\n /* GOOD - check each return value */\n ret = validate_input(cfg);\n if (ret != 0)\n return ret;\n ret = apply_config(cfg);\n if (ret != 0)\n return ret;\n ```\n Do NOT flag cases where the initial value is intentionally a default that may\n or may not be overwritten (e.g., `int ret = 0;` followed by a conditional assignment).\n Only flag unconditional overwrites where the first value can never be observed.\n- **Shared loop counter in nested loops**:\n Using the same variable as the loop counter in both an outer\n and inner loop causes the outer loop to malfunction because the inner loop modifies its counter.\n ```c\n /* BAD - inner loop clobbers outer loop counter */\n int i;\n for (i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (i = 0; i < nb_descs; i++) /* BUG: reuses i */\n init_desc(i);\n }\n\n /* GOOD - distinct loop counters */\n for (int i = 0; i < nb_queues; i++) {\n setup_queue(i);\n for (int j = 0; j < nb_descs; j++)\n init_desc(j);\n }\n ```\n- **`rte_mbuf_raw_free_bulk()` on mixed-pool mbuf arrays**:\n Tx burst functions and ring/queue dequeue paths receive mbufs\n that may originate from different mempools (applications are free to send mbufs from any pool).\n `rte_mbuf_raw_free_bulk()` takes an explicit mempool parameter\n and calls `rte_mempool_put_bulk()` directly --\n ALL mbufs in the array must come from that single pool.\n If mbufs come from different pools, they are returned to the wrong pool,\n corrupting pool accounting and causing hard-to-debug failures.\n Note: `rte_pktmbuf_free_bulk()` is safe for mixed pools --\n it batches mbufs by pool internally and flushes whenever the pool changes.\n ```c\n /* BAD - assumes all mbufs are from the same pool */\n /* (in tx_burst completion or ring dequeue error path) */\n rte_mbuf_raw_free_bulk(mp, mbufs, nb_mbufs);\n\n /* GOOD - rte_pktmbuf_free_bulk handles mixed pools correctly */\n rte_pktmbuf_free_bulk(mbufs, nb_mbufs);\n\n /* GOOD - free individually (each mbuf returned to its own pool) */\n for (i = 0; i < nb_mbufs; i++)\n rte_pktmbuf_free(mbufs[i]);\n ```\n This applies to any path that frees mbufs submitted by the application:\n Tx completion, Tx error cleanup, and ring/queue drain paths.\n `rte_mbuf_raw_free_bulk()` is an optimization for the fast-free case (`RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE`)\n where the application guarantees all mbufs come from a single pool with refcnt=1.\n- **MTU confused with Ethernet frame length**:\n Maximum Transmission Unit (MTU) is the maximum L3 payload size\n (e.g., 1500 bytes for standard Ethernet).\n The maximum Ethernet *frame length* includes L2 overhead:\n Ethernet header (14 bytes) + optional VLAN tags (4 bytes each) + CRC (4 bytes).\n The overhead varies per device depending on supported encapsulations (VLAN, QinQ, etc.).\n Confusing MTU with frame length produces off-by-14-to-22-byte errors in packet size limits,\n buffer sizing, and scattered Rx decisions.\n\n **VLAN tag accounting:** The outer VLAN tag is L2 overhead\n and does NOT count toward MTU (matching Linux and FreeBSD).\n A 1522-byte single-tagged frame is valid at MTU 1500.\n However, in QinQ the inner (customer) tag DOES consume MTU --\n it is part of the customer frame.\n So QinQ with MTU 1500 allows only 1496 bytes of L3 payload\n unless the port MTU is raised to 1504.\n\n **Using `rxmode.mtu` after configure:** After `rte_eth_dev_configure()` completes,\n the canonical MTU is stored in `dev->data->mtu`.\n The `dev->data->dev_conf.rxmode.mtu` field is the user's *request*\n and must not be read after configure --\n it becomes stale if `rte_eth_dev_set_mtu()` is called later.\n Both configure and set_mtu write to `dev->data->mtu`; PMDs should always read from there.\n\n **Overhead calculation:** Do not hardcode a single overhead constant.\n Use the device's own overhead calculation\n (typically available via `dev_info.max_rx_pktlen - dev_info.max_mtu`\n or an internal `eth_overhead` field).\n Different devices support different encapsulations, so the overhead is not a universal constant.\n\n **Scattered Rx decision:** PMDs compare maximum frame length (MTU + per-device overhead)\n against Rx buffer size to decide whether scattered Rx is needed.\n Comparing raw MTU against buffer size is wrong --\n it underestimates the actual frame size by the overhead.\n ```c\n /* BAD - MTU used where frame length is needed */\n if (dev->data->mtu > rxq->buf_size)\n enable_scattered_rx();\n\n /* BAD - hardcoded overhead, wrong for QinQ-capable devices */\n #define ETHER_OVERHEAD 18 /* may be 22 or 26 for VLAN/QinQ */\n max_frame = mtu + ETHER_OVERHEAD;\n\n /* BAD - reading rxmode.mtu after configure (stale if set_mtu called) */\n static int\n mydrv_rx_queue_setup(...) {\n mtu = dev->data->dev_conf.rxmode.mtu; /* WRONG - may be stale */\n ...\n }\n\n /* GOOD - use dev->data->mtu, the canonical post-configure value */\n static int\n mydrv_rx_queue_setup(...) {\n uint16_t mtu = dev->data->mtu;\n ...\n }\n\n /* GOOD - use per-device overhead for frame length calculation */\n uint32_t frame_overhead = dev_info.max_rx_pktlen - dev_info.max_mtu;\n uint32_t max_frame_len = dev->data->mtu + frame_overhead;\n if (max_frame_len > rxq->buf_size)\n enable_scattered_rx();\n\n /* GOOD - device-specific overhead constant derived from capabilities */\n static uint32_t\n mydrv_eth_overhead(struct rte_eth_dev *dev) {\n uint32_t overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_VLAN)\n overhead += RTE_VLAN_HLEN;\n if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_QINQ)\n overhead += RTE_VLAN_HLEN;\n return overhead;\n }\n ```\n Note: In `rte_eth_dev_configure()` itself, reading `rxmode.mtu` is correct --\n that is where the user's request is consumed and written to `dev->data->mtu`.\n Only flag reads of `rxmode.mtu` *outside* configure (queue setup, start, link update, MTU set, etc.).\n- **Missing scatter Rx for large MTU**:\n When the configured MTU produces a frame size (MTU + Ethernet overhead) larger\n than the mbuf data buffer size (`rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`),\n the PMD MUST either enable scatter Rx (multi-segment receive) or reject the configuration.\n Silently accepting the MTU and then truncating or dropping oversized packets is a correctness bug.\n ```c\n /* BAD - accepts MTU but will truncate packets that don't fit */\n static int\n mydrv_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)\n {\n /* No check against mbuf size or scatter capability */\n dev->data->mtu = mtu;\n return 0;\n }\n\n /* BAD - rejects valid MTU even though scatter is enabled */\n if (frame_size > mbuf_data_size)\n return -EINVAL; /* wrong: should allow if scatter is on */\n\n /* GOOD - check scatter and mbuf size */\n if (!dev->data->scattered_rx &&\n frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)\n return -EINVAL;\n\n /* GOOD - auto-enable scatter when needed */\n if (frame_size > mbuf_data_size) {\n if (!(dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SCATTER))\n return -EINVAL;\n dev->data->dev_conf.rxmode.offloads |=\n RTE_ETH_RX_OFFLOAD_SCATTER;\n dev->data->scattered_rx = 1;\n }\n ```\n Key relationships:\n - `dev_info.max_rx_pktlen`: maximum frame the hardware can receive\n - `dev_info.max_mtu`: maximum MTU = `max_rx_pktlen` - overhead\n - `dev_info.min_rx_bufsize`: minimum Rx buffer the HW requires\n - `dev_info.max_rx_bufsize`: maximum single-descriptor buffer size\n - `mbuf data size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM`\n - When scatter is off: frame length must fit in a single mbuf\n - When scatter is on: frame length can span multiple mbufs;\n the PMD selects a scattered Rx function\n\n This pattern should be checked in three places:\n 1. `dev_configure()` -- validate MTU against mbuf size / scatter\n 2. `rx_queue_setup()` -- select scattered vs non-scattered Rx path\n 3. `mtu_set()` -- runtime MTU change must re-validate\n- **Rx queue function selection ignoring scatter**:\n When a PMD has separate fast-path Rx functions for scalar (single-segment)\n and scattered (multi-segment) modes,\n it must select the scattered variant whenever `dev->data->scattered_rx` is set OR\n when the configured frame length exceeds the single mbuf data size.\n Failing to do so causes the scalar Rx function to silently drop or corrupt multi-segment packets.\n ```c\n /* BAD - only checks offload flag, ignores actual need */\n if (rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER)\n rx_func = mydrv_recv_scattered;\n else\n rx_func = mydrv_recv_single; /* will drop oversized pkts */\n\n /* GOOD - check both the flag and the size */\n mbuf_size = rte_pktmbuf_data_room_size(rxq->mp) -\n RTE_PKTMBUF_HEADROOM;\n max_pkt = dev->data->mtu + overhead;\n if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||\n max_pkt > mbuf_size) {\n dev->data->scattered_rx = 1;\n rx_func = mydrv_recv_scattered;\n } else {\n rx_func = mydrv_recv_single;\n }\n ```\n\n### Cryptographic and Security Code\n\nApplies only when the patch touches crypto PMDs (`drivers/crypto/`),\nthe security or ipsec libraries,\nor code handling key material, IVs, or authentication digests.\nStays silent on all other patches.\n\n- **Non-constant-time digest/tag comparison** (Error):\n When verifying an authentication tag, digest, MAC, or ICV --\n comparing a computed value against an attacker-supplied one to decide accept/reject --\n plain `memcmp()` leaks timing information.\n Use `rte_memeq_timingsafe()` instead.\n Return semantics differ: `memcmp()` returns 0 on equal,\n `rte_memeq_timingsafe()` returns true on equal.\n So `if (memcmp(tag, digest, len))` becomes `if (!rte_memeq_timingsafe(tag, digest, len))`.\n Do NOT flag `memcmp()` on non-secret data:\n algorithm IDs, key lengths, capability/feature structs, lookup keys.\n Only flag comparisons that gate acceptance of attacker-influenced data.\n\n- **Sensitive material not zeroed before free** (Error):\n Keys, expanded key schedules, HMAC ipad/opad, and session secrets must be wiped,\n not merely freed -- a plain free leaves secrets in the heap.\n - `rte_malloc`/`rte_zmalloc`'d secret: use `rte_free_sensitive()` instead of `rte_free()`\n - local or stack secret going out of scope: `rte_memzero_explicit()` before return;\n plain `memset()` may be optimized away\n Flag only where the patch itself allocates, copies, or frees the secret.\n Do NOT flag buffers that never held secrets (descriptors, `dev_info`),\n pre-existing free paths the patch does not touch,\n or `memset()` the compiler cannot elide because the pointer escapes to `free()`.\n `rte_memzero_explicit()` followed by a plain `rte_free()` is correct\n and is not a missing `rte_free_sensitive()`.\n\n- **Non-cryptographic RNG for keys/IVs** (Error):\n `rte_rand()` is seeded from system entropy (`getentropy()`, with RDSEED and TSC fallbacks),\n but the generator itself is an LFSR and is linear:\n an observer who sees enough output can recover the state and predict every later value.\n Good seeding is not sufficient for key, IV, or nonce generation.\n Use a cryptographic source instead:\n the device's own entropy source or DRBG,\n or `getrandom(2)` / `/dev/urandom` in the control path.\n `rand()` and `random()` are weaker still and must not appear in crypto or security code.\n Do NOT flag `rte_rand()` used for non-secret purposes:\n hash seeds (`lib/ipsec/ipsec_sad.c` uses it for `hash_func_init_val`),\n jitter, backoff, load balancing, or test data.\n\nDo NOT flag:\n- IV/nonce reuse -- a runtime property, not determinable from a patch.\n- `memcmp()` on lengths, algorithm selectors, or non-secret config.\n\n### Architecture & Patterns\n- Code that violates existing patterns in the code base\n- Missing error handling\n- Code that is not safe against signals\n\n### New Library API Design\n\nWhen a patch adds a new library under `lib/`, review API design in addition to correctness and style.\n\n**API boundary.** A library should be a compiler, not a framework.\nThe model is `rte_acl`: create a context, feed input, get structured output,\ncaller decides what to do with it.\nNo callbacks needed.\nIf the library requires callers to implement a callback table to function,\nthe boundary is wrong -- the library is asking the caller to be its backend.\n\n**Callback structs** (Warning / Error).\nAny function-pointer struct in an installed header is an ABI break waiting to happen.\nAdding or reordering a member breaks all consumers.\n- Prefer a single callback parameter over an ops table.\n- \\>5 callbacks: **Warning** -- likely needs redesign.\n- \\>20 callbacks: **Error** -- this is an app plugin API, not a library.\n- All callbacks must have Doxygen (contract, return values, ownership).\n- Void-returning callbacks for failable operations swallow errors -- flag as **Error**.\n- Callbacks serving app-specific needs (e.g. `verbose_level_get`)\n indicate wrong code was extracted into the library.\n\n**Extensible structures.** Prefer TLV / tagged-array patterns over enum + union,\nfollowing `rte_flow_item` and `rte_flow_action` as the model.\nType tag + pointer to type-specific data allows adding types without ABI breaks.\nFlag as **Warning**:\n- Large enums (100+) consumers must switch on.\n- Unions that grow with every new feature.\n- Ask: \"What changes when a feature is added next release?\"\n If \"add an enum value and union arm\" -- should be TLV.\n\n**Installed headers.** If it's in `headers` or `indirect_headers` in meson.build, it's public API.\nDon't call it \"private.\"\nIf truly internal, don't install it.\n\n**Global state.** Prefer handle-based API (`create`/`destroy`) over singletons.\n`rte_acl` allows multiple independent classifier instances; new libraries should do the same.\n\n**Output ownership.** Prefer caller-allocated or library-allocated- caller-freed\nover internal static buffers.\nIf static buffers are used, document lifetime\nand ensure Doxygen examples don't show stale-pointer usage.\n\n---\n\n## C Coding Style\n\n### General Formatting\n\n- **Tab width**:\n 8 characters (hard tabs for indentation, spaces for alignment)\n- **No trailing whitespace** on lines or at end of files\n- Files must end with a new line\n- Code style should be consistent within each file\n\n### Comments\n\n```c\n/* Most single-line comments look like this. */\n\n/*\n * VERY important single-line comments look like this.\n */\n\n/*\n * Multi-line comments look like this. Make them real sentences. Fill\n * them so they look like real paragraphs.\n */\n```\n\n### Header File Organization\n\nInclude order (each group separated by blank line):\n1. System/libc includes\n2. DPDK EAL includes\n3. DPDK misc library includes\n4. Application-specific includes\n\n```c\n#include \n#include \n\n#include \n\n#include \n#include \n\n#include \"application.h\"\n```\n\n### Header Guards\n\n```c\n#ifndef _FILE_H_\n#define _FILE_H_\n\n/* Code */\n\n#endif /* _FILE_H_ */\n```\n\n### Naming Conventions\n\n- **All external symbols** must have `RTE_` or `rte_` prefix\n- **Macros**:\n ALL_UPPERCASE with `RTE_` prefix\n- **Functions**:\n lowercase with underscores only (no CamelCase)\n- **Variables**:\n lowercase with underscores only\n- **Enum values**:\n ALL_UPPERCASE with `RTE__` prefix\n\n**Exception**: Driver base directories (`drivers/*/base/`) may use different naming conventions\nwhen sharing code across platforms or with upstream vendor code.\n\n#### Symbol Naming for Static Linking\n\nDrivers and libraries must not expose global variables that could clash\nwhen statically linked with other DPDK components or applications.\nUse consistent and unique prefixes for all exported symbols to avoid namespace collisions.\n\n**Good practice**: Use a driver-specific or library-specific prefix for all global variables:\n\n```c\n/* Good - virtio driver uses consistent \"virtio_\" prefix */\nconst struct virtio_ops virtio_legacy_ops = {\n\t.read = virtio_legacy_read,\n\t.write = virtio_legacy_write,\n\t.configure = virtio_legacy_configure,\n};\n\nconst struct virtio_ops virtio_modern_ops = {\n\t.read = virtio_modern_read,\n\t.write = virtio_modern_write,\n\t.configure = virtio_modern_configure,\n};\n\n/* Good - mlx5 driver uses consistent \"mlx5_\" prefix */\nstruct mlx5_flow_driver_ops mlx5_flow_dv_ops;\n```\n\n**Bad practice**: Generic names that may clash:\n\n```c\n/* Bad - \"ops\" is too generic, will clash with other drivers */\nconst struct virtio_ops ops = { ... };\n\n/* Bad - \"legacy_ops\" could clash with other legacy implementations */\nconst struct virtio_ops legacy_ops = { ... };\n\n/* Bad - \"driver_config\" is not unique */\nstruct driver_config config;\n```\n\n**Guidelines**:\n- Prefix all global variables with the driver or library name (e.g., `virtio_`, `mlx5_`, `ixgbe_`)\n- Prefix all global functions similarly unless they use the `rte_` namespace\n- Internal static variables do not require prefixes as they have file scope\n- Consider using the `RTE_` or `rte_` prefix only for symbols that are part of the public DPDK API\n\n#### Prohibited Terminology\n\nDo not use non-inclusive naming including:\n- `master/slave` -> Use: primary/secondary, controller/worker, leader/follower\n- `blacklist/whitelist` -> Use: denylist/allowlist, blocklist/passlist\n- `cripple` -> Use: impacted, degraded, restricted, immobilized\n- `tribe` -> Use: team, squad\n- `sanity check` -> Use: coherence check, test, verification\n\n\n### Comparisons and Boolean Logic\n\nDPDK style requires explicit comparison against `NULL`, `0`, or `'\\0'` rather than\ntruthiness on pointers, integers, and characters: write `if (p == NULL)` not `if (!p)`,\nand `if (a != 0)` not `if (a)`.\nDirect truthiness is acceptable only on actual `bool` types.\n`likely()` and `unlikely()` wrappers do not change whether a comparison is explicit\n(`if (likely(p != NULL))` is still explicit).\n\nMechanical rewrites for these are handled by coccinelle in `devtools/cocci/`;\nAI review need only catch cases cocci can't see (e.g. macro-expanded conditions).\n\n### Boolean Usage\n\nPrefer `bool` (from ``) over `int` for variables, parameters,\nand return values that are purely true/false.\nUsing `bool` makes intent explicit, enables compiler diagnostics for misuse, and is self-documenting.\n\n```c\n/* Bad - int used as boolean flag */\nint verbose = 0;\nint is_enabled = 1;\n\nint\ncheck_valid(struct item *item)\n{\n\tif (item->flags & ITEM_VALID)\n\t\treturn 1;\n\treturn 0;\n}\n\n/* Good - bool communicates intent */\nbool verbose = false;\nbool is_enabled = true;\n\nbool\ncheck_valid(struct item *item)\n{\n\treturn item->flags & ITEM_VALID;\n}\n```\n\n**Guidelines:**\n- Use `bool` for variables that only hold true/false values\n- Use `bool` return type for predicate functions (functions that answer a yes/no question,\n often named `is_*`, `has_*`, `can_*`)\n- Use `true`/`false` rather than `1`/`0` for boolean assignments\n- Boolean variables and parameters should not use explicit comparison:\n `if (verbose)` is correct, not `if (verbose == true)`\n- `int` is still appropriate when a value can be negative, is an error code,\n or carries more than two states\n\n**Structure fields:**\n- `bool` occupies 1 byte.\n In packed or cache-critical structures, consider using a bitfield or flags word instead\n- For configuration structures and non-hot-path data, `bool` is preferred over `int` for flag fields\n\n```c\n/* Bad - int flags waste space and obscure intent */\nstruct port_config {\n\tint promiscuous; /* 0 or 1 */\n\tint link_up; /* 0 or 1 */\n\tint autoneg; /* 0 or 1 */\n\tuint16_t mtu;\n};\n\n/* Good - bool for flag fields */\nstruct port_config {\n\tbool promiscuous;\n\tbool link_up;\n\tbool autoneg;\n\tuint16_t mtu;\n};\n\n/* Also good - bitfield for cache-critical structures */\nstruct fast_path_config {\n\tuint32_t flags; /* bitmask of CONFIG_F_* */\n\t/* ... hot-path fields ... */\n};\n```\n\n**Do NOT flag:**\n- `int` return type for functions that return error codes (0 for success, negative for error) --\n these are NOT boolean\n- `int` used for tri-state or multi-state values\n- `int` flags in existing code where changing the type would be a large, unrelated refactor\n- Bitfield or flags-word approaches in performance-critical structures\n\n### Indentation and Braces\n\n```c\n/* Control statements - no braces for single statements */\nif (val != NULL)\n\tval = realloc(val, newsize);\n\n/* Braces on same line as else */\nif (test)\n\tstmt;\nelse if (bar) {\n\tstmt;\n\tstmt;\n} else\n\tstmt;\n\n/* Switch statements - don't indent case */\nswitch (ch) {\ncase 'a':\n\taflag = 1;\n\t/* FALLTHROUGH */\ncase 'b':\n\tbflag = 1;\n\tbreak;\ndefault:\n\tusage();\n}\n\n/* Long conditions - double indent continuation */\nif (really_long_variable_name_1 == really_long_variable_name_2 &&\n\t\treally_long_variable_name_3 == really_long_variable_name_4)\n\tstmt;\n```\n\n### Variable Declarations\n\n- Prefer declaring variables inside the basic block where they are used\n- Variables may be declared either at the start of the block, or at point of first use (C99 style)\n- Both declaration styles are acceptable; consistency within a function is preferred\n- Initialize variables only when a meaningful value exists at declaration time\n- Use C99 designated initializers for structures\n\n```c\n/* Good - declaration at start of block */\nint ret;\nret = some_function();\n\n/* Also good - declaration at point of use (C99 style) */\nfor (int i = 0; i < count; i++)\n\tprocess(i);\n\n/* Good - declaration in inner block where variable is used */\nif (condition) {\n\tint local_val = compute();\n\tuse(local_val);\n}\n\n/* Bad - unnecessary initialization defeats compiler warnings */\nint ret = 0;\nret = some_function(); /* Compiler won't warn if assignment removed */\n```\n\n### Function Format\n\n- Return type on its own line\n- Opening brace on its own line\n- Place an empty line between declarations and statements\n\n```c\nstatic char *\nfunction(int a1, int b1)\n{\n\tchar *p;\n\n\tp = do_something(a1, b1);\n\treturn p;\n}\n```\n\n---\n\n## Unnecessary Code Patterns\n\nThe following patterns add unnecessary code, hide bugs, or reduce performance.\nAvoid them.\nSeveral closely related patterns are caught by coccinelle scripts in `devtools/cocci/`\n(unnecessary `void *` casts, NULL checks before `free()`/`rte_free()`,\n`memset()` before `free()`, zero-length array members, dead variable initialization);\nthose are not duplicated here.\n\n### Appropriate Use of rte_malloc()\n\n`rte_malloc()` allocates from hugepage memory.\nUse it only when required:\n\n- Memory that will be accessed by DMA (NIC descriptors, packet buffers)\n- Memory shared between primary and secondary DPDK processes\n- Memory requiring specific NUMA node placement\n\nFor general allocations, use standard `malloc()` which is faster\nand does not consume limited hugepage resources.\n\nQueue-related buffers (descriptor rings, Rx/Tx queue control structures)\nshould use `rte_zmalloc_socket()` rather\nthan plain `rte_malloc()`: zero-initialization avoids stale descriptor bugs,\nthe `_socket` variant ensures NUMA-local allocation,\nand hugepage backing makes the memory visible to secondary processes.\n\n```c\n/* Bad - rte_malloc for ordinary data structure */\nstruct config *cfg = rte_malloc(NULL, sizeof(*cfg), 0);\n\n/* Good - standard malloc for control structures */\nstruct config *cfg = malloc(sizeof(*cfg));\n\n/* Good - rte_zmalloc_socket for descriptor rings / queue structures */\nstruct my_rx_desc *ring = rte_zmalloc_socket(\"rx_ring\",\n n * sizeof(*ring), RTE_CACHE_LINE_SIZE, socket_id);\n```\n\n### Appropriate Use of rte_memcpy()\n\n`rte_memcpy()` is optimized for bulk data transfer in the fast path.\nFor general use, standard `memcpy()` is preferred because:\n\n- Modern compilers optimize `memcpy()` effectively\n- `memcpy()` includes bounds checking with `_FORTIFY_SOURCE`\n- `memcpy()` handles small fixed-size copies efficiently\n\n```c\n/* Bad - rte_memcpy in control path */\nrte_memcpy(&config, &default_config, sizeof(config));\n\n/* Good - standard memcpy for control path */\nmemcpy(&config, &default_config, sizeof(config));\n\n/* Good - rte_memcpy for packet data in fast path */\nrte_memcpy(rte_pktmbuf_mtod(m, void *), payload, len);\n```\n\n### Non-const Function Pointer Arrays\n\nArrays of function pointers (ops tables, dispatch tables, callback arrays) should be declared `const`\nwhen their contents are fixed at compile time.\nA non-`const` function pointer array can be overwritten by bugs or exploits,\nand prevents the compiler from placing the table in read-only memory.\n\n```c\n/* Bad - mutable when it doesn't need to be */\nstatic rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n\n/* Good - immutable dispatch table */\nstatic const rte_rx_burst_t rx_functions[] = {\n\trx_burst_scalar,\n\trx_burst_vec_avx2,\n\trx_burst_vec_avx512,\n};\n```\n\n**Exceptions** (do NOT flag):\n- Arrays modified at runtime for CPU feature detection or capability probing (e.g.,\n selecting a burst function based on `rte_cpu_get_flag_enabled()`)\n- Arrays containing mutable state (e.g., entries that are linked into lists)\n- Arrays populated dynamically via registration API\n- `dev_ops` or similar structures assigned per-device at init time\n\nOnly flag when the array is fully initialized at declaration with constant values\nand never modified thereafter.\n\n---\n\n## Forbidden Tokens\n\n### Functions\n\nForbidden function additions (`rte_panic()`, `rte_exit()`, `perror()`, `printf()`,\n`fprintf()`, `getenv()` in `lib/` and `drivers/`, with appropriate per-function\nexceptions for EAL, `examples/`, and `app/test/`) are caught by `devtools/checkpatches.sh`.\n\n### Atomics and Memory Barriers\n\nSubstitutions from the deprecated atomic API to the C11 DPDK wrappers\n(`__atomic_*`/`__ATOMIC_*` -> `rte_atomic_*_explicit()`/`rte_memory_order_*`,\n`__sync_*` -> `rte_atomic_*`, `rte_smp_{r,w,}mb()` -> `rte_atomic_thread_fence()`,\n`rte_atomic{16,32,64}_*()` -> `rte_atomic_*_explicit()`)\nare handled by coccinelle scripts in `devtools/cocci/`.\nAI review only needs to flag conceptual misuse, covered in the sections below.\n\n#### Shared Variable Access: volatile vs Atomics\n\nVariables shared between threads or between a thread and a signal handler **must** use atomic operations.\nThe C `volatile` keyword is NOT a substitute for atomics\n-- it prevents compiler optimization of accesses but provides no atomicity guarantees\nand no memory ordering between threads.\nOn some architectures, `volatile` reads and writes may tear on unaligned or multi-word values.\n\nDPDK provides C11 atomic wrappers that are portable across all supported compilers and architectures.\nAlways use these for shared state.\n\n**Reading shared variables:**\n\n```c\n/* BAD - volatile provides no atomicity or ordering guarantee */\nvolatile int stop_flag;\nif (stop_flag) /* data race, compiler/CPU can reorder */\n return;\n\n/* BAD - direct access to shared variable without atomic */\nif (shared->running) /* undefined behavior if another thread writes */\n process();\n\n/* GOOD - DPDK C11 atomic wrapper */\nif (rte_atomic_load_explicit(&shared->stop_flag, rte_memory_order_acquire))\n return;\n\n/* GOOD - relaxed is fine for statistics or polling a flag where\n * you don't need to synchronize other memory accesses */\ncount = rte_atomic_load_explicit(&shared->count, rte_memory_order_relaxed);\n```\n\n**Writing shared variables:**\n\n```c\n/* BAD - volatile write */\nvolatile int *flag = &shared->ready;\n*flag = 1;\n\n/* GOOD - atomic store with appropriate ordering */\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n```\n\n**Read-modify-write operations:**\n\n```c\n/* BAD - not atomic even with volatile */\nvolatile uint64_t *counter = &stats->packets;\n*counter += nb_rx; /* TOCTOU: load, add, store is 3 operations */\n\n/* GOOD - atomic add */\nrte_atomic_fetch_add_explicit(&stats->packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Memory Ordering Guide\n\nUse the weakest ordering that is correct.\nStronger ordering constrains hardware and compiler optimization unnecessarily.\n\n| DPDK Ordering | When to Use |\n|---------------|-------------|\n| `rte_memory_order_relaxed` | Statistics counters, polling flags where no other data depends on the value. Most common for simple counters. |\n| `rte_memory_order_acquire` | **Load** side of a flag/pointer that guards access to other shared data. Ensures subsequent reads see data published by the releasing thread. |\n| `rte_memory_order_release` | **Store** side of a flag/pointer that publishes shared data. Ensures all prior writes are visible to a thread that does an acquire load. |\n| `rte_memory_order_acq_rel` | Read-modify-write operations (e.g., `fetch_add`) that both consume and publish shared state in one operation. |\n| `rte_memory_order_seq_cst` | Rarely needed. Only when multiple independent atomic variables must be observed in a globally consistent total order. Avoid unless required. |\n\n**Common pattern -- producer/consumer flag:**\n\n```c\n/* Producer thread: fill buffer, then signal ready */\nfill_buffer(buf, data, len);\nrte_atomic_store_explicit(&shared->ready, 1, rte_memory_order_release);\n\n/* Consumer thread: wait for flag, then read buffer */\nwhile (!rte_atomic_load_explicit(&shared->ready, rte_memory_order_acquire))\n rte_pause();\nprocess_buffer(buf, len); /* guaranteed to see producer's writes */\n```\n\n**Common pattern -- statistics counter (no ordering needed):**\n\n```c\nrte_atomic_fetch_add_explicit(&port_stats->rx_packets, nb_rx,\n rte_memory_order_relaxed);\n```\n\n#### Standalone Fences\n\nPrefer ordering on the atomic operation itself (acquire load, release store) over standalone fences.\nStandalone fences (`rte_atomic_thread_fence()`) are a blunt instrument\nthat orders ALL memory accesses around the fence,\n\n\n# Claude Plugin Marketplace Catalog\n\nPath: `.claude-plugin/marketplace.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/marketplace.json)\n\n\n# claude-plugin Plugin Manifest\n\nPath: `.claude-plugin/plugin.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.claude-plugin/plugin.json)\n\n\n# Model Context Protocol (MCP) Configuration\n\nPath: `.cursor/mcp.json`\n\n[View Raw Content on GitHub](https://raw.githubusercontent.com/DPDK/dpdk/HEAD/.cursor/mcp.json)\n"}