Problems compiling on kernel-5.3

[ewildgoose]

Hi, I had a few problems compiling on kernel 5.3. The following changes resolve this for me. However, I think that getnstimeofday64 has disappeared since kernel 5.0? I am not 100% sure - I just clicked through the linux headers and couldn't see it defined in 5.0-5.3...

--- a/src/lib/ndpi_kernel_compat.c +++ b/src/lib/ndpi_kernel_compat.c @@ -367,7 +367,7 @@ int atoi(const char *buf) { return atol(buf); }

-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,4,0) +#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,3,0) static inline void getnstimeofday64(struct timespec64 *ts) { ktime_get_real_ts64(ts); }

[ewildgoose]

oh dear. Insufficient testing... No, the above does not resolve the issue entirely.

OK, going back a step, attempting to compile the flows-3.2 branch under kernel 5.3 is leading to lots of errors about non defined: getnstimeofday()

Now, this was working as little as a few weeks back, but with a different build environment, so my guess is that what changed is that I upgraded my linux headers to match the kernel? In any case I need to either properly fix kernel_compat (patch below) OR I recommend that nDPI changes all references to ktime_get_real_ts64, since that seems supported since at least kernel 4.14 according to a claim here: https://github.com/ccp-project/ccp-kernel/issues/22

The patch below should make things build though, but obviously alters upstream code

diff --git a/src/include/ndpi_kernel_compat.h b/src/include/ndpi_kernel_compat.h
index 2d98e5dc..37dce390 100644
--- a/src/include/ndpi_kernel_compat.h
+++ b/src/include/ndpi_kernel_compat.h
@@ -17,6 +17,11 @@ inet_ntop (int af, const void *src, char *dst, socklen_t size);
 int inet_pton(int af, const char *src, void *dst);
 int atoi(const char *);
 long int atol(const char *);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,0,0)
+static inline void getnstimeofday64(struct timespec64 *ts) {
+                ktime_get_real_ts64(ts);
+}
+#endif
 void gettimeofday64(struct timespec64 *tv, void *tz);
 char *strtok_r(char *str, const char *delim, char **saveptr);

diff --git a/src/lib/ndpi_kernel_compat.c b/src/lib/ndpi_kernel_compat.c
index dc1da853..38923ec1 100644
--- a/src/lib/ndpi_kernel_compat.c
+++ b/src/lib/ndpi_kernel_compat.c
@@ -367,12 +367,6 @@ int atoi(const char *buf) {
    return atol(buf);
 }

-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,4,0)
-static inline void getnstimeofday64(struct timespec64 *ts) {
-           ktime_get_real_ts64(ts);
-}
-#endif
-
 void gettimeofday64(struct timespec64 *tv, void *tz) {
    getnstimeofday64(tv);
 }

[vel21ripn]

Thanks for the patch. It is strange that "static inline void getnstimeofday64()" was defined in src/lib/ndpi_kernel_compat.c and not in src/include/ndpi_kernel_compat.h. This is a clear mistake. 5.6 was the first core where they solved the problem of 2038. Problem 2038 is not yet resolved in all supported kernels :( Successfully compiling nDPI with all supported kernels turned out to be not quite a trivial task after the release of kernel 5.6. I ran into problems when building kernel modules if kernel-include was different from the currently running kernel. But these are problems with build scripts and "Makefiles" that use "/lib/modules/uname-r/build" by default. If you need to build ndpi-netfilter for a kernel other than the currently running kernel, then you need to specify "KERNEL_DIR="

[ewildgoose]

Great advice - thanks. Actually, I'm cross compiling, so need the KERNEL_DIR thing. I have a gentoo ebuild for anyone reading this?

What direction do you think to go with this? My thought would be for "your code", to change all references to: "ktime_get_real_ts64" and then if desired to support older kernels, to add back a compat function for that? This should support future kernels best, with compat back for older?

Do you want me to submit a patch for this? Note that I think upstream nDPI code also has a few references, to probably we ALSO need the compat.h workaround sigh...

[vel21ripn]

Note that I think upstream nDPI code also has a few references, to probably we ALSO need the compat.h workaround sigh...

I have already changed everything “gettimeofday()” to “gettimeofday64()" in the main project. Depending on the kernel version, "gettimeofday64()" is implemented either through "getnstimeofday64()" or through "ktime_get_real_ts64()". Perhaps we need to bring everything to one implementation. If there is a desire and time, then offer your patch.

[ewildgoose]

OK, I've looked into this further and I think the correct method would be as follows:

Note, I've done a couple of things here: a) used ktime_get_real_seconds() where possible. According to this link it's faster and accurate to the most recent timer tick (which seems acceptable for most of the use cases I could see?)

• https://www.kernel.org/doc/html/latest/core-api/timekeeping.html#c.ktime_get_real_ts64

I've also gone straight to ktime_get_real_ts64(), since as near as I can see using the kernel history, there is only one kernel revision different between when this call was introduced and when timespec64 was introduced. Therefore I don't think we are practically losing much compatibility (and also I think the code has now effectively become 3.17 as min compatibility anyway)

• https://elixir.bootlin.com/linux/v3.17.8/source/include/linux/timekeeping.h#L94

ktime_get_real_seconds is first defined in 3.19

• https://elixir.bootlin.com/linux/v3.19.8/source/include/linux/timekeeping.h#L32

So we have compatibility back to oct 2014. I guess if you want to support 3.17-3.19 then we need a compat macro for those kernels, but supporting 3.16 and earlier will require macros around timespec64 since they didn't exist? My vote would be to keep things simple and set 3.19 as min kernel now?

Note the https://elixir.bootlin.com is great for checking symbol introduction as it has a search box top right! Super!

edit: I realise now that the kernel_compat.c file isn't from upstream

diff --git a/ndpi-netfilter/src/main.c b/ndpi-netfilter/src/main.c
index 33132a7e..44d9235d 100644
--- a/ndpi-netfilter/src/main.c
+++ b/ndpi-netfilter/src/main.c
@@ -1242,7 +1242,7 @@ ndpi_mt(const struct sk_buff *skb, struct xt_action_param *par)

    COUNTER(ndpi_pk);

-   getnstimeofday64(&tm);
+   ktime_get_real_ts64(&tm);

    ct = nf_ct_get (skb, &ctinfo);
    if (ct == NULL) {
@@ -1820,7 +1820,7 @@ static void bt_port_gc(unsigned long data) {
         struct ndpi_net *n = (struct ndpi_net *)data;
 #endif
         struct ndpi_detection_module_struct *ndpi_struct = n->ndpi_struct;
-   struct timespec64 tm;
+   time64_t tm;
    uint32_t now32;
    int i;
    uint32_t st_j;
@@ -1830,8 +1830,8 @@ static void bt_port_gc(unsigned long data) {
    if(!read_trylock(&n->ndpi_busy)) return; // ndpi_net_exit() started!

    st_j = READ_ONCE(jiffies);
-   getnstimeofday64(&tm);
-   now32 = (uint32_t)tm.tv_sec; // BUG AFTER YAER 2105
+   tm=ktime_get_real_seconds();
+   now32 = (uint32_t)tm; // BUG AFTER YEAR 2038
    {
        struct hash_ip4p_table *ht = READ_ONCE(ndpi_struct->bt_ht);
        if(ht) {
diff --git a/ndpi-netfilter/src/ndpi_proc_flow.c b/ndpi-netfilter/src/ndpi_proc_flow.c
index c0638ca4..f03cdf45 100644
--- a/ndpi-netfilter/src/ndpi_proc_flow.c
+++ b/ndpi-netfilter/src/ndpi_proc_flow.c
@@ -20,11 +20,11 @@
 #include "ndpi_proc_generic.h"

 void nflow_proc_read_start(struct ndpi_net *n) {
-   struct timespec64 tm;
+   time64_t tm;

-   getnstimeofday64(&tm);
+   tm=ktime_get_real_seconds();
    n->acc_end  = 0;
-   n->acc_open_time = tm.tv_sec;
+   n->acc_open_time = tm;
    n->flow_l   = NULL;
    memset(n->str_buf,0,NF_STR_LBUF);
    n->str_buf_len = 0;
diff --git a/ndpi-netfilter/src/ndpi_proc_info.c b/ndpi-netfilter/src/ndpi_proc_info.c
index 3e6e08e0..538158f0 100644
--- a/ndpi-netfilter/src/ndpi_proc_info.c
+++ b/ndpi-netfilter/src/ndpi_proc_info.c
@@ -107,13 +107,13 @@ ssize_t _ninfo_proc_read(struct ndpi_net *n, char __user *buf,
    spin_lock_bh(&t->lock);
    do {
        struct hash_ip4p_node *x = t->top;
-       struct timespec64 tm;
+       time64_t tm;

-           getnstimeofday64(&tm);
+           tm=ktime_get_real_seconds();
        while(x && p < count - 128) {
                l =  inet_ntop_port(family,&x->ip,x->port,lbuf,sizeof(lbuf)-2);
            l += snprintf(&lbuf[l],sizeof(lbuf)-l-1, " %d %x %u\n",
-               (int)(tm.tv_sec - x->lchg),x->flag,x->count);
+               (int)(tm - x->lchg),x->flag,x->count);

            if (!(ACCESS_OK(VERIFY_WRITE, buf+p, l) &&
                !__copy_to_user(buf+p, lbuf, l))) return -EFAULT;
diff --git a/src/include/ndpi_kernel_compat.h b/src/include/ndpi_kernel_compat.h
index e1f23fbd..c194e226 100644
--- a/src/include/ndpi_kernel_compat.h
+++ b/src/include/ndpi_kernel_compat.h
@@ -18,12 +18,6 @@ int inet_pton(int af, const char *src, void *dst);
 int atoi(const char *);
 long int atol(const char *);

-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,3,0)
-static inline void getnstimeofday64(struct timespec64 *ts) {
-               ktime_get_real_ts64(ts);
-}
-#endif
-
 void gettimeofday64(struct timespec64 *tv, void *tz);
 char *strtok_r(char *str, const char *delim, char **saveptr);

diff --git a/src/lib/ndpi_kernel_compat.c b/src/lib/ndpi_kernel_compat.c
index 38923ec1..7c9f937a 100644
--- a/src/lib/ndpi_kernel_compat.c
+++ b/src/lib/ndpi_kernel_compat.c
@@ -368,7 +368,7 @@ int atoi(const char *buf) {
 }

 void gettimeofday64(struct timespec64 *tv, void *tz) {
-   getnstimeofday64(tv);
+   ktime_get_real_ts64(tv);
 }

 char *strtok_r (char *s, const char *delim, char **save_ptr)

[vel21ripn]

Thanks for the help. I see no reason to support kernels below 4.4. The ndpi_detection_process_packet () function gets the packet arrival time with an accuracy of 1 millisecond and the getnstimeofday64 () function was used for this. However, this exact time is not used anywhere else in the future. I believe that in our case, you can use ktime_get_real_seconds () everywhere.

[ewildgoose]

I forgot to add: did you see this function "ktime_get_coarse_real_ts64": https://www.kernel.org/doc/html/latest/core-api/timekeeping.html#c.ktime_get_coarse_real_ts64

I didn't study the one use case where we need ms accuracy to see if it was appropriate, but this function avoids checking the clock hardware and hence could be a benefit in the event of many packets per second. It would mean a reduction of accuracy in the time to nearest kernel clock tick (which could be every 10ms), so you would have less than 1ms accuracy.

I haven't tried to benchmark this, but I would appreciate your thought if it's an acceptable optimisation? How accurate do we need the packet timers here?

On a wider note, if we were reviewing time use here. I think our time will be affected by NTP adjusting the clock? Or leap seconds, etc? Also suspend and resume might go wrong? Should we consider if the "boottime" variant would work for this?

(My situation is that I have some devices without RTC clocks. So when you boot them, as they connect to the internet, the clock can suddenly jump forward several years as chrony notices that we need a big time update - don't think that this will cause catastrophic problems compared with a jump backwards though?)