cvs commit: src/sys/net if_ethersubr.c

[Sam Leffler]

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