|WARNING| [v2] net/af_xdp: fix default device plugin path

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167471

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-11

# DPDK Patch Review

## Patch Summary
This patch fixes the default device plugin path for AF_XDP PMD by prioritizing the EAL runtime directory over the legacy `/tmp/afxdp_dp` path, while maintaining backward compatibility.

---

## Errors

None identified.

---

## Warnings

### 1. Missing release notes update

**Issue:** The patch makes a user-visible behavior change (default path lookup order) but does not update release notes.

**Why it matters:** Users and deployers need to know about the new path precedence and the fallback behavior.

**Fix:** Add an entry to `doc/guides/rel_notes/release_<version>.rst` under "New Features" or "Bug Fixes" describing:
- The new default path lookup order (runtime dir first, then `/tmp/afxdp_dp`)
- The notice-level log when the fallback is used
- That explicit `dp_path` still overrides both

---

### 2. Return value semantics inconsistency

**Issue:** `get_dflt_dp_path()` returns 0 on success and negative errno on failure, but the caller code paths after the two calls to this function are not symmetric. The first call (for `use_cni`) properly checks `if (ret < 0) return ret;`. The second call (for `use_pinned_map`) also checks, but there is no early-exit path verification needed here since both behave correctly. However, the comment above the function says "A runtime directory candidate that does not fit is skipped rather than rejected" but the code returns `-ENAMETOOLONG` when neither path fits.

**Clarification needed:** The function comment implies graceful degradation but the implementation returns an error when both paths exceed `size`. This is correct behavior (it's an actual error), but the comment could be clearer: "A runtime directory candidate that exceeds `size` is skipped in favor of trying the shorter legacy path. If the legacy path also does not fit, `-ENAMETOOLONG` is returned."

**Suggested fix:**
```c
/*
 * Build the default device plugin path for an interface.
 *
 * Unix domain sockets and pinned maps are runtime state, so the EAL runtime
 * directory is preferred: it is per user and per file prefix, and both of its
 * levels are created with mode 0700. The AF_XDP Device Plugin for Kubernetes
 * still creates and mounts these endpoints below DP_LEGACY_BASE_PATH, so that
 * location is used when the runtime directory holds no usable entry.
 *
 * The interface name is always part of the path: it keeps the endpoints of
 * several interfaces distinct when more than one is mounted in a single pod.
 *
 * "size" is the longest path the caller can use, which may be shorter than the
 * destination buffer. A runtime directory candidate that does not fit is
 * skipped in favor of the legacy path. If the legacy path also exceeds size,
 * -ENAMETOOLONG is returned.
 */
```

---

### 3. Documentation could clarify runtime directory variability more prominently

**Issue:** While the docs correctly note that `<runtime_dir>` depends on user ID and `--file-prefix`, this critical detail appears only in a note in `af_xdp.rst`. The `af_xdp_dp.rst` howto does not mention this variability at all.

**Why it matters:** Kubernetes pod deployments need to know they must mount the endpoint into a directory that matches the EAL runtime directory of the running DPDK application, which varies by user and prefix.

**Fix:** Add a note in `doc/guides/howto/af_xdp_dp.rst` similar to the one in `af_xdp.rst`:

```rst
.. note::

   The EAL runtime directory path depends on the user ID the application
   runs as and on the ``--file-prefix`` EAL option. Deployments that mount
   the endpoint below the runtime directory must ensure the mount point
   matches the runtime directory that will be used by the DPDK application.
```

Place this note after the first paragraph that introduces the runtime directory lookup.

---

## Info

### 1. Consider documenting the `access(dp_path, F_OK)` check rationale

The code uses `access(dp_path, F_OK) == 0` to decide whether the runtime directory path is usable. This check happens before falling back to the legacy path. A brief inline comment explaining that this allows the PMD to skip a non-existent runtime directory path even when it would fit, preferring the legacy path that may actually exist, would help future maintainers:

```c
/* Try the runtime directory first. Skip if it doesn't fit or doesn't exist. */
ret = snprintf(dp_path, size, "%s/%s/%s/%s", rte_eal_get_runtime_dir(),
               DP_DIR_NAME, if_name, entry);
if (ret >= 0 && (size_t)ret < size && access(dp_path, F_OK) == 0)
    return 0;
```

---

## Summary

**Correctness:** The patch is correct. Error paths properly propagate errors, resource handling is sound, and the new path construction logic correctly handles buffer sizes and validates lengths before use.

**Style:** Code follows DPDK style. No forbidden tokens. Comparisons are explicit.

**Process:** The patch needs a release notes entry (Warning severity). Documentation improvements are suggested for clarity (Warning/Info).

**Overall:** This is a well-written patch that correctly addresses a real issue (security and multi-tenancy benefits of using the EAL runtime directory) while maintaining backward compatibility. The main actionable item is adding release notes.