|SUCCESS| [v4,7/7] eal: configure initial device probing

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

Commit: [unable to determine - not in context]
Author: David Marchand <david.marchand at redhat.com>
Subject: [PATCH v4 1/7] devtools: check packet forwarding in null test

This patch series contains several changes related to device management,
bus operations, and testing in DPDK. The series aims to improve device
probing behavior and fix issues in bus cleanup operations.

> diff --git a/devtools/test-null.sh b/devtools/test-null.sh
> index e3ac168ce3..db16f36bfa 100755
> --- a/devtools/test-null.sh
> +++ b/devtools/test-null.sh
> @@ -26,8 +26,14 @@ else
>  	libs=
>  fi
>  
> +logfile=$build/test-null.log
>  (sleep 1 && echo stop) |
>  # testpmd only needs 20M, make it x2 (default number of cores) for NUMA systems
>  $testpmd -l $corelist --no-huge -m 40 \
>  	$libs -a 0:0.0 --vdev net_null1 --vdev net_null2 $eal_options -- \
> -	--no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
> +	--no-mlockall --total-num-mbufs=2048 $testpmd_options -ia | tee $logfile
> +
> +# we expect two ports and some traffic is received and transmitted
> +grep -q 'io packet forwarding - ports=2 -' $logfile
> +grep 'RX-packets: ' $logfile | tail -1 | grep -q 'RX-packets:[[:space:]]*[^0[:space:]]'
> +grep 'TX-packets: ' $logfile | tail -1 | grep -q 'TX-packets:[[:space:]]*[^0[:space:]]'

Does this code check the exit status of each grep command? If any grep
fails, the script should probably exit with a non-zero status.

[ ... ]

> diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
> index 550d4e0e8d..7daa18d850 100644
> --- a/drivers/bus/fslmc/fslmc_vfio.c
> +++ b/drivers/bus/fslmc/fslmc_vfio.c
> @@ -1393,7 +1393,7 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
>  {
>  	struct rte_dpaa2_object *object = NULL;
>  	struct rte_dpaa2_driver *drv;
> -	int ret, probe_all;
> +	int ret;
>  
>  	switch (dev->dev_type) {
>  	case DPAA2_IO:
> @@ -1411,22 +1411,9 @@ fslmc_close_iodevices(struct rte_dpaa2_device *dev,
>  	case DPAA2_ETH:
>  	case DPAA2_CRYPTO:
>  	case DPAA2_QDMA:
> -		probe_all = rte_fslmc_bus.bus.conf.scan_mode !=
> -			    RTE_BUS_SCAN_ALLOWLIST;
> -		TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
> -			if (drv->drv_type != dev->dev_type)
> -				continue;
> -			if (rte_dev_is_probed(&dev->device))
> -				continue;
> -			if (probe_all ||
> -			    (dev->device.devargs &&
> -			     dev->device.devargs->policy ==
> -			     RTE_DEV_ALLOWED)) {
> -				ret = drv->remove(dev);
> -				if (ret)
> -					DPAA2_BUS_ERR("Unable to remove");
> -			}
> -		}
> +		drv = dev->driver;
> +		if (drv && drv->remove && drv->remove(dev))
> +			DPAA2_BUS_ERR("Unable to remove");

Does this code handle the case where dev->driver is NULL for a probed
device? Could this happen if the device was probed but the driver
pointer was not set?

[ ... ]

> diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
> index d9830b68ca..5e0f32bfe8 100644
> --- a/drivers/bus/dpaa/dpaa_bus.c
> +++ b/drivers/bus/dpaa/dpaa_bus.c
> @@ -614,6 +614,7 @@ void
>  rte_dpaa_driver_register(struct rte_dpaa_driver *driver)
>  {
>  	RTE_VERIFY(driver);
> +	RTE_VERIFY(driver->probe != NULL);

[ ... ]

> diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
> index fdce1f6b1f..ca2812a960 100644
> --- a/drivers/bus/ifpga/ifpga_bus.c
> +++ b/drivers/bus/ifpga/ifpga_bus.c
> @@ -220,7 +220,7 @@ ifpga_scan(void)
>  	struct rte_afu_device *afu_dev = NULL;
>  
>  	/* for FPGA devices we scan the devargs_list populated via cmdline */
> -	RTE_EAL_DEVARGS_FOREACH(IFPGA_ARG_NAME, devargs) {
> +	RTE_EAL_DEVARGS_FOREACH(rte_ifpga_bus.name, devargs) {

[ ... ]

> @@ -516,5 +516,9 @@ static struct rte_bus rte_ifpga_bus = {
>  	.parse       = ifpga_parse,
>  };
>  
> -RTE_REGISTER_BUS(IFPGA_BUS_NAME, rte_ifpga_bus);
> +RTE_REGISTER_BUS(ifpga, rte_ifpga_bus);
> +RTE_INIT(ifpga_bus_init)
> +{
> +	RTE_VERIFY(strcmp(rte_ifpga_bus.name, RTE_STR(IFPGA_BUS_NAME)) == 0);
> +}

Does this verification run before or after the bus is registered? If the
verification fails after registration, does this leave the bus in a
partially initialized state?

[ ... ]

> diff --git a/lib/eal/common/eal_common_bus.c b/lib/eal/common/eal_common_bus.c
> index 0a2311a342..474fa9331e 100644
> --- a/lib/eal/common/eal_common_bus.c
> +++ b/lib/eal/common/eal_common_bus.c
> @@ -205,6 +206,33 @@ rte_bus_find_by_name(const char *busname)
>  	return rte_bus_find(NULL, cmp_bus_name, (const void *)busname);
>  }
>  
> +RTE_EXPORT_INTERNAL_SYMBOL(rte_bus_find_devargs)
> +struct rte_devargs *
> +rte_bus_find_devargs(const struct rte_bus *bus, const char *name)
> +{
> +	rte_bus_dev_compare_t cmp = bus->dev_compare;
> +	const char *bus_name = rte_bus_name(bus);
> +	struct rte_devargs *devargs;
> +
> +	if (cmp == NULL)
> +		cmp = strcmp;
> +
> +	RTE_EAL_DEVARGS_FOREACH(bus_name, devargs) {
> +		const char *devargs_name = devargs->name;
> +
> +		/* The name in the devargs is usually prefixed with <bus>: */
> +		if (strncmp(devargs_name, bus_name, strlen(bus_name)) == 0)
> +			devargs_name += strlen(bus_name) + 1;

Does this code overflow devargs_name if the string is exactly
strlen(bus_name) characters long and the next character is not ':'?

> +		if (cmp(name, devargs_name) != 0)
> +			continue;
> +		EAL_LOG(DEBUG, "found devargs for %s:%s", bus_name, name);
> +		return devargs;
> +	}
> +
> +	EAL_LOG(DEBUG, "no devargs for %s:%s", bus_name, name);
> +	return NULL;
> +}

[ ... ]

> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index aad676a004..290386dc63 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -233,10 +233,15 @@ eal_collate_args(int argc, char **argv)
>  		EAL_LOG(ERR, "Options allow (-a) and block (-b) can't be used at the same time");
>  		return -1;
>  	}
> +	if (args.no_auto_probing && !TAILQ_EMPTY(&args.block)) {
> +		EAL_LOG(ERR, "Options no-auto-probing and block (-b) can't be used at the same time");
> +		return -1;
> +	}