[dpdk-dev,10/11] net/failsafe: fix sub-device ownership race

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

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Thomas Monjalon May 9, 2018, 9:43 a.m. UTC
  From: Matan Azrad <matan@mellanox.com>

There is time between the sub-device port probing by the sub-device PMD
to the sub-device port ownership taking by a fail-safe port.

In this time, the port is available for the application usage. For
example, the port will be exposed to the applications which use
RTE_ETH_FOREACH_DEV iterator.

Thus, ownership unaware applications may manage the port in this time
what may cause a lot of problematic behaviors in the fail-safe
sub-device initialization.

Register to the ethdev NEW event to take the sub-device port ownership
before it becomes exposed to the application.

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/failsafe/failsafe.c         | 22 ++++++++++---
 drivers/net/failsafe/failsafe_eal.c     | 58 +++++++++++++++++++++------------
 drivers/net/failsafe/failsafe_ether.c   | 23 +++++++++++++
 drivers/net/failsafe/failsafe_private.h |  4 +++
 4 files changed, 83 insertions(+), 24 deletions(-)
  

Comments

Gaëtan Rivet May 9, 2018, 12:41 p.m. UTC | #1
Hello Matan,

Two nitpicks below:

On Wed, May 09, 2018 at 11:43:36AM +0200, Thomas Monjalon wrote:
> From: Matan Azrad <matan@mellanox.com>
> 
> There is time between the sub-device port probing by the sub-device PMD
> to the sub-device port ownership taking by a fail-safe port.
> 
> In this time, the port is available for the application usage. For
> example, the port will be exposed to the applications which use
> RTE_ETH_FOREACH_DEV iterator.
> 
> Thus, ownership unaware applications may manage the port in this time
> what may cause a lot of problematic behaviors in the fail-safe
> sub-device initialization.
> 
> Register to the ethdev NEW event to take the sub-device port ownership
> before it becomes exposed to the application.
> 
> Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")

This fix is relying on the RTE_ETH_EVENT_NEW, an API that I think is not
meant to be backported in the stable release that would be targetted by
this commit id.

I think this fix is useless without the rest of this series, so I don't
know what is exactly planned about the rest (whether it is backported,
and where), but I would only CC stable if this is planned, and only as
soon as the relevant APIs are introduced.

> Cc: stable@dpdk.org
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
>  drivers/net/failsafe/failsafe.c         | 22 ++++++++++---
>  drivers/net/failsafe/failsafe_eal.c     | 58 +++++++++++++++++++++------------
>  drivers/net/failsafe/failsafe_ether.c   | 23 +++++++++++++
>  drivers/net/failsafe/failsafe_private.h |  4 +++
>  4 files changed, 83 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> index fc989c4f5..c9d128de3 100644
> --- a/drivers/net/failsafe/failsafe.c
> +++ b/drivers/net/failsafe/failsafe.c
> @@ -204,16 +204,25 @@ fs_eth_dev_create(struct rte_vdev_device *vdev)
>  	}
>  	snprintf(priv->my_owner.name, sizeof(priv->my_owner.name),
>  		 FAILSAFE_OWNER_NAME);
> +	DEBUG("Failsafe port %u owner info: %s_%016"PRIX64, dev->data->port_id,
> +	      priv->my_owner.name, priv->my_owner.id);
> +	ret = rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
> +					    failsafe_eth_new_event_callback,
> +					    dev);
> +	if (ret) {
> +		ERROR("Failed to register NEW callback");
> +		goto free_args;
> +	}
>  	ret = failsafe_eal_init(dev);
>  	if (ret)
> -		goto free_args;
> +		goto unregister_new_callback;
>  	ret = fs_mutex_init(priv);
>  	if (ret)
> -		goto free_args;
> +		goto unregister_new_callback;
>  	ret = failsafe_hotplug_alarm_install(dev);
>  	if (ret) {
>  		ERROR("Could not set up plug-in event detection");
> -		goto free_args;
> +		goto unregister_new_callback;
>  	}
>  	mac = &dev->data->mac_addrs[0];
>  	if (mac_from_arg) {
> @@ -226,7 +235,7 @@ fs_eth_dev_create(struct rte_vdev_device *vdev)
>  							       mac);
>  			if (ret) {
>  				ERROR("Failed to set default MAC address");
> -				goto free_args;
> +				goto unregister_new_callback;
>  			}
>  		}
>  	} else {
> @@ -261,6 +270,9 @@ fs_eth_dev_create(struct rte_vdev_device *vdev)
>  	};
>  	rte_eth_dev_probing_finish(dev);
>  	return 0;
> +unregister_new_callback:
> +	rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
> +					failsafe_eth_new_event_callback, dev);
>  free_args:
>  	failsafe_args_free(dev);
>  free_subs:
> @@ -280,6 +292,8 @@ fs_rte_eth_free(const char *name)
>  	dev = rte_eth_dev_allocated(name);
>  	if (dev == NULL)
>  		return -ENODEV;
> +	rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
> +					failsafe_eth_new_event_callback, dev);
>  	ret = failsafe_eal_uninit(dev);
>  	if (ret)
>  		ERROR("Error while uninitializing sub-EAL");
> diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
> index ce767703f..8f1b9d845 100644
> --- a/drivers/net/failsafe/failsafe_eal.c
> +++ b/drivers/net/failsafe/failsafe_eal.c
> @@ -10,7 +10,7 @@
>  static int
>  fs_ethdev_portid_get(const char *name, uint16_t *port_id)
>  {
> -	uint16_t pid;
> +	uint32_t pid;

I do not see why the port_id is made uint32_t? Is there a reason?

Otherwise all seems fine. With the proper justification or with uin16_t
pid, and with a second pass on the backport tagging,

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

Thanks,
  
Matan Azrad May 9, 2018, 1:01 p.m. UTC | #2
Hi Gaetan

Regarding backporting.
This version should be bacported for 18.02.1.
There we have the new event.

Regarding uint32
The maximum port id number can be 0xffff.
In this case the loop will be infinite if we use uint16 to iterate over all the ports.

Hello Matan, Two nitpicks below: On Wed, May 09, 2018 at 11:43:36AM +0200, Thomas Monjalon wrote: > From: Matan Azrad > > There is time between the sub-device port probing by the sub-device PMD > to the sub-device port ownership taking by a fail-safe port. > > In this time, the port is available for the application usage. For > example, the port will be exposed to the applications which use > RTE_ETH_FOREACH_DEV iterator. > > Thus, ownership unaware applications may manage the port in this time > what may cause a lot of problematic behaviors in the fail-safe > sub-device initialization. > > Register to the ethdev NEW event to take the sub-device port ownership > before it becomes exposed to the application. > > Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD") This fix is relying on the RTE_ETH_EVENT_NEW, an API that I think is not meant to be backported in the stable release that would be targetted by this commit id. I think this fix is useless without the rest of this series, so I don't know what is exactly planned about the rest (whether it is backported, and where), but I would only CC stable if this is planned, and only as soon as the relevant APIs are introduced. > Cc: stable@dpdk.org > > Signed-off-by: Matan Azrad > --- > drivers/net/failsafe/failsafe.c | 22 ++++++++++--- > drivers/net/failsafe/failsafe_eal.c | 58 +++++++++++++++++++++------------ > drivers/net/failsafe/failsafe_ether.c | 23 +++++++++++++ > drivers/net/failsafe/failsafe_private.h | 4 +++ > 4 files changed, 83 insertions(+), 24 deletions(-) > > diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c > index fc989c4f5..c9d128de3 100644 > --- a/drivers/net/failsafe/failsafe.c > +++ b/drivers/net/failsafe/failsafe.c > @@ -204,16 +204,25 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) > } > snprintf(priv->my_owner.name, sizeof(priv->my_owner.name), > FAILSAFE_OWNER_NAME); > + DEBUG("Failsafe port %u owner info: %s_%016"PRIX64, dev->data->port_id, > + priv->my_owner.name, priv->my_owner.id); > + ret = rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, > + failsafe_eth_new_event_callback, > + dev); > + if (ret) { > + ERROR("Failed to register NEW callback"); > + goto free_args; > + } > ret = failsafe_eal_init(dev); > if (ret) > - goto free_args; > + goto unregister_new_callback; > ret = fs_mutex_init(priv); > if (ret) > - goto free_args; > + goto unregister_new_callback; > ret = failsafe_hotplug_alarm_install(dev); > if (ret) { > ERROR("Could not set up plug-in event detection"); > - goto free_args; > + goto unregister_new_callback; > } > mac = &dev->data->mac_addrs[0]; > if (mac_from_arg) { > @@ -226,7 +235,7 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) > mac); > if (ret) { > ERROR("Failed to set default MAC address"); > - goto free_args; > + goto unregister_new_callback; > } > } > } else { > @@ -261,6 +270,9 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) > }; > rte_eth_dev_probing_finish(dev); > return 0; > +unregister_new_callback: > + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, > + failsafe_eth_new_event_callback, dev); > free_args: > failsafe_args_free(dev); > free_subs: > @@ -280,6 +292,8 @@ fs_rte_eth_free(const char *name) > dev = rte_eth_dev_allocated(name); > if (dev == NULL) > return -ENODEV; > + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, > + failsafe_eth_new_event_callback, dev); > ret = failsafe_eal_uninit(dev); > if (ret) > ERROR("Error while uninitializing sub-EAL"); > diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c > index ce767703f..8f1b9d845 100644 > --- a/drivers/net/failsafe/failsafe_eal.c > +++ b/drivers/net/failsafe/failsafe_eal.c > @@ -10,7 +10,7 @@ > static int > fs_ethdev_portid_get(const char *name, uint16_t *port_id) > { > - uint16_t pid; > + uint32_t pid; I do not see why the port_id is made uint32_t? Is there a reason? Otherwise all seems fine. With the proper justification or with uin16_t pid, and with a second pass on the backport tagging, Acked-by: Gaetan Rivet Thanks, -- Gaëtan Rivet 6WIND
  
Thomas Monjalon May 9, 2018, 1:26 p.m. UTC | #3
09/05/2018 14:41, Gaëtan Rivet:
> > Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
> 
> This fix is relying on the RTE_ETH_EVENT_NEW, an API that I think is not
> meant to be backported in the stable release that would be targetted by
> this commit id.

This event was added in 18.02. So yes it can be backported.

> I think this fix is useless without the rest of this series, so I don't
> know what is exactly planned about the rest (whether it is backported,
> and where), but I would only CC stable if this is planned, and only as
> soon as the relevant APIs are introduced.

All the series is candidate for 18.02 backport.


> >  static int
> >  fs_ethdev_portid_get(const char *name, uint16_t *port_id)
> >  {
> > -	uint16_t pid;
> > +	uint32_t pid;
> 
> I do not see why the port_id is made uint32_t? Is there a reason?

Copying Matan's answer:
"
The maximum port id number can be 0xffff.
In this case the loop will be infinite if we use uint16 to iterate over all the ports.
"
  
Gaëtan Rivet May 9, 2018, 1:30 p.m. UTC | #4
On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote:
> Hi Gaetan
> 
> Regarding backporting.
> This version should be bacported for 18.02.1.
> There we have the new event.
> 

Then the fixline should probably reflect this instead.
Targetting the initial failsafe release won't work.

This patch also relies on probing_finish() being introduced, so I guess
the plan is to backport the whole series in 18.02.1?

If so, I think the whole series should target the same commit id within
this release, maybe the introduction of ownership or RTE_ETH_EVENT_NEW.

In any case, I think I recall being told to leave this to stable
maintainers to deal with. However, I do not see the benefit of having
a fixline if the information is meant to be discarded for someone to do
the work again.

> Regarding uint32
> The maximum port id number can be 0xffff.
> In this case the loop will be infinite if we use uint16 to iterate over all the ports.

If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff]
would be defined statically, and I think other issues would arise
before our being stuck in an infinite loop?

In any case, if this had to be fixed, then there should be a
BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of
librte_ethdev, instead of relying on librte_ethdev users skirting
shortfalls of the library. Anyone iterating on port IDs should expect the
port_id type to be sufficient to hold this information.
  
Thomas Monjalon May 9, 2018, 1:43 p.m. UTC | #5
09/05/2018 15:30, Gaëtan Rivet:
> On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote:
> > Hi Gaetan
> > 
> > Regarding backporting.
> > This version should be bacported for 18.02.1.
> > There we have the new event.
> > 
> 
> Then the fixline should probably reflect this instead.
> Targetting the initial failsafe release won't work.
> 
> This patch also relies on probing_finish() being introduced, so I guess
> the plan is to backport the whole series in 18.02.1?

Yes, I will provide a backported series for 18.02.

 
> If so, I think the whole series should target the same commit id within
> this release, maybe the introduction of ownership or RTE_ETH_EVENT_NEW.
> 
> In any case, I think I recall being told to leave this to stable
> maintainers to deal with. However, I do not see the benefit of having
> a fixline if the information is meant to be discarded for someone to do
> the work again.

The information in the Fixes line shows where the bug was introduced.
It is used to do our backports (to know if a fix is relevant)
but it can be used for other purposes like identifying known issues
in a given version.

So, in short, these bugs can be fixed easily in 18.02, but probably not
worth to backport in older releases (no 17.11 backport).

> > Regarding uint32
> > The maximum port id number can be 0xffff.
> > In this case the loop will be infinite if we use uint16 to iterate over all the ports.
> 
> If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff]
> would be defined statically, and I think other issues would arise
> before our being stuck in an infinite loop?
> 
> In any case, if this had to be fixed, then there should be a
> BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of
> librte_ethdev, instead of relying on librte_ethdev users skirting
> shortfalls of the library. Anyone iterating on port IDs should expect the
> port_id type to be sufficient to hold this information.

Interesting thought.
I vote for keeping Matan's option as it is correct,
and will accept a patch in 18.08 for your option (BUILD_BUG_ON).
Maybe we should send a deprecation notice before limiting the max
number of ports to 0xfffe? Or is it ridiculous for such unlikely constraint?
  
Gaëtan Rivet May 9, 2018, 2:03 p.m. UTC | #6
On Wed, May 09, 2018 at 03:43:31PM +0200, Thomas Monjalon wrote:
> 09/05/2018 15:30, Gaëtan Rivet:
> > On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote:
> > > Regarding uint32
> > > The maximum port id number can be 0xffff.
> > > In this case the loop will be infinite if we use uint16 to iterate over all the ports.
> > 
> > If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff]
> > would be defined statically, and I think other issues would arise
> > before our being stuck in an infinite loop?
> > 
> > In any case, if this had to be fixed, then there should be a
> > BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of
> > librte_ethdev, instead of relying on librte_ethdev users skirting
> > shortfalls of the library. Anyone iterating on port IDs should expect the
> > port_id type to be sufficient to hold this information.
> 
> Interesting thought.
> I vote for keeping Matan's option as it is correct,
> and will accept a patch in 18.08 for your option (BUILD_BUG_ON).
> Maybe we should send a deprecation notice before limiting the max
> number of ports to 0xfffe? Or is it ridiculous for such unlikely constraint?
> 
> 

No actually the issue is when RTE_MAX_ETHPORTS is equal (or superior) to
0x10000.

If this is an issue that you think is worth having a workaround here,
you should also consider that rte_eth_find_next_owned_by (and
rte_eth_find_next, even if this one should be phased out), would also
result in an overflow and an infinite loop.
  
Thomas Monjalon May 9, 2018, 9:59 p.m. UTC | #7
09/05/2018 16:03, Gaëtan Rivet:
> On Wed, May 09, 2018 at 03:43:31PM +0200, Thomas Monjalon wrote:
> > 09/05/2018 15:30, Gaëtan Rivet:
> > > On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote:
> > > > Regarding uint32
> > > > The maximum port id number can be 0xffff.
> > > > In this case the loop will be infinite if we use uint16 to iterate over all the ports.
> > > 
> > > If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff]
> > > would be defined statically, and I think other issues would arise
> > > before our being stuck in an infinite loop?
> > > 
> > > In any case, if this had to be fixed, then there should be a
> > > BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of
> > > librte_ethdev, instead of relying on librte_ethdev users skirting
> > > shortfalls of the library. Anyone iterating on port IDs should expect the
> > > port_id type to be sufficient to hold this information.
> > 
> > Interesting thought.
> > I vote for keeping Matan's option as it is correct,
> > and will accept a patch in 18.08 for your option (BUILD_BUG_ON).
> > Maybe we should send a deprecation notice before limiting the max
> > number of ports to 0xfffe? Or is it ridiculous for such unlikely constraint?
> > 
> > 
> 
> No actually the issue is when RTE_MAX_ETHPORTS is equal (or superior) to
> 0x10000.
> 
> If this is an issue that you think is worth having a workaround here,
> you should also consider that rte_eth_find_next_owned_by (and
> rte_eth_find_next, even if this one should be phased out), would also
> result in an overflow and an infinite loop.

You get a point. I will remove the workaround uint32_t in v2,
so all the related issues can be fixed at once in a separate patch
using BUILD_BUG_ON.
  

Patch

diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index fc989c4f5..c9d128de3 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -204,16 +204,25 @@  fs_eth_dev_create(struct rte_vdev_device *vdev)
 	}
 	snprintf(priv->my_owner.name, sizeof(priv->my_owner.name),
 		 FAILSAFE_OWNER_NAME);
+	DEBUG("Failsafe port %u owner info: %s_%016"PRIX64, dev->data->port_id,
+	      priv->my_owner.name, priv->my_owner.id);
+	ret = rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
+					    failsafe_eth_new_event_callback,
+					    dev);
+	if (ret) {
+		ERROR("Failed to register NEW callback");
+		goto free_args;
+	}
 	ret = failsafe_eal_init(dev);
 	if (ret)
-		goto free_args;
+		goto unregister_new_callback;
 	ret = fs_mutex_init(priv);
 	if (ret)
-		goto free_args;
+		goto unregister_new_callback;
 	ret = failsafe_hotplug_alarm_install(dev);
 	if (ret) {
 		ERROR("Could not set up plug-in event detection");
-		goto free_args;
+		goto unregister_new_callback;
 	}
 	mac = &dev->data->mac_addrs[0];
 	if (mac_from_arg) {
@@ -226,7 +235,7 @@  fs_eth_dev_create(struct rte_vdev_device *vdev)
 							       mac);
 			if (ret) {
 				ERROR("Failed to set default MAC address");
-				goto free_args;
+				goto unregister_new_callback;
 			}
 		}
 	} else {
@@ -261,6 +270,9 @@  fs_eth_dev_create(struct rte_vdev_device *vdev)
 	};
 	rte_eth_dev_probing_finish(dev);
 	return 0;
+unregister_new_callback:
+	rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
+					failsafe_eth_new_event_callback, dev);
 free_args:
 	failsafe_args_free(dev);
 free_subs:
@@ -280,6 +292,8 @@  fs_rte_eth_free(const char *name)
 	dev = rte_eth_dev_allocated(name);
 	if (dev == NULL)
 		return -ENODEV;
+	rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW,
+					failsafe_eth_new_event_callback, dev);
 	ret = failsafe_eal_uninit(dev);
 	if (ret)
 		ERROR("Error while uninitializing sub-EAL");
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce767703f..8f1b9d845 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -10,7 +10,7 @@ 
 static int
 fs_ethdev_portid_get(const char *name, uint16_t *port_id)
 {
-	uint16_t pid;
+	uint32_t pid;
 	size_t len;
 
 	if (name == NULL) {
@@ -18,8 +18,9 @@  fs_ethdev_portid_get(const char *name, uint16_t *port_id)
 		return -EINVAL;
 	}
 	len = strlen(name);
-	RTE_ETH_FOREACH_DEV(pid) {
-		if (!strncmp(name, rte_eth_devices[pid].device->name, len)) {
+	for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
+		if (rte_eth_dev_is_valid_port(pid) &&
+		    !strncmp(name, rte_eth_devices[pid].device->name, len)) {
 			*port_id = pid;
 			return 0;
 		}
@@ -41,6 +42,8 @@  fs_bus_init(struct rte_eth_dev *dev)
 			continue;
 		da = &sdev->devargs;
 		if (fs_ethdev_portid_get(da->name, &pid) != 0) {
+			struct rte_eth_dev_owner pid_owner;
+
 			ret = rte_eal_hotplug_add(da->bus->name,
 						  da->name,
 						  da->args);
@@ -55,12 +58,26 @@  fs_bus_init(struct rte_eth_dev *dev)
 				ERROR("sub_device %d init went wrong", i);
 				return -ENODEV;
 			}
+			/*
+			 * The NEW callback tried to take ownership, check
+			 * whether it succeed or didn't.
+			 */
+			rte_eth_dev_owner_get(pid, &pid_owner);
+			if (pid_owner.id != PRIV(dev)->my_owner.id) {
+				INFO("sub_device %d owner(%s_%016"PRIX64") is not my,"
+				     " owner(%s_%016"PRIX64"), will try again later",
+				     i, pid_owner.name, pid_owner.id,
+				     PRIV(dev)->my_owner.name,
+				     PRIV(dev)->my_owner.id);
+				continue;
+			}
 		} else {
+			/* The sub-device port was found. */
 			char devstr[DEVARGS_MAXLEN] = "";
 			struct rte_devargs *probed_da =
 					rte_eth_devices[pid].device->devargs;
 
-			/* Take control of device probed by EAL options. */
+			/* Take control of probed device. */
 			free(da->args);
 			memset(da, 0, sizeof(*da));
 			if (probed_da != NULL)
@@ -77,22 +94,23 @@  fs_bus_init(struct rte_eth_dev *dev)
 			}
 			INFO("Taking control of a probed sub device"
 			      " %d named %s", i, da->name);
-		}
-		ret = rte_eth_dev_owner_set(pid, &PRIV(dev)->my_owner);
-		if (ret < 0) {
-			INFO("sub_device %d owner set failed (%s),"
-			     " will try again later", i, strerror(-ret));
-			continue;
-		} else if (strncmp(rte_eth_devices[pid].device->name, da->name,
-			   strlen(da->name)) != 0) {
-			/*
-			 * The device probably was removed and its port id was
-			 * reallocated before ownership set.
-			 */
-			rte_eth_dev_owner_unset(pid, PRIV(dev)->my_owner.id);
-			INFO("sub_device %d was probably removed before taking"
-			     " ownership, will try again later", i);
-			continue;
+			ret = rte_eth_dev_owner_set(pid, &PRIV(dev)->my_owner);
+			if (ret < 0) {
+				INFO("sub_device %d owner set failed (%s), "
+				     "will try again later", i, strerror(-ret));
+				continue;
+			} else if (strncmp(rte_eth_devices[pid].device->name,
+				   da->name, strlen(da->name)) != 0) {
+				/*
+				 * The device probably was removed and its port
+				 * id was reallocated before ownership set.
+				 */
+				rte_eth_dev_owner_unset(pid,
+							PRIV(dev)->my_owner.id);
+				INFO("sub_device %d was removed before taking"
+				     " ownership, will try again later", i);
+				continue;
+			}
 		}
 		ETH(sdev) = &rte_eth_devices[pid];
 		SUB_ID(sdev) = i;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index b414a7884..ebce87841 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -463,3 +463,26 @@  failsafe_eth_lsc_event_callback(uint16_t port_id __rte_unused,
 	else
 		return 0;
 }
+
+/* Take sub-device ownership before it becomes exposed to the application. */
+int
+failsafe_eth_new_event_callback(uint16_t port_id,
+				enum rte_eth_event_type event __rte_unused,
+				void *cb_arg, void *out __rte_unused)
+{
+	struct rte_eth_dev *fs_dev = cb_arg;
+	struct sub_device *sdev;
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	size_t len = strlen(dev->device->name);
+	uint8_t i;
+
+	FOREACH_SUBDEV_STATE(sdev, i, fs_dev, DEV_PARSED) {
+		if (sdev->state >= DEV_PROBED)
+			continue;
+		if (strncmp(sdev->devargs.name, dev->device->name, len) != 0)
+			continue;
+		rte_eth_dev_owner_set(port_id, &PRIV(fs_dev)->my_owner);
+		break;
+	}
+	return 0;
+}
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index b54f8e336..cd8e0b8c9 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -220,6 +220,10 @@  int failsafe_eth_rmv_event_callback(uint16_t port_id,
 int failsafe_eth_lsc_event_callback(uint16_t port_id,
 				    enum rte_eth_event_type event,
 				    void *cb_arg, void *out);
+int
+failsafe_eth_new_event_callback(uint16_t port_id,
+				enum rte_eth_event_type event,
+				void *cb_arg, void *out);
 
 /* GLOBALS */