* Madhu <20240326.220015.873398620206616683.enometh@xxxxxxxx> Wrote on Tue, 26 Mar 2024 22:00:15 +0530 (IST) > Hello, I've only now updated from 2.4.9 to 2.5.0 (on gentoo) and am > noticing this non-deterministic behaviour, pppoe plugin, > > I call pppd with the nodetach option, and frequently terminate the > connection with ^C. Usually I get output like this > ``` > ^CTerminating on signal 2 > Connect time 26.4 minutes. > Sent 85749 bytes, received 1323321 bytes. > Connection terminated. > ``` > But sometimes I only get > ``` > ^CTerminating on signal 2 Connection terminated. > ``` > > i.e. connection time and send/recv stats are not printed > or sent to syslog, and the accounting information is being lost. > > Has this been encountered by others since the release? Any > suggestions on where to look for the changes which might cause this? I downgraded for a while to 2.4.9 and didn't see the problem on the same system, so it was def. to do with the changes in 2.5.0. Following on a hunch after seeing a log like this ``` May 3 05:35:30 localhost pppd[15375]: peer from calling number XX:XX:XX:XX:XX:XX authorized May 3 05:35:31 localhost pppd[15375]: local IP address n1.n2.n3.n4 May 3 05:35:31 localhost pppd[15375]: remote IP address m1.m2.m3.m4 May 3 05:35:31 localhost pppd[15375]: primary DNS address XXX May 3 05:35:31 localhost pppd[15375]: secondary DNS address XXX May 3 05:35:33 localhost pppd[15375]: Connect time 0.1 minutes. May 3 05:35:33 localhost pppd[15375]: Sent 14 bytes, received 10 bytes. May 3 05:35:33 localhost pppd[15375]: local IP address n1.n2.n3.n4 May 3 05:35:33 localhost pppd[15375]: remote IP address m1.m2.m3.m4 May 3 05:35:33 localhost pppd[15375]: primary DNS address XXX May 3 05:35:33 localhost pppd[15375]: secondary DNS address XXX May 3 08:33:50 localhost pppd[15375]: Terminating on signal 2 ``` (After the ipcp quirk in connection I was connected Between 5:35 and 8:33, but the ^C didn't print the connection information. This was without the persist option.) The problem seems to be caused by faulty logic with `link_stats_print' in pppd/main.c (which seems to have been separated out of link_stats_valid). It is initialized to 1 in main(), and reset to 0 in print_link_stats(). This means that print_link_stats() will only ever print once. This cannot possibly be correct with with persist option. I'm not sure if this covers my initial report, but I'm now running with the attached patch, which resets link_stats_print to 1 in reset_link_stats(). (please review, it also skips the conditional on the return value) -- Regards, Madhu
>From c7ce6adfc3d81cdc44291fe03f907844fc1318c6 Mon Sep 17 00:00:00 2001 From: Madhu <enometh@xxxxxxxx> Date: Fri, 3 May 2024 14:57:07 +0530 Subject: [PATCH] fix reliability of print_link_stats (with option persist) * pppd/ipcp.c: (ipcp_down): fix comment * pppd/main.c: (reset_link_stats): reset print_link_stats to 1, set start_time even if get_ppp_stats fails. --- pppd/ipcp.c | 2 +- pppd/main.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pppd/ipcp.c b/pppd/ipcp.c index 5d9ff11..b67f934 100644 --- a/pppd/ipcp.c +++ b/pppd/ipcp.c @@ -2026,7 +2026,7 @@ ipcp_down(fsm *f) sifvjcomp(f->unit, 0, 0, 0); print_link_stats(); /* _after_ running the notifiers and ip_down_hook(), - * because print_link_stats() sets link_stats_valid + * because print_link_stats() sets link_stats_print * to 0 (zero) */ /* diff --git a/pppd/main.c b/pppd/main.c index 8310c98..da68fc5 100644 --- a/pppd/main.c +++ b/pppd/main.c @@ -1331,9 +1331,9 @@ print_link_stats(void) void reset_link_stats(int u) { - if (!get_ppp_stats(u, &old_link_stats)) - return; + get_ppp_stats(u, &old_link_stats) ppp_get_time(&start_time); + link_stats_print = 1; } /* -- 2.39.2.101.g768bb238c4