-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ppp-2.5.0 Sent Bytes Counter Seems to Be Over-reporting #443
Comments
Interesting. I tried here with a couple of Linux machines connected via serial ports, and the number of bytes sent is reasonable. What is the nature of the underlying connection you're using - is it serial, PPPoE, or what? And did you change kernel version at the same time as you updated ppp? What OS is pppd running on? If you can find an easy way that I can replicate the problem, that would be useful. |
@gerickson , could you compare that with the figures as reported using for example "ip -s a" please? I did work to incorporate 64-bit counters and wondering if this possibly relates. Then please attach strace to the running process to determine how network statistics is gathered. It will either (1) send netlink messages for RTM_GETSTATS/RTM_NEWSTATS, or (2) open files in /sys, or (3) using iocl for SIOCGPPPSTATS. ioctl was the old mechanism pre 2.5 and only supports 32-bit counters. RTM_GETSTATS is only available on new enough kernels, and sysfs uses file parsing. Kernel version would also help. |
@gerickson: Have you seen comments from @paulusmack and @jkroonza? Thanks in advance. |
I hadn't seen the comments above until just now. The underlying kernel version is invariant, 4.9.212 from Digi Embedded Yocto (DEY) 2.6, across the changed in ppp version. The underlying connection is over a serial link (serial port I'll see about gathering the other data shortly. |
If it's a helpful bit of data, I've attached the following from the build:
|
I just ran a quick data test. Here are the system logs from
For a quick Here's the same output from
which seems slightly more credible. |
Honestly, it sounds like a regression and I am probably the one to blame. Haven't had any bandwidth to look at this yet. I'd still vouch for getting this fixed for 2.5.1 goes out the door |
@gerickson I actually wonder if I might not be the one to blame to be honest. I did the refactoring to allow for 64-bit counters, @enaess did a lot of work on ppp 2.5.0 as well, but this just feels more likely related to:
Could you please get me the output from strace attached to the ppp process during shutdown? Ie:
Then initiate a shutdown on the pppd process. Copy & paste the output on the strace side please. Possibly you could do something like (off the top of my head):
After which all invocations of pppd will result in the original pppd binary being run inside strace with the system calls logged to /tmp/strace-pppd-${timestamp}.txt To undo simply rename .real back to the original name. |
Regrettably, I haven't had the bandwidth to circle back to this as of yet. |
@gerickson: Have you progressed on it? |
Regrettably, the system that was using this has moved on to using the |
@gerickson: 2.5.1 has been released (2024-09-18): And new commits have been done since the release. |
This issue should be fixed by #528 in ppp-2.5.2. |
So whilst I agree with the changes in #528 - I'm trying to think technically why that would make a difference at the data volumes @gerickson showed here?
So that looks correct even without the patch. This is on a 64-bit host. Where we have:
So changing from var_arg(.., unsigned) to var_arg(..., unsigned long long) would pull 8 bytes rather than 4 from the variables args on the stack ... so why is there values at all in my output? Unless all values are 64-bit aligned on the stack anyway, and due to little endian just happening to get the right values, and on a 32-bit machine the int would in fact get 4 bytes and 4 bytes from one of the two counters? But then, why over-report, and not one value correct and the other zero, since one of the two output values would remain zero until such time as we exceed 4GB on the counter? |
This issue appears only on 32-bit architectures. This is because |
@tpaukrt: Good job! |
@gerickson: It is good for you with this @tpaukrt PR: Your ticket has been solved? |
I recently updated from ppp-2.4.7 to ppp-2.5.0 and have noted for another otherwise-fixed connect-transmit-disconnect sequence that ppp-2.5.0 seems to be dramatically over-reporting the number of sent bytes.
This log stream is demonstrative:
The text was updated successfully, but these errors were encountered: