tools/nut-usbinfo.pl: Use hwdb for UPower rules

[benzea]

A udev hwdb is faster to process, so switch to use that. Doing so means that the attributes are set further up in the chain, so we still need a rule to copy them over.

Alternatively, we could consume it differently in UPower (i.e. fetch the "usb" subsystem parent first), either works, but this was a bit more straight forward and it is hard to test otherwise.

[jimklimov]

It may also help to leave a persistent link to update the file "in-place":

Suggested change

	print $outputUPower "# or checkout the NUT repository and call 'tools/nut-usbinfo.pl'\n\n";
	print $outputUPower "# fetch https://raw.githubusercontent.com/networkupstools/nut/master/scripts/upower/95-upower-hid.hwdb\n";
	print $outputUPower "# or checkout the NUT repository and call 'tools/nut-usbinfo.pl'\n\n";

...or maybe not, since support of devices depends on the binary NUT code deployed on the system, and this list seems generated from C sources (not data/drivers.list like in some other cases).

[benzea]

We could also move the generation into the UPower repository. That is, if grabbing data/drivers.list from the NUT repository is enough to generate it there.

I suppose the reason for it being this way is that the data is parsed from the C source code in the first place.

[jimklimov]

Looking at the files more, I think the drivers.list is not suitable here, it is more about documenting (often post-factum) that certain drivers understand and manage certain devices - how-ever those get branded on the market, etc.

In particular, it is not about only USB, and does not convey VID/PID info systemically (for some devices these IDs are listed among comments or names, but not exhaustively nor in a parsable manner).

[benzea]

... support of devices depends on the binary NUT code deployed on the system ...

Not sure what you mean here. NUT does not need to be installed for UPower to show warnings and trigger an emergency shutdown eventually.

[jimklimov]

Guess I gotta read up more on what UPower does and how... I suppose it can deal with standard USB HID devices for at least basic commands/readings/alerts served on standard paths; however the USB_DEVICE macro also identifies the other USB connected UPSes, like those with effectively (protocol-wise) a built-in serial-to-USB converter and usually one of Megatec Qx protocol variations.

[benzea]

It implements standard USB HID for reading basic status information. As I understand it, the code in the perl script is filtering the information to only include USB HID devices.

[jimklimov]

Oh, right, found it at last:

                        # UPower device entry (only for USB/HID devices!)
                        if ($vendor{$vendorId}{$productId}{"driver"} eq "usbhid-ups") ...

Then, I guess, the suggestion to fetch the file is not a useless one :)

[jimklimov]

Generally looks useful, comments posted.

Is there any known limitation such as "hwdb starts with OS releases as recent as..." so we'd need the older style file generated for older releases that are still of practical interest (boxes used as minimally changed appliances)?

On the other hand, any rough estimate how much faster is the hwdb compared to text rules (like "1 sec vs 5 sec to parse")?

[benzea]

Generally looks useful, comments posted.

Is there any known limitation such as "hwdb starts with OS releases as recent as..." so we'd need the older style file generated for older releases that are still of practical interest (boxes used as minimally changed appliances)?

Looks like the hwdb support in udev must was added end of 2012 (systemd version 196). I suspect that is long enough ago that we can rely on it.

On the other hand, any rough estimate how much faster is the hwdb compared to text rules (like "1 sec vs 5 sec to parse")?

I don't have any estimate, it mostly seems like the "right" thing to do here. What we do is increase the size of the hwdb a bit for USB devices, but remove a lot of rules that need to be executed for hiddev devices.

hwdb is much faster in principle. For the rules, all udev rules need to be processed sequentially. The hwdb in contrast allows an optimised lookup to find entries that match the MODALIAS string.

[jimklimov]

@benzea : Note for possible future fixes and force-pushes - I added a couple of commits above ;)

[jimklimov]

At the moment I don't have access to USB connected HID UPSes on any Linux, so gotta trust trials and judgement from community members who do: has this change been tested in the field? Does a recent distro with systemd work well with the proposed HWDB file (looking at https://repology.org/project/systemd/versions - any distro with systemd has its version new enough)?

CC @bigon

[benzea]

I don't have any hardware either. I just checked the rules were working by testing them against another USB HID device that I have (a yubikey in this case).

EDIT: Oh, didn't realise you merged it!

[jimklimov]

@benzea : Cheers, got some second thoughts which I hope you can clarify here :)

One question is posted about the code separately. Another is more generic: the original issue discussion wording, and the https://www.freedesktop.org/software/systemd/man/hwdb.html man page, seem to imply that "hwdb" is an "udev" implementation detail, instead of udev.rules...

Why did this PR end up changing UPower configuration, which seems similar and is also from FreeDesktop project (as a sort-of-descendant of HAL and DeviceKit-power), but is not udev, right?.. I am not convinced now that this is the file we want edited about systemd udev/hwdb implementation details. Is it even compatible?

Alternately: are the same HWDB files reusable by both? And/or do we want a (different) HWDB file for actually udev? After all, we still do generate a full-scale "$TOP_BUILDDIR/scripts/udev/nut-usbups.rules.in"; - see "$TOP_BUILDDIR/scripts/udev/nut-usbups.rules.in"; and https://github.com/networkupstools/nut/blob/master/tools/nut-usbinfo.pl#L196-L200

• https://upower.freedesktop.org/
• https://gitlab.freedesktop.org/upower/upower
• https://www.freedesktop.org/software/systemd/man/hwdb.html
• https://www.freedesktop.org/software/systemd/man/udev.html

[jimklimov]

@benzea : cheers, circled back to this due to research on another issue. Comparing the old stack of rules to determine whether these UPower rules/hwdb apply for a device being checked (IIUC), to the new single-line rule:

SUBSYSTEM=="usbmisc", SUBSYSTEMS=="usb", KERNEL=="hiddev*", IMPORT{parent}="UPOWER_*", IMPORT{parent}="ID_VENDOR", IMPORT{parent}="ID_PRODUCT"

...and not being an expert in the area, I wonder if they are logically equivalent. At face value:

• Not sure how SUBSYSTEMS work - so if usbmisc + usb requirement is still honoured; supposing "yes" but better double-check
• Similar for hiddev* vs. usbraw*- supposing, but asking to check :)
• Finally, I don't see an exemption for usb_interface anymore. Does it matter? Can it be cause of some issues in the field? ;\

[benzea]

from udev(7)

       SUBSYSTEM
           Match the subsystem of the event device.

       SUBSYSTEMS
           Search the devpath upwards for a matching device subsystem name.     

• The hwdb tags USB devices (not the usbmisc device!)
• We then match a "usbmisc" device that has a "usb" parent (which is probably redundant), so yes, it is honoured
• then, with KERNEL=="hiddev*", exactly the same match as before, just inverted for obvious reasons
• then, with IMPORT{}, we copy those tags down to the hiddev

It is not 100% equivalent. As, it for example copies the ID_PRODUCT/ID_VENDOR entries down to the usbmisc device (not sure why I did that, that might actually be redundant and IMPORT{UPOWER_*} should be sufficient).

I think usb_interface is just an optimization. i.e. don't bother with executing hundreds of rules for every endpoint. So yes, the exemption is missing but it will not matter as it is only one simple match/import rule. But, we can also add it back without any negative side effect. After all, no one needs UPOWER_* variables on every endpoint.

[benzea]

Another difference is obviously that the USB device itself also gets the UPOWER_* variables, which before it had not.

EDIT: Also, I suppose you could do the same basic matching as before, and then trigger hwdb with a custom string (e.g. nut:pPIDvVID) and the hwdb would directly tag the correct device. Plenty of possibilities, but at the end of the day what matters is only two things:

• Fewer rules to execute
• The usbmisc device gets the variable assigned

[jimklimov]

My reading was that this original matching was a sort of "if/switch" statement:

• if SUBSYSTEM==usbmisc, go to mid-way label up_hid_chkdev
• if SUBSYSTEM==usb, go to mid-way label up_hid_chkdev (by default, because otherwise we bail)

So if SUBSYSTEM is any one of these two - try next steps (consider hiddev and interfaces); otherwise don't bother.

[jimklimov]

As for interfaces and settings on "every endpoint", not being a domain expert I am in fact not sure what we should want here technically. In the past couple of years more and more devices appear which serve UPS HID protocols on a non-zero USB interface number, through libusb. If these translate on Linux to different filesystem nodes with their separate access permissions, and if this is the filter which separates base device (and/or interface #0 used for ages) from the non-zero interfaces, then perhaps we do want to skip the filter. It might however break some other software, I guess (whose generic/storage/scanner/whatever endpoints would get owned by nut and inaccessible to their specific software).

I just don't know for sure here, so voicing what looks like reasonable-guess concerns.

[benzea]

One question is posted about the code separately. Another is more generic: the original issue discussion wording, and the https://www.freedesktop.org/software/systemd/man/hwdb.html man page, seem to imply that "hwdb" is an "udev" implementation detail, instead of udev.rules...

I am a bit confused. hwdb is just a way to set variables on devices that udev knows about. What we do here, is use an existing matching for USB devices that is run anyway, to inject our own variables. One can also trigger such a match against the hwdb from a udev rule.

What is absolutely true, is that having UPOWER_* variables here is odd. Ideally, we would use some more generic names for variables and use those. At which point, we might be able to ask for the hwdb to be included in upstream systemd.

[jimklimov]

Thanks for the prompt reply, even if it rather reinforced my recent concerns:

having UPOWER_* variables here is odd

...um, because it is an UPower configuration file? (FWIW, the older code you replaced did also set those ENV vars, that's all I know about it and assume the UPower code uses that somehow). We shot well, but picked the wrong target, it seems. At least if the talk is about "udev"... or not if they do use the same backend for configuration data (rules/hwdb/systemd-udev binary DB)?

[jimklimov]

Looking at code, where a copy of this database landed in v0.99.18 (by you, so I guess you know whatcha doing after all ;p)...

https://gitlab.freedesktop.org/search?group_id=1217&project_id=139&repository_ref=v0.99.18&scope=blobs&search=%22UPOWER_ literally does go for

type = g_udev_device_get_property (native, "UPOWER_BATTERY_TYPE");

and

	/* prefer UPOWER names */
	vendor = g_udev_device_get_property (native, "UPOWER_VENDOR");
	if (vendor == NULL)
		vendor = g_udev_device_get_property (native, "ID_VENDOR");
...

in https://gitlab.freedesktop.org/upower/upower/-/blob/v0.99.18/src/linux/up-device-hid.c

However this code may be tailored to what you have in the database. "WUP" uses a somewhat different name set, other files may be provided (or not) by distros or users - but are not in codebase here, and I have yet to find a 1-2-3 instruction on setting it up which would sort of imply a schema :D

[jimklimov]

On a side note, does "linux" in source name imply that others (*bsd) do not use this rule set at all?

[jimklimov]

Looking at how that relies on g_udev_device_get_property() - from https://gnome.pages.gitlab.gnome.org/libgudev/libgudev/GUdevDevice.html#g-udev-device-get-property I suppose - I harbor hope that the udev database is after all shared by different Freedesktop projects.

So, waiting for a definitive reply from an expert - should there be more work in this area? Any work -

• revert this (if upower after all does not play well with HWDB format)?
• change/replace the other rules file generation?
• do we need two rules files simultaneously? would they confuse end-user systems if both get installed? are there use-cases where only one gets installed? (I suppose if people lack an upower package, the systemd basics include udev engine anyway?)

Any conflicts possible if a NUT package installs same file as an upower package? (NUT's should win to match real currently available device support, I suppose).

[benzea]

I have the feeling that there must be a slight misunderstanding on how it all works (and why like this).

So, udev (part of systemd), is the infrastructure to discover devices on the system. udev uses rules to e.g. set properties, bind drivers, etc. The hwdb is just an optimisation at the end of the day. There is a set of default rules maintained by systemd upstream¹ and everyone is allowed to add their own rules as suits them.

Now, we have UPower and NUT, which both need the same information (what HID device is actually a UPS). We choose to set properties in udev as that is a really good solution to the discovery problem.

---

• revert this (if upower after all does not play well with HWDB format)?

I do not understand this point. Using the HWDB is a much superior solution here.

• change/replace the other rules file generation?

Sure, happy with that. We just need some hwdb that is getting applied. Doesn't matter what it looks like exactly.

• do we need two rules files simultaneously? …

• udev is pretty much guaranteed to be available (non-systemd distros also have it; OpenWRT might be a niche were it is different)
• Duplicating a small set of udev rules is completely fine, the performance impact is minimal.
• I think that two rule files make sense from a distribution point of view (they might even be identical). The reason is that a direct depdencency of UPower on NUT is just not fun for anyone.

So, if both UPower and nut ship the same hwdb (but different rules), that would be a good solution! systemd-hwdb would merge them and the resulting binary hwdb will be the same. Still really efficient for udev to process and avoids all of the distribution issues.

Footnotes

1. Systemd upstream again pulls rules/hwdb information from other sources. Upstreams for this information are e.g. ChromeOS and libfprint, IIRC. However, in that case it is mostly about power management, where having the rules/hwdb installed is always desirable as it might save power exactly when no one uses the hardware. ↩

[benzea]

However this code may be tailored to what you have in the database. "WUP" uses a somewhat different name set, other files may be provided (or not) by distros or users - but are not in codebase here, and I have yet to find a 1-2-3 instruction on setting it up which would sort of imply a schema :D

Just invent a new schema that everyone can use in the future. I would say what UPower does is quite solid, but maybe you disagree ;-)

It is trivial to change the strings that UPower looks for in a future version.

On a side note, does "linux" in source name imply that others (*bsd) do not use this rule set at all?

I suppose, *bsd does not have udev. So obviously, no. But, on linux you really should use udev.

[jimklimov]

Thanks for the detailed replies, I've learned something new in these few days :)

So to summarize back to my points:

• at least on Linux, upower relies on (systemd-)udev database and any data format good for that - goes well (no reverts needed, phew!);

• something new for me is about systemd merging different rules - I suppose this implies different basenames for the file (man page says how one instance of a filename is chosen from a prioritized list of directories).

  • Now wondering how it merges them - should two files provide exact same matching criteria (ATTRs, ...) for a device, and then the outcomes (ENV, GROUP, SYMLINK... setting, commands maybe...) are glued into one DB entry? Is it like "first hit wins"? Or there may be several partial definitions which all match by their few criteria, and each is processed? (Original rules file did that, I think, for vendor id=>name vs. their products which are UPSes; I guess this is also what happens now for "$outputUdev" rules generated by the same script, right?)

  • Conversely though, if two rules/hwdb files have same (or worse - slightly different, e.g. different GROUP) content and different base names - won't that double the run-time overheads, possibly open a door for race-conditions, etc?

• "non-systemd distros" also having udev still implied Linux, not other OSes (many of which have their own equivalents, like devd... does upower cooperate with that?)

[benzea]

• something new for me is about systemd merging different rules …

Not quite. Logically it is executing everything in sequence (including the hwdb). The different directories are just different sources, all files are treated equally and are sorted by name (hence they always have a 2 digit numeric prefix).

I was saying the merging about the hwdb. The rule here is also "last matching entry wins". hwdb is an optimised binary format for doing the matching in the hwdb file and systemd-hwdb should optimise it if the input had duplicate entries.

• Conversely though, if two rules/hwdb files have same (or worse - slightly different, e.g. different GROUP) content and different base names - won't that double the run-time overheads, possibly open a door for race-conditions, etc?

As said above, the result is simple: last one wins. There is no race, because everything waits for the udev daemon to be done processing all of the rules in sequence.

• "non-systemd distros" also having udev still implied Linux, not other OSes (many of which have their own equivalents, like devd... does upower cooperate with that?)

UPower has some code, but it is in a bad state and only handles a small subset of what the Linux codebase can do.