-Werror=incompatible-pointer-types in Linux v5.5

[stuart-little]

With the very latest version 5.5 of the kernel I am getting

/root/repos/r8168/src/r8168_n.c: In function ‘rtl8168_proc_init’:
/root/repos/r8168/src/r8168_n.c:1670:47: error: passing argument 4 of ‘proc_create_data’ from incompatible pointer type [-Werror=incompatible-pointer-types]
                                               &rtl8168_proc_fops, f->show)) {
                                               ^
In file included from /root/repos/r8168/src/r8168_n.c:92:0:
./include/linux/proc_fs.h:59:31: note: expected ‘const struct proc_ops *’ but argument is of type ‘const struct file_operations *’
 extern struct proc_dir_entry *proc_create_data(const char *, umode_t,
                               ^~~~~~~~~~~~~~~~
/root/repos/r8168/src/r8168_n.c: At top level:
/root/repos/r8168/src/r8168_n.c:25821:31: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
         .ndo_tx_timeout     = rtl8168_tx_timeout,
                               ^~~~~~~~~~~~~~~~~~
/root/repos/r8168/src/r8168_n.c:25821:31: note: (near initialization for ‘rtl8168_netdev_ops.ndo_tx_timeout’)
cc1: some warnings being treated as errors
scripts/Makefile.build:265: recipe for target '/root/repos/r8168/src/r8168_n.o' failed
make[3]: *** [/root/repos/r8168/src/r8168_n.o] Error 1
Makefile:1681: recipe for target '/root/repos/r8168/src' failed
make[2]: *** [/root/repos/r8168/src] Error 2
make[2]: Leaving directory '/root/repos/linux'
Makefile:142: recipe for target 'modules' failed
make[1]: *** [modules] Error 2
make[1]: Leaving directory '/root/repos/r8168/src'
Makefile:40: recipe for target 'modules' failed
make: *** [modules] Error 2

It would appear a struct type has changed in /include/linux/proc_fs.h, from file_operations to proc_ops or some such thing.

[stuart-little]

Yep, here's the commit (in the Linux repo) that effected the change:

diff --git a/0640be56dcbd b/3dfa92633af3
index 0640be56dcbd..3dfa92633af3 100644
--- a/0640be56dcbd
+++ b/3dfa92633af3
@@ -12,6 +12,21 @@ struct proc_dir_entry;
 struct seq_file;
 struct seq_operations;
 
+struct proc_ops {
+       int     (*proc_open)(struct inode *, struct file *);
+       ssize_t (*proc_read)(struct file *, char __user *, size_t, loff_t *);
+       ssize_t (*proc_write)(struct file *, const char __user *, size_t, loff_t *);
+       loff_t  (*proc_lseek)(struct file *, loff_t, int);
+       int     (*proc_release)(struct inode *, struct file *);
+       __poll_t (*proc_poll)(struct file *, struct poll_table_struct *);
+       long    (*proc_ioctl)(struct file *, unsigned int, unsigned long);
+#ifdef CONFIG_COMPAT
+       long    (*proc_compat_ioctl)(struct file *, unsigned int, unsigned long);
+#endif
+       int     (*proc_mmap)(struct file *, struct vm_area_struct *);
+       unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
+};
+
 #ifdef CONFIG_PROC_FS
 
 typedef int (*proc_write_t)(struct file *, char *, size_t);
@@ -43,10 +58,10 @@ struct proc_dir_entry *proc_create_single_data(const char *name, umode_t mode,
  
 extern struct proc_dir_entry *proc_create_data(const char *, umode_t,
                                               struct proc_dir_entry *,
-                                              const struct file_operations *,
+                                              const struct proc_ops *,
                                               void *);
 
-struct proc_dir_entry *proc_create(const char *name, umode_t mode, struct proc_dir_entry *parent, const struct file_operations *proc_fops);
+struct proc_dir_entry *proc_create(const char *name, umode_t mode, struct proc_dir_entry *parent, const struct proc_ops *proc_ops);
 extern void proc_set_size(struct proc_dir_entry *, loff_t);
 extern void proc_set_user(struct proc_dir_entry *, kuid_t, kgid_t);
 extern void *PDE_DATA(const struct inode *);
@@ -108,8 +123,8 @@ static inline struct proc_dir_entry *proc_mkdir_mode(const char *name,
 #define proc_create_seq(name, mode, parent, ops) ({NULL;})
 #define proc_create_single(name, mode, parent, show) ({NULL;})
 #define proc_create_single_data(name, mode, parent, show, data) ({NULL;})
-#define proc_create(name, mode, parent, proc_fops) ({NULL;})
-#define proc_create_data(name, mode, parent, proc_fops, data) ({NULL;})
+#define proc_create(name, mode, parent, proc_ops) ({NULL;})
+#define proc_create_data(name, mode, parent, proc_ops, data) ({NULL;})
 
 static inline void proc_set_size(struct proc_dir_entry *de, loff_t size) {}
 static inline void proc_set_user(struct proc_dir_entry *de, kuid_t uid, kgid_t gid) {}

[stuart-little]

The module now builds, but I have not tested it; and one of the modifications was just a hack.

There were two issues:

(1) As noted above, commit 3dfa92633af3 in Linux replaced the file_operations struct with proc_operations.

(2) Additionally, there's

diff --git a/9ef20389622d b/30745068fb39
index 9ef20389622d..30745068fb39 100644
--- a/9ef20389622d
+++ b/30745068fb39
@@ -1014,7 +1014,7 @@ int netdev_name_node_alt_destroy(struct net_device *dev, const char *name);
  *     Called when a user wants to change the Maximum Transfer Unit
  *     of a device.
  *
- * void (*ndo_tx_timeout)(struct net_device *dev);
+ * void (*ndo_tx_timeout)(struct net_device *dev, unsigned int txqueue);
  *     Callback used when the transmitter has not made any progress
  *     for dev->watchdog ticks.
  *
@@ -1281,7 +1281,8 @@ struct net_device_ops {
                                                  int new_mtu);
        int                     (*ndo_neigh_setup)(struct net_device *dev,
                                                   struct neigh_parms *);
-       void                    (*ndo_tx_timeout) (struct net_device *dev);
+       void                    (*ndo_tx_timeout) (struct net_device *dev,
+                                                  unsigned int txqueue);
 
        void                    (*ndo_get_stats64)(struct net_device *dev,
                                                   struct rtnl_link_stats64 *storage);

in ./include/linux/netdevice.h, which was causing the separate rtl8168_tx_timeout error.

r8168 now builds, having made the following changes:

Date:   Fri Feb 7 14:57:39 2020 +0000

    file_operations -> proc_ops in ./include/linux/proc_fs.h and also void (*ndo_tx_timeout)(struct net_device *dev) -> void (*ndo_tx_timeout)(struct net_device *dev, unsigned int txqueue) in ./include/linux/netdevice.h

diff --git a/src/r8168_n.c b/src/r8168_n.c
index 0df6041..d968099 100755
--- a/src/r8168_n.c
+++ b/src/r8168_n.c
@@ -456,7 +456,7 @@ static void rtl8168_hw_config(struct net_device *dev);
 static void rtl8168_hw_start(struct net_device *dev);
 static int rtl8168_close(struct net_device *dev);
 static void rtl8168_set_rx_mode(struct net_device *dev);
-static void rtl8168_tx_timeout(struct net_device *dev);
+static void rtl8168_tx_timeout(struct net_device *dev, unsigned int txqueue);
 static struct net_device_stats *rtl8168_get_stats(struct net_device *dev);
 static int rtl8168_rx_interrupt(struct net_device *, struct rtl8168_private *, napi_budget);
 static int rtl8168_change_mtu(struct net_device *dev, int new_mtu);
@@ -1616,11 +1616,11 @@ static int rtl8168_proc_open(struct inode *inode, struct file *file)
         return single_open(file, show, dev);
 }
 
-static const struct file_operations rtl8168_proc_fops = {
-        .open           = rtl8168_proc_open,
-        .read           = seq_read,
-        .llseek         = seq_lseek,
-        .release        = single_release,
+static const struct proc_ops rtl8168_proc_fops = {
+        .proc_open           = rtl8168_proc_open,
+        .proc_read           = seq_read,
+        .proc_lseek         = seq_lseek,
+        .proc_release        = single_release,
 };
 #endif
 
@@ -27844,7 +27844,7 @@ static void rtl8168_reset_task(struct work_struct *work)
 }
 
 static void
-rtl8168_tx_timeout(struct net_device *dev)
+  rtl8168_tx_timeout(struct net_device *dev, unsigned int txqueue)
 {
         struct rtl8168_private *tp = netdev_priv(dev);
         unsigned long flags;

Again, I have not tested the module after building, and in rtl8168_tx_timeout I simply added the extra argument unsigned int txqueue because I saw it in the Linux source, with no further use for it.

---

Edit:

Have now tested: it works fine, applying the patch pasted above.

[petehg]

Thanks for this. I'd got the struct prop_ops stuff but I couldn't work out why the tx_timeout was wrong. I applied that change to just the driver file (r8168_n.c - I'm running k5.6-rc1 on one of my systems). It works for the rc kernel as well, I can confirm.
So, thanks again.

[jonnyprouty]

Apologies in advance if this isn't the correct place to report this.

This bug also effects v5.8.0 of the kernel, but the patch provided by @stuart-little resolves the issue. The patch applied cleanly (offset a few lines though) to v8.048.00, the version packaged in the r8168-dkms package provided for Ubuntu 20.04 LTS.