Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try SDL_UDEV_deviceclass to detect joysticks even if in a container #8645

Merged
merged 1 commit into from
Dec 25, 2023

Conversation

twhitehead
Copy link
Contributor

Description

The udev container issue is mostly to do with device notifications and netlink. The device classification stuff just pokes file in /sys and /run/udev. Doesn't hurt to try it first for classifying joysticks and then fall to the guess heuristics if it fails.

Here is an example of the the sdl-jtest program compiled against the current SDL2 HEAD

tyson@tux ~> ./sdl2-orig/bin/sdl2-jstest -l
Found 1 joystick(s)

Joystick Name:     'PRODUCTS CH PRO PEDALS USB'
Joystick GUID:     0300b9e88e060000f200000000010000
Joystick Number:    0
Number of Axes:     3
Number of Buttons:  0
Number of Hats:     0
Number of Balls:    0
GameControllerConfig:
  missing (see 'gamecontrollerdb.txt' or SDL_GAMECONTROLLERCONFIG)

and here it is inside the steam pressure vessel container

tyson@tux ~> steam-run /home/tyson/.local/share/Steam/steamapps/common/SteamLinuxRuntime_sniper/_v2-entry-point -- ./sdl2-orig/bin/sdl2-jstest -l
...
No joysticks were found

The issue being the the heuristics fail because the CH PRO PEDALS are three axis with no buttons, which is the same as an accelerometer. It works outside the container because the udev hwdb has an entry for this device.

Now with it build against SDL with these changes (and the -DDEBUG_INPUT_EVENTS and DDEBUG_JOYSTICK turned on for some more clarity)

tyson@tux ~> ./sdl2-patch/bin/sdl2-jstest -l
INFO: Checking /dev/input/event6
INFO: Joystick: PRODUCTS CH PRO PEDALS USB, bustype = 3, vendor = 0x068e, product = 0x00f2, version = 256
INFO: found joystick: /dev/input/event6
INFO: Joystick has absolute axis: 0x00
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x01
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x02
INFO: Values = { val:124, min:0, max:255, fuzz:0, flat:15, res:0 }
Checking /dev/input/event6
Checking /dev/input/event6
Checking /dev/input/js0
Found 1 joystick(s)

INFO: Joystick has absolute axis: 0x00
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x01
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x02
INFO: Values = { val:125, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick : Re-read Axis 0 (0) val= -32768
INFO: Joystick : Re-read Axis 1 (1) val= -32768
INFO: Joystick : Re-read Axis 2 (2) val= -643
Joystick Name:     'PRODUCTS CH PRO PEDALS USB'
Joystick GUID:     0300b9e88e060000f200000000010000
Joystick Number:    0
Number of Axes:     3
Number of Buttons:  0
Number of Hats:     0
Number of Balls:    0
GameControllerConfig:
  missing (see 'gamecontrollerdb.txt' or SDL_GAMECONTROLLERCONFIG)

and inside the container

tyson@tux ~> steam-run /home/tyson/.local/share/Steam/steamapps/common/SteamLinuxRuntime_sniper/_v2-entry-point -- ./sdl2-patch/bin/sdl2-jstest -l
...
INFO: Checking /dev/input/event6
INFO: Joystick: PRODUCTS CH PRO PEDALS USB, bustype = 3, vendor = 0x068e, product = 0x00f2, version = 256
INFO: found joystick: /dev/input/event6
INFO: Joystick has absolute axis: 0x00
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x01
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x02
INFO: Values = { val:124, min:0, max:255, fuzz:0, flat:15, res:0 }
Checking /dev/input/event6
Checking /dev/input/event6
Checking /dev/input/js0
Found 1 joystick(s)

INFO: Joystick has absolute axis: 0x00
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x01
INFO: Values = { val:0, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick has absolute axis: 0x02
INFO: Values = { val:125, min:0, max:255, fuzz:0, flat:15, res:0 }
INFO: Joystick : Re-read Axis 0 (0) val= -32768
INFO: Joystick : Re-read Axis 1 (1) val= -32768
INFO: Joystick : Re-read Axis 2 (2) val= -643
Joystick Name:     'PRODUCTS CH PRO PEDALS USB'
Joystick GUID:     0300b9e88e060000f200000000010000
Joystick Number:    0
Number of Axes:     3
Number of Buttons:  0
Number of Hats:     0
Number of Balls:    0
GameControllerConfig:
  missing (see 'gamecontrollerdb.txt' or SDL_GAMECONTROLLERCONFIG)

Existing Issue(s)

This should close #7500.

Other

This cherry picks reasonably well onto the master SDL3 branch as well. If you find it acceptable, I will do another pull request for SDL3 too.

@twhitehead
Copy link
Contributor Author

twhitehead commented Nov 30, 2023

For further details, see @smcv's commit 13e7d1a. It explains the container udev/netlink/api issues

Device enumeration via libudev can fail in a container for two reasons:

  • the netlink protocol between udevd and libudev is considered private, so there is no API guarantee that the version of libudev in a container will understand netlink messages from a dissimilar version of udevd on the host system;
  • the netlink protocol between udevd and libudev relies for security on being able to check the uid of each message, but in a container with a user namespace where host uid 0 is not mapped, the libudev client cannot distinguish between messages from host uid 0 and messages from a different, malicious user on the host

As I mentioned, the udev device lookup code doesn't use netlink. Instead it pokes around it /sys and then goes and checks the device database in /run/udev/data/<device>. While a kernel change could break the /sys part, or a udev change could break the /run/udev/data/<device>, the code still falls back to the heuristics if the lookup fails (or succeeds and doesn't return anything useful), so worst case it should be no worse than what we currently have.

On the question of the likelyhood of /run/udev/data/<device> breaking, it is worth noting that these are just plain text type:key=value files, so there isn't really too much to break. For the CH PRO PEDALS, for example,

tyson@tux ~> cat /run/udev/data/c13:70 
S:input/by-path/pci-0000:00:14.0-usb-0:14:1.0-event-joystick
S:input/by-id/usb-CH_PRODUCTS_CH_PRO_PEDALS_USB-event-joystick
I:9481792
E:PATH=/nix/store/8x78lgbda9b8wcr0mnl1vgbvdv3kiya3-udev-path/bin:/nix/store/8x78lgbda9b8wcr0mnl1vgbvdv3kiya3-udev-path/sbin
E:ID_INPUT=1
E:ID_INPUT_ACCELEROMETER=0
E:ID_INPUT_JOYSTICK=1
E:ID_BUS=usb
E:ID_MODEL=CH_PRO_PEDALS_USB
E:ID_MODEL_ENC=CH\x20PRO\x20PEDALS\x20USB\x20
E:ID_MODEL_ID=00f2
E:ID_SERIAL=CH_PRODUCTS_CH_PRO_PEDALS_USB
E:ID_VENDOR=CH_PRODUCTS
E:ID_VENDOR_ENC=CH\x20PRODUCTS
E:ID_VENDOR_ID=068e
E:ID_REVISION=0000
E:ID_TYPE=hid
E:ID_USB_MODEL=CH_PRO_PEDALS_USB
E:ID_USB_MODEL_ENC=CH\x20PRO\x20PEDALS\x20USB\x20
E:ID_USB_MODEL_ID=00f2
E:ID_USB_SERIAL=CH_PRODUCTS_CH_PRO_PEDALS_USB
E:ID_USB_VENDOR=CH_PRODUCTS
E:ID_USB_VENDOR_ENC=CH\x20PRODUCTS
E:ID_USB_VENDOR_ID=068e
E:ID_USB_REVISION=0000
E:ID_USB_TYPE=hid
E:ID_USB_INTERFACES=:030000:
E:ID_USB_INTERFACE_NUM=00
E:ID_USB_DRIVER=usbhid
E:ID_PATH=pci-0000:00:14.0-usb-0:14:1.0
E:ID_PATH_TAG=pci-0000_00_14_0-usb-0_14_1_0
E:ID_FOR_SEAT=input-pci-0000_00_14_0-usb-0_14_1_0
E:LIBINPUT_DEVICE_GROUP=3/68e/f2:usb-0000:00:14.0-14
G:uaccess
G:seat
Q:uaccess
Q:seat
V:1

@slouken
Copy link
Collaborator

slouken commented Dec 3, 2023

@smcv, is this a good change for SDL?

@twhitehead
Copy link
Contributor Author

Thanks for taking a look!

I compiled up new SDL2 packages for the steam runtimes and asked people to check them out on some of the "does not recognize my device correctly" bug reports. I am currently using them as well.

Hoping that will provide some testing and feedback.

@smcv
Copy link
Contributor

smcv commented Dec 4, 2023

@smcv, is this a good change for SDL?

Right now the most definitive answer I can give is "maybe". I'll try to dig into the finer details at some point.

src/core/linux/SDL_udev.c Outdated Show resolved Hide resolved
src/core/linux/SDL_evdev_capabilities.h Outdated Show resolved Hide resolved
src/core/linux/SDL_udev.c Outdated Show resolved Hide resolved
src/joystick/linux/SDL_sysjoystick.c Outdated Show resolved Hide resolved
src/joystick/linux/SDL_sysjoystick.c Outdated Show resolved Hide resolved
@smcv
Copy link
Contributor

smcv commented Dec 5, 2023

I compiled up new SDL2 packages for the steam runtimes and asked people to check them out on some of the "does not recognize my device correctly" bug reports. I am currently using them as well.

In case it's not obvious, this falls into the category of unsupported/unsupportable changes to the Steam Runtime; while it's great as a way to prototype and evaluate changes, please make it obvious to users of these builds that Valve cannot provide support for a runtime that has been modified in this way.

@smcv
Copy link
Contributor

smcv commented Dec 5, 2023

As I mentioned, the udev device lookup code doesn't use netlink. Instead it pokes around it /sys and then goes and checks the device database in /run/udev/data/. While a kernel change could break the /sys part, or a udev change could break the /run/udev/data/, the code still falls back to the heuristics if the lookup fails (or succeeds and doesn't return anything useful), so worst case it should be no worse than what we currently have.

The /sys part is part of the ABI presented to user-space by the kernel, so Linux kernel policy is that any breaking change to that should be reverted as soon as anyone notices it. So that part is fine.

The issue here is /run/udev/data, which is explicitly private to udev: the udev/systemd maintainers reserve the right to break those at any time.

On the question of the likelyhood of /run/udev/data/ breaking, it is worth noting that these are just plain text type:key=value files, so there isn't really too much to break

They are plain text key/value files today. The concern is that tomorrow, the udev/systemd maintainers could replace them with JSON or XML or some binary format, and if we complained that this caused SDL to regress, the response would be "you shouldn't have been parsing those, they're private".

More realistically, and more concerningly (because it would be harder for SDL to detect), they could keep the format intact but make incompatible changes to the semantics.

The question, then, is how likely this is to really happen in practice. In the Steam Runtime containers (Steam Linux Runtime), the position we've taken in the past is to say "it seems unlikely", and share /run/udev/data with the container even though it is officially private. For SDL and Proton, there are specific code paths that bypass libudev and do the device lookup from first principles, but some native Linux game engines use libudev directly, and sharing /run/udev/data makes those "mostly work" (albeit without the ability to hotplug).

However, in Flatpak, the position that has been taken is to say "/run/udev/data is officially not a stable interface, so we're not going to share it with the sandbox", and all game engines that will run inside a Flatpak app are expected to adapt to this.

I think on balance, this PR is probably a positive change: while /run/udev/data is officially not a stable interface, what we have observed in the real world is that it's generally "stable enough". However, we will need to be on the lookout for regressions if the format or content of /run/udev/data genuinely does change in an incompatible way. The only way I can see to respond to those regressions would be to stop sharing /run/udev/data with the container, which would make the changes in this PR ineffective, and as a result we'd fall back to GuessIsJoystick() as before.

This means that we should be adding known mis-detected joysticks like yours to a hard-coded (or possibly environment-variable-overridable) table of known joysticks in SDL, as well as trying to use libudev to bypass the mis-detection.

@smcv
Copy link
Contributor

smcv commented Dec 5, 2023

This means that we should be adding known mis-detected joysticks like yours to a hard-coded (or possibly environment-variable-overridable) table of known joysticks in SDL, as well as trying to use libudev to bypass the mis-detection.

Since you have Steam installed, the easiest way to collect fairly complete information about this device is #7801 (comment). Please do that if you can - that way, we can put information about your device in the automated tests, and it will be available to any SDL contributor, even those of us who don't have an example of it physically available.

@twhitehead
Copy link
Contributor Author

Thank you for the very detailed review. I will make those corrections and make sure to make it clean to people that neither myself nor the patched packages are in any way associated with Valve, and that they should net not report an issues to Valve while running under them. I'll get you that dump on the CH Pro Pedals too. 👍

@twhitehead
Copy link
Contributor Author

I have update the README.txt file to be very clear that the test packages are not affiliated or support by Valve, and no problems should be reported while running them.

@twhitehead
Copy link
Contributor Author

Here is the json blob returned for the CH Pro Pedals.

@twhitehead
Copy link
Contributor Author

twhitehead commented Dec 6, 2023

Note that I haven't yet had a chance to test it. I will let you know when I have.

@twhitehead
Copy link
Contributor Author

I tested it, and it still works as I previously reported (i.e., sdl2-jstest lists the CH Pro Pedals as a joystick, both when invoked directly and when invoked inside the sniper runtime, when built against a patched SDL2).

src/core/linux/SDL_udev.c Outdated Show resolved Hide resolved
@slouken slouken added this to the 2.30.0 milestone Dec 24, 2023
The udev container issue is mostly to do with device notifications
and netlink. The device classification stuff just pokes file in /sys
and /run/udev. Doesn't hurt to try it first for classifying joysticks
and then fall to the guess heuristics if it fails.
@slouken slouken merged commit 3b1e0e1 into libsdl-org:SDL2 Dec 25, 2023
@slouken
Copy link
Collaborator

slouken commented Dec 25, 2023

Thanks!

@slouken
Copy link
Collaborator

slouken commented Dec 25, 2023

Can you make a PR for main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants