Changes in Linux 6.6

[CherineK]

Hi,

There have been some changes in Linux 6.6 breaking the build. Details here: https://elixir.bootlin.com/linux/v6.6.1/source/include/linux/tty_ldisc.h

I have tested with Linux 6.1.62 and 6.6.1, it builds and works fine.

Best regards, CK

[ppisa]

Thanks for testing and contribution update for newer kernels.

I agree generally with changes but I suggest to combine them in a single patch because the first patch introduces non-bisectable/broken state for bug hunting when some older stable kernel is used for testing where all versions should build.

I suggest to skip #if around netdev_dbg. Even change to %ul is not correct, correct would be for kernel %zu. But I suggest to stay away from #if where possible and use

netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %lu\n", (unsigned long)count);

which should be correct and without warning for all cases.

I will merge changes |or somebody other can) if these details are resolved and patch updated. But I am on busyness trip next week, so it can take week till my return.

[CherineK]

Thank you for your feedback.

I merged all my changes into a single commit.

I also removed the #if around netdev_dbg by using the code you suggested. It builds without warnings on Linux 6.1 and 6.6.

[ppisa]

Thanks. If I look again at the code I have another idea how to solve the problem to minimize code duplication. Sorry that I did not see that at the first glimpse. But if only type of count fled is changed, I would suggest to make change more compact.

What do you think about next solution which minimizes conditional sections to single one

size_t count
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
typedef size_t sllin_tty_count_t;
#else
typedef int sllin_tty_count_t;
#endif

then all functions can be updated to use sllin_tty_count_t

static void sllin_slave_receive_buf(struct tty_struct *tty,
                  const unsigned char *cp, const char *fp,  sllin_tty_count_t count);

That is not a peak of elegance but it would keep burden of maintenance lower.

[CherineK]

Unfortunately, it is not just the change from int to size_t that is problematic. There is also the change from char to u8 there:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
static void sllin_receive_buf(struct tty_struct *tty, const u8 *cp, const u8 *fp, size_t count)
#else
static void sllin_receive_buf(struct tty_struct *tty, const unsigned char *cp, sllin_receive_buf_fp_const char *fp, int count)
#endif

While looking at it a second time, there is actually a possible implicit conversion from u8 to char I missed there, so I guess these functions also need to be updated from char to u8 for Linux 6.6:

if (sl->lin_master)
    sllin_master_receive_buf(tty, cp, fp, count);
else
    sllin_slave_receive_buf(tty, cp, fp, count);

However, I do agree that your solution would lower the maintenance burden lower. Based on your solution (while adapting the function prototypes accordingly), this would be something like:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
typedef size_t sllin_tty_count_t;
typedef u8 sllin_tty_char_t;
typedef u8 sllin_tty_uchar_t;
#else
typedef int sllin_tty_count_t;
typedef char sllin_tty_char_t;
typedef unsigned char sllin_tty_uchar_t;
#endif

It is even less elegant, but still keeps only one #if.

How does that sound?

[ppisa]

Thanks for thinking about alternative approach.

I have no stronger opinion now, when more types have to be adjusted.

I would incline to minimize preprocessor ifs count. On the other hand, defining types and infrastructure for compatibility with multiple kernel versions is unacceptable for possible inclusion in mainline kernel in the future. So all such layers and ifdefs has to be removed and only support for the latest kernel should be presented then. May it be removing lines according to #if LINUX_VERSION_CODE ... is faster in such case.

I am ready to accept both approaches according to your and others preferences. Every feedback, opinion is welcomed.

[CherineK]

Here is the proposed patch for both options.

I tend to prefer the one with all the preprocessor ifs as it would make for an easy removal if such need arises and it makes data types more explicit. However, I can do both, so based on your feedback, I can modify the PR.

First case, without modifying the data types and keeping all preprocessor ifs:

--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -225,11 +225,17 @@ struct sllin {

 static struct net_device **sllin_devs;
 static int sllin_configure_frame_cache(struct sllin *sl, struct can_frame *cf);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+static void sllin_slave_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count);
+static void sllin_master_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count);
+#else
 static void sllin_slave_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count);
 static void sllin_master_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count);
-
+#endif

 /* Values of two parity bits in LIN Protected
    Identifier for each particular LIN ID */
@@ -667,8 +673,13 @@ static void sll_setup(struct net_device *dev)
 /******************************************
   Routines looking at TTY side.
  ******************************************/
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+static void sllin_master_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count)
+#else
 static void sllin_master_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count)
+#endif
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;

@@ -910,9 +921,13 @@ static void sllin_slave_finish_rx_msg(struct sllin *sl)
        sl->rx_len_unknown = false; /* We do know exact length of the header */
        sl->header_received = false;
 }
-
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+static void sllin_slave_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count)
+#else
 static void sllin_slave_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count)
+#endif
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;
        int lin_id;
@@ -1024,12 +1039,16 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 #define sllin_receive_buf_fp_const
 #endif

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+static void sllin_receive_buf(struct tty_struct *tty, const u8 *cp,
+                              const u8 *fp, size_t count)
+#else
 static void sllin_receive_buf(struct tty_struct *tty, const unsigned char *cp,
                               sllin_receive_buf_fp_const char *fp, int count)
+#endif
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;
-       netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
-
+       netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %lu\n", (unsigned long)count);
        if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
                return;

Second case, with the data type modification and only one preprocessor if:

--- a/sllin/sllin.c
+++ b/sllin/sllin.c
@@ -223,13 +223,22 @@ struct sllin {
 #endif
 };

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+typedef size_t sllin_tty_count_t;
+typedef u8 sllin_tty_char_t;
+typedef u8 sllin_tty_uchar_t;
+#else
+typedef int sllin_tty_count_t;
+typedef char sllin_tty_char_t;
+typedef unsigned char sllin_tty_uchar_t;
+#endif
+
 static struct net_device **sllin_devs;
 static int sllin_configure_frame_cache(struct sllin *sl, struct can_frame *cf);
 static void sllin_slave_receive_buf(struct tty_struct *tty,
-                             const unsigned char *cp, const char *fp, int count);
+                             const sllin_tty_uchar_t *cp, const sllin_tty_char_t *fp, sllin_tty_count_t count);
 static void sllin_master_receive_buf(struct tty_struct *tty,
-                             const unsigned char *cp, const char *fp, int count);
-
+                             const sllin_tty_uchar_t *cp, const sllin_tty_char_t *fp, sllin_tty_count_t count);

 /* Values of two parity bits in LIN Protected
    Identifier for each particular LIN ID */
@@ -668,7 +677,7 @@ static void sll_setup(struct net_device *dev)
   Routines looking at TTY side.
  ******************************************/
 static void sllin_master_receive_buf(struct tty_struct *tty,
-                             const unsigned char *cp, const char *fp, int count)
+                             const sllin_tty_uchar_t *cp, const sllin_tty_char_t *fp, sllin_tty_count_t count)
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;

@@ -912,7 +921,7 @@ static void sllin_slave_finish_rx_msg(struct sllin *sl)
 }

 static void sllin_slave_receive_buf(struct tty_struct *tty,
-                             const unsigned char *cp, const char *fp, int count)
+                             const sllin_tty_uchar_t *cp, const sllin_tty_char_t *fp, sllin_tty_count_t count)
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;
        int lin_id;
@@ -1024,12 +1033,11 @@ static void sllin_slave_receive_buf(struct tty_struct *tty,
 #define sllin_receive_buf_fp_const
 #endif

-static void sllin_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-                              sllin_receive_buf_fp_const char *fp, int count)
+static void sllin_receive_buf(struct tty_struct *tty, const sllin_tty_uchar_t *cp,
+                              const sllin_tty_char_t *fp, sllin_tty_count_t count)
 {
        struct sllin *sl = (struct sllin *) tty->disc_data;
-       netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %u\n", count);
-
+       netdev_dbg(sl->dev, "sllin_receive_buf invoked, count = %lu\n", (unsigned long)count);
        if (!sl || sl->magic != SLLIN_MAGIC || !netif_running(sl->dev))
                return;

[ppisa]

OK, I will apply pull request as it is with more ifdefs. I agree that it is easier to find what could be removed one day when support for version for 6.6.0 is no more required.

[ppisa]

Because it is single commit I have done simple rebased merge. Check that result is correct and switch to master for possible future updates.

Thanks much for patch preparation and cooperative conversation during decision about appropriate solution choice.

[CherineK]

I am sorry, I did not think you would merge the PR right away.

The change from char to u8 is missing in the commit for these two functions:

+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 6, 0)
+static void sllin_slave_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count);
+static void sllin_master_receive_buf(struct tty_struct *tty,
+                             const u8 *cp, const u8 *fp, size_t count);
+#else
 static void sllin_slave_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count);
 static void sllin_master_receive_buf(struct tty_struct *tty,
                              const unsigned char *cp, const char *fp, int count);
-
+#endif

I am truly sorry, how can I fix this?

[ppisa]

No problem, switch to master, please, and prepare new pull request. The intermediate state does not make troubles to versions before 6.6.0 and older caused warnings for 6.6.0+ anyway.