Hello Aaron,
On Tue, Oct 17, 2017 at 02:16:59PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.ri...@6wind.com> writes:
>
> > The devtype is now entirely defined by the device bus. As such, it is
> > already characterized by the bus identifier within an rte_devargs.
> >
> > The rte_devtype enum can disappear, along with crutches added during
> > this transition.
> >
> > rte_eal_devargs_type_count becomes useless and is removed.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.ri...@6wind.com>
> > ---
< ... >
> > @@ -380,11 +371,8 @@ rte_pci_probe(void)
> > probed++;
> >
> > devargs = dev->device.devargs;
> > - /* probe all or only whitelisted devices */
> > - if (probe_all)
> > - ret = pci_probe_all_drivers(dev);
> > - else if (devargs != NULL &&
> > - devargs->policy == RTE_DEV_WHITELISTED)
> > + /* probe all or only declared devices */
> > + if (probe_all ^ (devargs != NULL))
>
> What is the intent of this? If probe_all is true, and devargs != null,
> I think this branch isn't taken.
>
> Shouldn't this be ||? Maybe I missed something?
>
Here are the possible choices:
probe_all \ devargs !NULL NULL
--------------------------+----------------------+----------------------------+
true |blacklist and the |blacklist mode and no |
|devargs describes a |devargs, meaning the device |
|blacklisted device |is not blacklisted |
|--> do not probe |--> do probe |
--------------------------+----------------------+----------------------------+
false |whitelist mode and |whitelist mode and no |
|whitelisted device |devargs, device has been |
|using a devargs |scanned but not whitelisted |
|--> do probe |--> do not probe |
--------------------------+----------------------+----------------------------+
A XOR is thus the logical expression of this decision.
What I could be doubtful about here is that using a xor anywhere can be
confusing for some people and it could all be expressed in two simpler
ifs.
Also, probe_all could be renamed as "blacklist_mode" for example to be
more descriptive.
But unless I'm mistaken, the logic should be correct.
> > ret = pci_probe_all_drivers(dev);
> > if (ret < 0) {
> > RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
< ... >
--
Gaëtan Rivet
6WIND
