More backport woes on RHEL9

[cyqsimon]

I ran into this issue today on my RHEL9 box after updating to kernel version 5.14.0-427.16.1.el9_4:

  CC [M]  /var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.o
/var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.c: In function ‘gasket_register_device’:
/var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.c:1846:47: error: passing argument 1 of ‘class_create’ from incompatible pointer type [-Werror=incompatible-pointer-types]
 1846 |     internal->class = class_create(driver_desc->module, driver_desc->name);
      |                                    ~~~~~~~~~~~^~~~~~~~
      |                                               |
      |                                               struct module *
In file included from ./include/linux/device.h:31,
                 from ./include/linux/cdev.h:8,
                 from /var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.h:11,
                 from /var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.c:12:
./include/linux/device/class.h:230:54: note: expected ‘const char *’ but argument is of type ‘struct module *’
  230 | struct class * __must_check class_create(const char *name);
      |                                          ~~~~~~~~~~~~^~~~
/var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.c:1846:23: error: too many arguments to function ‘class_create’
 1846 |     internal->class = class_create(driver_desc->module, driver_desc->name);
      |                       ^~~~~~~~~~~~
In file included from ./include/linux/device.h:31,
                 from ./include/linux/cdev.h:8,
                 from /var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.h:11,
                 from /var/lib/dkms/gasket/0.0.git.256.b6a7c6d8/build/gasket_core.c:12:
./include/linux/device/class.h:230:29: note: declared here
  230 | struct class * __must_check class_create(const char *name);
      |                             ^~~~~~~~~~~~
cc1: some warnings being treated as errors

This is essentially the same issue as KyleGospo#7, only that it's now happening on RHEL. Again, it's these damned kernel backports.

Is there any way to rewrite this to be more flexible (i.e. not depend on a kernel version check)?

    /* Function signature for `class_create()` is changed in kernel >= 6.4.x
     * to only accept a single argument.
     * */
#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 4, 0)
    internal->class = class_create(driver_desc->module, driver_desc->name);
#else
    internal->class = class_create(driver_desc->name);
#endif

[cyqsimon]

I asked around and @gub11 over on ArchLinux discord suggested using dynamic function resolution.

gub11: Replied to your post on the arch server. Here’s a more in depth breakdown
Identify the target function's symbol name, which may vary based on the kernel version or other runtime factors.
Dynamically load the target library using dlopen
Resolve the function symbol using dlsym or GetProcAddress, obtaining a function pointer that can be used to call the function dynamically.
gub11: Not sure if this approach would work for your use case, but I hope it’s helpful

I have too little experience with C to be able to add meaningfully to this conversation, so I've pasted his comment here verbatim.

[robertzaage]

I will take a look. I think we have at least two occurrences of version based conditions, a thing I'm really no fan of. This always breaks on backports.

[cyqsimon]

Today I did some brief digging in /usr/src/kernels/ and stumbled upon this: /usr/src/kernels/5.14.0-427.20.1.el9_4.x86_64/include/generated/uapi/linux/version.h

#define LINUX_VERSION_CODE 331264
#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255 : (c)))
#define LINUX_VERSION_MAJOR 5
#define LINUX_VERSION_PATCHLEVEL 14
#define LINUX_VERSION_SUBLEVEL 0
#define RHEL_MAJOR 9
#define RHEL_MINOR 4
#define RHEL_RELEASE_VERSION(a,b) (((a) << 8) + (b))
#define RHEL_RELEASE_CODE 2308
#define RHEL_RELEASE "427.20.1"

It appears that although RedHat does not bump its kernel version, at least it has the decency to include its own set of macros for their custom release versioning scheme. I dislike hard-coded version-based conditional compilation stuff as well, but in lieu of a better solution, maybe we can use this to handle the backport instead?

And just to confirm that for this issue specifically, the breaking changes were backported in 9.4.

[cyqsimon]

Well, this is ugly but it does work:

diff --git a/src/gasket_core.c b/src/gasket_core.c
index b1c2726..70e6472 100644
--- a/src/gasket_core.c
+++ b/src/gasket_core.c
@@ -1840,12 +1840,14 @@ int gasket_register_device(const struct gasket_driver_desc *driver_desc)
 	memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX);
 
     /* Function signature for `class_create()` is changed in kernel >= 6.4.x
-     * to only accept a single argument.
+     * to only accept a single argument. This change was also backported to
+     * RHEL 9.4, whose kernel is nominally versioned 5.14.0.
      * */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 4, 0)
-    internal->class = class_create(driver_desc->module, driver_desc->name);
-#else
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0) || \
+    (defined RHEL_RELEASE_CODE && RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 4))
     internal->class = class_create(driver_desc->name);
+#else
+    internal->class = class_create(driver_desc->module, driver_desc->name);
 #endif
 
 	if (IS_ERR(internal->class)) {

Edit: I inverted the logic to make it perhaps a bit more intuitive.

[robertzaage]

This solves the RHEL backport problems for the moment, but other backported kernels would still fail. I sadly hadn't the time to work out a solution which doesn't rely on versions. I thought about using a template specialization, but it is a hen egg problem to guess if class_create needs one or more args.

[cyqsimon]

Yeah my patch sucks I know 🙁. I don't like it either but what else.

I'm curious though, what other notable distros do you know of that do kernel backports AND include breaking changes AND keeps the old version numbers? From the top of my head I actually can't think of any other than RHEL. As long as this practice is not very common, I think this is fine. There aren't many version-based conditional compilation stuff within this codebase anyways (3 in total I think).

[robertzaage]

Your patch doesnt suck, it works and solves a problem! 😄 Afaik at least the Proxmox VE kernel versions arent consistent either(?) This isn't the first time a version condition breaks in this driver. In a perfect world a package maintainer would care about the distro specific quirks...

[cyqsimon]

Yeah okay I'll make a PR for this I guess. Better than nothing.

[FrankelJb]

Are there any plans to merge this?