Date: Tue, 15 Feb 2005 09:53:28 +0200 From: Ruslan Ermilov <ru@freebsd.org> To: Sam Leffler <sam@errno.com> Cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/net if_ethersubr.c Message-ID: <20050215075328.GD6781@ip.net.ua> In-Reply-To: <20050215074226.GA6781@ip.net.ua> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--2iBwrppp/7QCDedR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 15, 2005 at 09:42:26AM +0200, 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=20 > > >>>>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) !=3D 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=20 > > >>not one should inspect the tag chain for vlan tags. This effectively= =20 > > >>renders that assumption invalid. I think it would better to discard= =20 > > >>these frames in the driver rather than allocate a tag, pass it up, th= en=20 > > >>discard it in ether_demux. I think you could encapsulate the check i= n=20 > > >>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. > >=20 > > And when you said this before my reply was: don't penalize non-netgraph= =20 > > use of the system. If netgraph truly needs to violate this underlying= =20 > > assumption of the vlan code then please do it with an ifdef. Otherwise= =20 > > let's find a better solution. And if that's not possible then we shoul= d=20 > > rethink having if_nvlans at all as this change renders it meaningless. > >=20 > 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. >=20 How about if we mark mbuf as having a VLAN mtag, in VLAN_INPUT_TAG() macro, with a simple bit flag? Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --2iBwrppp/7QCDedR Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQFCEap4qRfpzJluFF4RAgbzAJ0a38D7B5QiYzwt+6JfBKvZTorNcgCcCxn+ vXXYsKVm/WfuzLjbvYCRUIU= =NeWF -----END PGP SIGNATURE----- --2iBwrppp/7QCDedR--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050215075328.GD6781>