[dpdk-dev,v1,4/8] bus: add probe mode setter
Checks
Commit Message
Introduce new rte_bus operation to configure the probe policy.
Implementation is required from buses interested in supporting
this configuration element.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
lib/librte_eal/common/eal_common_bus.c | 24 ++++++++++++++++++++++++
lib/librte_eal/common/eal_common_options.c | 17 ++++-------------
lib/librte_eal/common/include/rte_bus.h | 16 ++++++++++++++++
3 files changed, 44 insertions(+), 13 deletions(-)
Comments
On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
> Introduce new rte_bus operation to configure the probe policy.
>
> Implementation is required from buses interested in supporting
> this configuration element.
>
[...]
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index e40c049..630c9d2 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -997,29 +997,24 @@ int
> eal_parse_common_option(int opt, const char *optarg,
> struct internal_config *conf)
> {
> - static int b_used;
> - static int w_used;
> -
> switch (opt) {
> /* blacklist */
> case 'b':
> - if (w_used)
> - goto bw_used;
> + if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
> + return -1;
Generic layer shouldn't be concerned about "pci" or other bus.
Problem would be to find which bus this option needs to be set.
What I can think of as options is:
1. Storing this configuration until we can parse the argument for which
<bus> the argument has been created. That would mean changing the way
the "-b" and "-w" are passed and to allow non-PCI device identifier to
be passed.
2. Call each bus bus->ctrl and let it decide what to do based on the args.
- so, have a wrapper over rte_bus_probe_mode_set for all buses rather
than taking any one bus as option.
(2) sounds most plausible for now as the application will not send the
bus name as argument.
And if brute force is not required, we need to split the argument to
know the bus - after making the device naming standardized (<bus>:<...>)
Even before that, we need to agree that "-w' and "-b" are not more valid
only for PCIs.
Above is more of loud thinging - I don't have concrete thought on this
for now. I'll revisit this after reviewing the patches in this series.
> if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
> optarg) < 0) {
> return -1;
> }
> - b_used = 1;
> break;
> /* whitelist */
> case 'w':
> - if (b_used)
> - goto bw_used;
> + if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
> + return -1;
> if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
> optarg) < 0) {
> return -1;
> }
> - w_used = 1;
> break;
> /* coremask */
> case 'c':
> @@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char *optarg,
[...]
On Monday 11 December 2017 06:09 PM, Shreyansh Jain wrote:
> On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
>> Introduce new rte_bus operation to configure the probe policy.
>>
>> Implementation is required from buses interested in supporting
>> this configuration element.
>>
>
> [...]
>
>> diff --git a/lib/librte_eal/common/eal_common_options.c
>> b/lib/librte_eal/common/eal_common_options.c
>> index e40c049..630c9d2 100644
>> --- a/lib/librte_eal/common/eal_common_options.c
>> +++ b/lib/librte_eal/common/eal_common_options.c
>> @@ -997,29 +997,24 @@ int
>> eal_parse_common_option(int opt, const char *optarg,
>> struct internal_config *conf)
>> {
>> - static int b_used;
>> - static int w_used;
>> -
>> switch (opt) {
>> /* blacklist */
>> case 'b':
>> - if (w_used)
>> - goto bw_used;
>> + if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
>> + return -1;
>
> Generic layer shouldn't be concerned about "pci" or other bus.
> Problem would be to find which bus this option needs to be set.
>
> What I can think of as options is:
> 1. Storing this configuration until we can parse the argument for which
> <bus> the argument has been created. That would mean changing the way
> the "-b" and "-w" are passed and to allow non-PCI device identifier to
> be passed.
> 2. Call each bus bus->ctrl and let it decide what to do based on the args.
> - so, have a wrapper over rte_bus_probe_mode_set for all buses rather
> than taking any one bus as option.
>
> (2) sounds most plausible for now as the application will not send the
> bus name as argument.
> And if brute force is not required, we need to split the argument to
> know the bus - after making the device naming standardized (<bus>:<...>)
>
> Even before that, we need to agree that "-w' and "-b" are not more valid
> only for PCIs.
>
> Above is more of loud thinging - I don't have concrete thought on this
> for now. I'll revisit this after reviewing the patches in this series.
Apologies. I see that some work has been done for this in devargs series
- I was too quick to reply on this.
Let me come back to you on this after reading through that series.
>
>> if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
>> optarg) < 0) {
>> return -1;
>> }
>> - b_used = 1;
>> break;
>> /* whitelist */
>> case 'w':
>> - if (b_used)
>> - goto bw_used;
>> + if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
>> + return -1;
>> if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
>> optarg) < 0) {
>> return -1;
>> }
>> - w_used = 1;
>> break;
>> /* coremask */
>> case 'c':
>> @@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char
>> *optarg,
>
> [...]
>
@@ -120,6 +120,30 @@ rte_bus_probe(void)
return 0;
}
+/* Configure the probing policy of a bus */
+int
+rte_bus_probe_mode_set(const char *busname,
+ enum rte_bus_probe_mode mode)
+{
+ struct rte_bus *bus;
+ rte_bus_ctrl_t probe_mode_set;
+
+ bus = rte_bus_find_by_name(busname);
+ if (bus == NULL) {
+ RTE_LOG(ERR, EAL, "Bus %s not found.\n",
+ busname);
+ return -EFAULT;
+ }
+ probe_mode_set = bus->ctrl(RTE_BUS_CTRL_SET,
+ RTE_BUS_CTRL_PROBE_MODE);
+ if (probe_mode_set == NULL) {
+ RTE_LOG(ERR, EAL, "Bus %s: probe policy cannot be configured.\n",
+ busname);
+ return -ENOTSUP;
+ }
+ return probe_mode_set(&mode);
+}
+
/* Dump information of a single bus */
static int
bus_dump_one(FILE *f, struct rte_bus *bus)
@@ -997,29 +997,24 @@ int
eal_parse_common_option(int opt, const char *optarg,
struct internal_config *conf)
{
- static int b_used;
- static int w_used;
-
switch (opt) {
/* blacklist */
case 'b':
- if (w_used)
- goto bw_used;
+ if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0)
+ return -1;
if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
optarg) < 0) {
return -1;
}
- b_used = 1;
break;
/* whitelist */
case 'w':
- if (b_used)
- goto bw_used;
+ if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0)
+ return -1;
if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
optarg) < 0) {
return -1;
}
- w_used = 1;
break;
/* coremask */
case 'c':
@@ -1165,10 +1160,6 @@ eal_parse_common_option(int opt, const char *optarg,
}
return 0;
-bw_used:
- RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
- "cannot be used at the same time\n");
- return -1;
}
static void
@@ -289,6 +289,22 @@ int rte_bus_scan(void);
int rte_bus_probe(void);
/**
+ * Configure bus probe policy.
+ *
+ * @param busname
+ * Name of the bus to configure.
+ *
+ * @param mode
+ * Configure the designated bus probe policy to this value.
+ *
+ * @return
+ * 0 on success
+ * !0 otherwise
+ */
+int rte_bus_probe_mode_set(const char *busname,
+ enum rte_bus_probe_mode mode);
+
+/**
* Dump information of all the buses registered with EAL.
*
* @param f