From owner-freebsd-net@FreeBSD.ORG Wed Jul 9 08:50:30 2008 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 91418106568E; Wed, 9 Jul 2008 08:50:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx07.syd.optusnet.com.au (fallbackmx07.syd.optusnet.com.au [211.29.132.9]) by mx1.freebsd.org (Postfix) with ESMTP id 2C2148FC44; Wed, 9 Jul 2008 08:50:30 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail06.syd.optusnet.com.au (mail06.syd.optusnet.com.au [211.29.132.187]) by fallbackmx07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id m656gD4e024931; Sat, 5 Jul 2008 16:42:13 +1000 Received: from c220-239-252-11.carlnfd3.nsw.optusnet.com.au (c220-239-252-11.carlnfd3.nsw.optusnet.com.au [220.239.252.11]) by mail06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id m656g9vR017221 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 5 Jul 2008 16:42:11 +1000 Date: Sat, 5 Jul 2008 16:42:09 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: John Baldwin In-Reply-To: <200807041748.m64HmZur018637@svn.freebsd.org> Message-ID: <20080705161831.F13262@delplex.bde.org> References: <200807041748.m64HmZur018637@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: net@freebsd.org Subject: Re: svn commit: r180256 - head/sys/dev/arl 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, 09 Jul 2008 08:50:30 -0000 On Fri, 4 Jul 2008, John Baldwin wrote: > Author: jhb > Date: Fri Jul 4 17:48:34 2008 > New Revision: 180256 > URL: http://svn.freebsd.org/changeset/base/180256 > > Log: > Make arl(4) MPSAFE: > ... > - ifp->if_snd.ifq_maxlen = IFQ_MAXLEN; > + IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN); Why do we obfuscate setting of ifq_maxlen using a macro, especially when the setting is to a wrong default value? The macro was introduced with ALTQ changes, but seems to have never done anything different for ALTQ. ALTQ also introduced an ifq_drv_maxlen field, but the macro provides no help for managing this. Drivers that support ALTQ end up with 2 settings of ifq_*maxlen, one direct one for ifq_drv_maxlen and one obfuscated one of ifq_maxlen. arl apparently doesn't support ALTQ, and you didn't fix this -- it still doesn't set ifq_drv_maxlen. if_attach() uses the correct default value of ifqmaxlen if the driver leaves ifp->if_snd.ifq_maxlen set to 0, but prints a bogus warning about this. Non-driver code under net/ still mostly doesn't use the obfuscation, but uses IFQ_MAXLEN and ifqmaxlen almost perfectly randomly to have about 50% of each. Since ifqmaxlen isn't a tuneable or sysctl, and is statically initialized to IFQ_MAXLEN, not using only makes a difference if someone iniitalizes it diffently using a debugger, so these bugs are normally just spelling errors. IFQ_MAXLEN is also too small for 1Gbps or even 100Nbps hardware devices, so only drivers for old hardware and some software drivers can use it anyway. Bruce