From owner-freebsd-net@FreeBSD.ORG Wed Oct 13 13:43:38 2010 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8529B1065670; Wed, 13 Oct 2010 13:43:38 +0000 (UTC) (envelope-from emaste@freebsd.org) Received: from mail1.sandvine.com (Mail1.sandvine.com [64.7.137.134]) by mx1.freebsd.org (Postfix) with ESMTP id 263488FC16; Wed, 13 Oct 2010 13:43:38 +0000 (UTC) Received: from labgw2.phaedrus.sandvine.com (192.168.222.22) by WTL-EXCH-1.sandvine.com (192.168.196.31) with Microsoft SMTP Server id 14.0.694.0; Wed, 13 Oct 2010 09:43:37 -0400 Received: by labgw2.phaedrus.sandvine.com (Postfix, from userid 10332) id 6561E33C00; Wed, 13 Oct 2010 09:43:37 -0400 (EDT) Date: Wed, 13 Oct 2010 09:43:37 -0400 From: Ed Maste To: Attilio Rao Message-ID: <20101013134337.GA30028@sandvine.com> Mail-Followup-To: Ed Maste , Attilio Rao , Robert Watson , FreeBSD Current , freebsd-net@freebsd.org, Ryan Stone References: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.1i Cc: freebsd-net@freebsd.org, Ryan Stone , Robert Watson , FreeBSD Current Subject: Re: [PATCH] Netdump for review and testing -- preliminary version X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Oct 2010 13:43:38 -0000 On Wed, Oct 13, 2010 at 01:04:54PM +0200, Attilio Rao wrote: > 2010/10/9 Robert Watson : > > (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