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

fix: dvb extension as it was missing modules #575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samip5
Copy link
Contributor

@samip5 samip5 commented Dec 28, 2024

It seems that sound related modules are indeed required.

@frezbo
Copy link
Member

frezbo commented Dec 28, 2024

Make sure all modules listed here are added: siderolabs/pkgs#1102 (comment)

That's all the new modules that got added, that's the right way to pull in an extension providing modules

@frezbo
Copy link
Member

frezbo commented Dec 28, 2024

Make sure all modules listed here are added: siderolabs/pkgs#1102 (comment)

That's all the new modules that got added, that's the right way to pull in an extension providing modules

you could also add v4l as a depedency for dvb so when it's selected those gets pulled in automatically too

@samip5
Copy link
Contributor Author

samip5 commented Dec 28, 2024

that's the right way to pull in an extension providing modules

Yes, but as this extension is related to cx23885, not all of those should be needed. The idea of mine was to provide bare minimal modules. Not every single built one.

@frezbo
Copy link
Member

frezbo commented Dec 28, 2024

that's the right way to pull in an extension providing modules

Yes, but as this extension is related to cx23885, not all of those should be needed. The idea of mine was to provide bare minimal modules. Not every single built one.

ahh then you'll have to split the pkgs PR, find the base list of dvb modules (there might be a lot of dependencies), then another diff when cx23885 is enabled

@samip5 samip5 force-pushed the dvb-fixing branch 2 times, most recently from e11f247 to c808ebd Compare December 28, 2024 14:39
@samip5
Copy link
Contributor Author

samip5 commented Dec 28, 2024

This is easier.
@frezbo I added all of the modules and the dep but I'm not exactly sure if the dep thing was done right.

@samip5
Copy link
Contributor Author

samip5 commented Dec 31, 2024

Okay, so progress but it still seems to be unable to fully work, possibly related to the fact there's no /dev/dvb?
Do we have a way to enable debug for kernel module?

I did try this in machine config, but did not see anywhere that it was applied:

kernel:
  modules:
    - name: cx23885
      parameters:
        - debug=1
[2024-12-31T15:47:03.065271513Z]: [talos] [initramfs] enabling system extension dvb-cx23885 1.9.0
[2024-12-31T15:47:21.665787513Z]: cx23885: CORE cx23885[0]: subsystem: 0070:6a28, board: Hauppauge WinTV-QuadHD-DVB [card=56,autodetected]
[2024-12-31T15:47:22.167955513Z]: tveeprom: Hauppauge model 166100, rev B4I6, serial# <snip>
[2024-12-31T15:47:22.167958513Z]: tveeprom: MAC address is <censored>
[2024-12-31T15:47:22.167959513Z]: tveeprom: tuner model is SiLabs Si2157 (idx 186, type 4)
[2024-12-31T15:47:22.167960513Z]: tveeprom: TV standards PAL(B/G) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)
[2024-12-31T15:47:22.167962513Z]: tveeprom: audio processor is CX23888 (idx 40)
[2024-12-31T15:47:22.167963513Z]: tveeprom: decoder processor is CX23888 (idx 34)
[2024-12-31T15:47:22.167963513Z]: tveeprom: has no radio, has IR receiver, has no IR transmitter
[2024-12-31T15:47:23.245490513Z]: cx23885: cx23885_dvb_register() allocating 1 frontend(s)
[2024-12-31T15:47:23.314938513Z]: cx23885: cx23885[0]: cx23885 based dvb card
[2024-12-31T15:47:23.372015513Z]: cx23885: dvb_register(): board=56 port=1
[2024-12-31T15:47:23.426914513Z]: cx23885: cx23885_dvb_register() dvb_register failed err = -22
[2024-12-31T15:47:23.501127513Z]: cx23885: cx23885_dev_setup() Failed to register dvb adapters on VID_B
[2024-12-31T15:47:23.582891513Z]: cx23885: cx23885_dvb_register() allocating 1 frontend(s)
[2024-12-31T15:47:23.652344513Z]: cx23885: cx23885[0]: cx23885 based dvb card
[2024-12-31T15:47:23.709443513Z]: cx23885: dvb_register(): board=56 port=2
[2024-12-31T15:47:23.764231513Z]: cx23885: cx23885_dvb_register() dvb_register failed err = -22
[2024-12-31T15:47:23.838399513Z]: cx23885: cx23885_dev_setup() Failed to register dvb on VID_C
[2024-12-31T15:47:24.092773513Z]: cx23885: CORE cx23885[1]: subsystem: 0070:6b28, board: Hauppauge WinTV-QuadHD-DVB [card=56,autodetected]
[2024-12-31T15:47:24.548192513Z]: tveeprom: Hauppauge model 166101, rev B4I6, serial# <snip>
[2024-12-31T15:47:24.625485513Z]: tveeprom: MAC address is <censored>
[2024-12-31T15:47:24.683730513Z]: tveeprom: tuner model is SiLabs Si2157 (idx 186, type 4)
[2024-12-31T15:47:24.755292513Z]: tveeprom: TV standards PAL(B/G) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xf4)
[2024-12-31T15:47:24.861957513Z]: tveeprom: audio processor is CX23888 (idx 40)
[2024-12-31T15:47:24.922992513Z]: tveeprom: decoder processor is CX23888 (idx 34)
[2024-12-31T15:47:24.985953513Z]: tveeprom: has no radio
[2024-12-31T15:47:26.135716513Z]: cx23885: cx23885_dvb_register() allocating 1 frontend(s)
[2024-12-31T15:47:26.207342513Z]: cx23885: cx23885[1]: cx23885 based dvb card
[2024-12-31T15:47:26.266554513Z]: cx23885: dvb_register(): board=56 port=1
[2024-12-31T15:47:26.323425513Z]: cx23885: cx23885_dvb_register() dvb_register failed err = -22
[2024-12-31T15:47:26.399702513Z]: cx23885: cx23885_dev_setup() Failed to register dvb adapters on VID_B
[2024-12-31T15:47:26.483580513Z]: cx23885: cx23885_dvb_register() allocating 1 frontend(s)
[2024-12-31T15:47:26.555050513Z]: cx23885: cx23885[1]: cx23885 based dvb card
[2024-12-31T15:47:26.614184513Z]: cx23885: dvb_register(): board=56 port=2
[2024-12-31T15:47:26.671022513Z]: cx23885: cx23885_dvb_register() dvb_register failed err = -22
[2024-12-31T15:47:26.747254513Z]: cx23885: cx23885_dev_setup() Failed to register dvb on VID_C

@samip5 samip5 force-pushed the dvb-fixing branch 2 times, most recently from cd8cecc to f0039e6 Compare December 31, 2024 17:44
@samip5 samip5 marked this pull request as draft January 1, 2025 12:24
@samip5
Copy link
Contributor Author

samip5 commented Jan 1, 2025

I found that we are actually missing a driver.

siderolabs/pkgs#1132

It seems that some modules were missing and it has a dependency on v4l-uvc-drivers.

Signed-off-by: Skyler Mäntysaari <sm+git@skym.fi>
@samip5
Copy link
Contributor Author

samip5 commented Jan 5, 2025

Happy to report it works, even if the group on the devices is root. That should be video, but I don't know what's the best way to have it do that.

I'm also not sure if we should let the blacklists stay, as I think a user might want udev to load the driver.

@samip5 samip5 marked this pull request as ready for review January 5, 2025 13:50
@samip5
Copy link
Contributor Author

samip5 commented Jan 6, 2025

@frezbo What do you think about if we should let udev handle the loading as the devices should have proper groups than just root applied to them..?

@frezbo
Copy link
Member

frezbo commented Jan 6, 2025

@frezbo What do you think about if we should let udev handle the loading as the devices should have proper groups than just root applied to them..?

it's not about udev, since talos doesn't have /etc/passwd and /etc/group udev cannot apply any info regarding that

@@ -1,11 +1,188 @@
modules.order
modules.alias
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need these, it'll be generated by depmod

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modinfo would disagree.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a reproducer, since depmod should re-create a final data based on the modules available

@samip5
Copy link
Contributor Author

samip5 commented Jan 6, 2025

@frezbo What do you think about if we should let udev handle the loading as the devices should have proper groups than just root applied to them..?

it's not about udev, since talos doesn't have /etc/passwd and /etc/group udev cannot apply any info regarding that

But it should still be able to use the numeric value for group?

This udev rule seems to do nothing:

SUBSYSTEM=="dvb", GROUP="44", MODE="0660"

@frezbo
Copy link
Member

frezbo commented Jan 6, 2025

@frezbo What do you think about if we should let udev handle the loading as the devices should have proper groups than just root applied to them..?

it's not about udev, since talos doesn't have /etc/passwd and /etc/group udev cannot apply any info regarding that

But it should still be able to use the numeric value for group?

This udev rule seems to do nothing:

SUBSYSTEM=="dvb", GROUP="44", MODE="0660"

@dsseng do we disable group/user handling in udevd?

@dsseng
Copy link
Member

dsseng commented Jan 6, 2025

I believe we do not explicitly disable anything. Yet what's the purpose of adding groups to these nodes? Video group is how it (was) managed on desktop, now I guess logind also overtook this (or about to do so). In Kubernetes environment groups would likely be annoying rather than useful as you give access with volumes when running pods

@samip5
Copy link
Contributor Author

samip5 commented Jan 6, 2025

I believe we do not explicitly disable anything. Yet what's the purpose of adding groups to these nodes? Video group is how it (was) managed on desktop, now I guess logind also overtook this (or about to do so). In Kubernetes environment groups would likely be annoying rather than useful as you give access with volumes when running pods

If the device nodes don't get group properly set, I cannot run a workload unprivileged and just have it use the group and have access to the tuners. As the situation stands, tvheadend (my workload) cannot see (or to actually use) the devices due to permissions if not running privileged.

Just mounting them as volumes will not be sufficient due to owner AND group being root aka uid 0 / gid 0.

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