* rose timer t error displayed in /proc/net/rose @ 2022-07-30 15:08 Bernard f6bvp 2022-08-01 0:39 ` Francois Romieu 0 siblings, 1 reply; 12+ messages in thread From: Bernard f6bvp @ 2022-07-30 15:08 UTC (permalink / raw) To: Eric Dumazet; +Cc: linux-hams, Thomas Osterried DL9SAU, netdev Rose proc timer t error Timer t is decremented one by one during normal operations. When decreasing from 1 to 0 it displays a very large number until next clock tic as demonstrated below. t1, t2 and t3 are correctly handled. root@ubuntu-f6bvp:/home/bernard# cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 002 00001 2 0 0 0 6 200 180 180 5 0/000 0 0 68541 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68448 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68447 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68433 root@ubuntu-f6bvp:/home/bernard# cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 002 00001 2 0 0 0 73786976294838206 200 180 180 5 0/000 0 0 68541 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68448 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68447 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68433 root@ubuntu-f6bvp:/home/bernard# cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68448 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68447 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 68433 Bernard ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-07-30 15:08 rose timer t error displayed in /proc/net/rose Bernard f6bvp @ 2022-08-01 0:39 ` Francois Romieu 2022-08-01 8:06 ` Thomas Osterried ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Francois Romieu @ 2022-08-01 0:39 UTC (permalink / raw) To: Bernard f6bvp; +Cc: Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev Bernard f6bvp <f6bvp@free.fr> : > Rose proc timer t error > > Timer t is decremented one by one during normal operations. > > When decreasing from 1 to 0 it displays a very large number until next clock > tic as demonstrated below. > > t1, t2 and t3 are correctly handled. "t" is ax25_display_timer(&rose->timer) / HZ whereas "tX" are rose->tX / HZ. ax25_display_timer() does not like jiffies > timer->expires (and it should probably return plain seconds btw). You may try the hack below. diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c index 85865ebfdfa2..b77433fff0c9 100644 --- a/net/ax25/ax25_timer.c +++ b/net/ax25/ax25_timer.c @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) unsigned long ax25_display_timer(struct timer_list *timer) { - if (!timer_pending(timer)) - return 0; + long delta = timer->expires - jiffies; - return timer->expires - jiffies; + return jiffies_delta_to_clock_t(delta) * HZ; } EXPORT_SYMBOL(ax25_display_timer); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 0:39 ` Francois Romieu @ 2022-08-01 8:06 ` Thomas Osterried 2022-08-01 9:19 ` Bernard f6bvp 2022-08-01 15:44 ` Francois Romieu 2022-08-01 8:54 ` rose timer t error displayed in /proc/net/rose Bernard f6bvp 2022-08-01 10:43 ` Bernard f6bvp 2 siblings, 2 replies; 12+ messages in thread From: Thomas Osterried @ 2022-08-01 8:06 UTC (permalink / raw) To: Francois Romieu Cc: Bernard f6bvp, Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev Hello, > Am 01.08.2022 um 02:39 schrieb Francois Romieu <romieu@fr.zoreil.com>: > > Bernard f6bvp <f6bvp@free.fr> : >> Rose proc timer t error >> >> Timer t is decremented one by one during normal operations. >> >> When decreasing from 1 to 0 it displays a very large number until next clock >> tic as demonstrated below. >> >> t1, t2 and t3 are correctly handled. > > "t" is ax25_display_timer(&rose->timer) / HZ whereas "tX" are rose->tX / HZ. > > ax25_display_timer() does not like jiffies > timer->expires (and it should > probably return plain seconds btw). > > You may try the hack below. > > diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c > index 85865ebfdfa2..b77433fff0c9 100644 > --- a/net/ax25/ax25_timer.c > +++ b/net/ax25/ax25_timer.c > @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) > > unsigned long ax25_display_timer(struct timer_list *timer) > { > - if (!timer_pending(timer)) > - return 0; > + long delta = timer->expires - jiffies; > > - return timer->expires - jiffies; > + return jiffies_delta_to_clock_t(delta) * HZ; > } > > EXPORT_SYMBOL(ax25_display_timer); 1. why do you check for pending timer anymore? 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ? jiffies_delta_to_clock_t() returns clock_t. ax25_display_timer() is used in ax25, netrom and rose, mostly for displaying states in /proc. In ax25_subr.c, ax25_calculate_rtt() it is used for rtt calculation. Is it proven that the the values that ax25_display_timer returns are still as expected? I ask, because we see there a substraction ax25_display_timer(&ax25->t1timer) from ax25->t1, and we need to be sure, that your change will not break he ax.25 stack. # grep ax25_display_timer ../*/*c|cut -d/ -f2- ax25/af_ax25.c: ax25_info.t1timer = ax25_display_timer(&ax25->t1timer) / HZ; ax25/af_ax25.c: ax25_info.t2timer = ax25_display_timer(&ax25->t2timer) / HZ; ax25/af_ax25.c: ax25_info.t3timer = ax25_display_timer(&ax25->t3timer) / HZ; ax25/af_ax25.c: ax25_info.idletimer = ax25_display_timer(&ax25->idletimer) / (60 * HZ); ax25/af_ax25.c: ax25_display_timer(&ax25->t1timer) / HZ, ax25->t1 / HZ, ax25/af_ax25.c: ax25_display_timer(&ax25->t2timer) / HZ, ax25->t2 / HZ, ax25/af_ax25.c: ax25_display_timer(&ax25->t3timer) / HZ, ax25->t3 / HZ, ax25/af_ax25.c: ax25_display_timer(&ax25->idletimer) / (60 * HZ), ax25/ax25.mod.c:SYMBOL_CRC(ax25_display_timer, 0x14cecd59, ""); ax25/ax25_subr.c: ax25->rtt = (9 * ax25->rtt + ax25->t1 - ax25_display_timer(&ax25->t1timer)) / 10; ax25/ax25_timer.c:unsigned long ax25_display_timer(struct timer_list *timer) ax25/ax25_timer.c:EXPORT_SYMBOL(ax25_display_timer); netrom/af_netrom.c: ax25_display_timer(&nr->t1timer) / HZ, netrom/af_netrom.c: ax25_display_timer(&nr->t2timer) / HZ, netrom/af_netrom.c: ax25_display_timer(&nr->t4timer) / HZ, netrom/af_netrom.c: ax25_display_timer(&nr->idletimer) / (60 * HZ), netrom/netrom.mod.c: { 0x14cecd59, "ax25_display_timer" }, rose/af_rose.c: ax25_display_timer(&rose->timer) / HZ, rose/af_rose.c: ax25_display_timer(&rose->idletimer) / (60 * HZ), rose/rose.mod.c: { 0x14cecd59, "ax25_display_timer" }, rose/rose_route.c: ax25_display_timer(&rose_neigh->t0timer) / HZ, rose/rose_route.c: ax25_display_timer(&rose_neigh->ftimer) / HZ); 3. Back to the initial problem: >> When decreasing from 1 to 0 it displays a very large number until next clock >> tic as demonstrated below. I assume it's the information when timer for rose expired. If it has been expired 1s ago, the computed time diff diff becomes negative -> -(jiffies). We are unsigned long (and imho need to b), but the "underflow" result is something like (2**64)-1-jiffies -- a very large positive number that represents a small negative number. => If my assumptions for rose behavior are correct: 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug? 2. The time window with that large value is large. 3. Are negative numbers (-> timer expired) are of interest? Else, 0 should be enough to indicate that the timer has expired. linux/jiffies.h: extern clock_t jiffies_to_clock_t(unsigned long x); static inline clock_t jiffies_delta_to_clock_t(long delta) { return jiffies_to_clock_t(max(0L, delta)); } => Negative may be handled due to Francois' patch now correctly. delta as signed long may be negative. max(0L, -nnnn) sould result to 0L. This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 8:06 ` Thomas Osterried @ 2022-08-01 9:19 ` Bernard f6bvp 2022-08-01 15:44 ` Francois Romieu 1 sibling, 0 replies; 12+ messages in thread From: Bernard f6bvp @ 2022-08-01 9:19 UTC (permalink / raw) To: Thomas Osterried, Francois Romieu Cc: Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev The reason I am looking at /proc/net/rose is for I want to check if a rose connect request is pending. I am investigating a bug in rose socket connect that was ok until kernel 5.4.79 and occured with kernel 5.4.83. All ROSE network client applications I am playing with (fpad, fpacwpd, wplist, wpedit...) need connect rose socket and are failing since that kernel bug. The connect request triggers rose->timer from 200 until it counts down to 0 and fails due to timeout. This let me think that there is a bug somewhere in library that handles rose socket ? I have been told to perform a kernel git bisect from 5.4.79 (good) to 5.4.83 (bad) but I am facing difficulties rebooting after kernel is compiled on my Ubuntu machine. The reason is that with Ubuntu boot is very complex to me. I can see that new vmlinuz is present in /boot but boot process is rather using /boot/efi/EFI/ that I don't know how to manage... rw-r--r-- 1 root root 173964 Jul 29 19:04 config-5.18.11-F6BVP-4 -rw-r--r-- 1 root root 176329 Aug 1 10:44 config-5.19.0-rc8-next-20220728-F6BVP-next-patchs+ drwx------ 3 root root 4096 Jan 1 1970 efi/ drwxr-xr-x 5 root root 4096 Aug 1 10:44 grub/ lrwxrwxrwx 1 root root 54 Jul 30 10:46 initrd.img -> initrd.img-5.19.0-rc8-next-20220728-F6BVP-next-patchs+ -rw-r--r-- 1 root root 62359030 Jul 26 17:38 initrd.img-5.15.0-41-generic -rw-r--r-- 1 root root 17810668 Jul 29 19:04 initrd.img-5.18.11-F6BVP-4 -rw-r--r-- 1 root root 18144144 Aug 1 10:44 initrd.img-5.19.0-rc8-next-20220728-F6BVP-next-patchs+ -rw-r--r-- 1 root root 182800 Feb 6 21:35 memtest86+.bin -rw-r--r-- 1 root root 184476 Feb 6 21:35 memtest86+.elf -rw-r--r-- 1 root root 184980 Feb 6 21:35 memtest86+_multiboot.bin lrwxrwxrwx 1 root root 51 Aug 1 10:44 vmlinuz -> vmlinuz-5.19.0-rc8-next-20220728-F6BVP-next-patchs+ -rw------- 1 root root 11086240 Jun 22 15:24 vmlinuz-5.15.0-41-generic -rw-r--r-- 1 root root 9686976 Jul 29 19:04 vmlinuz-5.18.11-F6BVP-4 -rw-r--r-- 1 root root 10211648 Aug 1 10:44 vmlinuz-5.19.0-rc8-next-20220728-F6BVP-next-patchs+ -rw-r--r-- 1 root root 10211648 Jul 30 10:46 vmlinuz-5.19.0-rc8-next-20220728-F6BVP-next-patchs+.old lrwxrwxrwx 1 root root 55 Aug 1 10:44 vmlinuz.old -> vmlinuz-5.19.0-rc8-next-20220728-F6BVP-next-patchs+.old Le 01/08/2022 à 10:06, Thomas Osterried a écrit : > Hello, > > 1. why do you check for pending timer anymore? > 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ? > jiffies_delta_to_clock_t() returns clock_t. > > ax25_display_timer() is used in ax25, netrom and rose, mostly for displaying states in /proc. > > In ax25_subr.c, ax25_calculate_rtt() it is used for rtt calculation. Is it proven that the the values that ax25_display_timer returns are still as expected? > I ask, because we see there a substraction ax25_display_timer(&ax25->t1timer) from ax25->t1, and we need to be sure, that your change will not break he ax.25 stack. > > # grep ax25_display_timer ../*/*c|cut -d/ -f2- > ax25/af_ax25.c: ax25_info.t1timer = ax25_display_timer(&ax25->t1timer) / HZ; > ax25/af_ax25.c: ax25_info.t2timer = ax25_display_timer(&ax25->t2timer) / HZ; > ax25/af_ax25.c: ax25_info.t3timer = ax25_display_timer(&ax25->t3timer) / HZ; > ax25/af_ax25.c: ax25_info.idletimer = ax25_display_timer(&ax25->idletimer) / (60 * HZ); > ax25/af_ax25.c: ax25_display_timer(&ax25->t1timer) / HZ, ax25->t1 / HZ, > ax25/af_ax25.c: ax25_display_timer(&ax25->t2timer) / HZ, ax25->t2 / HZ, > ax25/af_ax25.c: ax25_display_timer(&ax25->t3timer) / HZ, ax25->t3 / HZ, > ax25/af_ax25.c: ax25_display_timer(&ax25->idletimer) / (60 * HZ), > ax25/ax25.mod.c:SYMBOL_CRC(ax25_display_timer, 0x14cecd59, ""); > ax25/ax25_subr.c: ax25->rtt = (9 * ax25->rtt + ax25->t1 - ax25_display_timer(&ax25->t1timer)) / 10; > ax25/ax25_timer.c:unsigned long ax25_display_timer(struct timer_list *timer) > ax25/ax25_timer.c:EXPORT_SYMBOL(ax25_display_timer); > netrom/af_netrom.c: ax25_display_timer(&nr->t1timer) / HZ, > netrom/af_netrom.c: ax25_display_timer(&nr->t2timer) / HZ, > netrom/af_netrom.c: ax25_display_timer(&nr->t4timer) / HZ, > netrom/af_netrom.c: ax25_display_timer(&nr->idletimer) / (60 * HZ), > netrom/netrom.mod.c: { 0x14cecd59, "ax25_display_timer" }, > rose/af_rose.c: ax25_display_timer(&rose->timer) / HZ, > rose/af_rose.c: ax25_display_timer(&rose->idletimer) / (60 * HZ), > rose/rose.mod.c: { 0x14cecd59, "ax25_display_timer" }, > rose/rose_route.c: ax25_display_timer(&rose_neigh->t0timer) / HZ, > rose/rose_route.c: ax25_display_timer(&rose_neigh->ftimer) / HZ); > > > 3. Back to the initial problem: > >>> When decreasing from 1 to 0 it displays a very large number until next clock >>> tic as demonstrated below. > I assume it's the information when timer for rose expired. > If it has been expired 1s ago, the computed time diff diff becomes negative -> -(jiffies). > We are unsigned long (and imho need to b), but the "underflow" result is something like (2**64)-1-jiffies -- a very large positive number that represents a small negative number. > > => If my assumptions for rose behavior are correct: > 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug? > 2. The time window with that large value is large. > 3. Are negative numbers (-> timer expired) are of interest? Else, 0 should be enough to indicate that the timer has expired. > linux/jiffies.h: > extern clock_t jiffies_to_clock_t(unsigned long x); > static inline clock_t jiffies_delta_to_clock_t(long delta) > { > return jiffies_to_clock_t(max(0L, delta)); > } > > => Negative may be handled due to Francois' patch now correctly. delta as signed long may be negative. max(0L, -nnnn) sould result to 0L. > This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed? > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 8:06 ` Thomas Osterried 2022-08-01 9:19 ` Bernard f6bvp @ 2022-08-01 15:44 ` Francois Romieu 2022-08-01 19:06 ` Thomas Osterried ` (3 more replies) 1 sibling, 4 replies; 12+ messages in thread From: Francois Romieu @ 2022-08-01 15:44 UTC (permalink / raw) To: Thomas Osterried Cc: Bernard f6bvp, Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev [-- Attachment #1: Type: text/plain, Size: 1917 bytes --] Thomas Osterried <thomas@osterried.de> : [...] > 1. why do you check for pending timer anymore? s/do/don't/ ? I don't check for pending timer because: - the check is racy - jiffies_delta_to_clock_t maxes negative delta to zero > 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ? > jiffies_delta_to_clock_t() returns clock_t. I completely messed this part :o/ clock_t relates to USER_HZ like jiffies does to HZ. [don't break the ax25] Sure, we are in violent agreement here. [...] > 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug? The relevant rose state in Bernard's sample was ROSE_STATE_2. net/rose/rose_timer.c::rose_timer_expiry would had called rose_disconnect(sk, ETIMEDOUT, -1, -1) so there should not be any timer restart (afaiu, etc.). [...] > => Negative may be handled due to Francois' patch now correctly. > delta as signed long may be negative. max(0L, -nnnn) sould result to 0L. > This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed? I must confess that I was not terribly professional this morning past 2AM. The attached snippet illustrates the behavior wrt negative values (make; insmod foo.ko ; sleep 1; rmmod foo.ko; dmesg | tail -n 2). It also illustrates that I got the unit wrong. This should be better: diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c index 85865ebfdfa2..3c94e5a2d098 100644 --- a/net/ax25/ax25_timer.c +++ b/net/ax25/ax25_timer.c @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) unsigned long ax25_display_timer(struct timer_list *timer) { - if (!timer_pending(timer)) - return 0; + long delta = timer->expires - jiffies; - return timer->expires - jiffies; + return max(0L, delta); } EXPORT_SYMBOL(ax25_display_timer); [-- Attachment #2: Makefile --] [-- Type: text/plain, Size: 129 bytes --] obj-m := foo.o KDIR := /lib/modules/$(shell uname -r)/build PWD := $(shell pwd) default: $(MAKE) -C $(KDIR) M=$(PWD) modules [-- Attachment #3: foo.c --] [-- Type: text/plain, Size: 512 bytes --] #include <linux/module.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/jiffies.h> static unsigned long expires; static void bar(void) { long delta = expires - jiffies; printk(KERN_INFO "%lu %lu %lx\n", jiffies_delta_to_clock_t(delta), jiffies_delta_to_clock_t(jiffies), delta); } static int __init foo_start(void) { expires = jiffies; bar(); return 0; } static void __exit foo_end(void) { bar(); } module_init(foo_start); module_exit(foo_end); MODULE_LICENSE("GPL"); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 15:44 ` Francois Romieu @ 2022-08-01 19:06 ` Thomas Osterried 2022-08-01 19:33 ` Bernard f6bvp ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Thomas Osterried @ 2022-08-01 19:06 UTC (permalink / raw) To: Francois Romieu Cc: Bernard f6bvp, Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev > Am 01.08.2022 um 17:44 schrieb Francois Romieu <romieu@fr.zoreil.com>: > > Thomas Osterried <thomas@osterried.de> : > [...] >> 1. why do you check for pending timer anymore? > > s/do/don't/ ? Yes, don't; sorry ;) > I don't check for pending timer because: > - the check is racy Just arguing: If no timer is running, we previously returned 0: if (!timer_pending(timer)) return 0; If we omit this check, timer->expires is the jiffies of last timer expiration. What if jiffies overflows due to long uptime? Example: if jiffies is at "maxvalue - 100" and timer is set to i.e. jiffies+10000, timer->expires is set to 9900 (overflow of the unsigned long). Original code: return timer->expires - jiffies; == return 9900-(maxvalue - 100) -> large negative value, but our small positive difference, because we are in unsigned long. But original code bugs (just found it out) if expires is i.e. 100 and jiffies i.E. 150: then we get a large unsigned result. => This always will happen if you cat /proc and timer has just expired and the new timer timerhas not started. if (!timer_pending(timer)) prevented that false value. Your patch makes this good. Prove: include <stdio.h> int main() { unsigned long foo = 0; foo = foo -100; unsigned long bar = foo + 10000; unsigned long foo2 = foo+50; printf("jifffiessWhenTimerSet %lu; timerExpires(jifffiessWhenTimerSet+10000) %lu; jifffiessWhenTimerSet+50 %lu, diff to expiry %lu\n", foo, bar, foo2, bar-foo2); long delta = bar-foo2; printf("with signed long delta:\n"); printf("jifffiessWhenTimerSet %lu; timerExpires(jifffiessWhenTimerSet+10000) %lu; jifffiessWhenTimerSet+50 %lu, diff to expiry %ld\n", foo, bar, foo2, delta); foo2 = foo2 + 10000; printf("jiffies += 10000 = %lu:\n", foo2); printf("jifffiessWhenTimerSet %lu; timerExpires(jifffiessWhenTimerSet+10000) %lu; jifffiessWhenTimerSet+50+10000 %lu, diff to expiry %lu\n", foo, bar, foo2, bar-foo2); delta = bar-foo2; printf("with signed long delta:\n"); printf("jifffiessWhenTimerSet %lu; timerExpires(jifffiessWhenTimerSet+10000) %lu; jifffiessWhenTimerSet+50+10000 %lu, diff to expiry %ld\n", foo, bar, foo2, delta); } jifffiessWhenTimerSet 18446744073709551516; timerExpires(jifffiessWhenTimerSet+10000) 9900; jifffiessWhenTimerSet+50 18446744073709551566, diff to expiry 9950 with signed long delta: jifffiessWhenTimerSet 18446744073709551516; timerExpires(jifffiessWhenTimerSet+10000) 9900; jifffiessWhenTimerSet+50 18446744073709551566, diff to expiry 9950 jiffies += 10000 = 9950: jifffiessWhenTimerSet 18446744073709551516; timerExpires(jifffiessWhenTimerSet+10000) 9900; jifffiessWhenTimerSet+50+10000 9950, diff to expiry 18446744073709551566 with signed long delta: jifffiessWhenTimerSet 18446744073709551516; timerExpires(jifffiessWhenTimerSet+10000) 9900; jifffiessWhenTimerSet+50+10000 9950, diff to expiry -50 => ;)) Nevertheless, I feel better to return 0 if no timer is running, than computing on a timer with an ancient timer->expiry. Because in some time gap, we might get a positive result and display that - but since timer is not running the result should be 0. You argued on timer_pending(timer): "the check is racy". Perhaps. But the time window is small: you do a cat /proc/... while timer is stopped. you check timer_pending(timer) -> false. other cpu starts the timer. you'll returrn 0 instead of maxtimervalue. If you'd issued the command a few microseconds before or after, the value would be correct. This is, imho, absolutely ok. But if timer is stopped at jiffies = maxvalue-100, and we now are at jffies == 100, then we get with maxvalue-100-100 a very large number (instead of 0 - timer is not running. And this will happen the next 49 days (on 64bit). I hope I've not overseen / misinterpreted something. But as far as I can see, a propably racy timer_pending(timer) is better than the risk of returning numbers - where user thinks it's a running timer, but it's from a previously stopped one - that may become very high > - jiffies_delta_to_clock_t maxes negative delta to zero I also came to this opinion for jiffies_delta_to_clock_t; but Bernard posted this morning he tested, and still got negative values. > >> 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ? >> jiffies_delta_to_clock_t() returns clock_t. > > I completely messed this part :o/ > > clock_t relates to USER_HZ like jiffies does to HZ. > > [don't break the ax25] > > Sure, we are in violent agreement here. > > [...] >> 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug? > > The relevant rose state in Bernard's sample was ROSE_STATE_2. > > net/rose/rose_timer.c::rose_timer_expiry would had called > rose_disconnect(sk, ETIMEDOUT, -1, -1) so there should not > be any timer restart (afaiu, etc.). > > [...] >> => Negative may be handled due to Francois' patch now correctly. >> delta as signed long may be negative. max(0L, -nnnn) sould result to 0L. >> This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed? > > I must confess that I was not terribly professional this morning past 2AM. > > The attached snippet illustrates the behavior wrt negative values > (make; insmod foo.ko ; sleep 1; rmmod foo.ko; dmesg | tail -n 2). > It also illustrates that I got the unit wrong. ;)tnx > > This should be better: > > diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c > index 85865ebfdfa2..3c94e5a2d098 100644 > --- a/net/ax25/ax25_timer.c > +++ b/net/ax25/ax25_timer.c > @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) > > unsigned long ax25_display_timer(struct timer_list *timer) > { > - if (!timer_pending(timer)) > - return 0; > + long delta = timer->expires - jiffies; > > - return timer->expires - jiffies; > + return max(0L, delta); > } > > EXPORT_SYMBOL(ax25_display_timer); I'd prefere to keep the !timer_pending(timer)) check. Anyone else on the list has an opinion about this? vy 73, - Thomas dl9sau > > <Makefile.txt><foo.c> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 15:44 ` Francois Romieu 2022-08-01 19:06 ` Thomas Osterried @ 2022-08-01 19:33 ` Bernard f6bvp 2022-08-07 18:04 ` [PATCH] AX25 rose_call - replacing carriage return by newlines f6bvp 2022-08-07 18:21 ` Resend : " f6bvp 3 siblings, 0 replies; 12+ messages in thread From: Bernard f6bvp @ 2022-08-01 19:33 UTC (permalink / raw) To: Francois Romieu, Thomas Osterried Cc: Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev François, your new patch is working fine in both state 1 and 2 of rose. Congrats and thanks ! By the way, it is a very old display issue... Already present in 5.4.79 (32 bytes OS) for example and probably since much earlier. Le 01/08/2022 à 17:44, Francois Romieu a écrit : > Thomas Osterried <thomas@osterried.de> : > [...] >> 1. why do you check for pending timer anymore? > s/do/don't/ ? > > I don't check for pending timer because: > - the check is racy > - jiffies_delta_to_clock_t maxes negative delta to zero > >> 2. I'm not really sure what value jiffies_delta_to_clock_t() returns. jiffies / HZ? >> jiffies_delta_to_clock_t() returns clock_t. > I completely messed this part :o/ > > clock_t relates to USER_HZ like jiffies does to HZ. > > [don't break the ax25] > > Sure, we are in violent agreement here. > > [...] >> 1. I expect rose->timer to be restarted soon. If it does not happen, is there a bug? > The relevant rose state in Bernard's sample was ROSE_STATE_2. > > net/rose/rose_timer.c::rose_timer_expiry would had called > rose_disconnect(sk, ETIMEDOUT, -1, -1) so there should not > be any timer restart (afaiu, etc.). > > [...] >> => Negative may be handled due to Francois' patch now correctly. >> delta as signed long may be negative. max(0L, -nnnn) sould result to 0L. >> This would result to 0. Perhaps proven by Francois, because he used this function and achieved a correct display of that idle value. Francois, am I correct, is "0" really displayed? > I must confess that I was not terribly professional this morning past 2AM. > > The attached snippet illustrates the behavior wrt negative values > (make; insmod foo.ko ; sleep 1; rmmod foo.ko; dmesg | tail -n 2). > It also illustrates that I got the unit wrong. > > This should be better: > > diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c > index 85865ebfdfa2..3c94e5a2d098 100644 > --- a/net/ax25/ax25_timer.c > +++ b/net/ax25/ax25_timer.c > @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) > > unsigned long ax25_display_timer(struct timer_list *timer) > { > - if (!timer_pending(timer)) > - return 0; > + long delta = timer->expires - jiffies; > > - return timer->expires - jiffies; > + return max(0L, delta); > } > > EXPORT_SYMBOL(ax25_display_timer); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] AX25 rose_call - replacing carriage return by newlines 2022-08-01 15:44 ` Francois Romieu 2022-08-01 19:06 ` Thomas Osterried 2022-08-01 19:33 ` Bernard f6bvp @ 2022-08-07 18:04 ` f6bvp 2022-08-07 18:21 ` Resend : " f6bvp 3 siblings, 0 replies; 12+ messages in thread From: f6bvp @ 2022-08-07 18:04 UTC (permalink / raw) To: linux-hams; +Cc: Thomas Osterried DL9SAU, netdev, Francois Romieu [-- Attachment #1: Type: text/plain, Size: 1290 bytes --] [PATCH] AX25 rose_call - replacing carriage return by newlines I have been using intensively rose_call application (part of ax25tools/user_call library) while debugging rose connect issue. However once connected rose_call displays remote message without linefeed. Consequently it is impossible to read messages. For example calling local node : # rose_call rose0 f6bvp f6bvp-4 2080175524 Connecting to f6bvp-4 @ 2080175524 ... *** Connected F6BVP-4 (Commands = ?) : Aug 5 2022) for LINUX (help = h) Then issuing command P to the connected local node, all answer lines are superimposed. F6BVP-4 (Commands = ?) : Switch Port Now with the proposed patch is the complete info displayed: # ./rose_call rose0 f6bvp f6bvp-4 2080175524 Connecting to f6bvp-4 @ 2080175524 ... *** Connected User call : F6BVP-0 Welcome to the last release of Fpac! This file is fpac.hello and is displayed when a user connects to the node. FPAC-Node v 4.1.3 (built Aug 5 2022) for LINUX (help = h) F6BVP-4 (Commands = ?) : In file rose_call.c carriage returns are also replaced by newlines in order to let error messages to be correctly displayed. Cc: Thomas DL9SAU Osterried <thomas@osterried.de> Cc: Francois Romieu <romieu@fr.zoreil.com> Signed-off-by: Bernard Pidoux <f6bvp@free.fr> [-- Attachment #2: replace_return-linefeed_in_rose_call.patch --] [-- Type: text/x-patch, Size: 5274 bytes --] diff --git a/linuxax25-master-f6bvp/ax25tools/user_call/user_io.c b/linuxax25-master/ax25tools/user_call/user_io.c index 3bd6a26..92c47a6 100644 --- a/linuxax25-master/ax25tools/user_call/user_io.c +++ b/linuxax25-master/ax25tools/user_call/user_io.c @@ -174,6 +174,16 @@ int user_read(int fd, void *buf, size_t count) #endif } +void linefeed_buffer(char *buf, size_t count) +{ + int i; + + for (i=0 ; i < count ; i++) { + if(buf[i] == 0x0D) + buf[i] = 0x0A; + } +} + int select_loop(int s) { fd_set read_fd; @@ -199,8 +209,10 @@ int select_loop(int s) select(s + 1, &read_fd, NULL, NULL, NULL); if (FD_ISSET(s, &read_fd)) { - while ((n = user_read(s, buf, BUFLEN)) > 0) + while ((n = user_read(s, buf, BUFLEN)) > 0) { + linefeed_buffer(buf, BUFLEN); user_write(STDOUT_FILENO, buf, n); + } if (n == 0 || (n < 0 && errno != EAGAIN)) { close(s); break; diff --git a/linuxax25-master-f6bvp/ax25tools/user_call/rose_call.c b/linuxax25-master/ax25tools/user_call/rose_call.c index 03bba08..7a7dea1 100644 --- a/linuxax25-master-f6bvp/ax25tools/user_call/rose_call.c +++ b/linuxax25-master/ax25tools/user_call/rose_call.c @@ -44,13 +44,13 @@ int main(int argc, char **argv) break; case ':': case '?': - err("ERROR: invalid option usage\n"); + err("ERROR: invalid option usage\r"); return 1; } } if (paclen_in < 1 || paclen_out < 1) { - err("ERROR: invalid paclen\n"); + err("ERROR: invalid paclen\r"); return 1; } @@ -58,12 +58,12 @@ int main(int argc, char **argv) * Arguments should be "rose_call port mycall remcall remaddr" */ if ((argc - optind) != 4) { - strcpy(buffer, "ERROR: invalid number of parameters\n"); + strcpy(buffer, "ERROR: invalid number of parameters\r"); err(buffer); } if (rs_config_load_ports() == 0) { - strcpy(buffer, "ERROR: problem with rsports file\n"); + strcpy(buffer, "ERROR: problem with rsports file\r"); err(buffer); } @@ -75,27 +75,27 @@ int main(int argc, char **argv) addr = rs_config_get_addr(argv[optind]); if (addr == NULL) { - sprintf(buffer, "ERROR: invalid Rose port name - %s\n", argv[optind]); + sprintf(buffer, "ERROR: invalid Rose port name - %s\r", argv[optind]); err(buffer); } if (rose_aton(addr, rosebind.srose_addr.rose_addr) == -1) { - sprintf(buffer, "ERROR: invalid Rose port address - %s\n", argv[optind]); + sprintf(buffer, "ERROR: invalid Rose port address - %s\r", argv[optind]); err(buffer); } if (ax25_aton_entry(argv[optind + 1], rosebind.srose_call.ax25_call) == -1) { - sprintf(buffer, "ERROR: invalid callsign - %s\n", argv[optind + 1]); + sprintf(buffer, "ERROR: invalid callsign - %s\r", argv[optind + 1]); err(buffer); } if (ax25_aton_entry(argv[optind + 2], roseconnect.srose_call.ax25_call) == -1) { - sprintf(buffer, "ERROR: invalid callsign - %s\n", argv[optind + 2]); + sprintf(buffer, "ERROR: invalid callsign - %s\r", argv[optind + 2]); err(buffer); } if (rose_aton(argv[optind + 3], roseconnect.srose_addr.rose_addr) == -1) { - sprintf(buffer, "ERROR: invalid Rose address - %s\n", argv[optind + 3]); + sprintf(buffer, "ERROR: invalid Rose address - %s\r", argv[optind + 3]); err(buffer); } @@ -104,7 +104,7 @@ int main(int argc, char **argv) */ s = socket(AF_ROSE, SOCK_SEQPACKET, 0); if (s < 0) { - sprintf(buffer, "ERROR: cannot open Rose socket, %s\n", strerror(errno)); + sprintf(buffer, "ERROR: cannot open Rose socket, %s\r", strerror(errno)); err(buffer); } @@ -112,11 +112,11 @@ int main(int argc, char **argv) * Set our AX.25 callsign and Rose address accordingly. */ if (bind(s, (struct sockaddr *)&rosebind, addrlen) != 0) { - sprintf(buffer, "ERROR: cannot bind Rose socket, %s\n", strerror(errno)); + sprintf(buffer, "ERROR: cannot bind Rose socket, %s\r", strerror(errno)); err(buffer); } - sprintf(buffer, "Connecting to %s @ %s ...\n", argv[optind + 2], argv[optind + 3]); + sprintf(buffer, "Connecting to %s @ %s ...\r", argv[optind + 2], argv[optind + 3]); user_write(STDOUT_FILENO, buffer, strlen(buffer)); /* @@ -132,16 +132,16 @@ int main(int argc, char **argv) if (connect(s, (struct sockaddr *)&roseconnect, addrlen) != 0) { switch (errno) { case ECONNREFUSED: - strcpy(buffer, "*** Connection refused - aborting\n"); + strcpy(buffer, "*** Connection refused - aborting\r"); break; case ENETUNREACH: - strcpy(buffer, "*** No known route - aborting\n"); + strcpy(buffer, "*** No known route - aborting\r"); break; case EINTR: - strcpy(buffer, "*** Connection timed out - aborting\n"); + strcpy(buffer, "*** Connection timed out - aborting\r"); break; default: - sprintf(buffer, "ERROR: cannot connect to Rose address, %s\n", strerror(errno)); + sprintf(buffer, "ERROR: cannot connect to Rose address, %s\r", strerror(errno)); break; } @@ -153,12 +153,12 @@ int main(int argc, char **argv) */ alarm(0); - strcpy(buffer, "*** Connected\n"); + strcpy(buffer, "*** Connected\r"); user_write(STDOUT_FILENO, buffer, strlen(buffer)); select_loop(s); - strcpy(buffer, "\n*** Disconnected\n"); + strcpy(buffer, "\r*** Disconnected\r"); user_write(STDOUT_FILENO, buffer, strlen(buffer)); end_compress(); ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Resend : [PATCH] AX25 rose_call - replacing carriage return by newlines 2022-08-01 15:44 ` Francois Romieu ` (2 preceding siblings ...) 2022-08-07 18:04 ` [PATCH] AX25 rose_call - replacing carriage return by newlines f6bvp @ 2022-08-07 18:21 ` f6bvp [not found] ` <ABFC096C-8F65-49C9-8BB9-7B75B3CE30B7@osterried.de> 3 siblings, 1 reply; 12+ messages in thread From: f6bvp @ 2022-08-07 18:21 UTC (permalink / raw) To: linux-hams; +Cc: Thomas Osterried DL9SAU, netdev, Francois Romieu [-- Attachment #1: Type: text/plain, Size: 1345 bytes --] [PATCH] AX25 rose_call - replacing carriage return by newlines Previous patch was reversed... resending correct one. I have been using intensively rose_call application (part of ax25tools/user_call library) while debugging rose connect issue. However once connected rose_call displays remote message without linefeed. Consequently it is impossible to read messages. For example calling local node : # rose_call rose0 f6bvp f6bvp-4 2080175524 Connecting to f6bvp-4 @ 2080175524 ... *** Connected F6BVP-4 (Commands = ?) : Aug 5 2022) for LINUX (help = h) Then issuing command P to the connected local node, all answer lines are superimposed. F6BVP-4 (Commands = ?) : Switch Port Now with the proposed patch is the complete info displayed: # ./rose_call rose0 f6bvp f6bvp-4 2080175524 Connecting to f6bvp-4 @ 2080175524 ... *** Connected User call : F6BVP-0 Welcome to the last release of Fpac! This file is fpac.hello and is displayed when a user connects to the node. FPAC-Node v 4.1.3 (built Aug 5 2022) for LINUX (help = h) F6BVP-4 (Commands = ?) : In file rose_call.c carriage returns are also replaced by newlines in order to let error messages to be correctly displayed. Cc: Thomas DL9SAU Osterried <thomas@osterried.de> Cc: Francois Romieu <romieu@fr.zoreil.com> Signed-off-by: Bernard Pidoux <f6bvp@free.fr> [-- Attachment #2: replace_return-linefeed_in_rose_call.patch --] [-- Type: text/x-patch, Size: 5274 bytes --] diff --git a/linuxax25-master-f6bvp/ax25tools/user_call/user_io.c b/linuxax25-master/ax25tools/user_call/user_io.c index 3bd6a26..92c47a6 100644 --- a/linuxax25-master/ax25tools/user_call/user_io.c +++ b/linuxax25-master/ax25tools/user_call/user_io.c @@ -174,6 +174,16 @@ int user_read(int fd, void *buf, size_t count) #endif } +void linefeed_buffer(char *buf, size_t count) +{ + int i; + + for (i=0 ; i < count ; i++) { + if(buf[i] == 0x0D) + buf[i] = 0x0A; + } +} + int select_loop(int s) { fd_set read_fd; @@ -199,8 +209,10 @@ int select_loop(int s) select(s + 1, &read_fd, NULL, NULL, NULL); if (FD_ISSET(s, &read_fd)) { - while ((n = user_read(s, buf, BUFLEN)) > 0) + while ((n = user_read(s, buf, BUFLEN)) > 0) { + linefeed_buffer(buf, BUFLEN); user_write(STDOUT_FILENO, buf, n); + } if (n == 0 || (n < 0 && errno != EAGAIN)) { close(s); break; diff --git a/linuxax25-master/ax25tools/user_call/rose_call.c b/linuxax25-master-f6bvp/ax25tools/user_call/rose_call.c index 7a7dea1..03bba08 100644 --- a/linuxax25-master/ax25tools/user_call/rose_call.c +++ b/linuxax25-master-f6bvp/ax25tools/user_call/rose_call.c @@ -44,13 +44,13 @@ int main(int argc, char **argv) break; case ':': case '?': - err("ERROR: invalid option usage\r"); + err("ERROR: invalid option usage\n"); return 1; } } if (paclen_in < 1 || paclen_out < 1) { - err("ERROR: invalid paclen\r"); + err("ERROR: invalid paclen\n"); return 1; } @@ -58,12 +58,12 @@ int main(int argc, char **argv) * Arguments should be "rose_call port mycall remcall remaddr" */ if ((argc - optind) != 4) { - strcpy(buffer, "ERROR: invalid number of parameters\r"); + strcpy(buffer, "ERROR: invalid number of parameters\n"); err(buffer); } if (rs_config_load_ports() == 0) { - strcpy(buffer, "ERROR: problem with rsports file\r"); + strcpy(buffer, "ERROR: problem with rsports file\n"); err(buffer); } @@ -75,27 +75,27 @@ int main(int argc, char **argv) addr = rs_config_get_addr(argv[optind]); if (addr == NULL) { - sprintf(buffer, "ERROR: invalid Rose port name - %s\r", argv[optind]); + sprintf(buffer, "ERROR: invalid Rose port name - %s\n", argv[optind]); err(buffer); } if (rose_aton(addr, rosebind.srose_addr.rose_addr) == -1) { - sprintf(buffer, "ERROR: invalid Rose port address - %s\r", argv[optind]); + sprintf(buffer, "ERROR: invalid Rose port address - %s\n", argv[optind]); err(buffer); } if (ax25_aton_entry(argv[optind + 1], rosebind.srose_call.ax25_call) == -1) { - sprintf(buffer, "ERROR: invalid callsign - %s\r", argv[optind + 1]); + sprintf(buffer, "ERROR: invalid callsign - %s\n", argv[optind + 1]); err(buffer); } if (ax25_aton_entry(argv[optind + 2], roseconnect.srose_call.ax25_call) == -1) { - sprintf(buffer, "ERROR: invalid callsign - %s\r", argv[optind + 2]); + sprintf(buffer, "ERROR: invalid callsign - %s\n", argv[optind + 2]); err(buffer); } if (rose_aton(argv[optind + 3], roseconnect.srose_addr.rose_addr) == -1) { - sprintf(buffer, "ERROR: invalid Rose address - %s\r", argv[optind + 3]); + sprintf(buffer, "ERROR: invalid Rose address - %s\n", argv[optind + 3]); err(buffer); } @@ -104,7 +104,7 @@ int main(int argc, char **argv) */ s = socket(AF_ROSE, SOCK_SEQPACKET, 0); if (s < 0) { - sprintf(buffer, "ERROR: cannot open Rose socket, %s\r", strerror(errno)); + sprintf(buffer, "ERROR: cannot open Rose socket, %s\n", strerror(errno)); err(buffer); } @@ -112,11 +112,11 @@ int main(int argc, char **argv) * Set our AX.25 callsign and Rose address accordingly. */ if (bind(s, (struct sockaddr *)&rosebind, addrlen) != 0) { - sprintf(buffer, "ERROR: cannot bind Rose socket, %s\r", strerror(errno)); + sprintf(buffer, "ERROR: cannot bind Rose socket, %s\n", strerror(errno)); err(buffer); } - sprintf(buffer, "Connecting to %s @ %s ...\r", argv[optind + 2], argv[optind + 3]); + sprintf(buffer, "Connecting to %s @ %s ...\n", argv[optind + 2], argv[optind + 3]); user_write(STDOUT_FILENO, buffer, strlen(buffer)); /* @@ -132,16 +132,16 @@ int main(int argc, char **argv) if (connect(s, (struct sockaddr *)&roseconnect, addrlen) != 0) { switch (errno) { case ECONNREFUSED: - strcpy(buffer, "*** Connection refused - aborting\r"); + strcpy(buffer, "*** Connection refused - aborting\n"); break; case ENETUNREACH: - strcpy(buffer, "*** No known route - aborting\r"); + strcpy(buffer, "*** No known route - aborting\n"); break; case EINTR: - strcpy(buffer, "*** Connection timed out - aborting\r"); + strcpy(buffer, "*** Connection timed out - aborting\n"); break; default: - sprintf(buffer, "ERROR: cannot connect to Rose address, %s\r", strerror(errno)); + sprintf(buffer, "ERROR: cannot connect to Rose address, %s\n", strerror(errno)); break; } @@ -153,12 +153,12 @@ int main(int argc, char **argv) */ alarm(0); - strcpy(buffer, "*** Connected\r"); + strcpy(buffer, "*** Connected\n"); user_write(STDOUT_FILENO, buffer, strlen(buffer)); select_loop(s); - strcpy(buffer, "\r*** Disconnected\r"); + strcpy(buffer, "\n*** Disconnected\n"); user_write(STDOUT_FILENO, buffer, strlen(buffer)); end_compress(); ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <ABFC096C-8F65-49C9-8BB9-7B75B3CE30B7@osterried.de>]
* Re: Resend : [PATCH] AX25 rose_call - replacing carriage return by newlines [not found] ` <ABFC096C-8F65-49C9-8BB9-7B75B3CE30B7@osterried.de> @ 2022-08-08 12:31 ` f6bvp 0 siblings, 0 replies; 12+ messages in thread From: f6bvp @ 2022-08-08 12:31 UTC (permalink / raw) To: Thomas Osterried Cc: linux-hams, Francois Romieu, David Ranch, F3KT, Eric Dumazet, k4gbb1, Woldanski Lee Hi Thomas, Many thanks for providing your feelings about what I have done. I understand you don't want to change old AX.25 tools that are convenient for packet radio. No problem. As I needed a simple user application to connect ROSE network I will still use my application for rose kernel module debugging. "call" is a multi protocol application that is working for AX25 or Netrom but does abolutely not work for rose as it is quite buggy. This is why I preferred to adapt rose_call that is a single protocol simple application. I already repaired rose module in (net-next kernel 5.19+) that had been broken by a patch applied two years ago without any user checking. Very bad behavior indeed from AX25 and ROSE so called maintainers. Reverting the patch did the job. Without all rose users have to keep using 5.4.79 kernel. I spend the last hours to repair call application and rose protocol is now working. However it will take a few days to certify the whole. 73 de Bernard, f6bvp / ai7bg link to commit : https://lore.kernel.org/all/20201119191043.28813-1-anmol.karan123@gmail.com/ link to call screen image http://f6bvp.org/call_screen.png Le 07/08/2022 à 21:07, Thomas Osterried a écrit : > Hello, > > I don't think netev@ is the right addressee here. netdev is for kernel, not for userspace programs like telnet, libreoffice or X11. Or am I wrong? I removed netdev from the list of receipients. > > > Your patch makes things bad. > Reason: see man page: > > ax25_call, netrom_call, rose_call and tcp_call establish an AX.25, NET/ROM, ROSE or > a TCP connection in a manner suitable for calling from either the ax25d program > directly, or from the node program as an external command. By setting the command > line arguments appropriately it is possible to set the local callsign from which > the call will be made. No translation of the end of lines is performed by this pro‐ > gram. > > > Explanation: > EOL convention in PacketRadio is '\r'. Not '\n', and not '\r\n'. There's no algorithm possible for autodetection - so many people have tried over the years, all have failed. > The last sentense, "No translation of the end of lines is performed by this program." is important; it explains the purpose of the program and why it is implemented this way. > > The sense of ax25_call is, for example, that it is called from ax25d (i.e. if user connects you to -10, you might pass the connection over to your local mailbox in the LAN). > Same for netrom_call, rose_call. > These commands are not intended for a unix user to be executed directly on the shell. That's why they reside in /usr/sbin. > > If the program is used as intended, but with your changes, the packet-radio-user sees now in his terminal: > === > Connecting to %s @ %s ...[linefeed] > *** Connected[linefeed] > Hello user xy, welcome to our BBS. > Last login... > mailboxprompt> [linefeed] > *** Disconnected[linefeed] > === > > => his session is messed up. > > > Users from shell normally use the call command, which does the correct EOL conversion. > > > If you have a special usecase, that's out of the scope of the current demands, you may implement a comandline option for changing the EOL character behavior. > But rose_call becomes with such an addition more and more like call. I see no reason why this is useful. > > Apart from EOL in the messages, you implemented an EOL-changer for the input buffer. > You missed implementing an EOL changer for what unix user enters (normaly, his lines end with \n): Unix users press enter and don't know, that they have to enter ^M so that remote side understands your line correctly. > > > > A site note on your remark >> However once connected rose_call displays remote message without linefeed. Consequently it is impossible to read messages. > > If the remote site is using a software that does not behave like a normal packet-radio-software, > then you could either try to change all client terminal softwares in the world and make \n the new EOL standard, > or please the remote site to correct the EOL according to what's everywhere else used in the packet-radio world. > > > ..and one comment inline: > >> Am 07.08.2022 um 20:21 schrieb f6bvp <f6bvp@free.fr>: >> >> [PATCH] AX25 rose_call - replacing carriage return by newlines >> >> Previous patch was reversed... resending correct one. >> >> I have been using intensively rose_call application (part of ax25tools/user_call library) while debugging rose connect issue. >> >> However once connected rose_call displays remote message without linefeed. Consequently it is impossible to read messages. >> >> For example calling local node : >> >> # rose_call rose0 f6bvp f6bvp-4 2080175524 >> Connecting to f6bvp-4 @ 2080175524 ... >> >> *** Connected >> >> F6BVP-4 (Commands = ?) : Aug 5 2022) for LINUX (help = h) >> >> >> Then issuing command P to the connected local node, all answer >> lines are superimposed. >> >> F6BVP-4 (Commands = ?) : Switch Port >> >> >> Now with the proposed patch is the complete info displayed: >> >> # ./rose_call rose0 f6bvp f6bvp-4 2080175524 >> >> Connecting to f6bvp-4 @ 2080175524 ... >> >> *** Connected >> >> User call : F6BVP-0 >> >> Welcome to the last release of Fpac! >> >> This file is fpac.hello and is displayed when >> >> a user connects to the node. > > I see > > many more > > newlines. > >> >> >> >> FPAC-Node v 4.1.3 (built Aug 5 2022) for LINUX (help = h) >> >> F6BVP-4 (Commands = ?) : >> >> >> In file rose_call.c carriage returns are also replaced by newlines >> in order to let error messages to be correctly displayed. >> >> Cc: Thomas DL9SAU Osterried <thomas@osterried.de> >> Cc: Francois Romieu <romieu@fr.zoreil.com> >> Signed-off-by: Bernard Pidoux <f6bvp@free.fr> >> >> <replace_return-linefeed_in_rose_call.patch> > > > vy 73, > - Thomas dl9sau ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 0:39 ` Francois Romieu 2022-08-01 8:06 ` Thomas Osterried @ 2022-08-01 8:54 ` Bernard f6bvp 2022-08-01 10:43 ` Bernard f6bvp 2 siblings, 0 replies; 12+ messages in thread From: Bernard f6bvp @ 2022-08-01 8:54 UTC (permalink / raw) To: Francois Romieu; +Cc: Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev After applying patch rose->timer displays still t underflow value dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 002 00001 1 0 0 0 122 200 180 180 5 0/000 0 0 37356 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 2080835201 WP-0 2080175524 WP-0 rose0 032 00002 1 0 0 0 1 200 180 180 5 0/000 0 0 41389 2080175520 WP-0 2080175524 WP-0 rose0 032 00003 1 0 0 0 1 200 180 180 5 0/000 0 0 41388 2080175527 WP-0 2080175524 WP-0 rose0 032 00004 1 0 0 0 1 200 180 180 5 0/000 0 0 41387 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 2080175524 WP-0 2080175524 FPAD-0 rose0 001 00001 1 0 0 0 73786976294838206 200 180 180 5 0/000 0 0 36437 * * 2080175524 ??????-? rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36436 root@ubuntu-f6bvp:/home/bernard# cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 002 00001 1 0 0 0 115 200 180 180 5 0/000 0 0 37356 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 2080835201 WP-0 2080175524 WP-0 rose0 032 00002 2 0 0 0 178 200 180 180 5 0/000 0 0 41389 2080175520 WP-0 2080175524 WP-0 rose0 032 00003 2 0 0 0 178 200 180 180 5 0/000 0 0 41388 2080175527 WP-0 2080175524 WP-0 rose0 032 00004 2 0 0 0 178 200 180 180 5 0/000 0 0 41387 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 2080175524 WP-0 2080175524 FPAD-0 rose0 001 00001 2 0 0 0 178 200 180 180 5 0/000 0 0 36437 * * 2080175524 ??????-? rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36436 Le 01/08/2022 à 02:39, Francois Romieu a écrit : > Bernard f6bvp <f6bvp@free.fr> : >> Rose proc timer t error >> >> Timer t is decremented one by one during normal operations. >> >> When decreasing from 1 to 0 it displays a very large number until next clock >> tic as demonstrated below. >> >> t1, t2 and t3 are correctly handled. > "t" is ax25_display_timer(&rose->timer) / HZ whereas "tX" are rose->tX / HZ. > > ax25_display_timer() does not like jiffies > timer->expires (and it should > probably return plain seconds btw). > > You may try the hack below. > > diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c > index 85865ebfdfa2..b77433fff0c9 100644 > --- a/net/ax25/ax25_timer.c > +++ b/net/ax25/ax25_timer.c > @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) > > unsigned long ax25_display_timer(struct timer_list *timer) > { > - if (!timer_pending(timer)) > - return 0; > + long delta = timer->expires - jiffies; > > - return timer->expires - jiffies; > + return jiffies_delta_to_clock_t(delta) * HZ; > } > > EXPORT_SYMBOL(ax25_display_timer); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: rose timer t error displayed in /proc/net/rose 2022-08-01 0:39 ` Francois Romieu 2022-08-01 8:06 ` Thomas Osterried 2022-08-01 8:54 ` rose timer t error displayed in /proc/net/rose Bernard f6bvp @ 2022-08-01 10:43 ` Bernard f6bvp 2 siblings, 0 replies; 12+ messages in thread From: Bernard f6bvp @ 2022-08-01 10:43 UTC (permalink / raw) To: Francois Romieu; +Cc: Eric Dumazet, linux-hams, Thomas Osterried DL9SAU, netdev I used concurrently ~/ax25tools/user_call/rose_call in order to perform a connect request to my local node : ./rose_call rose0 f6bvp f6bvp-4 2080175524 Looking again at /proc/net/rose it seems that rose->timer is not stopped immediately when 0 value is reached and counts down result to an underflow (long -1 value). After a few more clock tics rose->timer is reinitialized to 180 and rose state st changes from 1 to 2. dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 1 0 0 0 6 200 180 180 5 0/000 0 0 77873 2080175524 F6BVP-4 2080175524 F6BVP-0 rose0 002 00001 2 0 0 0 147 200 180 180 5 0/000 0 0 76283 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 bernard@ubuntu-f6bvp:~$ cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 1 0 0 0 73786976294838200 200 180 180 5 0/000 0 0 77873 2080175524 F6BVP-4 2080175524 F6BVP-0 rose0 002 00001 2 0 0 0 135 200 180 180 5 0/000 0 0 76283 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 bernard@ubuntu-f6bvp:~$ cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 1 0 0 0 73786976294838195 200 180 180 5 0/000 0 0 77873 2080175524 F6BVP-4 2080175524 F6BVP-0 rose0 002 00001 2 0 0 0 131 200 180 180 5 0/000 0 0 76283 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 bernard@ubuntu-f6bvp:~$ cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 2 0 0 0 178 200 180 180 5 0/000 0 0 77873 2080175524 F6BVP-4 2080175524 F6BVP-0 rose0 002 00001 2 0 0 0 129 200 180 180 5 0/000 0 0 76283 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 At the end of state st 2 count down rose->timer is correctly stopped while displaying 0 and rose client applications time out. dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 2 0 0 0 2 200 180 180 5 0/000 0 0 77873 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 bernard@ubuntu-f6bvp:~$ cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 2 0 0 0 1 200 180 180 5 0/000 0 0 77873 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 bernard@ubuntu-f6bvp:~$ cat /proc/net/rose dest_addr dest_call src_addr src_call dev lci neigh st vs vr va t t1 t2 t3 hb idle Snd-Q Rcv-Q inode 2080175524 WP-0 2080175524 NODE-0 rose0 001 00001 2 0 0 0 0 200 180 180 5 0/000 0 0 77873 * * 2080175524 ROUTE-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35195 * * 2080175524 F6BVP-15 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 35194 * * 2080175524 WP-0 rose0 000 00000 0 0 0 0 0 200 180 180 5 0/000 0 0 36456 Le 01/08/2022 à 02:39, Francois Romieu a écrit : > Bernard f6bvp <f6bvp@free.fr> : >> Rose proc timer t error >> >> Timer t is decremented one by one during normal operations. >> >> When decreasing from 1 to 0 it displays a very large number until next clock >> tic as demonstrated below. >> >> t1, t2 and t3 are correctly handled. > "t" is ax25_display_timer(&rose->timer) / HZ whereas "tX" are rose->tX / HZ. > > ax25_display_timer() does not like jiffies > timer->expires (and it should > probably return plain seconds btw). > > You may try the hack below. > > diff --git a/net/ax25/ax25_timer.c b/net/ax25/ax25_timer.c > index 85865ebfdfa2..b77433fff0c9 100644 > --- a/net/ax25/ax25_timer.c > +++ b/net/ax25/ax25_timer.c > @@ -108,10 +108,9 @@ int ax25_t1timer_running(ax25_cb *ax25) > > unsigned long ax25_display_timer(struct timer_list *timer) > { > - if (!timer_pending(timer)) > - return 0; > + long delta = timer->expires - jiffies; > > - return timer->expires - jiffies; > + return jiffies_delta_to_clock_t(delta) * HZ; > } > > EXPORT_SYMBOL(ax25_display_timer); -- 73 de Bernard f6bvp / ai7bg http://radiotelescope-lavillette.fr/au-jour-le-jour/ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-08-08 12:31 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-30 15:08 rose timer t error displayed in /proc/net/rose Bernard f6bvp
2022-08-01 0:39 ` Francois Romieu
2022-08-01 8:06 ` Thomas Osterried
2022-08-01 9:19 ` Bernard f6bvp
2022-08-01 15:44 ` Francois Romieu
2022-08-01 19:06 ` Thomas Osterried
2022-08-01 19:33 ` Bernard f6bvp
2022-08-07 18:04 ` [PATCH] AX25 rose_call - replacing carriage return by newlines f6bvp
2022-08-07 18:21 ` Resend : " f6bvp
[not found] ` <ABFC096C-8F65-49C9-8BB9-7B75B3CE30B7@osterried.de>
2022-08-08 12:31 ` f6bvp
2022-08-01 8:54 ` rose timer t error displayed in /proc/net/rose Bernard f6bvp
2022-08-01 10:43 ` Bernard f6bvp
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.