From owner-cvs-src@FreeBSD.ORG Mon Feb 14 19:12:37 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 F0C7816A4FE; Mon, 14 Feb 2005 19:12:36 +0000 (GMT) Received: from ebb.errno.com (ebb.errno.com [66.127.85.87]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0A69743D45; Mon, 14 Feb 2005 19:12:34 +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 j1EJCVWi059025 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 Feb 2005 11:12:31 -0800 (PST) (envelope-from sam@errno.com) Message-ID: <4210F849.8060005@errno.com> Date: Mon, 14 Feb 2005 11:13:13 -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> In-Reply-To: <20050214181431.GA69635@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: Mon, 14 Feb 2005 19:12:37 -0000 Ruslan Ermilov wrote: > Hi Sam, > > On Mon, Feb 14, 2005 at 08:30:08AM -0800, Sam Leffler wrote: > >>Ruslan Ermilov wrote: >> >>>ru 2005-02-14 08:29:42 UTC >>> >>> FreeBSD src repository >>> >>> Modified files: >>> sys/net if_ethersubr.c >>> Log: >>> If no vlan(4) interfaces are configured for the interface, and the >>> driver did VLAN decapsulation in hardware, we were passing a frame >>> as if it came for the parent (non-VLAN) interface. Stop this from >>> happening. >>> >>> Reminded by: glebius >>> Security: This could pose a security risk in some setups >>> >>> Revision Changes Path >>> 1.183 +10 -3 src/sys/net/if_ethersubr.c >>>http://cvsweb.FreeBSD.org/src/sys/net/if_ethersubr.c.diff?r1=1.182&r2=1.183 >>> >>> >> >>Looks like you should use m_tag_find instead of >>m_tag_first+m_tag_locate. >> > > Unfortunately m_tag_find() is only the compatibility function and > doesn't work for FreeBSD mtags. Er, yes, thanks. > > >>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. As to performance ether_demux can be called from many places and it's unclear whether there will be non-vlan tags present that will add overhead for the common case. That is the reason why if_nvlans exist; otherwise I'd have just checked for the tag. Sam