|WARNING| [v2] eal/linux: harden uevent recv error handling

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Summary
Patch improves error handling in Linux uevent receiver for hot plug/unplug scenarios. Overall quality is good, but there are a few issues to address.

---

## Errors

### 1. Missing errno constant in comparison (Error)

**File:** `lib/eal/linux/eal_dev.c`  
**Line:** `if (ret < 0 && (errno == EAGAIN || errno == EWOULDBLOCK || EINTR)) {`

The condition `|| EINTR` is missing `errno ==`. This compares the errno constant value directly instead of checking the errno variable, which is always true (EINTR is non-zero).

**Fix:**
```c
if (ret < 0 &&
    (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)) {
```

---

## Warnings

### 1. Inconsistent error classification (Warning)

**File:** `lib/eal/linux/eal_dev.c`  
**Lines:** `else if (ret < 0 && (errno == ENOBUFS)) { ... EAL_LOG(ERR, ...`

The code treats ENOBUFS as a "non-fatal transient memory condition" and returns (continuing operation), yet logs it at ERR level. If it's non-fatal and transient, it should probably be logged at WARNING or INFO level to avoid alarming operators. ERR-level logs typically indicate conditions requiring attention.

**Suggested fix:**
```c
/* non-fatal transient memory condition */
EAL_LOG(WARNING, "transient buffer shortage on uevent recv: %s",
        strerror(errno));
```

### 2. Unnecessary parentheses (Warning - style)

**File:** `lib/eal/linux/eal_dev.c`  
**Line:** `else if (ret < 0 && (errno == ENOBUFS)) {`

The inner parentheses around `errno == ENOBUFS` are unnecessary and inconsistent with the style of the previous condition.

**Suggested fix:**
```c
} else if (ret < 0 && errno == ENOBUFS) {
```

---

## Info

### 1. Consider errno clarity on ret == 0 path (Info)

When `recv()` returns 0, it indicates orderly shutdown (EOF), not an error. The current code logs "uevent socket connection is broken" and calls `dev_delayed_unregister`. This is likely correct behavior, but the `ret <= 0` path conflates two cases:
- `ret == 0`: orderly shutdown (no errno)  
- `ret < 0` with unexpected errno: actual error

Consider separating these for clarity in logs or comments, e.g.:
```c
} else if (ret == 0) {
        /* orderly shutdown: peer closed connection */
        EAL_LOG(ERR, "uevent socket closed by peer.");
        rte_eal_alarm_set(1, dev_delayed_unregister, NULL);
} else {
        /* ret < 0, unexpected error */
        EAL_LOG(ERR, "uevent recv error: %s", strerror(errno));
        rte_eal_alarm_set(1, dev_delayed_unregister, NULL);
}
```

This is optional--the current logic is not incorrect, just less explicit.

---

## Checklist Review

- [x] Code compiles independently  
- [x] Proper indentation (tabs)  
- [x] No trailing whitespace (not visible in diff context)  
- [x] Comments are appropriate  
- [x] No forbidden tokens  
- [x] No resource leaks introduced  
- [x] No use-after-free  
- [x] Error paths handled (early returns are correct)  
- [x] No new API (no release notes needed)  
- [x] Commit message formatting (out of scope per instructions)  

---

## Required Changes

1. **Fix the EINTR comparison** (missing `errno ==`) -- this is a critical correctness bug.

## Recommended Changes

1. Change ENOBUFS log level from ERR to WARNING.
2. Remove unnecessary parentheses around `errno == ENOBUFS`.