From owner-cvs-src@FreeBSD.ORG Wed Mar 26 10:33:52 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 6891537B404; Wed, 26 Mar 2003 10:33:52 -0800 (PST) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.FreeBSD.org (Postfix) with ESMTP id E7F6C43F85; Wed, 26 Mar 2003 10:33:51 -0800 (PST) (envelope-from mux@freebsd.org) Received: by elvis.mu.org (Postfix, from userid 1920) id C6D892ED430; Wed, 26 Mar 2003 10:33:51 -0800 (PST) Date: Wed, 26 Mar 2003 19:33:51 +0100 From: Maxime Henrion To: Mike Silbersack Message-ID: <20030326183351.GJ57674@elvis.mu.org> References: <200303260452.h2Q4quap015364@www.ambrisko.com> <20030326114030.U2075@odysseus.silby.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20030326114030.U2075@odysseus.silby.com> User-Agent: Mutt/1.4i X-Spam-Status: No, hits=-32.5 required=5.0 tests=EMAIL_ATTRIBUTION,IN_REP_TO,QUOTED_EMAIL_TEXT,REFERENCES, REPLY_WITH_QUOTES,USER_AGENT_MUTT autolearn=ham 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 18:33:54 -0000 Mike Silbersack wrote: > > On Tue, 25 Mar 2003, Doug Ambrisko wrote: > > > | I think that we'll end up being even better off by just making a > > | m_defragment function, thereby reducing code duplication even more. I'll > > | make sure to test any such function more than I tested the MBUF_FRAG_TEST > > | code, however. :) > > > > Yes, I was thinking that as well. > > Ok, here's my attempt at a m_defragment function. It seems to work well > here, although I have only tested it with the if_xl driver. Could you > guys give it a quick lookover to see if it has any bugs? I'll add the > ability to copy mbuf chains longer than MCLBYTES once I know all the > packet header handling / etc is correct. > > Also note that the patch contains my changes to if_xl, which are mostly > debugging code. You can ignore most of that. One change I _will_ make to > if_xl, however, is to walk the chain and find out its length _before_ we > do any busdma calls; this moves the defragmentation occur earlier, thereby > avoiding the need for a complex error recovery case. I suspect we'll want > to do the same in the other network drivers. 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? - I believe an mbuf chain passed to a network driver should always have a packet header. - 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