[PATCH v20 11/25] net/pcap: consolidate boolean flag handling

[Bruce Richardson]

On Tue, Mar 10, 2026 at 09:09:49AM -0700, Stephen Hemminger wrote:
> Convert internal flag fields from int/unsigned int to bool for clarity
> and reduced structure size.
> 
> Merge the separate select_phy_mac() and get_infinite_rx_arg() functions
> into a single process_bool_flag() handler. The new function also adds
> proper validation, rejecting values other than "0", "1", or empty (which
> defaults to true).
> 
> Also change num_of_queue from unsigned int to uint16_t since it cannot
> exceed RTE_PMD_PCAP_MAX_QUEUES.
> 
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson at intel.com>

One minor comment inline below.

> ---
>  drivers/net/pcap/pcap_ethdev.c | 69 +++++++++++++++-------------------
>  1 file changed, 30 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index 258cffb813..c3270a676c 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -7,6 +7,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <stdbool.h>
>  #include <time.h>
>  #include <inttypes.h>
>  #include <errno.h>
> @@ -102,9 +103,9 @@ struct pmd_internals {
>  	char devargs[ETH_PCAP_ARG_MAXLEN];
>  	struct rte_ether_addr eth_addr;
>  	int if_index;
> -	int single_iface;
> -	int phy_mac;
> -	unsigned int infinite_rx;
> +	bool single_iface;
> +	bool phy_mac;
> +	bool infinite_rx;
>  };
>  
>  struct pmd_process_private {
> @@ -114,25 +115,25 @@ struct pmd_process_private {
>  };
>  
>  struct pmd_devargs {
> -	unsigned int num_of_queue;
> +	uint16_t num_of_queue;
> +	bool phy_mac;
>  	struct devargs_queue {
>  		pcap_dumper_t *dumper;
>  		pcap_t *pcap;
>  		const char *name;
>  		const char *type;
>  	} queue[RTE_PMD_PCAP_MAX_QUEUES];
> -	int phy_mac;
>  };
>  
>  struct pmd_devargs_all {
>  	struct pmd_devargs rx_queues;
>  	struct pmd_devargs tx_queues;
> -	int single_iface;
> -	unsigned int is_tx_pcap;
> -	unsigned int is_tx_iface;
> -	unsigned int is_rx_pcap;
> -	unsigned int is_rx_iface;
> -	unsigned int infinite_rx;
> +	bool single_iface;
> +	bool is_tx_pcap;
> +	bool is_tx_iface;
> +	bool is_rx_pcap;
> +	bool is_rx_iface;
> +	bool infinite_rx;
>  };
>  
>  static const char *valid_arguments[] = {
> @@ -884,7 +885,7 @@ eth_dev_close(struct rte_eth_dev *dev)
>  		}
>  	}
>  
> -	if (internals->phy_mac == 0)
> +	if (!internals->phy_mac)
>  		/* not dynamically allocated, must not be freed */
>  		dev->data->mac_addrs = NULL;
>  
> @@ -1209,29 +1210,19 @@ open_tx_iface(const char *key, const char *value, void *extra_args)
>  }
>  
>  static int
> -select_phy_mac(const char *key __rte_unused, const char *value,
> -		void *extra_args)
> +process_bool_flag(const char *key, const char *value, void *extra_args)
>  {
> -	if (extra_args) {
> -		const int phy_mac = atoi(value);
> -		int *enable_phy_mac = extra_args;
> -
> -		if (phy_mac)
> -			*enable_phy_mac = 1;
> -	}
> -	return 0;
> -}
> -
> -static int
> -get_infinite_rx_arg(const char *key __rte_unused,
> -		const char *value, void *extra_args)
> -{
> -	if (extra_args) {
> -		const int infinite_rx = atoi(value);
> -		int *enable_infinite_rx = extra_args;
> -
> -		if (infinite_rx > 0)
> -			*enable_infinite_rx = 1;
> +	bool *flag = extra_args;
> +
> +	if (value == NULL || *value == '\0') {
> +		*flag = true; /* default with no additional argument */
> +	} else if (strcmp(value, "0") == 0) {
> +		*flag = false;
> +	} else if (strcmp(value, "1") == 0) {
> +		*flag = true;
> +	} else {
> +		PMD_LOG(ERR, "Invalid '%s' value '%s'", key, value);
> +		return -1;
>  	}
>  	return 0;
>  }
> @@ -1507,7 +1498,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  		dumpers.queue[0] = pcaps.queue[0];
>  
>  		ret = rte_kvargs_process(kvlist, ETH_PCAP_PHY_MAC_ARG,
> -				&select_phy_mac, &pcaps.phy_mac);
> +					 &process_bool_flag, &pcaps.phy_mac);
>  		if (ret < 0)
>  			goto free_kvlist;
>  
> @@ -1546,9 +1537,9 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>  
>  		if (infinite_rx_arg_cnt == 1) {
>  			ret = rte_kvargs_process(kvlist,
> -					ETH_PCAP_INFINITE_RX_ARG,
> -					&get_infinite_rx_arg,
> -					&devargs_all.infinite_rx);
> +						 ETH_PCAP_INFINITE_RX_ARG,
> +						 &process_bool_flag,
> +						 &devargs_all.infinite_rx);

You can shrink the diff by leaving the indentation alone, there is nothing
wrong with it! :-)

>  			if (ret < 0)
>  				goto free_kvlist;
>  			PMD_LOG(INFO, "infinite_rx has been %s for %s",
> @@ -1698,5 +1689,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_pcap,
>  	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
>  	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
>  	ETH_PCAP_IFACE_ARG "=<ifc> "
> -	ETH_PCAP_PHY_MAC_ARG "=<int>"
> +	ETH_PCAP_PHY_MAC_ARG "=<0|1> "
>  	ETH_PCAP_INFINITE_RX_ARG "=<0|1>");
> -- 
> 2.51.0
>