|SUCCESS| net/tap: avoid using netlink socket if closed

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162855

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-06

Below is my review of the patch:

commit <series.patch>
Author: Stephen Hemminger <stephen at networkplumber.org>

net/tap: avoid using netlink socket if closed

This patch adds a check to ensure the netlink socket is valid before
attempting to use it during device close.

> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index a5d460a0b3..ba9aef74c0 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1124,7 +1124,7 @@ tap_dev_close(struct rte_eth_dev *dev)
>  		}
>  	}
>  
> -	if (internals->remote_if_index) {
> +	if (internals->remote_if_index != 0 && internals->nlsk_fd != -1) {
>  		/* Restore initial remote state */
>  		int ret = tap_nl_set_flags(internals->nlsk_fd, internals->remote_if_index,
>  					   internals->remote_initial_flags, 1);

The change looks correct in fixing the Coverity issue. However, this
raises a question about coding style:

According to DPDK coding style guidelines, direct comparisons should be
used for testing numeric values. The original code uses an implicit
comparison "if (internals->remote_if_index)" which the style guide
considers acceptable for testing if a value is non-zero. However, the
new condition mixes styles: "!= 0" is explicit while "!= -1" is also
explicit.

For consistency with DPDK coding style, should the condition be written
as either:

  if (internals->remote_if_index && internals->nlsk_fd != -1)

or fully explicit as:

  if (internals->remote_if_index != 0 && internals->nlsk_fd != -1)

The mixed style could confuse readers about whether there's a semantic
difference being conveyed. Which style is preferred here?