From owner-cvs-src@FreeBSD.ORG Wed Mar 26 11:15:54 2003 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 31D3A37B404 for ; Wed, 26 Mar 2003 11:15:54 -0800 (PST) Received: from relay.pair.com (relay.pair.com [209.68.1.20]) by mx1.FreeBSD.org (Postfix) with SMTP id 501C443F93 for ; Wed, 26 Mar 2003 11:15:52 -0800 (PST) (envelope-from silby@silby.com) Received: (qmail 788 invoked from network); 26 Mar 2003 19:15:51 -0000 Received: from niwun.pair.com (HELO localhost) (209.68.2.70) by relay.pair.com with SMTP; 26 Mar 2003 19:15:51 -0000 X-pair-Authenticated: 209.68.2.70 Date: Wed, 26 Mar 2003 13:12:25 -0600 (CST) From: Mike Silbersack To: Maxime Henrion In-Reply-To: <20030326183351.GJ57674@elvis.mu.org> Message-ID: <20030326130903.G2075@odysseus.silby.com> References: <200303260452.h2Q4quap015364@www.ambrisko.com> <20030326183351.GJ57674@elvis.mu.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Status: No, hits=-25.9 required=5.0 tests=AWL,EMAIL_ATTRIBUTION,IN_REP_TO,QUOTED_EMAIL_TEXT, REFERENCES,REPLY_WITH_QUOTES version=2.50 X-Spam-Level: X-Spam-Checker-Version: SpamAssassin 2.50 (1.173-2003-02-20-exp) cc: Sam Leffler cc: src-committers@FreeBSD.org cc: Doug Ambrisko cc: cvs-all@FreeBSD.org cc: cvs-src@FreeBSD.org Subject: Re: cvs commit: src/sys/conf options src/sys/netinet ip_output.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Mar 2003 19:15:56 -0000 On Wed, 26 Mar 2003, Maxime Henrion wrote: > Nice work! This will really be very useful to have for the network > interface drivers. I have a few comments : > > - You removed the m_getcl() optimization that has been added in if_xl.c > recently. You should use m_getcl() if len is > MHLEN, because m_getcl() > grabs both a header and a cluster. If len is <= MHLEN, you can just use > m_gethdr(). I guess you did this because you have been hitting the > KASSERT() in m_dup_pkthdr(), and I think this KASSERT() is a bit bogus. > It seems to me there should be a way to do it without having to delay > the cluster allocation. Maybe Sam or Robert could comment on this? You hit the nail on the head. Sam says that Bosko may enhance m_dup_pkthdr() to handle the M_EXT case soon; if and when that occurs, I'll fix up m_defrag to use it. Until then, I'll stick with what I have so as to avoid getting sidetracked. > - I believe an mbuf chain passed to a network driver should always have > a packet header. Perhaps true, but I want this routine to not blow up if it's used elsewhere. (I may use it in the IP reassembly code, and it could end up elsewhere.) > - Minor style(9) nits: you should test pointers against NULL and not using them > as booleans. Also, there's a trailing newline in the m_defragment() > function. > > Thanks for doing this work! > > Cheers, > Maxime Ok, I'll make those changes in a future revision. Sam had some comments as well, I hope to be able to get around to putting out pass3 later today. Thanks for the comments, Mike "Silby" Silbersack