Date: Wed, 13 Oct 2010 09:43:37 -0400 From: Ed Maste <emaste@freebsd.org> To: Attilio Rao <attilio@freebsd.org> Cc: freebsd-net@freebsd.org, Ryan Stone <rstone@freebsd.org>, Robert Watson <rwatson@freebsd.org>, FreeBSD Current <current@freebsd.org> Subject: Re: [PATCH] Netdump for review and testing -- preliminary version Message-ID: <20101013134337.GA30028@sandvine.com> In-Reply-To: <AANLkTikaUL5g5ViaG763yq2=NGw9V-%2B6JL_%2BWxum8iAB@mail.gmail.com> References: <AANLkTikA5OUYD1A9pqCqVEZ5qk%2BVECq8x-fnRXnpp0KE@mail.gmail.com> <AANLkTikau6omhWrXVM13zonFEPCxXM%2B8EqJauovDu0OU@mail.gmail.com> <alpine.BSF.2.00.1010090121310.1232@fledge.watson.org> <AANLkTikaUL5g5ViaG763yq2=NGw9V-%2B6JL_%2BWxum8iAB@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 13, 2010 at 01:04:54PM +0200, Attilio Rao wrote: > 2010/10/9 Robert Watson <rwatson@freebsd.org>: > > (1) Did you consider using tftp as the network dump protocol, rather than a > > custom protocol? ??It's also a simple UDP-based, ACKed file transfer > > protocol, with the advantage that it's widely supported by exiting tftp > > daemons, packet sniffers, security devices, etc. ??This wouldn't require > > using a stock tftpd, although that would certainly be a benefit as well. > > ??The post-processing of crashdumps that seems to happen in the netdump > > server could, presumably, happen "offline" as easily...? > > I don't understand the "offline" question but, really, I don't know > why tftp wasn't considered in the first place. > Let me do some small search and see how much we could benefit from it. I think Robert means having something other than the netdump server doing the post-processing - e.g., a cron job could look in the directory where a tftp server saved a core file and run a script to extract the desired information. TFTP has a number of advantages; I'm not sure if the original author of the netdump code considered it and rejected it for some reason. I think the original implementation used a broadcast packet to locate the server though, so a custom server was required in any case. That said, I don't think it matters too much if we go ahead with the current version and switch to TFTP later on. > > (2) Have you thought about adding a checksum to the dump format, since > > packets are going over the wire on UDP, etc? ??Even an MD5 sum wouldn't be > > too hard to arrange: all the code is in the kernel already, requires > > relatively little state storage, and is designed for streamed data. > > Someone else already brought this point and I agree, we could use a > checksum here. Both a whole dump and per-packet checksum could be valuable. If we want the former I think this should be done in the dump infrastructure, so that disk dumps get it too. The latter conflicts somewhat with a desire to switch to TFTP though (other than a standard UDP packet checksum). > > The sysctl parts of the patch have a number of issues: > > Sysctl are still not overhauled just because I'm not sure we want to > keep them. That is one of the points I raised in the main e-mail and > I'd really would like to hear opinions about if we should keep them > rather than having a setup userland process, etc. > I'm sorry about this, but please keep in mind that the file still > needs a lot of cleanup so some comments are a bit out of date and > other parts may not be still perfectly overhauled. Attilio suggested adding a userland tool to configure netdump in lieu of the current sysctls. I don't have a strong opinion either way; the only real advantage to the sysctl approach that I see is that the use of the mirrored loader tunables is more obvious. > > The idea of calling into the mbuf allocator in this context is just freaky, > > and may have some truly awful side effects. ??I suppose this is the cost of > > trying to combine code paths in the network device driver rather than have > > an independent path in the netdump case, but it's quite unfortunate and will > > significantly reduce the robustness of netdumps in the face of, for example, > > mbuf starvation. > > I'm not sure in which other way we could fix that actually. Maybe a > pre-allocated pool of mbufs? Despite the freakiness, I can only offer the observation that it has worked very well for us in practise. > > + ?? ?? ?? if (ntohs(ah->ar_hrd) != ARPHRD_ETHER && > > + ?? ?? ?? ?? ?? ntohs(ah->ar_hrd) != ARPHRD_IEEE802 && > > ... > > > > Are you sure you don't want to just check for ETHER here? > > I'd really like to hear Ryan's or Ed's idea here. Interesting, I can't find this in our local version. It looks like this check was copied from if_ether.c::arpintr(). > > + ?? ?? ?? /* XXX: Probably should check if we're the recipient MAC address */ > > + ?? ?? ?? /* Done ethernet processing. Strip off the ethernet header */ > > > > Yes, quite probably. ??What if you panic in promiscuous mode? Well, the packet would get dropped at the next layer up, in nd_handle_ip. -Ed
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101013134337.GA30028>