From owner-cvs-src@FreeBSD.ORG Tue Feb 15 17:48:11 2005 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 3C36216A4CE; Tue, 15 Feb 2005 17:48:11 +0000 (GMT) Received: from ebb.errno.com (ebb.errno.com [66.127.85.87]) by mx1.FreeBSD.org (Postfix) with ESMTP id DADD543D49; Tue, 15 Feb 2005 17:48:10 +0000 (GMT) (envelope-from sam@errno.com) Received: from [66.127.85.91] (sam@[66.127.85.91]) (authenticated bits=0) by ebb.errno.com (8.12.9/8.12.6) with ESMTP id j1FHmAWi064840 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 15 Feb 2005 09:48:10 -0800 (PST) (envelope-from sam@errno.com) Message-ID: <42123604.9070002@errno.com> Date: Tue, 15 Feb 2005 09:48:52 -0800 From: Sam Leffler User-Agent: Mozilla Thunderbird 1.0RC1 (X11/20041208) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Ruslan Ermilov References: <200502140829.j1E8TgDs086634@repoman.freebsd.org> <4210D210.3080700@errno.com> <20050214181431.GA69635@ip.net.ua> <4210F849.8060005@errno.com> <20050214195558.GD69635@ip.net.ua> <421104C7.4070709@errno.com> <20050215074226.GA6781@ip.net.ua> In-Reply-To: <20050215074226.GA6781@ip.net.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/net if_ethersubr.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: Tue, 15 Feb 2005 17:48:11 -0000 Ruslan Ermilov wrote: > On Mon, Feb 14, 2005 at 12:06:31PM -0800, Sam Leffler wrote: > >>Ruslan Ermilov wrote: >> >>>On Mon, Feb 14, 2005 at 11:13:13AM -0800, Sam Leffler wrote: >>> >>> >>>>>>This also has the potential to noticeably >>>>>>affect performance so I think a better solution is needed. >>>>> >>>>>Here are my thoughts. On a typical input path, there will be >>>>>either one or zero mtags, one if driver provided us with the >>>>>VLAN mtag, so effectively we replaced "ifp->if_nvlans" with >>>>>"m_tag_first(m) != NULL", and this doesn't look like a huge >>>>>performance downgrade to me, if at all. >>>> >>>>The intent was/is that if_nvlans be the definitive check for whether or >>>>not one should inspect the tag chain for vlan tags. This effectively >>>>renders that assumption invalid. I think it would better to discard >>>>these frames in the driver rather than allocate a tag, pass it up, then >>>>discard it in ether_demux. I think you could encapsulate the check in >>>>VLAN_INPUT_TAG. >>>> >>> >>>I said this before: vlan(4) is not the only consumer of VLAN >>>frames in FreeBSD. VLAN frames are also accepted by ng_vlan(4), >>>so using the vlan(4)-specific if_nvlans to decide whether we >>>should accept VLAN frames (in the driver) just isn't appropriate. >> >>And when you said this before my reply was: don't penalize non-netgraph >>use of the system. If netgraph truly needs to violate this underlying >>assumption of the vlan code then please do it with an ifdef. Otherwise >>let's find a better solution. And if that's not possible then we should >>rethink having if_nvlans at all as this change renders it meaningless. >> > > Netgraph worked before and after this change, because Netgraph > (ng_ether) processing happens earlier. What this change does > is to fix a bug where stripped VLAN frames are passed to the > parent interface when they shouldn't be (i.e., when no vlan(4) > interfaces are configured on this Ethernet). Given that there > may be more than just vlan(4) consumers of VLANs in the system, > if you really think this change pessimizes performance, a better > solution is needed, but this solution shouldn't hurt Netgraph > either, and if we used an if_nvlans to indicate whether we > should accept VLANs, this would break it. I don't know at > this point what a better solution could be. If you have ideas, > let's discuss them. So far, the solutions proposed don't seem > to work well. If the intent is simply to eliminate the dispatch of frames out of the driver when there are no vlan consumers then it would seem another option is to do this at the source. I suggested this initially--that VLAN_INPUT_TAG be augmented to discard frames when if_nvlans is zero (this would also eliminate gratuitous tag allocation). I believe you're saying that if_nvlans can be zero and there may still be consumers for the packets in which case we might look at a better way of indicating there are consumers for the packets assuming we can be sure they won't reach ether_demux (which might be hard). As to your other suggestion of allocating an mbuf flag bit that'd be fine with me. I didn't offer that because I thought we were out of free bits but I see m_flags got changed to an int a while back and only 16-bits are assigned. Sam