diff mbox

[dpdk-dev,02/11] net/failsafe: fix sub-device visibility

Message ID 20180509094337.26112-3-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon May 9, 2018, 9:43 a.m. UTC 
  The iterator function rte_eth_find_next_owned_by(), used by the
iterator macro RTE_ETH_FOREACH_DEV_OWNED_BY, are ignoring the devices
which are neither ATTACHED nor REMOVED. Thus sub-devices, having
the state DEFERRED, cannot be seen with the ethdev iterator.
The state RTE_ETH_DEV_DEFERRED can be replaced by
RTE_ETH_DEV_ATTACHED + owner.

Fixes: dcd0c9c32b8d ("net/failsafe: use ownership mechanism for slaves")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/failsafe/failsafe_eal.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Gaëtan Rivet May 9, 2018, 12:13 p.m. UTC | #1 
I think this patch should be swapped with the next commit
"ethdev: add doxygen comments for each state",
While the comment about "DEFERRED" would be edited in this one to become:
/** The deferred state is deprecated and replaced by ownership. */
                          ^^^^^^^^^^
Otherwise I agree that the device state is not to be used anymore.

On Wed, May 09, 2018 at 11:43:28AM +0200, Thomas Monjalon wrote:
> The iterator function rte_eth_find_next_owned_by(), used by the
> iterator macro RTE_ETH_FOREACH_DEV_OWNED_BY, are ignoring the devices
> which are neither ATTACHED nor REMOVED. Thus sub-devices, having
> the state DEFERRED, cannot be seen with the ethdev iterator.
> The state RTE_ETH_DEV_DEFERRED can be replaced by
> RTE_ETH_DEV_ATTACHED + owner.
> 
> Fixes: dcd0c9c32b8d ("net/failsafe: use ownership mechanism for slaves")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Matan Azrad <matan@mellanox.com>

Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

> ---
>  drivers/net/failsafe/failsafe_eal.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
> index ee89236f1..ce767703f 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -98,7 +98,6 @@ fs_bus_init(struct rte_eth_dev *dev)
>  		SUB_ID(sdev) = i;
>  		sdev->fs_dev = dev;
>  		sdev->dev = ETH(sdev)->device;
> -		ETH(sdev)->state = RTE_ETH_DEV_DEFERRED;
>  		sdev->state = DEV_PROBED;
>  	}
>  	return 0;
> -- 
> 2.16.2
>
Thomas Monjalon May 9, 2018, 12:21 p.m. UTC | #2 
09/05/2018 14:13, Gaëtan Rivet:
> I think this patch should be swapped with the next commit
> "ethdev: add doxygen comments for each state",
> While the comment about "DEFERRED" would be edited in this one to become:
> /** The deferred state is deprecated and replaced by ownership. */
>                           ^^^^^^^^^^

I prefer considering the DEFERRED state as useless in this series.
We can discuss about deprecation and removal, separately with a
deprecation notice.

> Otherwise I agree that the device state is not to be used anymore.
diff mbox

Patch

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ee89236f1..ce767703f 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -98,7 +98,6 @@  fs_bus_init(struct rte_eth_dev *dev)
 		SUB_ID(sdev) = i;
 		sdev->fs_dev = dev;
 		sdev->dev = ETH(sdev)->device;
-		ETH(sdev)->state = RTE_ETH_DEV_DEFERRED;
 		sdev->state = DEV_PROBED;
 	}
 	return 0;