linux-kernel - Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Sep 2013 16:59:24 +0200
From:	Tom Gundersen <teg@...m.no>
To:	Martin Mares <mj@....cz>
Cc:	linux-pci@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

On Wed, Sep 4, 2013 at 3:57 PM, Martin Mares <mj@....cz> wrote:
> Hello!
>
> First of all: Sorry for not replying to the first mail. I do not follow
> linux-pci too much these days (or, I do that in big batches).

No problem, I guessed as much.

>> This lets you select hwdb support at compile time.
>>
>> hwdb is an efficient hardware database shipped with recent versions of udev.
>> It contains among other sources pci.ids so querying hwdb rather than reading
>> pci.ids directly should give the same result.
>>
>> Ideally Linux distros using udev could stop shipping pci.ids, but use hwdb
>> as the only source of this information, which this patch allows.
>
> Generally, I will be glad to include hwdb support in libpci.

Great.

>> +     if [ -f /usr/include/libudev.h -o -f /usr/local/include/libudev.h ] ; then
>> +             HWDB=yes
>> +     else
>> +             HWDB=no
>> +     fi
>
> Does this make sense? Does every version of libudev support hwdb?

Good point. I'll replace it with a pkg-config call, is that acceptable?

>> @@ -86,8 +91,58 @@ char
>>  *pci_id_lookup(struct pci_access *a, int flags, int cat, int id1, int id2, int id3, int id4)
>>  {
>>    struct id_entry *n, *best;
>> -  u32 id12 = id_pair(id1, id2);
>> -  u32 id34 = id_pair(id3, id4);
>> +  u32 id12, id34;
>> +
>> +#ifdef PCI_HAVE_HWDB
>> +  if (!(flags & PCI_LOOKUP_SKIP_LOCAL))
>> +    {
>
> As you wrote it, hwdb has always priority over pci.ids (unless local lookup is
> disabled). As a user, I would expect that pci.ids (being a part of the pciutils)
> is the primary source of data and other sources (network lookups, hwdb) are
> used only if pci.ids do not match or if explicitly requested.

Hm, this was actually intentional. The reason being that I'd like to
avoid reading in the pci.ids db in the common case, as using the hwdb
should be much more efficient (it is most likely already in memory and
lookup is constant time), and also we (at the distro level) want to
move away from the {usb,pci}.ids and rather default to hwdb
everywhere.

My original intention was to make hwdb a replacement for pci.ids, but
I ended up going the less invasive route, would making it a
replacement be more acceptable?

If not, I'll just swap around the priority, not a problem.

Cheers,

Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/