From owner-svn-src-all@FreeBSD.ORG Thu Nov 21 01:27:24 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 720F0200; Thu, 21 Nov 2013 01:27:24 +0000 (UTC) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id DEF0E2754; Thu, 21 Nov 2013 01:27:23 +0000 (UTC) Received: from julian-mbp3.pixel8networks.com (50-196-156-133-static.hfc.comcastbusiness.net [50.196.156.133]) (authenticated bits=0) by vps1.elischer.org (8.14.7/8.14.7) with ESMTP id rAL1RKmp006812 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Wed, 20 Nov 2013 17:27:22 -0800 (PST) (envelope-from julian@freebsd.org) Message-ID: <528D6173.4080406@freebsd.org> Date: Wed, 20 Nov 2013 17:27:15 -0800 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 MIME-Version: 1.0 To: Adrian Chadd , "George V. Neville-Neil" , "freebsd-arch@freebsd.org" , FreeBSD Net Subject: Re: svn commit: r258328 - head/sys/net References: <201311182258.rAIMwEFd048783@svn.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Nov 2013 01:27:24 -0000 On 11/20/13, 2:02 PM, Adrian Chadd wrote: > Can we revert this and just quickly put together something else that > won't potentially come back to bite us with weird side effects? > > My suggestions (and I'm happy to do this work if needed): > > * create a lightweight mbuf queue representation so an mbuf list isn't > just the mbuf header pointer; > * create a new ether input (say, ether_input_multi) that takes an mbuf > list, so existing driver code does the right thing. > > After that it'd be nice to write a set of mbuf list macros for > abstract the whole queue, dequeue, concat, iterate, etc (like > sys/queue.h, but for mbufs.) > > What do people think? > > (I've been doing it for m->next chained things, but not m->m_nextpkt things..) I was thinking: new interfaces.. (your -multi names sound good). macros for handling said lists so that people don't screw them up Old drivers run with no change. > > > > -adrian > > > On 18 November 2013 14:58, George V. Neville-Neil wrote: >> Author: gnn >> Date: Mon Nov 18 22:58:14 2013 >> New Revision: 258328 >> URL: http://svnweb.freebsd.org/changeset/base/258328 >> >> Log: >> 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 >> >> 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; >> + } >> } >> >> /*