Re: ping.c minor bug discrepancy between reported size of icmp packet
Hi, On 9/06/2018, at 7:52 AM, Tom Smyth wrote: > Hello I see a small discrepancy between the measurement > of sent and received packets as displayed by ping command > > on the wire the sent and received packets are the same size > I had a brief go > > foo# ping 5.134.88.1 > PING 5.134.88.1 (5.134.88.1): 56 data bytes > 64 bytes from 5.134.88.1: icmp_seq=0 ttl=53 time=91.719 ms > > it would appear that one measurement is the ICMP Payload only > and the other is the ICMP payload + ICMP header (8 byte difference) Note that these values do not necessarily differ by 8 as one may receive something other than an ECHOREPLY in return e.g. $ ping -s 40 -v 192.168.1.99 PING 192.168.1.99 (192.168.1.99): 40 data bytes 36 bytes from 192.168.5.1: Destination Host Unreachable To my mind the two measurements are distinct, distinguished, and useful in a diagnostic tool. As an aside, I have long hankered for ping(1) to advise me on how to hit or exceed MTU but it is unable (and ought to be unable) to do so, as that would involve multiple layer violations. For the same reason, it is unable to indicate total bytes sent. See the second paragraph of icmp(4) for starters. best, Richard. > > > foo# grep -n " data bytes" /root/ping.c > 760:printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), datalen); > foo# grep -n " bytes from" /root/ping.c > 1248: printf("%d bytes from %s: icmp_seq=%u", cc, > 1292: printf("%d bytes from %s: ", cc, pr_addr(from, fromlen)); > > looking at the source code it looks like the size = %d but %d is presenting > different values > I didnt see where %d was being changed between line 760 and line 1248 > It has been a while since I looked at C programming in anger and im a bit > rusty... > any pointers on where i should be looking so that I can submit a patch > > -- > Kindest regards, > Tom Smyth >
Re: ping.c minor bug discrepancy between reported size of icmp packet
Hello Paul, All, Thanks for your clarification, I appreciate your help on this please find my reponses in line On 9 June 2018 at 09:40, Paul de Weerd wrote: > Hi Tom, > > This is documented in ping(8): > > -s packetsize > Specify the number of data bytes to be sent. The default > is 56, which translates into 64 ICMP data bytes when > combined with the 8 bytes of ICMP header data. The > maximum packet size is 65467 for IPv4 and 65527 for IPv6. > > You can play around with the 56 bytes, but those 8 are non-negotiable: > they're always added. I get you ... the ICMP data bytes can be adjusted but not the ICMP header bytes, Note that it says 56 *data* bytes versus 64 > (total) bytes. the issue I was highlighting is that when the user specifies a certain size in the command , the command says sending 1000 bytes but then says the reply 1008 bytes I think this is confusing to the user of the system. also I think the total bytes is incorrect because it only takes account of the ICMP header and not the IP header, Perhaps the Approach I should take to fix this (my confusion) ( and possibly future users confusion) would be to make the displayed values consistent and the displayed label consistent > > You seem to be misunderstanding the format string passed that you > found. %d is part of a format string, see printf(3). The "%" > indicates a conversion specification, the "d" indicates the type of > conversion, in this case a signed integer. The arguments following > the format string are filled in where a conversion specification is > given in the format string, in the order given. So in this example: > > #include ; > > int main() { > int number; > const char* test; > > text = "Hi Tom!"; > number = 42; > > printf("string: %s\nnumber: %d\n", string, number) > return 0; > } > > The printf(3) function gets called with three arguments. The first is > the format string: "string: %s\nnumber: %d\n". As you can see, > there's two conversion specifications in there, "%s" and "%d". "%s" > is for a "char*" argument, it gets replaced with the second argument > to the function (the variable 'string', which we gave the value "Hi > Tom!"). "%d" is for the integer argument, it gets replaced with a > decimal representation of the value of the third argument to the > function (the variable 'number', which we gave the value 42). > Thanks for your help on printf(3) :) Ill work on this > Applying this knowledge to the three lines you found in the ping > source: > In line 760, %d gets the value from variable 'datalen'. > In lines 1248 and 1292, %d gets the value from variable 'cc'. > > Note that 'cc' could be changed between those two lines, so the value > printed in the end doesn't *have* to be the same - that depends on the > rest of the code. I think we could display (datalen+8) bytes in line 760 or preferably display (cc -8) bytes in lines 1248 and 1292 and do a ping man page update based on the agreed approach thanks for your help and input Tom Smyth > > Cheers, > > Paul > > -- >>[<++>-]<+++.>+++[<-->-]<.>+++[<+ > +++>-]<.>++[<>-]<+.--.[-] > http://www.weirdnet.nl/ -- Kindest regards, Tom Smyth
Re: ping.c minor bug discrepancy between reported size of icmp packet
On Fri, Jun 08, 2018 at 08:52:17PM +0100, Tom Smyth wrote: > Hello I see a small discrepancy between the measurement > of sent and received packets as displayed by ping command > > on the wire the sent and received packets are the same size > I had a brief go > > foo# ping 5.134.88.1 > PING 5.134.88.1 (5.134.88.1): 56 data bytes > 64 bytes from 5.134.88.1: icmp_seq=0 ttl=53 time=91.719 ms > > it would appear that one measurement is the ICMP Payload only > and the other is the ICMP payload + ICMP header (8 byte difference) > This analysis is correct. This discrepancy always bugged me but I suspect that people got so used to it that changing it is a bad idea. I certainly do the math automatically in my head. I'd like to improve this but I don't think it's possible, this stuff is just around for too long to change. > > foo# grep -n " data bytes" /root/ping.c > 760:printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), datalen); > foo# grep -n " bytes from" /root/ping.c > 1248: printf("%d bytes from %s: icmp_seq=%u", cc, > 1292: printf("%d bytes from %s: ", cc, pr_addr(from, fromlen)); > > looking at the source code it looks like the size = %d but %d is presenting > different values > I didnt see where %d was being changed between line 760 and line 1248 > It has been a while since I looked at C programming in anger and im a bit > rusty... > any pointers on where i should be looking so that I can submit a patch > > -- > Kindest regards, > Tom Smyth > -- I'm not entirely sure you are real.
Re: ping.c minor bug discrepancy between reported size of icmp packet
Hi Tom, This is documented in ping(8): -s packetsize Specify the number of data bytes to be sent. The default is 56, which translates into 64 ICMP data bytes when combined with the 8 bytes of ICMP header data. The maximum packet size is 65467 for IPv4 and 65527 for IPv6. You can play around with the 56 bytes, but those 8 are non-negotiable: they're always added. Note that it says 56 *data* bytes versus 64 (total) bytes. On Fri, Jun 08, 2018 at 08:52:17PM +0100, Tom Smyth wrote: | Hello I see a small discrepancy between the measurement | of sent and received packets as displayed by ping command | | on the wire the sent and received packets are the same size | I had a brief go | | foo# ping 5.134.88.1 | PING 5.134.88.1 (5.134.88.1): 56 data bytes | 64 bytes from 5.134.88.1: icmp_seq=0 ttl=53 time=91.719 ms | | it would appear that one measurement is the ICMP Payload only | and the other is the ICMP payload + ICMP header (8 byte difference) | | | foo# grep -n " data bytes" /root/ping.c | 760:printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), datalen); | foo# grep -n " bytes from" /root/ping.c | 1248: printf("%d bytes from %s: icmp_seq=%u", cc, | 1292: printf("%d bytes from %s: ", cc, pr_addr(from, fromlen)); | | looking at the source code it looks like the size = %d but %d is presenting | different values | I didnt see where %d was being changed between line 760 and line 1248 | It has been a while since I looked at C programming in anger and im a bit | rusty... | any pointers on where i should be looking so that I can submit a patch You seem to be misunderstanding the format string passed that you found. %d is part of a format string, see printf(3). The "%" indicates a conversion specification, the "d" indicates the type of conversion, in this case a signed integer. The arguments following the format string are filled in where a conversion specification is given in the format string, in the order given. So in this example: #include ; int main() { int number; const char* test; text = "Hi Tom!"; number = 42; printf("string: %s\nnumber: %d\n", string, number) return 0; } The printf(3) function gets called with three arguments. The first is the format string: "string: %s\nnumber: %d\n". As you can see, there's two conversion specifications in there, "%s" and "%d". "%s" is for a "char*" argument, it gets replaced with the second argument to the function (the variable 'string', which we gave the value "Hi Tom!"). "%d" is for the integer argument, it gets replaced with a decimal representation of the value of the third argument to the function (the variable 'number', which we gave the value 42). Applying this knowledge to the three lines you found in the ping source: In line 760, %d gets the value from variable 'datalen'. In lines 1248 and 1292, %d gets the value from variable 'cc'. Note that 'cc' could be changed between those two lines, so the value printed in the end doesn't *have* to be the same - that depends on the rest of the code. Cheers, Paul -- >[<++>-]<+++.>+++[<-->-]<.>+++[<+ +++>-]<.>++[<>-]<+.--.[-] http://www.weirdnet.nl/
ping.c minor bug discrepancy between reported size of icmp packet
Hello I see a small discrepancy between the measurement of sent and received packets as displayed by ping command on the wire the sent and received packets are the same size I had a brief go foo# ping 5.134.88.1 PING 5.134.88.1 (5.134.88.1): 56 data bytes 64 bytes from 5.134.88.1: icmp_seq=0 ttl=53 time=91.719 ms it would appear that one measurement is the ICMP Payload only and the other is the ICMP payload + ICMP header (8 byte difference) foo# grep -n " data bytes" /root/ping.c 760:printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), datalen); foo# grep -n " bytes from" /root/ping.c 1248: printf("%d bytes from %s: icmp_seq=%u", cc, 1292: printf("%d bytes from %s: ", cc, pr_addr(from, fromlen)); looking at the source code it looks like the size = %d but %d is presenting different values I didnt see where %d was being changed between line 760 and line 1248 It has been a while since I looked at C programming in anger and im a bit rusty... any pointers on where i should be looking so that I can submit a patch -- Kindest regards, Tom Smyth