|WARNING| 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/167416

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch fixes the default device plugin path for the AF_XDP PMD to use the EAL runtime directory instead of `/tmp`, with fallback for compatibility. The implementation is mostly correct but has several issues that need to be addressed.

---

## Errors

### 1. Resource leak on error path in `init_uds_sock()`
**File:** `drivers/net/af_xdp/rte_eth_af_xdp.c`
**Location:** New validation code in `init_uds_sock()`

The new path length validation added before the `socket()` call returns `-1` on error, but this function is expected to return the socket fd or a negative error code. The return value should be a proper errno value.

```c
/* Current code */
if (strnlen(dp_path, PATH_MAX) >= sizeof(server->sun_path)) {
    AF_XDP_LOG_LINE(ERR, "Device plugin socket path '%s' is too long", dp_path);
    return -1;
}

/* Should be */
if (strnlen(dp_path, PATH_MAX) >= sizeof(server->sun_path)) {
    AF_XDP_LOG_LINE(ERR, "Device plugin socket path '%s' is too long", dp_path);
    return -ENAMETOOLONG;
}
```

### 2. Redundant path length checks
**File:** `drivers/net/af_xdp/rte_eth_af_xdp.c`
**Location:** `rte_pmd_af_xdp_probe()`

The patch adds path length validation in two places:
1. In `init_uds_sock()` checking against `sizeof(server->sun_path)`
2. Later in `rte_pmd_af_xdp_probe()` checking against `RTE_SIZEOF_FIELD(struct sockaddr_un, sun_path)`

These are the same check (both evaluate to 108 bytes on Linux). The second check is unreachable for the `use_cni` case because `init_uds_sock()` would have already been called and failed. For the `use_pinned_map` case, the path is never copied into a `sockaddr_un`, so this check is misplaced.

**Suggested fix:** Remove the second check and rely on the validation in `init_uds_sock()`. If validation is needed for the pinned map path, it should be done where that path is actually used.

---

## Warnings

### 1. Missing documentation update for pinned map path length
The documentation changes in `af_xdp.rst` and `af_xdp_dp.rst` explain the new default path behavior, but don't mention that overly long paths will now be rejected. Users should know this limitation exists.

**Suggested addition to `af_xdp.rst`:**
```rst
.. note::

   The device plugin socket path must not exceed the maximum unix domain
   socket path length (typically 108 bytes). Paths that are too long
   will be rejected.
```

### 2. `access()` race condition in `get_dflt_dp_path()`
Using `access()` to check file existence followed by actual use is a TOCTOU (time-of-check-time-of-use) pattern. However, in this specific case the race is not exploitable: the check is only to decide which default path to try, and the actual open/use happens later where proper error handling exists. This is acceptable but worth noting.

Not flagging as an error because:
- The race window is harmless (just affects which default is chosen)
- The subsequent operations properly handle missing files
- This is control-plane code, not a security boundary

### 3. Format string could use `PATH_MAX` constant name
In the error messages that reference path length limits, using the symbolic constant would be clearer than "is too long":

```c
/* Current */
AF_XDP_LOG_LINE(ERR, "Device plugin path for %s is too long", if_name);

/* Clearer */
AF_XDP_LOG_LINE(ERR, "Device plugin path for %s exceeds PATH_MAX", if_name);
```

---

## Info

### 1. Documentation examples could show both locations
The updated documentation examples in `af_xdp.rst` now only show the runtime directory path (`/var/run/dpdk/rte/afxdp_dp/...`). Consider adding a note showing what the legacy path format looks like for users migrating existing deployments:

```rst
Legacy deployment (compatibility mode):
   /tmp/afxdp_dp/<<interface name>>/afxdp.sock

Recommended deployment:
   /var/run/dpdk/rte/afxdp_dp/<<interface name>>/afxdp.sock
```

### 2. Comment style
The comment for `DP_DIR_NAME` is single-line but could use the preferred multi-line format for consistency with DPDK style:

```c
/* Current */
/* Directory holding the per interface device plugin endpoints. */

/* DPDK style */
/*
 * Directory holding the per interface device plugin endpoints.
 */
```

---

## Code Style - Compliant

The following aspects of the patch comply with DPDK coding standards:
- Proper use of `rte_eal_get_runtime_dir()`
- Appropriate return value propagation (`-ENAMETOOLONG`)
- Correct use of `RTE_SIZEOF_FIELD` macro
- Logging uses `AF_XDP_LOG_LINE` consistently
- No forbidden tokens detected
- Documentation updates accompany code changes
- Release notes would need update (check if included in commit message)