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: sta...@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
