[dpdk-dev,v3,3/8] devargs: introduce insert function
Checks
Commit Message
Some buses will operate either in whitelist or blacklist mode.
This mode is currently passed down by the rte_eal_devargs_add function
with the devtype argument.
When inserting devices using the hotplug API, the implicit assumption is
that this device is being whitelisted, meaning that it is explicitly
requested by the application to be used. This can conflict with the
initial bus configuration.
While the rte_eal_devargs_add API is being deprecated soon, it cannot
be modified at the moment to accomodate this situation.
As such, this new experimental API offers a bare interface for inserting
rte_devargs without directly manipulating the global rte_devargs list.
This new function expects a fully-formed rte_devargs, previously parsed
and allocated.
It does not check whether the new rte_devargs is compatible with current
bus configuration, but will replace any eventual existing one for the same
device, allowing the hotplug operation to proceed. i.e. a previously
blacklisted device can be redefined as being whitelisted.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
lib/librte_eal/common/eal_common_devargs.c | 12 ++++++++++++
lib/librte_eal/common/include/rte_devargs.h | 13 +++++++++++++
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
4 files changed, 27 insertions(+)
Comments
On Tue, Jul 11, 2017 at 7:25 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> Some buses will operate either in whitelist or blacklist mode.
> This mode is currently passed down by the rte_eal_devargs_add function
> with the devtype argument.
>
> When inserting devices using the hotplug API, the implicit assumption is
> that this device is being whitelisted, meaning that it is explicitly
> requested by the application to be used. This can conflict with the
> initial bus configuration.
Actually I don't think that this is correct. If I blacklist a device
via devargs I don't want this to be probed in case my udev helper is
calling hotplug add.
Maybe it is better to just update the args field in case the devargs
instance is already found.
>
> While the rte_eal_devargs_add API is being deprecated soon, it cannot
> be modified at the moment to accomodate this situation.
> As such, this new experimental API offers a bare interface for inserting
> rte_devargs without directly manipulating the global rte_devargs list.
>
> This new function expects a fully-formed rte_devargs, previously parsed
> and allocated.
>
> It does not check whether the new rte_devargs is compatible with current
> bus configuration, but will replace any eventual existing one for the same
> device, allowing the hotplug operation to proceed. i.e. a previously
> blacklisted device can be redefined as being whitelisted.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> lib/librte_eal/common/eal_common_devargs.c | 12 ++++++++++++
> lib/librte_eal/common/include/rte_devargs.h | 13 +++++++++++++
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> 4 files changed, 27 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 40cd523..8b24309 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -206,6 +206,7 @@ DPDK_17.08 {
> EXPERIMENTAL {
> global:
>
> + rte_eal_devargs_insert;
> rte_eal_devargs_parse;
> rte_eal_devargs_remove;
> rte_eal_hotplug_add;
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index bcdee13..ff6c2a8 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -138,6 +138,18 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
> return 0;
> }
>
> +int
> +rte_eal_devargs_insert(struct rte_devargs *da)
> +{
> + int ret;
> +
> + ret = rte_eal_devargs_remove(da->bus->name, da->name);
> + if (ret < 0)
> + return ret;
> + TAILQ_INSERT_TAIL(&devargs_list, da, next);
> + return 0;
> +}
> +
> /* store a whitelist parameter for later parsing */
> int
> rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 36453b6..7b63fa3 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -139,6 +139,19 @@ rte_eal_devargs_parse(const char *dev,
> struct rte_devargs *da);
>
> /**
> + * Insert an rte_devargs in the global list.
> + *
> + * @param da
> + * The devargs structure to insert.
> + *
> + * @return
> + * - 0 on success
> + * - Negative on error.
> + */
> +int
> +rte_eal_devargs_insert(struct rte_devargs *da);
> +
> +/**
> * Add a device to the user device list
> *
> * For PCI devices, the format of arguments string is "PCI_ADDR" or
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index a8ee349..81f6af3 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -211,6 +211,7 @@ DPDK_17.08 {
> EXPERIMENTAL {
> global:
>
> + rte_eal_devargs_insert;
> rte_eal_devargs_parse;
> rte_eal_devargs_remove;
> rte_eal_hotplug_add;
> --
> 2.1.4
>
On Wed, Jul 12, 2017 at 04:20:48AM -0400, Jan Blunck wrote:
> On Tue, Jul 11, 2017 at 7:25 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > Some buses will operate either in whitelist or blacklist mode.
> > This mode is currently passed down by the rte_eal_devargs_add function
> > with the devtype argument.
> >
> > When inserting devices using the hotplug API, the implicit assumption is
> > that this device is being whitelisted, meaning that it is explicitly
> > requested by the application to be used. This can conflict with the
> > initial bus configuration.
>
> Actually I don't think that this is correct. If I blacklist a device
> via devargs I don't want this to be probed in case my udev helper is
> calling hotplug add.
>
This would be good for a udev handler yes. But should we expect this
behavior from all potential hotplug initiators?
To put it another way: should this rule be enforced at the EAL level or
from those using it? And is it impossible to imagine a system that would
actually need to be able to update a device, changing it from
blacklisted to whitelisted on some specific condition?
The fail-safe at least makes use of this ability. This is at the moment
the only hotplug user.
> Maybe it is better to just update the args field in case the devargs
> instance is already found.
>
> >
> > While the rte_eal_devargs_add API is being deprecated soon, it cannot
> > be modified at the moment to accomodate this situation.
> > As such, this new experimental API offers a bare interface for inserting
> > rte_devargs without directly manipulating the global rte_devargs list.
> >
> > This new function expects a fully-formed rte_devargs, previously parsed
> > and allocated.
> >
> > It does not check whether the new rte_devargs is compatible with current
> > bus configuration, but will replace any eventual existing one for the same
> > device, allowing the hotplug operation to proceed. i.e. a previously
> > blacklisted device can be redefined as being whitelisted.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
> > lib/librte_eal/common/eal_common_devargs.c | 12 ++++++++++++
> > lib/librte_eal/common/include/rte_devargs.h | 13 +++++++++++++
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
> > 4 files changed, 27 insertions(+)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index 40cd523..8b24309 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -206,6 +206,7 @@ DPDK_17.08 {
> > EXPERIMENTAL {
> > global:
> >
> > + rte_eal_devargs_insert;
> > rte_eal_devargs_parse;
> > rte_eal_devargs_remove;
> > rte_eal_hotplug_add;
> > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> > index bcdee13..ff6c2a8 100644
> > --- a/lib/librte_eal/common/eal_common_devargs.c
> > +++ b/lib/librte_eal/common/eal_common_devargs.c
> > @@ -138,6 +138,18 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
> > return 0;
> > }
> >
> > +int
> > +rte_eal_devargs_insert(struct rte_devargs *da)
> > +{
> > + int ret;
> > +
> > + ret = rte_eal_devargs_remove(da->bus->name, da->name);
> > + if (ret < 0)
> > + return ret;
> > + TAILQ_INSERT_TAIL(&devargs_list, da, next);
> > + return 0;
> > +}
> > +
> > /* store a whitelist parameter for later parsing */
> > int
> > rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> > index 36453b6..7b63fa3 100644
> > --- a/lib/librte_eal/common/include/rte_devargs.h
> > +++ b/lib/librte_eal/common/include/rte_devargs.h
> > @@ -139,6 +139,19 @@ rte_eal_devargs_parse(const char *dev,
> > struct rte_devargs *da);
> >
> > /**
> > + * Insert an rte_devargs in the global list.
> > + *
> > + * @param da
> > + * The devargs structure to insert.
> > + *
> > + * @return
> > + * - 0 on success
> > + * - Negative on error.
> > + */
> > +int
> > +rte_eal_devargs_insert(struct rte_devargs *da);
> > +
> > +/**
> > * Add a device to the user device list
> > *
> > * For PCI devices, the format of arguments string is "PCI_ADDR" or
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index a8ee349..81f6af3 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -211,6 +211,7 @@ DPDK_17.08 {
> > EXPERIMENTAL {
> > global:
> >
> > + rte_eal_devargs_insert;
> > rte_eal_devargs_parse;
> > rte_eal_devargs_remove;
> > rte_eal_hotplug_add;
> > --
> > 2.1.4
> >
On Wed, Jul 12, 2017 at 1:20 PM, Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> On Wed, Jul 12, 2017 at 04:20:48AM -0400, Jan Blunck wrote:
>> On Tue, Jul 11, 2017 at 7:25 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
>> > Some buses will operate either in whitelist or blacklist mode.
>> > This mode is currently passed down by the rte_eal_devargs_add function
>> > with the devtype argument.
>> >
>> > When inserting devices using the hotplug API, the implicit assumption is
>> > that this device is being whitelisted, meaning that it is explicitly
>> > requested by the application to be used. This can conflict with the
>> > initial bus configuration.
>>
>> Actually I don't think that this is correct. If I blacklist a device
>> via devargs I don't want this to be probed in case my udev helper is
>> calling hotplug add.
>>
>
> This would be good for a udev handler yes. But should we expect this
> behavior from all potential hotplug initiators?
>
> To put it another way: should this rule be enforced at the EAL level or
> from those using it? And is it impossible to imagine a system that would
> actually need to be able to update a device, changing it from
> blacklisted to whitelisted on some specific condition?
>
> The fail-safe at least makes use of this ability. This is at the moment
> the only hotplug user.
>
In tree .... I have hotplug functionality available for quite some time already.
>> Maybe it is better to just update the args field in case the devargs
>> instance is already found.
>>
>> >
>> > While the rte_eal_devargs_add API is being deprecated soon, it cannot
>> > be modified at the moment to accomodate this situation.
>> > As such, this new experimental API offers a bare interface for inserting
>> > rte_devargs without directly manipulating the global rte_devargs list.
>> >
>> > This new function expects a fully-formed rte_devargs, previously parsed
>> > and allocated.
>> >
>> > It does not check whether the new rte_devargs is compatible with current
>> > bus configuration, but will replace any eventual existing one for the same
>> > device, allowing the hotplug operation to proceed. i.e. a previously
>> > blacklisted device can be redefined as being whitelisted.
>> >
>> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> > ---
>> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 +
>> > lib/librte_eal/common/eal_common_devargs.c | 12 ++++++++++++
>> > lib/librte_eal/common/include/rte_devargs.h | 13 +++++++++++++
>> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 +
>> > 4 files changed, 27 insertions(+)
>> >
>> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> > index 40cd523..8b24309 100644
>> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>> > @@ -206,6 +206,7 @@ DPDK_17.08 {
>> > EXPERIMENTAL {
>> > global:
>> >
>> > + rte_eal_devargs_insert;
>> > rte_eal_devargs_parse;
>> > rte_eal_devargs_remove;
>> > rte_eal_hotplug_add;
>> > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
>> > index bcdee13..ff6c2a8 100644
>> > --- a/lib/librte_eal/common/eal_common_devargs.c
>> > +++ b/lib/librte_eal/common/eal_common_devargs.c
>> > @@ -138,6 +138,18 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
>> > return 0;
>> > }
>> >
>> > +int
>> > +rte_eal_devargs_insert(struct rte_devargs *da)
>> > +{
>> > + int ret;
>> > +
>> > + ret = rte_eal_devargs_remove(da->bus->name, da->name);
>> > + if (ret < 0)
>> > + return ret;
>> > + TAILQ_INSERT_TAIL(&devargs_list, da, next);
>> > + return 0;
>> > +}
>> > +
>> > /* store a whitelist parameter for later parsing */
>> > int
>> > rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>> > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
>> > index 36453b6..7b63fa3 100644
>> > --- a/lib/librte_eal/common/include/rte_devargs.h
>> > +++ b/lib/librte_eal/common/include/rte_devargs.h
>> > @@ -139,6 +139,19 @@ rte_eal_devargs_parse(const char *dev,
>> > struct rte_devargs *da);
>> >
>> > /**
>> > + * Insert an rte_devargs in the global list.
>> > + *
>> > + * @param da
>> > + * The devargs structure to insert.
>> > + *
>> > + * @return
>> > + * - 0 on success
>> > + * - Negative on error.
>> > + */
>> > +int
>> > +rte_eal_devargs_insert(struct rte_devargs *da);
>> > +
>> > +/**
>> > * Add a device to the user device list
>> > *
>> > * For PCI devices, the format of arguments string is "PCI_ADDR" or
>> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> > index a8ee349..81f6af3 100644
>> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> > @@ -211,6 +211,7 @@ DPDK_17.08 {
>> > EXPERIMENTAL {
>> > global:
>> >
>> > + rte_eal_devargs_insert;
>> > rte_eal_devargs_parse;
>> > rte_eal_devargs_remove;
>> > rte_eal_hotplug_add;
>> > --
>> > 2.1.4
>> >
>
> --
> Gaėtan Rivet
> 6WIND
@@ -206,6 +206,7 @@ DPDK_17.08 {
EXPERIMENTAL {
global:
+ rte_eal_devargs_insert;
rte_eal_devargs_parse;
rte_eal_devargs_remove;
rte_eal_hotplug_add;
@@ -138,6 +138,18 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
return 0;
}
+int
+rte_eal_devargs_insert(struct rte_devargs *da)
+{
+ int ret;
+
+ ret = rte_eal_devargs_remove(da->bus->name, da->name);
+ if (ret < 0)
+ return ret;
+ TAILQ_INSERT_TAIL(&devargs_list, da, next);
+ return 0;
+}
+
/* store a whitelist parameter for later parsing */
int
rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
@@ -139,6 +139,19 @@ rte_eal_devargs_parse(const char *dev,
struct rte_devargs *da);
/**
+ * Insert an rte_devargs in the global list.
+ *
+ * @param da
+ * The devargs structure to insert.
+ *
+ * @return
+ * - 0 on success
+ * - Negative on error.
+ */
+int
+rte_eal_devargs_insert(struct rte_devargs *da);
+
+/**
* Add a device to the user device list
*
* For PCI devices, the format of arguments string is "PCI_ADDR" or
@@ -211,6 +211,7 @@ DPDK_17.08 {
EXPERIMENTAL {
global:
+ rte_eal_devargs_insert;
rte_eal_devargs_parse;
rte_eal_devargs_remove;
rte_eal_hotplug_add;