Date: Mon, 14 Feb 2005 11:13:13 -0800 From: Sam Leffler <sam@errno.com> To: Ruslan Ermilov <ru@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/net if_ethersubr.c Message-ID: <4210F849.8060005@errno.com> In-Reply-To: <20050214181431.GA69635@ip.net.ua> References: <200502140829.j1E8TgDs086634@repoman.freebsd.org> <4210D210.3080700@errno.com> <20050214181431.GA69635@ip.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4210F849.8060005>