From owner-svn-src-head@FreeBSD.ORG Tue Nov 19 11:04:59 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 4314411F; Tue, 19 Nov 2013 11:04:59 +0000 (UTC) Received: from cyrus.watson.org (cyrus.watson.org [198.74.231.69]) by mx1.freebsd.org (Postfix) with ESMTP id 209E2276D; Tue, 19 Nov 2013 11:04:59 +0000 (UTC) Received: from fledge.watson.org (fledge.watson.org [198.74.231.63]) by cyrus.watson.org (Postfix) with ESMTPS id 21F6146B0C; Tue, 19 Nov 2013 06:04:58 -0500 (EST) Date: Tue, 19 Nov 2013 11:04:57 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: "George V. Neville-Neil" Subject: Re: svn commit: r258328 - head/sys/net In-Reply-To: <201311182258.rAIMwEFd048783@svn.freebsd.org> Message-ID: References: <201311182258.rAIMwEFd048783@svn.freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Nov 2013 11:04:59 -0000 On Mon, 18 Nov 2013, George V. Neville-Neil wrote: > Allow ethernet drivers to pass in packets connected via the nextpkt pointer. > Handling packets in this way allows drivers to amortize work during packet reception. > > Submitted by: Vijay Singh > Sponsored by: NetApp Currently, it is quite easy to make mistakes regarding individual mbuf chains vs. lists of mbuf chains. This leads me to wonder whether a new type, perhaps simply constructed on the stack before passing in, should be used for KPIs that accept lists of packets. E.g., /* * This structure is almost always allocated on a caller stack, so * cannot itself be queued without memory allocation in most cases. */ struct mbuf_queue { struct mbuf *mq_head; }; int ether_input(struct ifnet *ifp, struct mbuf_queue *m) { ... } ... struct mbuf_queue mq = { m }; return (ether_input(ifp, &mq)); ... That way the compiler can help us figure out where we expect an individual packet but have accidentally leaked a queue. Functions that accept only a single packet could also more agressively assert that m->m_nextpkt is NULL: M_ASSERT_ONEPACKET(m); Robert > > Modified: > head/sys/net/if_ethersubr.c > > Modified: head/sys/net/if_ethersubr.c > ============================================================================== > --- head/sys/net/if_ethersubr.c Mon Nov 18 22:55:50 2013 (r258327) > +++ head/sys/net/if_ethersubr.c Mon Nov 18 22:58:14 2013 (r258328) > @@ -708,13 +708,25 @@ static void > ether_input(struct ifnet *ifp, struct mbuf *m) > { > > + struct mbuf *mn; > + > /* > - * We will rely on rcvif being set properly in the deferred context, > - * so assert it is correct here. > + * The drivers are allowed to pass in a chain of packets linked with > + * m_nextpkt. We split them up into separate packets here and pass > + * them up. This allows the drivers to amortize the receive lock. > */ > - KASSERT(m->m_pkthdr.rcvif == ifp, ("%s: ifnet mismatch", __func__)); > + while (m) { > + mn = m->m_nextpkt; > + m->m_nextpkt = NULL; > > - netisr_dispatch(NETISR_ETHER, m); > + /* > + * We will rely on rcvif being set properly in the deferred context, > + * so assert it is correct here. > + */ > + KASSERT(m->m_pkthdr.rcvif == ifp, ("%s: ifnet mismatch", __func__)); > + netisr_dispatch(NETISR_ETHER, m); > + m = mn; > + } > } > > /* >