[dpdk-dev,v2,5/6] net/failsafe: use ownership mechanism to own ports
Checks
Commit Message
Fail-safe PMD sub devices management is based on ethdev port mechanism.
So, the sub-devices management structures are exposed to other DPDK
entities which may use them in parallel to fail-safe PMD.
Use the new port ownership mechanism to avoid multiple managments of
fail-safe PMD sub-devices.
Signed-off-by: Matan Azrad <matan@mellanox.com>
---
drivers/net/failsafe/failsafe.c | 7 +++++++
drivers/net/failsafe/failsafe_eal.c | 10 ++++++++++
drivers/net/failsafe/failsafe_private.h | 2 ++
3 files changed, 19 insertions(+)
Comments
Hi Matan,
Thanks for the patches. A remark however:
On Sun, Jan 07, 2018 at 09:45:50AM +0000, Matan Azrad wrote:
> Fail-safe PMD sub devices management is based on ethdev port mechanism.
> So, the sub-devices management structures are exposed to other DPDK
> entities which may use them in parallel to fail-safe PMD.
>
> Use the new port ownership mechanism to avoid multiple managments of
> fail-safe PMD sub-devices.
>
I think your implementation does not work with several fail-safe
instances, have you tested this configuration?
It should be possible for a user to create any number of fail-safe
instances. The minimum would be to allow for multiple fail-safe
side-by-side, but ideally it should also support a recursive
configuration:
+-----------+
|fail-safe |
| |
| |
+-+ +--+
| | | |
| +-----------+ |
| |
+-------v----+ +-----v-----+
|fail-safe | | |
| | | |
| | | |
| | | |
+-+ +-+ | |
| +------------+ | +-----------+
| |
+-----v-----+ +-----v-----+
| | | |
| | | |
| | | |
| | | |
| | | |
+-----------+ +-----------+
If I am not mistaken on this, then you need to generate different
owner-ids for each fail-safe instances.
I suggest using the full fail-safe instance name, as they are already
assured to be different from each other by the EAL, and you thus won't
need to generate IDs on the fly, as well as declare a global owner-id
prefix.
Hi Gaetan
From: Gaëtan Rivet, Sent: Monday, January 8, 2018 12:33 PM
> Hi Matan,
>
> Thanks for the patches. A remark however:
>
> On Sun, Jan 07, 2018 at 09:45:50AM +0000, Matan Azrad wrote:
> > Fail-safe PMD sub devices management is based on ethdev port
> mechanism.
> > So, the sub-devices management structures are exposed to other DPDK
> > entities which may use them in parallel to fail-safe PMD.
> >
> > Use the new port ownership mechanism to avoid multiple managments of
> > fail-safe PMD sub-devices.
> >
>
> I think your implementation does not work with several fail-safe instances,
> have you tested this configuration?
>
Why not? Each instance calls to fs_eth_dev_create and there the unique owner id allocation is called.
So, Any instance should get a unique owner id.
> It should be possible for a user to create any number of fail-safe instances.
> The minimum would be to allow for multiple fail-safe side-by-side, but ideally
> it should also support a recursive
> configuration:
>
> +-----------+
> |fail-safe |
> | |
> | |
> +-+ +--+
> | | | |
> | +-----------+ |
> | |
> +-------v----+ +-----v-----+
> |fail-safe | | |
> | | | |
> | | | |
> | | | |
> +-+ +-+ | |
> | +------------+ | +-----------+
> | |
> +-----v-----+ +-----v-----+
> | | | |
> | | | |
> | | | |
> | | | |
> | | | |
> +-----------+ +-----------+
>
> If I am not mistaken on this, then you need to generate different owner-ids
> for each fail-safe instances.
>
it is already done as I wrote above.
> I suggest using the full fail-safe instance name, as they are already assured to
> be different from each other by the EAL, and you thus won't need to
> generate IDs on the fly, as well as declare a global owner-id prefix.
>
The ID generation should be initiated by the DPDK entity itself(the fail-safe instance in this case).
The prefix can be changed to the EAL full fail-safe instance name, but it is not must, because the owner IDs are different.
> --
> Gaëtan Rivet
> 6WIND
On Mon, Jan 08, 2018 at 11:16:37AM +0000, Matan Azrad wrote:
> Hi Gaetan
>
> From: Gaëtan Rivet, Sent: Monday, January 8, 2018 12:33 PM
> > Hi Matan,
> >
> > Thanks for the patches. A remark however:
> >
> > On Sun, Jan 07, 2018 at 09:45:50AM +0000, Matan Azrad wrote:
> > > Fail-safe PMD sub devices management is based on ethdev port
> > mechanism.
> > > So, the sub-devices management structures are exposed to other DPDK
> > > entities which may use them in parallel to fail-safe PMD.
> > >
> > > Use the new port ownership mechanism to avoid multiple managments of
> > > fail-safe PMD sub-devices.
> > >
> >
> > I think your implementation does not work with several fail-safe instances,
> > have you tested this configuration?
> >
>
> Why not? Each instance calls to fs_eth_dev_create and there the unique owner id allocation is called.
> So, Any instance should get a unique owner id.
>
Ah, sure, I forgot about the actual owner-id.
No problem then, as you said it should work.
Regarding the fail-safe part:
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
@@ -191,6 +191,13 @@
ret = failsafe_args_parse(dev, params);
if (ret)
goto free_subs;
+ ret = rte_eth_dev_owner_new(&priv->my_owner.id);
+ if (ret) {
+ ERROR("Failed to get unique owner identifier");
+ goto free_args;
+ }
+ snprintf(priv->my_owner.name, sizeof(priv->my_owner.name),
+ FAILSAFE_OWNER_NAME);
ret = failsafe_eal_init(dev);
if (ret)
goto free_args;
@@ -69,6 +69,16 @@
ERROR("sub_device %d init went wrong", i);
return -ENODEV;
}
+ ret = rte_eth_dev_owner_set(j, &PRIV(dev)->my_owner);
+ if (ret) {
+ /*
+ * It is unexpected for a fail-safe sub-device
+ * to be owned by another DPDK entity.
+ */
+ ERROR("sub_device %d owner set failed (%s)", i,
+ strerror(ret));
+ return ret;
+ }
SUB_ID(sdev) = i;
sdev->fs_dev = dev;
sdev->dev = ETH(sdev)->device;
@@ -42,6 +42,7 @@
#include <rte_devargs.h>
#define FAILSAFE_DRIVER_NAME "Fail-safe PMD"
+#define FAILSAFE_OWNER_NAME "Fail-safe"
#define PMD_FAILSAFE_MAC_KVARG "mac"
#define PMD_FAILSAFE_HOTPLUG_POLL_KVARG "hotplug_poll"
@@ -139,6 +140,7 @@ struct fs_priv {
uint32_t mac_addr_pool[FAILSAFE_MAX_ETHADDR];
/* current capabilities */
struct rte_eth_dev_info infos;
+ struct rte_eth_dev_owner my_owner; /* Unique owner. */
/*
* Fail-safe state machine.
* This level will be tracking state of the EAL and eth