ni / linux

Linux kernel source for NI Linux Real-Time
Other
81 stars 78 forks source link

nati_x86_64_defconfig: enable nftables support #170

Closed rtollert closed 2 months ago

rtollert commented 2 months ago

Manually enabled CONFIGNFT and selected CONFIGNF and CONFIG_IPNF options via menuconfig.

Somewhere between menuconfig and savedefconfig, CONFIG_DRM_VMWGFX_FBCON=y was removed. I didn't do that.

Testing: rebuilt nilrt-base-system-image; haven't attempted runtime testing.

gratian commented 2 months ago

Also can you add a "Testing" section to your PR where you describe what testing you've done if any.

I would also like to see the disk size increase this adds. It don't expect it to gate the PR but it would be good to know for future reference.

rtollert commented 2 months ago

I would also like to see the disk size increase this adds. It don't expect it to gate the PR but it would be good to know for future reference.

nilrt-base-system-image-x64.tar:

Note that I haven't actually added any kernel modules, userspace packages etc. to the packagegroups yet.

rtollert commented 2 months ago

For the 'CONFIG_DRM_VMWGFX_FBCON=y' diff I would recommend creating a separate commit where you just regenerate the defconfig (before making your nftables changes). The commit description can be just "Regenerate. No functional changes." (with the usual subject/signoff).

But shouldn't the defconfigs always be regenerated rather than hand-edited? In any case, I'll split up the commit while trying to connote that's what's going on.

rtollert commented 2 months ago

@gratian v2 is up and I think I've addressed everything?

gratian commented 2 months ago

For the 'CONFIG_DRM_VMWGFX_FBCON=y' diff I would recommend creating a separate commit where you just regenerate the defconfig (before making your nftables changes). The commit description can be just "Regenerate. No functional changes." (with the usual subject/signoff).

But shouldn't the defconfigs always be regenerated rather than hand-edited? In any case, I'll split up the commit while trying to connote that's what's going on.

Yes, defconfigs should always be regenerated and not hand-edited. In this case what it appears happened is commit 67adcfae2eb177303ceba1fbcb0935909c6fecfa ("drm/vmwgfx: Port the framebuffer code to drm fb helpers") removed the DRM_VMWGFX_FBCON config option. We picked it up with the upstream merge v6.1.99-rt36 in PR https://github.com/ni/linux/pull/165 but neglected to regenerate our defconfig. I will make sure our future work templates will call out checking the nati defconfig after merges.

That's why I was saying above you should regenerate the defconfig before you make the other changes. With the split commits the order is reversed. I'll fix it when I pull it.

gratian commented 2 months ago

I would also like to see the disk size increase this adds. It don't expect it to gate the PR but it would be good to know for future reference.

nilrt-base-system-image-x64.tar:

* latest autobuild: 282.685 KB.

* test build with my branch: 282.736 KB (+0.02%).

Note that I haven't actually added any kernel modules, userspace packages etc. to the packagegroups yet.

Checking the actual modules folder in a local build:

/lib/modules$ du -cs *
46948   6.1.105-rt38-00106-ga86ee30b566a
47996   6.1.105-rt38-00108-g616ddd777a62

A little over 1 MB.

Note that currently all enabled kernel modules get installed by packagegroup-ni-base: https://github.com/ni/meta-nilrt/blob/805579d6d77a08630f25273274cc50a03c017523/recipes-core/packagegroups/packagegroup-ni-base.bb#L57

We do have plans to add optional modules but they are still in the planning stage.

gratian commented 2 months ago

Merge completed outside GitHub UI. Reversed order of commits as described above.

f947d0d0bdb86ea86173ce966c3adf86317e0ec8 (HEAD -> nilrt/master/6.1, origin/nilrt/master/6.1) nati_x86_64_defconfig: enable nftables support
8309134753e01d2074fc977785e6f3ee41b4ced1 nati_x86_64_defconfig: regenerate; no functional changes
a86ee30b566af1563ceca9e69d5d99144f57cd09 (rtollert/nilrt/master/6.1, erickshepherd/nilrt/master/6.1) Merge tag 'v6.1.105-rt38' into nilrt/master/6.1

Verified there are no superfluous changes after each commit by regenerating the defconfig during the rebase.

gratian commented 2 months ago

Cherry-picked in nilrt/master/6.6.