rose timer t error displayed in /proc/net/rose

rose timer t error displayed in /proc/net/rose

[Bernard f6bvp]

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

Re: rose timer t error displayed in /proc/net/rose

[Francois Romieu]

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);

Re: rose timer t error displayed in /proc/net/rose

[Thomas Osterried]

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?

Re: rose timer t error displayed in /proc/net/rose

[Bernard f6bvp]

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);

Re: rose timer t error displayed in /proc/net/rose

[Bernard f6bvp]

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?
>
>
>

Re: rose timer t error displayed in /proc/net/rose

[Bernard f6bvp]

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/

Re: rose timer t error displayed in /proc/net/rose

[Francois Romieu]

[-- 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");

Re: rose timer t error displayed in /proc/net/rose

[Thomas Osterried]

> 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>

Re: rose timer t error displayed in /proc/net/rose

[Bernard f6bvp]

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);
>

[PATCH] AX25 rose_call - replacing carriage return by newlines

[f6bvp]

[-- 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();

Resend : [PATCH] AX25 rose_call - replacing carriage return by newlines

[f6bvp]

[-- 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();

Re: Resend : [PATCH] AX25 rose_call - replacing carriage return by newlines

[f6bvp]

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