|WARNING| net/tap: close old LSC socket on netns change

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch Summary
This patch fixes a file descriptor leak in the net/tap driver when an interface is moved to another namespace. The LSC netlink socket is recreated without closing the old one.

## CORRECTNESS BUGS

### Errors

**Error: Potential double-close on error path**

Line: `tap_nl_final(rte_intr_fd_get(pmd->intr_handle));` (new line in patch)

The new code closes the old socket before updating the interrupt handle. However, if `rte_intr_callback_register()` fails at the end of the function, the error handling will call `tap_lsc_intr_handle_set(dev, 0)` which will attempt to close the fd again via the `else` branch that calls `tap_nl_final()`.

The existing code after this patch has:
```c
return rte_intr_callback_register(
    pmd->intr_handle, tap_dev_intr_handler, dev);
```

If this fails, the caller typically tries to clean up by calling `tap_lsc_intr_handle_set(dev, 0)`, which would execute:
```c
} else {
    /* ... */
    tap_nl_final(rte_intr_fd_get(pmd->intr_handle));
    rte_intr_fd_set(pmd->intr_handle, -1);
}
```

Since the handle was already updated to the new `fd`, this would close the new socket that was just created, and return it to the state where `intr_handle` references a closed fd (until the `rte_intr_fd_set(..., -1)` line).

**Suggested fix:** Only close the old socket after successful callback registration, or set the handle to -1 before closing the old socket:

```c
if (set) {
    int fd, old_fd;

    fd = tap_nl_init(RTMGRP_LINK);
    if (unlikely(fd == -1))
        return -EBADF;
    
    old_fd = rte_intr_fd_get(pmd->intr_handle);
    rte_intr_fd_set(pmd->intr_handle, fd);
    
    ret = rte_intr_callback_register(
        pmd->intr_handle, tap_dev_intr_handler, dev);
    if (ret != 0) {
        rte_intr_fd_set(pmd->intr_handle, old_fd);
        tap_nl_final(fd);
        return ret;
    }
    
    /* Close old socket only after successful registration */
    if (old_fd >= 0)
        tap_nl_final(old_fd);
    
    return 0;
}
```

This ensures the old socket remains valid if registration fails, and only closes it after success.

---

## C CODING STYLE

### Warnings

**Warning: Missing explicit comparison**

Line: `if (rte_intr_fd_get(pmd->intr_handle) >= 0)`

DPDK coding style requires explicit comparison for integers. The code correctly uses `>= 0`, so this is actually correct. No issue here.

---

## FINAL ASSESSMENT

The patch correctly identifies and attempts to fix a real file descriptor leak. However, it introduces a new bug on the error path where the newly created socket could be closed twice if callback registration fails.

Review-Result: ERROR