[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

David/Thomas,  in-line  -Roger


On 11/18/15 5:13 PM, Roger B. Melton wrote:
> Hi Thomas, in-line  -Roger
>
> On 11/17/15 10:46 AM, Thomas Monjalon wrote:
>> 2015-11-17 08:56, Roger B. Melton:
>>> Hi David,  in-line -Roger
>>>
>>> On 11/16/15 4:46 AM, David Marchand wrote:
 Hello Roger,

 On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton >>> > wrote:

  I like the "-b all" and "-w none" idea, but I think it might be
  complicated to implement it the way we would need it to work.  
 The
  existing -b and -w options  persist for the duration of the
  application, and we would need the "-b all"/"-w none" to persists
  only through rte_eal_init() time.  Otherwise our attempt to to
  attach a device at a later time would be blocked by the option.

 I agree, the black/white lists should only apply to initial scan.
 I forgot about this problem ...
 I had started some cleanup in the pci scan / attach code but this is
 too late for 2.2, I will post this in the next merge window.


  Wouldn't it be simpler to have an option to disable the
  rte_eal_init() time the probe.  Would that address the issue with
  VFIO, prevent automatically attaching to devices while permitting
  on demand attach?


 I suppose we can do this yes (I think Thomas once proposed off-list an
 option like --no-pci-scan).
 Do you think you can send a patch ?
>>> What about --no-pci-init-probe?  I know it's long, but it is more
>>> descriptive of it's purpose to disable only the init time pci probe.
>> Why not a "-b all"?
>> Making it work would also solve the case where you to scan only part of
>> the devices and initialize the blacklisted ones later.
>> .
>>
> Do you envision "-b all" setting a flag that would be used to block 
> rte_eal_init() invocation of rte_eal_pci_probe()?  e.g. If we have a 
> new API, *rte_eal_pci_blacklist_all_get()* that returns a non-zero 
> value if "-b all" was specified, then in rte_eal_init() we would have 
> something like:
>
> ...
> /* Probe & Initialize PCI devices */
> *if (!rte_eal_pci_blacklist_all_get())**  <--- New check*
> if (rte_eal_pci_probe())
> rte_panic("Cannot probe PCI\n");
> ...
>
>
> Or setting a flag that would be checked in rte_eal_probe_one() similar 
> to the existing per device blacklist check?  e.g. Again with a new 
> API, *rte_eal_pci_blacklist_all_get()* that returns a non-zero value 
> if "-b all" was specified, then in rte_eal_pci_probe_one_driver() we 
> would have something like:
>
> /* no initialization when blacklisted, return without error */
> if (*rte_eal_pci_blacklist_all_get() ||  <--- New check*
> (dev->devargs != NULL &&
>  dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)) {
> RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not 
> initializing\n");
> return 1;
> }
>
> The former would work, but I think it would be confusing for "-b all" 
> to only apply to init.
>
> The latter would be consistent with how "-b " works, but in 
> order to initialize devices at a later time, we would need a way to 
> clear the blacklist all state at run time so that 
> *rte_eal_pci_blacklist_all()* would return false. For example, 
> something like *rte_eal_pci_blacklist_all_clear()*.
>
> Or do you have something else in mind entirely?
>
> .
>

How about something like this:

  * Add pci_blacklist_all member to eal internal config.
  * When non-zero, pci_blacklist_all will prevent PCI probing. Result is
that probes will find no match.
  * pci_blacklist_all is temporal
  o Can be set at rte_eal_init() time with "-b all" switch.
  o Can be set or cleared at run time with
rte_eal_pci_blacklist_all_set() or
rte_eal_pci_blacklist_all_clear() respectively.
  o State can be queried with rte_eal_pci_blacklist_all_get()
  * Blacklist and whitelist switches with PCI DBDF are still permitted
as before, but they are used only when the blacklist all override is
cleared.

I've tested the implementation from the diff below in our application 
and it works well.  Note our application falls under linuxapp.  I 
haven't tested bsdapp yet.

If it looks good to you, I will update test application and update if 
necessary with logic to test the capability, and complete testing of the 
bsdapp.

Thanks,  -Roger

On branch DPDK2.2Integration
Changes not staged for commit:
   (use "git add ..." to update what will be committed)
   (use "git checkout -- ..." to discard changes in working directory)

modified:   lib/librte_eal/bsdapp/eal/eal.c
modified:   lib/librte_eal/common/eal_common_options.c
modified:   lib/librte_eal/common/eal_common_pci.c
modified:   lib/librte_eal/common/include/rte_eal.h
modified:   lib/librte_eal/linuxapp/eal/eal.c

no changes added to 

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

Hi Thomas, in-line  -Roger

On 11/17/15 10:46 AM, Thomas Monjalon wrote:
> 2015-11-17 08:56, Roger B. Melton:
>> Hi David,  in-line -Roger
>>
>> On 11/16/15 4:46 AM, David Marchand wrote:
>>> Hello Roger,
>>>
>>> On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton >> > wrote:
>>>
>>>  I like the "-b all" and "-w none" idea, but I think it might be
>>>  complicated to implement it the way we would need it to work.  The
>>>  existing -b and -w options  persist for the duration of the
>>>  application, and we would need the "-b all"/"-w none" to persists
>>>  only through rte_eal_init() time.  Otherwise our attempt to to
>>>  attach a device at a later time would be blocked by the option.
>>>
>>> I agree, the black/white lists should only apply to initial scan.
>>> I forgot about this problem ...
>>> I had started some cleanup in the pci scan / attach code but this is
>>> too late for 2.2, I will post this in the next merge window.
>>>
>>>
>>>  Wouldn't it be simpler to have an option to disable the
>>>  rte_eal_init() time the probe.  Would that address the issue with
>>>  VFIO, prevent automatically attaching to devices while permitting
>>>  on demand attach?
>>>
>>>
>>> I suppose we can do this yes (I think Thomas once proposed off-list an
>>> option like --no-pci-scan).
>>> Do you think you can send a patch ?
>> What about --no-pci-init-probe?  I know it's long, but it is more
>> descriptive of it's purpose to disable only the init time pci probe.
> Why not a "-b all"?
> Making it work would also solve the case where you to scan only part of
> the devices and initialize the blacklisted ones later.
> .
>
Do you envision "-b all" setting a flag that would be used to block 
rte_eal_init() invocation of rte_eal_pci_probe()?  e.g. If we have a new 
API, *rte_eal_pci_blacklist_all_get()* that returns a non-zero value if 
"-b all" was specified, then in rte_eal_init() we would have something like:

...
 /* Probe & Initialize PCI devices */
*if (!rte_eal_pci_blacklist_all_get())**  <--- New check*
 if (rte_eal_pci_probe())
 rte_panic("Cannot probe PCI\n");
...


Or setting a flag that would be checked in rte_eal_probe_one() similar 
to the existing per device blacklist check?  e.g. Again with a new API, 
*rte_eal_pci_blacklist_all_get()* that returns a non-zero value if "-b 
all" was specified, then in rte_eal_pci_probe_one_driver() we would have 
something like:

 /* no initialization when blacklisted, return without error */
 if (*rte_eal_pci_blacklist_all_get() ||  <--- New check*
 (dev->devargs != NULL &&
  dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)) {
 RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not 
initializing\n");
 return 1;
 }

The former would work, but I think it would be confusing for "-b all" to 
only apply to init.

The latter would be consistent with how "-b " works, but in 
order to initialize devices at a later time, we would need a way to 
clear the blacklist all state at run time so that 
*rte_eal_pci_blacklist_all()* would return false. For example, something 
like *rte_eal_pci_blacklist_all_clear()*.

Or do you have something else in mind entirely?

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Thomas Monjalon]

2015-11-17 08:56, Roger B. Melton:
> Hi David,  in-line -Roger
> 
> On 11/16/15 4:46 AM, David Marchand wrote:
> > Hello Roger,
> >
> > On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton  > > wrote:
> >
> > I like the "-b all" and "-w none" idea, but I think it might be
> > complicated to implement it the way we would need it to work.  The
> > existing -b and -w options  persist for the duration of the
> > application, and we would need the "-b all"/"-w none" to persists
> > only through rte_eal_init() time.  Otherwise our attempt to to
> > attach a device at a later time would be blocked by the option.
> >
> > I agree, the black/white lists should only apply to initial scan.
> > I forgot about this problem ...
> > I had started some cleanup in the pci scan / attach code but this is 
> > too late for 2.2, I will post this in the next merge window.
> >
> >
> > Wouldn't it be simpler to have an option to disable the
> > rte_eal_init() time the probe.  Would that address the issue with
> > VFIO, prevent automatically attaching to devices while permitting
> > on demand attach?
> >
> >
> > I suppose we can do this yes (I think Thomas once proposed off-list an 
> > option like --no-pci-scan).
> > Do you think you can send a patch ?
> 
> What about --no-pci-init-probe?  I know it's long, but it is more 
> descriptive of it's purpose to disable only the init time pci probe.

Why not a "-b all"?
Making it work would also solve the case where you to scan only part of
the devices and initialize the blacklisted ones later.

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

Hi David,  in-line -Roger

On 11/16/15 4:46 AM, David Marchand wrote:
> Hello Roger,
>
> On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton  > wrote:
>
> I like the "-b all" and "-w none" idea, but I think it might be
> complicated to implement it the way we would need it to work.  The
> existing -b and -w options  persist for the duration of the
> application, and we would need the "-b all"/"-w none" to persists
> only through rte_eal_init() time.  Otherwise our attempt to to
> attach a device at a later time would be blocked by the option.
>
>
> I agree, the black/white lists should only apply to initial scan.
> I forgot about this problem ...
> I had started some cleanup in the pci scan / attach code but this is 
> too late for 2.2, I will post this in the next merge window.
>
>
> Wouldn't it be simpler to have an option to disable the
> rte_eal_init() time the probe.  Would that address the issue with
> VFIO, prevent automatically attaching to devices while permitting
> on demand attach?
>
>
> I suppose we can do this yes (I think Thomas once proposed off-list an 
> option like --no-pci-scan).
> Do you think you can send a patch ?

What about --no-pci-init-probe?  I know it's long, but it is more 
descriptive of it's purpose to disable only the init time pci probe.

I code and test and have it ready.  I'm still working through internal 
processes to allow me to submit patches, but I hope to have that 
resolved in the next few weeks and at that time I can submit the patch.
>
>
> -- 
> David Marchand
>

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[David Marchand]

Hello Roger,

On Sun, Nov 15, 2015 at 3:45 PM, Roger B. Melton  wrote:

> I like the "-b all" and "-w none" idea, but I think it might be
> complicated to implement it the way we would need it to work.  The existing
> -b and -w options  persist for the duration of the application, and we
> would need the "-b all"/"-w none" to persists only through rte_eal_init()
> time.  Otherwise our attempt to to attach a device at a later time would be
> blocked by the option.
>

I agree, the black/white lists should only apply to initial scan.
I forgot about this problem ...
I had started some cleanup in the pci scan / attach code but this is too
late for 2.2, I will post this in the next merge window.


Wouldn't it be simpler to have an option to disable the rte_eal_init() time
> the probe.  Would that address the issue with VFIO, prevent automatically
> attaching to devices while permitting on demand attach?
>

I suppose we can do this yes (I think Thomas once proposed off-list an
option like --no-pci-scan).
Do you think you can send a patch ?


-- 
David Marchand

>

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

Hi David,

We are on a very old kernel (2.6.xx) that lacks VFIO.  In the future 
however, after migration to a newer kernel it will be an option.

I like the "-b all" and "-w none" idea, but I think it might be 
complicated to implement it the way we would need it to work.  The 
existing -b and -w options  persist for the duration of the application, 
and we would need the "-b all"/"-w none" to persists only through 
rte_eal_init() time.  Otherwise our attempt to to attach a device at a 
later time would be blocked by the option.

Wouldn't it be simpler to have an option to disable the rte_eal_init() 
time the probe.  Would that address the issue with VFIO, prevent 
automatically attaching to devices while permitting on demand attach?

Thanks again.

Regards,
-Roger



On 11/14/15 12:51 PM, David Marchand wrote:
> Hello Roger,
>
> On Sat, Nov 14, 2015 at 4:55 PM, Roger B. Melton  > wrote:
>
> Agreed.  For our application, the debug case would be to _enable_
> the PCI scan.
>
> Again, thanks David for pointing it out.  It did solve our problem.
>
>
> The only problem with --no-pci is that I think that vfio won't work 
> properly if used.
>
> Did you try to blacklist all your devices then attach them later ?
> I would say what you need here is to "blacklist all" or "whitelist 
> none" at startup, so maybe a special keyword for -b/-w options.
>
>
> -- 
> David Marchand

-- 
  
|Roger B. Melton|  |  Cisco Systems  |
|CPP Software  :|::|: 7100 Kit Creek Rd  |
|+1.919.476.2332 phone:|||:  :|||:RTP, NC 27709-4987 |
|+1.919.392.1094 fax   .:|||:..:|||:. rmelton at cisco.com  |
||
| This email may contain confidential and privileged material for the|
| sole use of the intended recipient. Any review, use, distribution  |
| or disclosure by others is strictly prohibited. If you are not the |
| intended recipient (or authorized to receive for the recipient),   |
| please contact the sender by reply email and delete all copies of  |
| this message.  |
||
| For corporate legal information go to: |
| http://www.cisco.com/web/about/doing_business/legal/cri/index.html |
|__ http://www.cisco.com |

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[David Marchand]

Hello Roger,

On Sat, Nov 14, 2015 at 4:55 PM, Roger B. Melton  wrote:

> Agreed.  For our application, the debug case would be to _enable_ the PCI
> scan.
>
> Again, thanks David for pointing it out.  It did solve our problem.


The only problem with --no-pci is that I think that vfio won't work
properly if used.

Did you try to blacklist all your devices then attach them later ?
I would say what you need here is to "blacklist all" or "whitelist none" at
startup, so maybe a special keyword for -b/-w options.


-- 
David Marchand

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

Agreed.  For our application, the debug case would be to _enable_ the 
PCI scan.

Again, thanks David for pointing it out.  It did solve our problem.

Regards,
Roger


On 11/13/15 5:58 PM, Don Provan wrote:
> --no-pci is cool. I'm pretty sure that wasn't there when the PCI scan was 
> first added to
> the library init routine. I'm glad to see it, so thanks for pointing it out.
>
> Just for the record: The comment says, "for debug purposes, PCI can be 
> disabled".
> This exhibits one of those classic DPDK blindspots. The library can be used 
> for many
> things entirely unrelated to hardware. My project has several DPDK 
> applications
> intended to be used by users that do not have privs to mess around with PCI
> hardware, so, for them, turning off PCI wouldn't be just for debugging 
> purposes.
>
> -don provan
> dprovan at bivio.net
>
> -Original Message-
> From: David Marchand [mailto:david.marchand at 6wind.com]
> Sent: Friday, November 13, 2015 12:50 AM
> To: Roger B Melton 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?
>
> ...
> Did you try the --no-pci option ?
> It avoids the initial sysfs scan, so with no pci device, the initial 
> pci_probe should do nothing.
> ...
>

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Jan Viktorin]

On Fri, 13 Nov 2015 22:58:04 +
Don Provan  wrote:

> --no-pci is cool. I'm pretty sure that wasn't there when the PCI scan was 
> first added to
> the library init routine. I'm glad to see it, so thanks for pointing it out.
> 
> Just for the record: The comment says, "for debug purposes, PCI can be 
> disabled".
> This exhibits one of those classic DPDK blindspots. The library can be used 
> for many
> things entirely unrelated to hardware. My project has several DPDK 
> applications
> intended to be used by users that do not have privs to mess around with PCI
> hardware, so, for them, turning off PCI wouldn't be just for debugging 
> purposes.

And ARMv7 depends on this option a bit... just for the record ;).

Jan

> 
> -don provan
> dprovan at bivio.net
> 
> -Original Message-
> From: David Marchand [mailto:david.marchand at 6wind.com] 
> Sent: Friday, November 13, 2015 12:50 AM
> To: Roger B Melton 
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?
> 
> ...
> Did you try the --no-pci option ?
> It avoids the initial sysfs scan, so with no pci device, the initial 
> pci_probe should do nothing.
> ...
> 

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Don Provan]

--no-pci is cool. I'm pretty sure that wasn't there when the PCI scan was first 
added to
the library init routine. I'm glad to see it, so thanks for pointing it out.

Just for the record: The comment says, "for debug purposes, PCI can be 
disabled".
This exhibits one of those classic DPDK blindspots. The library can be used for 
many
things entirely unrelated to hardware. My project has several DPDK applications
intended to be used by users that do not have privs to mess around with PCI
hardware, so, for them, turning off PCI wouldn't be just for debugging purposes.

-don provan
dprovan at bivio.net

-Original Message-
From: David Marchand [mailto:david.march...@6wind.com] 
Sent: Friday, November 13, 2015 12:50 AM
To: Roger B Melton 
Cc: dev at dpdk.org
Subject: Re: [dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

...
Did you try the --no-pci option ?
It avoids the initial sysfs scan, so with no pci device, the initial pci_probe 
should do nothing.
...

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[David Marchand]

Hello Roger,

On Thu, Nov 12, 2015 at 11:43 PM, Roger B Melton  wrote:

> Hi folks,
>
> With the addition of hot plug support we have been migrating away from
> device discovery and attach at initialization time to a model where it is
> controlled from a separate process.  The separate process manages the
> binding of devices to UIO and instructs the DPDK process when to attach.
> One of the problems we stumbled onto was that if our control process
> discovered devices and bound them to UIO before our DPDK process started,
> then rte_eal_init() would discover and attach to those devices via the
> rte_eal_pci_probe() invocation. This caused problems later on when when our
> control process, instructed our DPDK process to attach to a device.
>
> There are a number of ways we could address this, but the simplest is to
> prevent the rte_eal_pci_probe() at rte_eal_init() time.  In our model we
> will never need it and I suspect others may also be in that boat.
>
> What are your thoughts on adding an argument to instruct rte_eal_init() to
> skip the PCI probe?
>

Did you try the --no-pci option ?
It avoids the initial sysfs scan, so with no pci device, the initial
pci_probe should do nothing.

Attaching devices later will trigger this sysfs scan and only probe the
requested device.
I am not totally happy with the way it is done right now, but I think this
should work for you.


-- 
David Marchand

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B. Melton]

Hi David,


On 11/13/15 3:49 AM, David Marchand wrote:
> Hello Roger,
>
> On Thu, Nov 12, 2015 at 11:43 PM, Roger B Melton  > wrote:
>
> Hi folks,
>
> With the addition of hot plug support we have been migrating away
> from device discovery and attach at initialization time to a model
> where it is controlled from a separate process.  The separate
> process manages the binding of devices to UIO and instructs the
> DPDK process when to attach.  One of the problems we stumbled onto
> was that if our control process discovered devices and bound them
> to UIO before our DPDK process started, then rte_eal_init() would
> discover and attach to those devices via the rte_eal_pci_probe()
> invocation. This caused problems later on when when our control
> process, instructed our DPDK process to attach to a device.
>
> There are a number of ways we could address this, but the simplest
> is to prevent the rte_eal_pci_probe() at rte_eal_init() time.  In
> our model we will never need it and I suspect others may also be
> in that boat.
>
> What are your thoughts on adding an argument to instruct
> rte_eal_init() to skip the PCI probe?
>
>
> Did you try the --no-pci option ?
> It avoids the initial sysfs scan, so with no pci device, the initial 
> pci_probe should do nothing.
>
> Attaching devices later will trigger this sysfs scan and only probe 
> the requested device.
> I am not totally happy with the way it is done right now, but I think 
> this should work for you.

I saw it, but I was so caught up in the probe that I didn't consider 
that delaying the scan until attach time might solve the problem.

I'll give it shot.  Thanks for pointing it out David.

Regards,
-Roger

>
>
> -- 
> David Marchand

-- 
  
|Roger B. Melton|  |  Cisco Systems  |
|CPP Software  :|::|: 7100 Kit Creek Rd  |
|+1.919.476.2332 phone:|||:  :|||:RTP, NC 27709-4987 |
|+1.919.392.1094 fax   .:|||:..:|||:. rmelton at cisco.com  |
||
| This email may contain confidential and privileged material for the|
| sole use of the intended recipient. Any review, use, distribution  |
| or disclosure by others is strictly prohibited. If you are not the |
| intended recipient (or authorized to receive for the recipient),   |
| please contact the sender by reply email and delete all copies of  |
| this message.  |
||
| For corporate legal information go to: |
| http://www.cisco.com/web/about/doing_business/legal/cri/index.html |
|__ http://www.cisco.com |

[dpdk-dev] Making rte_eal_pci_probe() in rte_eal_init() optional?

[Roger B Melton]

Hi folks,

With the addition of hot plug support we have been migrating away from 
device discovery and attach at initialization time to a model where it 
is controlled from a separate process.  The separate process manages the 
binding of devices to UIO and instructs the DPDK process when to 
attach.  One of the problems we stumbled onto was that if our control 
process discovered devices and bound them to UIO before our DPDK process 
started, then rte_eal_init() would discover and attach to those devices 
via the rte_eal_pci_probe() invocation. This caused problems later on 
when when our control process, instructed our DPDK process to attach to 
a device.

There are a number of ways we could address this, but the simplest is to 
prevent the rte_eal_pci_probe() at rte_eal_init() time.  In our model we 
will never need it and I suspect others may also be in that boat.

What are your thoughts on adding an argument to instruct rte_eal_init() 
to skip the PCI probe?

Thanks,
-Roger