[v2,03/22] ethdev: add function to release port in local process
Checks
Commit Message
Add driver API rte_eth_release_port_private to support the
requirement that an ethdev only be released on secondary process,
so only local state be set to unused , share data will not be
reset so primary process can still use it.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- rename rte_eth_release_port_local to rte_eth_release_port_private.
lib/librte_ethdev/rte_ethdev.c | 24 +++++++++++++++++++++---
lib/librte_ethdev/rte_ethdev_driver.h | 13 +++++++++++++
2 files changed, 34 insertions(+), 3 deletions(-)
Comments
On 21-Jun-18 3:00 AM, Qi Zhang wrote:
> Add driver API rte_eth_release_port_private to support the
> requirement that an ethdev only be released on secondary process,
> so only local state be set to unused , share data will not be
> reset so primary process can still use it.
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
<snip>
>
> /**
> * @internal
> + * Release the specified ethdev port in local process, only set to ethdev
> + * state to unused, but not reset share data since it assume other process
> + * is still using it, typically it is called by secondary process.
> + *
> + * @param eth_dev
> + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure.
> + * @return
> + * - 0 on success, negative on error
> + */
> +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev);
> +
As far as i can tell, even though the function is marked as internal, it
should still be exported in the .map file (see rte_eth_dev_allocate()
for example).
Thomas and others, does this count as new API? Should this be marked as
__rte_experimental? Presumably, we guarantee ABI stability for internal
functions too, so my expectation would be yes.
> +/**
> + * @internal
> * Release device queues and clear its configuration to force the user
> * application to reconfigure it. It is for internal use only.
> *
>
21/06/2018 10:06, Burakov, Anatoly:
> On 21-Jun-18 3:00 AM, Qi Zhang wrote:
> > Add driver API rte_eth_release_port_private to support the
> > requirement that an ethdev only be released on secondary process,
> > so only local state be set to unused , share data will not be
> > reset so primary process can still use it.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
>
> <snip>
>
> >
> > /**
> > * @internal
> > + * Release the specified ethdev port in local process, only set to ethdev
> > + * state to unused, but not reset share data since it assume other process
> > + * is still using it, typically it is called by secondary process.
> > + *
> > + * @param eth_dev
> > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure.
> > + * @return
> > + * - 0 on success, negative on error
> > + */
> > +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev);
> > +
>
> As far as i can tell, even though the function is marked as internal, it
> should still be exported in the .map file (see rte_eth_dev_allocate()
> for example).
>
> Thomas and others, does this count as new API? Should this be marked as
> __rte_experimental? Presumably, we guarantee ABI stability for internal
> functions too, so my expectation would be yes.
You know the A in ABI stands for Application :)
If it is not called by application, it has no impact on ABI.
However, I am not sure about having this function at all.
Who is calling it?
> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Thursday, June 21, 2018 4:06 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [PATCH v2 03/22] ethdev: add function to release port in local
> process
>
> On 21-Jun-18 3:00 AM, Qi Zhang wrote:
> > Add driver API rte_eth_release_port_private to support the requirement
> > that an ethdev only be released on secondary process, so only local
> > state be set to unused , share data will not be reset so primary
> > process can still use it.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
>
> <snip>
>
> >
> > /**
> > * @internal
> > + * Release the specified ethdev port in local process, only set to
> > +ethdev
> > + * state to unused, but not reset share data since it assume other
> > +process
> > + * is still using it, typically it is called by secondary process.
> > + *
> > + * @param eth_dev
> > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure.
> > + * @return
> > + * - 0 on success, negative on error
> > + */
> > +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev);
> > +
>
> As far as i can tell, even though the function is marked as internal, it should still
> be exported in the .map file (see rte_eth_dev_allocate() for example).
>
> Thomas and others, does this count as new API? Should this be marked as
> __rte_experimental? Presumably, we guarantee ABI stability for internal
> functions too, so my expectation would be yes.
Sorry, I not intent to mark this as experimental, I must forgot to remove this
It should rte_eth_dev_attach/detach_private and rte_eth_dev_lock/unlock .
I guess internal API is not necessary to have this.
I will remove it in v3
Thanks
Qi
>
> > +/**
> > + * @internal
> > * Release device queues and clear its configuration to force the user
> > * application to reconfigure it. It is for internal use only.
> > *
> >
>
>
> --
> Thanks,
> Anatoly
@@ -359,6 +359,23 @@ rte_eth_dev_attach_secondary(const char *name)
}
int
+rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev)
+{
+ if (eth_dev == NULL)
+ return -EINVAL;
+
+ _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY, NULL);
+
+ rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
+
+ eth_dev->state = RTE_ETH_DEV_UNUSED;
+
+ rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
+
+ return 0;
+}
+
+int
rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
{
if (eth_dev == NULL)
@@ -370,9 +387,10 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
- eth_dev->state = RTE_ETH_DEV_UNUSED;
-
- memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));
+ if (eth_dev->state != RTE_ETH_DEV_UNUSED) {
+ eth_dev->state = RTE_ETH_DEV_UNUSED;
+ memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));
+ }
rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
@@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev);
/**
* @internal
+ * Release the specified ethdev port in local process, only set to ethdev
+ * state to unused, but not reset share data since it assume other process
+ * is still using it, typically it is called by secondary process.
+ *
+ * @param eth_dev
+ * The *eth_dev* pointer is the address of the *rte_eth_dev* structure.
+ * @return
+ * - 0 on success, negative on error
+ */
+int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev);
+
+/**
+ * @internal
* Release device queues and clear its configuration to force the user
* application to reconfigure it. It is for internal use only.
*