Date: Sat, 5 Jul 2008 16:42:09 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@freebsd.org> Cc: net@freebsd.org Subject: Re: svn commit: r180256 - head/sys/dev/arl Message-ID: <20080705161831.F13262@delplex.bde.org> In-Reply-To: <200807041748.m64HmZur018637@svn.freebsd.org>
index | next in thread | previous in thread | raw e-mail
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. Brucehome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080705161831.F13262>