[1/5] bus/vmbus: add devargs support
Checks
Commit Message
From: Stephen Hemminger <sthemmin@microsoft.com>
Take device arguments from command line and put
them in the device devargs.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/bus/vmbus/linux/vmbus_bus.c | 2 ++
drivers/bus/vmbus/private.h | 3 +++
drivers/bus/vmbus/vmbus_common.c | 22 +++++++++++++++++++++-
3 files changed, 26 insertions(+), 1 deletion(-)
Comments
On 8/30/2018 11:35 PM, Stephen Hemminger wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> Take device arguments from command line and put
> them in the device devargs.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
<...>
> @@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
> return ret;
> }
>
> +/*
> + * scan for matching device args on command line
> + * example:
> + * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
This is just in comment but,
I guess latest syntax is:
-w "vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20"
@Gaetan, is latest devarg syntax documented somewhere?
Hi,
On Fri, Sep 14, 2018 at 01:46:59PM +0100, Ferruh Yigit wrote:
> On 8/30/2018 11:35 PM, Stephen Hemminger wrote:
> > From: Stephen Hemminger <sthemmin@microsoft.com>
> >
> > Take device arguments from command line and put
> > them in the device devargs.
> >
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>
> <...>
>
> > @@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
> > return ret;
> > }
> >
> > +/*
> > + * scan for matching device args on command line
> > + * example:
> > + * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
>
> This is just in comment but,
>
> I guess latest syntax is:
> -w "vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20"
>
> @Gaetan, is latest devarg syntax documented somewhere?
That's the current syntax indeed. Some documentation is found at
lib/librte_eal/common/include/rte_devargs.h:100
Where it is specified that the bus name can be either omitted or
followed by any character, to separate it from the device identifier.
This means that using ':' is fine, as well as '('. As long as the device
PMD afterward ignore the dangling ')' during devargs parsing, this should
be fine.
I don't think this is very clean, but it works.
On 9/14/2018 2:06 PM, Gaëtan Rivet wrote:
> Hi,
>
> On Fri, Sep 14, 2018 at 01:46:59PM +0100, Ferruh Yigit wrote:
>> On 8/30/2018 11:35 PM, Stephen Hemminger wrote:
>>> From: Stephen Hemminger <sthemmin@microsoft.com>
>>>
>>> Take device arguments from command line and put
>>> them in the device devargs.
>>>
>>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>>
>> <...>
>>
>>> @@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
>>> return ret;
>>> }
>>>
>>> +/*
>>> + * scan for matching device args on command line
>>> + * example:
>>> + * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
>>
>> This is just in comment but,
>>
>> I guess latest syntax is:
>> -w "vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20"
>>
>> @Gaetan, is latest devarg syntax documented somewhere?
>
> That's the current syntax indeed. Some documentation is found at
>
> lib/librte_eal/common/include/rte_devargs.h:100
>
> Where it is specified that the bus name can be either omitted or
> followed by any character, to separate it from the device identifier.
>
> This means that using ':' is fine, as well as '('. As long as the device
> PMD afterward ignore the dangling ')' during devargs parsing, this should
> be fine.
>
> I don't think this is very clean, but it works.
Thanks for the info, I see how "(" works, but ")" is takes as part of argument
and causing problem, I think better to not give "()" as supported syntax at all.
btw, now both -w and --vdev are valid and can be used interchangeably, right? I
mean all following are valid?
-w pci:0000:86:06.0,enable_floating_veb=1
-w vdev:net_pcap,iface=lo
--vdev pci:0000:86:06.0,enable_floating_veb=1
--vdev vdev:net_pcap,iface=lo
On Fri, Sep 14, 2018 at 02:19:19PM +0100, Ferruh Yigit wrote:
> On 9/14/2018 2:06 PM, Gaëtan Rivet wrote:
> > Hi,
> >
> > On Fri, Sep 14, 2018 at 01:46:59PM +0100, Ferruh Yigit wrote:
> >> On 8/30/2018 11:35 PM, Stephen Hemminger wrote:
> >>> From: Stephen Hemminger <sthemmin@microsoft.com>
> >>>
> >>> Take device arguments from command line and put
> >>> them in the device devargs.
> >>>
> >>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> >>
> >> <...>
> >>
> >>> @@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
> >>> return ret;
> >>> }
> >>>
> >>> +/*
> >>> + * scan for matching device args on command line
> >>> + * example:
> >>> + * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
> >>
> >> This is just in comment but,
> >>
> >> I guess latest syntax is:
> >> -w "vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20"
> >>
> >> @Gaetan, is latest devarg syntax documented somewhere?
> >
> > That's the current syntax indeed. Some documentation is found at
> >
> > lib/librte_eal/common/include/rte_devargs.h:100
> >
> > Where it is specified that the bus name can be either omitted or
> > followed by any character, to separate it from the device identifier.
> >
> > This means that using ':' is fine, as well as '('. As long as the device
> > PMD afterward ignore the dangling ')' during devargs parsing, this should
> > be fine.
> >
> > I don't think this is very clean, but it works.
>
> Thanks for the info, I see how "(" works, but ")" is takes as part of argument
> and causing problem, I think better to not give "()" as supported syntax at all.
>
> btw, now both -w and --vdev are valid and can be used interchangeably, right? I
> mean all following are valid?
> -w pci:0000:86:06.0,enable_floating_veb=1
> -w vdev:net_pcap,iface=lo
> --vdev pci:0000:86:06.0,enable_floating_veb=1
> --vdev vdev:net_pcap,iface=lo
>
>
They are both valid but cannot be used interchangeably.
This was the case at one point, between two rcs, because I had removed
the devtype and the bus black/white-listing was configured another way.
A user complained about the API change because it was not announced in
time IIRC, and this was scrapped. Since then, the effort has been to on
the new syntax instead of cleaning the old system.
They cannot be used interchangeably because an rte_devtype is defined by
the parameter choosen. BLACKLISTED_PCI for -b, WHITELISTED_PCI for -w,
and VIRTUAL for --vdev.
-b will actually change the bus configuration to "blacklist mode", -w to
"whitelist mode", and --vdev will do nothing.
So a PCI device declared using --vdev, will actually be skipped during
PCI probe because by default it operates in blacklist mode, and the
device policy is not WHITELISTED. This could surprise the one attempting
this trick.
This semantic is subtle and bug-prone. I'm adding it to the pile of
reasons I'd like to remove the blacklist mode altogether.
On Fri, 14 Sep 2018 15:58:04 +0200
Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> On Fri, Sep 14, 2018 at 02:19:19PM +0100, Ferruh Yigit wrote:
> > On 9/14/2018 2:06 PM, Gaëtan Rivet wrote:
> > > Hi,
> > >
> > > On Fri, Sep 14, 2018 at 01:46:59PM +0100, Ferruh Yigit wrote:
> > >> On 8/30/2018 11:35 PM, Stephen Hemminger wrote:
> > >>> From: Stephen Hemminger <sthemmin@microsoft.com>
> > >>>
> > >>> Take device arguments from command line and put
> > >>> them in the device devargs.
> > >>>
> > >>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> > >>
> > >> <...>
> > >>
> > >>> @@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
> > >>> return ret;
> > >>> }
> > >>>
> > >>> +/*
> > >>> + * scan for matching device args on command line
> > >>> + * example:
> > >>> + * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
> > >>
> > >> This is just in comment but,
> > >>
> > >> I guess latest syntax is:
> > >> -w "vmbus:635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20"
> > >>
> > >> @Gaetan, is latest devarg syntax documented somewhere?
> > >
> > > That's the current syntax indeed. Some documentation is found at
> > >
> > > lib/librte_eal/common/include/rte_devargs.h:100
> > >
> > > Where it is specified that the bus name can be either omitted or
> > > followed by any character, to separate it from the device identifier.
> > >
> > > This means that using ':' is fine, as well as '('. As long as the device
> > > PMD afterward ignore the dangling ')' during devargs parsing, this should
> > > be fine.
> > >
> > > I don't think this is very clean, but it works.
> >
> > Thanks for the info, I see how "(" works, but ")" is takes as part of argument
> > and causing problem, I think better to not give "()" as supported syntax at all.
> >
> > btw, now both -w and --vdev are valid and can be used interchangeably, right? I
> > mean all following are valid?
> > -w pci:0000:86:06.0,enable_floating_veb=1
> > -w vdev:net_pcap,iface=lo
> > --vdev pci:0000:86:06.0,enable_floating_veb=1
> > --vdev vdev:net_pcap,iface=lo
> >
> >
>
> They are both valid but cannot be used interchangeably.
>
> This was the case at one point, between two rcs, because I had removed
> the devtype and the bus black/white-listing was configured another way.
>
> A user complained about the API change because it was not announced in
> time IIRC, and this was scrapped. Since then, the effort has been to on
> the new syntax instead of cleaning the old system.
>
> They cannot be used interchangeably because an rte_devtype is defined by
> the parameter choosen. BLACKLISTED_PCI for -b, WHITELISTED_PCI for -w,
> and VIRTUAL for --vdev.
>
> -b will actually change the bus configuration to "blacklist mode", -w to
> "whitelist mode", and --vdev will do nothing.
>
> So a PCI device declared using --vdev, will actually be skipped during
> PCI probe because by default it operates in blacklist mode, and the
> device policy is not WHITELISTED. This could surprise the one attempting
> this trick.
>
> This semantic is subtle and bug-prone. I'm adding it to the pile of
> reasons I'd like to remove the blacklist mode altogether.
>
It would have been nice if there was a --devargs instead of overloading -w and -b.
Especially when just changing parameter on a device.
@@ -276,6 +276,8 @@ vmbus_scan_one(const char *name)
dev->device.numa_node = SOCKET_ID_ANY;
}
+ dev->device.devargs = vmbus_devargs_lookup(dev);
+
/* device is valid, add in list (sorted) */
VMBUS_LOG(DEBUG, "Adding vmbus device %s", name);
@@ -66,6 +66,9 @@ struct vmbus_channel {
#define VMBUS_MAX_CHANNELS 64
+struct rte_devargs *
+vmbus_devargs_lookup(struct rte_vmbus_device *dev);
+
int vmbus_chan_create(const struct rte_vmbus_device *device,
uint16_t relid, uint16_t subid, uint8_t monitor_id,
struct vmbus_channel **new_chan);
@@ -85,7 +85,6 @@ vmbus_match(const struct rte_vmbus_driver *dr,
return false;
}
-
/*
* If device ID match, call the devinit() function of the driver.
*/
@@ -204,6 +203,27 @@ vmbus_parse(const char *name, void *addr)
return ret;
}
+/*
+ * scan for matching device args on command line
+ * example:
+ * -w 'vmbus(635a7ae3-091e-4410-ad59-667c4f8c04c3,latency=20)'
+ */
+struct rte_devargs *
+vmbus_devargs_lookup(struct rte_vmbus_device *dev)
+{
+ struct rte_devargs *devargs;
+ rte_uuid_t addr;
+
+ RTE_EAL_DEVARGS_FOREACH("vmbus", devargs) {
+ vmbus_parse(devargs->name, &addr);
+
+ if (rte_uuid_compare(dev->device_id, addr) == 0)
+ return devargs;
+ }
+ return NULL;
+
+}
+
/* register vmbus driver */
void
rte_vmbus_register(struct rte_vmbus_driver *driver)