On Thursday 21 August 2008 22:31, Doug Graham wrote:
> Does the following code from networking/ping.c (appears in two places)
> serve any useful purpose?
> 
>         /* set recv buf for broadcast pings */
>         sockopt = 48 * 1024; /* explain why 48k? */
>         setsockopt(pingsock, SOL_SOCKET, SO_RCVBUF, &sockopt, 
> sizeof(sockopt));
> 
> On some kernels (our 2.6.16.26 mips kernel for example), it breaks large
> like this:
> 
>       ping -s 64000 -c1 <dest>
> 
> This problem can be fixed either removing this setsockopt entirely or
> by using a 96k receive bufsize instead of 48k.  I picked 96k because
> that's what the ping on my Redhat desktop (from iputils-20020927) uses.
> 
> I'm not sure exactly what this number is used for.  You'd think it would
> set the maximum size of a packet that could be received on the socket, but
> on the above mentioned kernel, I can use a ping size of up to 57712 before
> it breaks, and on a  2.6.9 x86 desktop kernel, it doesn't break at all.
> 
> It's not really a big deal that really large pings break, unless you
> don't know that the bug is in ping and not in your networking stack,
> and then proceed to waste a large chunk of time debugging the latter :-)
> 
> So, as the comment asks, can anybody "explain why 48k?".

The comment was mine. I don't know :)

In order to not waste too much of buffer space, I propose using
(datalen | 0x7ff) + 1025 bytes instead of fixed 48k or 96k constant.

Please try attached patch.
--
vda
diff -d -urpN busybox.2/networking/ping.c busybox.3/networking/ping.c
--- busybox.2/networking/ping.c	2008-08-06 00:55:59.000000000 +0200
+++ busybox.3/networking/ping.c	2008-08-24 01:43:08.000000000 +0200
@@ -577,7 +577,7 @@ static void ping4(len_and_sockaddr *lsa)
 	setsockopt_broadcast(pingsock);
 
 	/* set recv buf for broadcast pings */
-	sockopt = 48 * 1024; /* explain why 48k? */
+	sockopt = (datalen | 0x7ff) + 1025; /* giving it a bit of extra room */
 	setsockopt(pingsock, SOL_SOCKET, SO_RCVBUF, &sockopt, sizeof(sockopt));
 
 	signal(SIGINT, print_stats_and_exit);
@@ -641,7 +641,7 @@ static void ping6(len_and_sockaddr *lsa)
 	setsockopt_broadcast(pingsock);
 
 	/* set recv buf for broadcast pings */
-	sockopt = 48 * 1024; /* explain why 48k? */
+	sockopt = (datalen | 0x7ff) + 1025; /* giving it a bit of extra room */
 	setsockopt(pingsock, SOL_SOCKET, SO_RCVBUF, &sockopt, sizeof(sockopt));
 
 	sockopt = offsetof(struct icmp6_hdr, icmp6_cksum);
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Reply via email to