Date: Tue, 15 Feb 2005 09:48:52 -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: <42123604.9070002@errno.com> 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
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?42123604.9070002>