2013-09-04 13:46:14

by Tom Gundersen

[permalink] [raw]
Subject: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

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.

Cc: Martin Mares <[email protected]>
---
lib/configure | 17 ++++++++++++++++
lib/names-hash.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/lib/configure b/lib/configure
index 27388bc..bb9a983 100755
--- a/lib/configure
+++ b/lib/configure
@@ -141,6 +141,23 @@ esac
echo >>$c '#define PCI_HAVE_PM_DUMP'
echo " dump"

+echo_n "Checking for udev hwdb support... "
+if [ "$HWDB" = yes -o "$HWDB" = no ] ; then
+ echo "$HWDB (set manually)"
+else
+ if [ -f /usr/include/libudev.h -o -f /usr/local/include/libudev.h ] ; then
+ HWDB=yes
+ else
+ HWDB=no
+ fi
+ echo "$HWDB (auto-detected)"
+fi
+if [ "$HWDB" = yes ] ; then
+ echo >>$c '#define PCI_HAVE_HWDB'
+ echo >>$m 'LIBUDEV=-ludev'
+ echo >>$m 'WITH_LIBS+=$(LIBUDEV)'
+fi
+
echo_n "Checking for zlib support... "
if [ "$ZLIB" = yes -o "$ZLIB" = no ] ; then
echo "$ZLIB (set manually)"
diff --git a/lib/names-hash.c b/lib/names-hash.c
index 33f3c11..9661d03 100644
--- a/lib/names-hash.c
+++ b/lib/names-hash.c
@@ -11,6 +11,11 @@
#include "internal.h"
#include "names.h"

+#ifdef PCI_HAVE_HWDB
+#include <libudev.h>
+#include <stdio.h>
+#endif
+
struct id_bucket {
struct id_bucket *next;
unsigned int full;
@@ -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))
+ {
+ char modalias[64];
+ const char *key = NULL;
+ struct udev *udev = udev_new();
+ struct udev_hwdb *hwdb = udev_hwdb_new(udev);
+ struct udev_list_entry *entry;
+
+ switch(cat)
+ {
+ case ID_VENDOR:
+ sprintf(modalias, "pci:v%08X*", id1);
+ key = "ID_VENDOR_FROM_DATABASE";
+ break;
+ case ID_DEVICE:
+ sprintf(modalias, "pci:v%08Xd%08X*", id1, id2);
+ key = "ID_MODEL_FROM_DATABASE";
+ break;
+ case ID_SUBSYSTEM:
+ sprintf(modalias, "pci:v%08Xd%08Xsv%08Xsd%08X*", id1, id2, id3, id4);
+ key = "ID_MODEL_FROM_DATABASE";
+ break;
+ case ID_GEN_SUBSYSTEM:
+ sprintf(modalias, "pci:v*d*sv%08Xsd%08X*", id1, id2);
+ key = "ID_MODEL_FROM_DATABASE";
+ break;
+ case ID_CLASS:
+ sprintf(modalias, "pci:v*d*sv*sd*bc%02X*", id1);
+ key = "ID_PCI_CLASS_FROM_DATABASE";
+ break;
+ case ID_SUBCLASS:
+ sprintf(modalias, "pci:v*d*sv*sd*bc%02Xsc%02X*", id1, id2);
+ key = "ID_PCI_SUBCLASS_FROM_DATABASE";
+ break;
+ case ID_PROGIF:
+ sprintf(modalias, "pci:v*d*sv*sd*bc%02Xsc%02Xi%02X*", id1, id2, id3);
+ key = "ID_PCI_INTERFACE_FROM_DATABASE";
+ break;
+ }
+
+ if (key)
+ udev_list_entry_foreach(entry, udev_hwdb_get_properties_list_entry(hwdb, modalias, 0))
+ if (strcmp(udev_list_entry_get_name(entry), key) == 0)
+ return udev_list_entry_get_value(entry);
+ }
+#endif
+
+ id12 = id_pair(id1, id2);
+ id34 = id_pair(id3, id4);

if (a->id_hash)
{
--
1.8.4


2013-09-04 14:07:17

by Martin Mares

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

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).

> 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.

There is a couple of details I am concerned with:

> + 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?

> @@ -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.

Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Quidquid latine dictum sit, altum videtur.

2013-09-04 14:59:47

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

On Wed, Sep 4, 2013 at 3:57 PM, Martin Mares <[email protected]> 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

2013-09-13 11:08:34

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

On Wed, Sep 4, 2013 at 4:59 PM, Tom Gundersen <[email protected]> wrote:
> On Wed, Sep 4, 2013 at 3:57 PM, Martin Mares <[email protected]> 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.

Hi Martin,

Any comments on the above before I resubmit?

Cheers,

Tom

2013-10-25 09:02:42

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

On Fri, Sep 13, 2013 at 1:08 PM, Tom Gundersen <[email protected]> wrote:
> On Wed, Sep 4, 2013 at 4:59 PM, Tom Gundersen <[email protected]> wrote:
>> On Wed, Sep 4, 2013 at 3:57 PM, Martin Mares <[email protected]> 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.
>
> Hi Martin,
>
> Any comments on the above before I resubmit?

Ping?

-t

2014-02-10 10:26:25

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

Ping?

On Wed, Jan 29, 2014 at 5:13 PM, Tom Gundersen <[email protected]> wrote:
> Ping?
>
> On 30 Dec 2013 19:53, "Tom Gundersen" <[email protected]> wrote:
>>
>> Hi Martin,
>>
>> On Sun, Dec 15, 2013 at 12:33 PM, Martin Mares <[email protected]> wrote:
>> > I see that a mechanism for fast lookup of hardware identification data
>> > is needed. However, why should such a mechanism depend on udev, systemd,
>> > or Linux in general?
>> >
>> > What I would really like to have is a universal library for HW lookup,
>> > independent of anything else and widely portable. All the hardware data
>> > would be provided by other packages -- pci.ids, usb.ids, kernel modules,
>> > etc. -- and compiled to a binary format available for instant queries.
>>
>> I'd like to reach an agreement on this patch, so I'll resend it with
>> the changes we agreed on so far. Please let me know if further changes
>> are needed.
>>
>> If I understand correctly, you are ok with the functionality hwdb
>> provides, but would have preferred an OS-agnostic solution. However,
>> as no such solution currently exist, and the patch adding hwdb support
>> is pretty minimal and self-contained, would you be happy taking that
>> for now, and rather revisit the topic if and when a generic solution
>> appears?
>>
>> FWIW, I made the analogous change to lsusb some time ago [0], so would
>> be cool to get this all done.
>>
>> Cheers,
>>
>> Tom
>>
>> [0]:
>> <https://github.com/gregkh/usbutils/commit/c6282a7a0e20fbb9c56837f055843635a43ba8b4>

2014-02-12 20:05:53

by Martin Mares

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

Hi!

> Ping?

Sorry for the delay, I was alternatingly ill and overloaded with other stuff...

As I said before, I do not like the current implementation of hwdb much (it's
too much tied to Linux, and hardware description strings, originally intended
for internal use between the kernel and its modules, are far from a nice API).

I would prefer a more general HW identification library and I am willing to
write it, but I have to admit that with my current load, it will be hardly
finished before this summer.

I will merge your patches. It's better to have the current hwdb than nothing.

Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
First law of socio-genetics: Celibacy is not hereditary.

2014-02-12 20:05:50

by Tom Gundersen

[permalink] [raw]
Subject: Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support

On Wed, Feb 12, 2014 at 8:58 PM, Martin Mares <[email protected]> wrote:
> Hi!
>
>> Ping?
>
> Sorry for the delay, I was alternatingly ill and overloaded with other stuff...

No worries. I hope you are better now.

> As I said before, I do not like the current implementation of hwdb much (it's
> too much tied to Linux, and hardware description strings, originally intended
> for internal use between the kernel and its modules, are far from a nice API).
>
> I would prefer a more general HW identification library and I am willing to
> write it, but I have to admit that with my current load, it will be hardly
> finished before this summer.

Looking forward to seeing that.

> I will merge your patches. It's better to have the current hwdb than nothing.

Thanks!

Cheers,

Tom

2014-11-01 17:57:15

by Martin Mares

[permalink] [raw]
Subject: udev/hwdb support in pciutils (Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support)

Hi Tom!

The support for hwdb has finally landed in mainline pciutils.

I have modified your patch to bring it closer to the rest of device
naming logic. Namely:

• The interface to HWDB now lives in a separate source file
lib/names-hwdb.c. It is called from the main entry point
to the name resolver (lib/names.c).

• Resolved names are cached in the hash table and they participate
in the system of priorities. They have lower priority than pci.ids,
but higher than remote lookups via DNS. (Therefore if a distro
decides not to ship pci.ids, hwdb is used exclusively for all
local lookups.)

• There is a flag to suppress the use of HWDB (PCI_LOOKUP_NO_HWDB)
and also a configuration parameter for the same (hwdb.disable).

Aside of the README, everything should be ready for a public release.
Could you please verify that it still works for you?

Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Uncle Ed's Rule of Thumb: Never use your thumb for a rule.

2014-11-03 13:54:33

by Tom Gundersen

[permalink] [raw]
Subject: Re: udev/hwdb support in pciutils (Re: [PATCH RESEND][pciutils] libpci: pci_id_lookup - add udev/hwdb support)

Hi Martin,

On Sat, Nov 1, 2014 at 6:51 PM, Martin Mares <[email protected]> wrote:
> The support for hwdb has finally landed in mainline pciutils.
>
> I have modified your patch to bring it closer to the rest of device
> naming logic. Namely:

Great! Thanks for merging this, and for working on it.

> Aside of the README, everything should be ready for a public release.
> Could you please verify that it still works for you?

I can verify that it still works as expected, so it is good to go as
far as I'm concerned.

Cheers,

Tom