kernel: include metadata for built-in modules by guidosarducci · Pull Request #14218 · openwrt/openwrt

guidosarducci · 2023-12-14T06:36:15Z

Description

Add modules.builtin and modules.builtin.modinfo to the kernel package for improved handling of built-in modules. As with other distros, this enables modprobe <module> to consistently return success for both loaded/built-in modules, a useful feature for presence-testing. and allows modinfo to print related module details.

Given OpenWrt's few built-in modules, this change and the related kmodloader support only add ~2.5 KB to a compressed filesystem image size.

Examples

Some samples of new and changed functionality (with updated kmodloader) are below.

# Print parameters for existing loadable modules:

root@OpenWrt:/# modinfo mac80211
filename:       /lib/modules/6.1.65/mac80211.ko
license:        GPL
depends:        cfg80211,compat
name:           mac80211
vermagic:       6.1.65 SMP mod_unload MIPS32_R2 32BIT
parm:           minstrel_vht_only (bool)
parm:           max_nullfunc_tries (int)
parm:           max_probe_tries (int)
parm:           beacon_loss_count (int)
parm:           probe_wait_ms (int)
parm:           ieee80211_default_rc_algo (charp)


# Recognize built-in modules, treat consistent with loadable ones

root@OpenWrt:/# rmmod sch_fq_codel
module is builtin

root@OpenWrt:/# modinfo sch_cake
filename:       /lib/modules/6.1.65/sch_cake.ko
license:        Dual BSD/GPL
depends:
intree:         Y
name:           sch_cake
vermagic:       6.1.65 SMP mod_unload MIPS32_R2 32BIT

root@OpenWrt:/# modprobe sch_fq_codel && echo SUCCESS || echo FAIL
SUCCESS
root@OpenWrt:/# modprobe sch_cake && echo SUCCESS || echo FAIL
SUCCESS


# Recognize aliases for built-in modules, treat consistent with loadable ones

root@OpenWrt:/# modinfo unix
name:           unix
filename:       (builtin)
alias:          net-pf-1
license:        GPL

root@OpenWrt:/# modprobe net-pf-1 && echo SUCCESS || echo FAIL
SUCCESS

Testing

Compile and run-tested against master on malta/mipseb32.
Confirmed kmodloader continues to function with or without modules.builtin and modules.builtin.modinfo.
Checked size impact of changes: updates to kmodloader, with inclusion of modules.builtin and modules.builtin.modinfo, increased OpenWrt compressed filesystem image size by ~2.5 KB.
Additional testing details in related kmodloader PR above.

Review

CC previous contributors: @blogic @nbd168 @hauke @Ansuel @neheb @yousong
Thanks in advance for your feedback...

Ansuel · 2023-12-14T10:57:22Z

@guidosarducci From what I'm reading this seems optional... Wonder if an idea might be make this configurable, enable by default and disable for tiny targets?

guidosarducci · 2023-12-15T01:56:29Z

@guidosarducci From what I'm reading this seems optional... Wonder if an idea might be make this configurable, enable by default and disable for tiny targets?

@Ansuel Hmmm, I considered doing that but concluded it wasn't worth it and simply caused more problems.

Total 2.5 KB size increase of all compressed files (modules.builtin, modules.builtin.modinfo, /sbin/kmodloader) doesn't seem to justify a SMALL_FLASH variant, and likewise having a "tiny" variant of ubox/kmodloader feels overcomlicated.
For programs/scripts to rely on these changes (e.g. for presence-testing) they do need to be universal and not optional. On most Linux distros, modprobe -q <builtin-module> just works and confirms a built-in module is available, but otherwise people resort to using terrible kludges such as trying to use a module and checking for failure (e.g. https://github.com/tohojo/sqm-scripts/blob/master/src/functions.sh#L388 on OpenWrt).
Past feature/size trade-offs that employ SMALL_FLASH conditionals typically deal with much larger sizes than 2.5 KB (e.g. 3d66f55). Also, these changes are on user filesystem flash, and face less constraints than would on kernel partitions.

Are there other major considerations? What would be your idea of a negligible size increase? Maybe mine doesn't align...

Ansuel · 2023-12-16T00:32:15Z

Are there other major considerations? What would be your idea of a negligible size increase? Maybe mine doesn't align...

All good point and correct. My idea wasn't of a tiny variant of ubox, just a config to disable the inclusion of these additional files in the final image. If it does add major complication and it's not worth it... Totally ok with this.

Anyway aside from this small thing the change looks good to me. I think we first have to merge ubox before this correct?

I would love some feedback from @hauke or @ynezz on this topic.

guidosarducci · 2023-12-16T01:17:51Z

I've tried to make this independent of the ubox PR, so kmodloader will check for the presence of modules.builtin and not abort if missing, just continue as before. This is the same behavior I see on Ubuntu. Also, updating kmodloader immediately gives you modinfo parameters data, since this is already included in our current loadable modules. Makes it easy to test only ubox PR with e.g. 'modinfo mac80211'.

…

________________________________ From: Christian Marangi ***@***.***> Sent: Friday, December 15, 2023 4:32:30 PM To: openwrt/openwrt ***@***.***> Cc: guidosarducci ***@***.***>; Mention ***@***.***> Subject: Re: [openwrt/openwrt] kernel: include metadata for built-in modules (PR #14218) Are there other major considerations? What would be your idea of a negligible size increase? Maybe mine doesn't align... All good point and correct. My idea wasn't of a tiny variant of ubox, just a config to disable the inclusion of these additional files in the final image. If it does add major complication and it's not worth it... Totally ok with this. Anyway aside from this small thing the change looks good to me. I think we first have to merge ubox before this correct? I would love some feedback from @hauke<https://github.com/hauke> or @ynezz<https://github.com/ynezz> on this topic. — Reply to this email directly, view it on GitHub<#14218 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADNNWOOQGKQ5REKT6SIMRXTYJTTZXAVCNFSM6AAAAABAUKIXZ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJYGYZTQOJRGY>. You are receiving this because you were mentioned.Message ID: ***@***.***>

guidosarducci · 2023-12-16T01:21:32Z

Thanks.BTW, I noticed some pre-existing code duplication in get_module_info() and print_modinfo() that we might look at in the future to shrink ubox a little. Would like to hear others thoughts too.

…

________________________________ From: Christian Marangi ***@***.***> Sent: Friday, December 15, 2023 4:32:40 PM To: openwrt/openwrt ***@***.***> Cc: guidosarducci ***@***.***>; Mention ***@***.***> Subject: Re: [openwrt/openwrt] kernel: include metadata for built-in modules (PR #14218) @Ansuel approved this pull request. — Reply to this email directly, view it on GitHub<#14218 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADNNWOLC2ZNH75L5KG5GG4LYJTT2RAVCNFSM6AAAAABAUKIXZ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOBVGA3TOMZRHE>. You are receiving this because you were mentioned.Message ID: ***@***.***>

guidosarducci · 2023-12-17T04:44:38Z

Rebased against master and fixed wrong handle for @nbd168 .

guidosarducci · 2023-12-19T02:32:01Z

Rebased against master again.

guidosarducci · 2023-12-20T21:48:28Z

And another rebase.

guidosarducci · 2024-01-04T00:33:51Z

Thanks @Ansuel for reviewing this. Could you also please take a look and comment @nbd168 @hauke and @blogic ? Best in the New Year to all...

Ansuel · 2024-01-05T15:16:11Z

@guidosarducci I want to merge this... can you rebase?

guidosarducci · 2024-01-06T10:25:29Z

@Ansuel OK, rebased again. Thanks for reviewing and merging! Would you have a chance to look at openwrt/ubox#1 as well?

Add modules.builtin to the kernel package for improved handling of loadable and builtin modules. As with other distros, this allows 'modprobe <module>' to consistently return success for both loaded/built-in modules, a useful feature for presence-testing. Given OpenWrt's few built-in modules, this change and related kmodloader support add ~1 KB to the compressed image size. Using sch_fq_codel (builtin) and sch_cake (loadable) for example: root@OpenWrt:/# modprobe sch_fq_codel && echo SUCCESS || echo FAIL SUCCESS root@OpenWrt:/# modprobe sch_cake && echo SUCCESS || echo FAIL SUCCESS root@OpenWrt:/# rmmod sch_fq_codel module is builtin Signed-off-by: Tony Ambardar <itugrok@yahoo.com>

Add modules.builtin.modinfo to the kernel package, to support presence testing using module aliases and printing module details with 'modinfo'. With related kmodloader changes this adds ~2 KB to compressed image sizes. root@OpenWrt:/# modinfo unix name: unix filename: (builtin) alias: net-pf-1 license: GPL root@OpenWrt:/# modprobe net-pf-1 && echo SUCCESS || echo FAIL SUCCESS Signed-off-by: Tony Ambardar <itugrok@yahoo.com>

j4cbo · 2024-01-07T14:17:48Z

I think this may be breaking the build for me when CONFIG_COLLECT_KERNEL_DEBUG is set. Specifically, this line fails:

openwrt/include/kernel-build.mk

Line 68 in 9658050

    
           $(FIND) $(KERNEL_BUILD_DIR)/debug -type f | $(XARGS) $(KERNEL_CROSS)strip --only-keep-debug

find /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug -type f | xargs -r mips-openwrt-linux-musl-strip --only-keep-debug
mips-openwrt-linux-musl-strip: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug/modules/modules.builtin.modinfo: file format not recognized
mips-openwrt-linux-musl-strip: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug/modules/modules.builtin: file format not recognized
make[4]: *** [Makefile:62: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/linux-5.15.146/.image] Error 123

ynezz · 2024-01-07T18:17:10Z

I think this may be breaking the build for me when CONFIG_COLLECT_KERNEL_DEBUG is set. Specifically, this line fails:

openwrt/include/kernel-build.mk

Line 68 in 9658050

$(FIND) $(KERNEL_BUILD_DIR)/debug -type f | $(XARGS) $(KERNEL_CROSS)strip --only-keep-debug
find /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug -type f | xargs -r mips-openwrt-linux-musl-strip --only-keep-debug
mips-openwrt-linux-musl-strip: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug/modules/modules.builtin.modinfo: file format not recognized
mips-openwrt-linux-musl-strip: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/debug/modules/modules.builtin: file format not recognized
make[4]: *** [Makefile:62: /[...]/openwrt/build_dir/target-mips_4kec_musl/linux-realtek_rtl838x/linux-5.15.146/.image] Error 123

@j4cbo can you check @981213 's fix 066b0fe ?

Merging pull request #14218 (with all 190 CI checks green) resulted in buildbot failures in `Kernel/CollectDebug` step, likely due to the fact, that CI builds don't use same config options as on buildbot, specifically CI builds currently don't have `COLLECT_KERNEL_DEBUG` config option enabled. So lets try to prevent those regressions in the future by aligning the CI build process with the build config options used later on the buildbots. References: 066b0fee7668 ("kernel: copy only *.ko for debug info") References: openwrt/openwrt#14218 Signed-off-by: Petr Štetiar <ynezz@true.cz>

j4cbo · 2024-01-07T19:25:34Z

Yep, that fixes it, thanks!

guidosarducci · 2024-01-07T22:42:38Z

@981213 @ynezz Thanks for investigating. Any reason not to keep the modinfo files for debug purposes in 066b0fe? Changing the find in

openwrt/include/kernel-build.mk

Line 68 in 9658050

    
           $(FIND) $(KERNEL_BUILD_DIR)/debug -type f | $(XARGS) $(KERNEL_CROSS)strip --only-keep-debug

to include -name '*.ko' would target only modules for stripping.

981213 · 2024-01-08T03:13:06Z

Any reason not to keep the modinfo files for debug purposes in 066b0fe?

We need this separated debug info archive because we strip debug info out of modules in firmwares.
The files added here are available in the final firmware and we can just grab it there if it's needed.

Merging pull request #14218 (with all 190 CI checks green) resulted in buildbot failures in `Kernel/CollectDebug` step, likely due to the fact, that CI builds don't use same config options as on buildbot, specifically CI builds currently don't have `COLLECT_KERNEL_DEBUG` config option enabled. So lets try to prevent those regressions in the future by aligning the CI build process with the build config options used later on the buildbots. References: 066b0fee7668 ("kernel: copy only *.ko for debug info") References: openwrt/openwrt#14218 Signed-off-by: Petr Štetiar <ynezz@true.cz>

github-actions bot added kernel pull request/issue with Linux kernel related changes core packages pull request/issue for core (in-tree) packages labels Dec 14, 2023

guidosarducci mentioned this pull request Dec 14, 2023

kmodloader: add general support for built-in modules and modinfo openwrt/ubox#1

Merged

Ansuel approved these changes Dec 16, 2023

View reviewed changes

guidosarducci force-pushed the master-update-kmodloader branch from 812dc89 to 70ed367 Compare December 17, 2023 04:43

guidosarducci force-pushed the master-update-kmodloader branch from 70ed367 to 2722ea7 Compare December 19, 2023 02:25

guidosarducci force-pushed the master-update-kmodloader branch from 2722ea7 to fe68769 Compare December 20, 2023 21:46

guidosarducci force-pushed the master-update-kmodloader branch 5 times, most recently from 80dee83 to ea89f18 Compare January 4, 2024 00:27

guidosarducci force-pushed the master-update-kmodloader branch from ea89f18 to 4e9f9d9 Compare January 6, 2024 10:24

guidosarducci added 2 commits January 6, 2024 08:34

guidosarducci force-pushed the master-update-kmodloader branch from 4e9f9d9 to e1d8e57 Compare January 6, 2024 16:36

openwrt-bot merged commit e1d8e57 into openwrt:main Jan 6, 2024

guidosarducci deleted the master-update-kmodloader branch January 6, 2024 16:42

ynezz mentioned this pull request Jan 7, 2024

Try to prevent harder buildbot failures openwrt/actions-shared-workflows#5

Draft

Uh oh!

Conversation

guidosarducci commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related

Examples

Testing

Review

Uh oh!

Ansuel commented Dec 14, 2023

Uh oh!

guidosarducci commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ansuel commented Dec 16, 2023

Uh oh!

guidosarducci commented Dec 16, 2023 via email

Uh oh!

guidosarducci commented Dec 16, 2023 via email

Uh oh!

guidosarducci commented Dec 17, 2023

Uh oh!

guidosarducci commented Dec 19, 2023

Uh oh!

guidosarducci commented Dec 20, 2023

Uh oh!

guidosarducci commented Jan 4, 2024

Uh oh!

Ansuel commented Jan 5, 2024

Uh oh!

guidosarducci commented Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

j4cbo commented Jan 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ynezz commented Jan 7, 2024

Uh oh!

j4cbo commented Jan 7, 2024

Uh oh!

guidosarducci commented Jan 7, 2024

Uh oh!

981213 commented Jan 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

guidosarducci commented Dec 14, 2023 •

edited

Loading

guidosarducci commented Dec 15, 2023 •

edited

Loading

guidosarducci commented Jan 6, 2024 •

edited

Loading

j4cbo commented Jan 7, 2024 •

edited

Loading