|WARNING| [v3] 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/167538

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch updates the AF_XDP PMD to look for device plugin endpoints in the EAL runtime directory before falling back to `/tmp/afxdp_dp`. Overall code quality is good with proper error handling and documentation updates.

---

## Errors

None found.

---

## Warnings

### 1. Potential resource leak on early error path in `get_dflt_dp_path()`

**Location:** `drivers/net/af_xdp/rte_eth_af_xdp.c:2492-2496`

The first `snprintf()` error path returns 0 (success) when the runtime directory entry doesn't exist (`access()` fails), which is correct. However, when `snprintf()` itself fails (`ret < 0`), the function continues to the legacy path instead of returning an error immediately. This is acceptable fallback behavior, but the second `snprintf()` could theoretically also fail with `ret < 0`, yet the code only checks for truncation. If both fail with negative return values, the function still returns `-ENAMETOOLONG`, which may not accurately describe a `snprintf()` encoding error.

**Suggested fix:**
While the current logic works for practical cases, consider explicitly handling `snprintf()` errors:

```c
ret = snprintf(dp_path, size, "%s/%s/%s/%s", rte_eal_get_runtime_dir(),
               DP_DIR_NAME, if_name, entry);
if (ret < 0) {
    AF_XDP_LOG_LINE(ERR, "Failed to format device plugin path");
    return -EINVAL;
}
if ((size_t)ret < size && access(dp_path, F_OK) == 0)
    return 0;

ret = snprintf(dp_path, size, "%s/%s/%s", DP_LEGACY_BASE_PATH, if_name,
               entry);
if (ret < 0) {
    AF_XDP_LOG_LINE(ERR, "Failed to format legacy device plugin path");
    return -EINVAL;
}
if ((size_t)ret >= size) {
    AF_XDP_LOG_LINE(ERR, "Device plugin path for %s is too long", if_name);
    return -ENAMETOOLONG;
}
```

### 2. Missing explicit bounds check on `if_name` length

**Location:** `drivers/net/af_xdp/rte_eth_af_xdp.c:2568-2569`

The code checks `strlen(if_name) == 0` but doesn't verify an upper bound. While `snprintf()` will truncate, a maliciously long `if_name` could cause `get_dflt_dp_path()` to always build truncated paths that fail. Consider adding an explicit maximum length check (e.g., `IFNAMSIZ` from `<net/if.h>`) to fail fast with a clear error message.

**Suggested fix:**

```c
if (strlen(if_name) == 0 || strlen(if_name) >= IFNAMSIZ) {
    AF_XDP_LOG_LINE(ERR, "Invalid network interface name length");
    return -EINVAL;
}
```

---

## Info

### 1. Code organization: early validation

**Location:** `drivers/net/af_xdp/rte_eth_af_xdp.c:2568-2572`

The interface name validation was moved earlier in the function (before path construction), which is good practice. This is correctly documented in the commit message and improves code clarity.

### 2. Documentation quality

The updates to `af_xdp.rst`, `af_xdp_dp.rst`, and release notes are thorough and clearly explain both the new behavior and the backward compatibility considerations. The notes about bind mounting for pinned maps and the need to preserve the `<if_name>` component are valuable operational guidance.

### 3. Release notes placement

The release notes entry is appropriately placed under "New Features" with a clear description of the behavior change and its impact on existing deployments.

---

## Observations (not issues)

1. **Good:** Error messages are clear and actionable (e.g., specifying which parameter is invalid and why).

2. **Good:** The fallback logic preserves backward compatibility while encouraging migration to the runtime directory.

3. **Good:** Use of `RTE_SIZEOF_FIELD` macro for consistent size calculations.

4. **Good:** The `NOTICE` level log for fallback usage provides visibility into which path is being used without being overly verbose.