Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 7 Jul 2003 16:45:11 -0700 (PDT)
From:      wpaul@FreeBSD.ORG (Bill Paul)
To:        wollman@lcs.mit.edu (Garrett Wollman)
Cc:        cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/net if_vlan.c
Message-ID:  <20030707234511.3B0FB37B401@hub.freebsd.org>
In-Reply-To: <200307071845.h67Ij1N1016542@khavrinen.lcs.mit.edu> from Garrett Wollman at "Jul 7, 2003 02:45:01 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
> <<On Sat, 5 Jul 2003 20:24:25 -0700 (PDT), Bill Paul <wpaul@FreeBSD.org> said:
> 
> >     I'm not quite sure what the right thing is to do here. Neither the
> >     vlan(4) nor ifconfig(8) man pages suggest which way to go. For now,
> >     I've removed this use of EVL_VLANOFTAG() so that the tag will match
> >     correctly in all cases. I will not get upset if somebody makes a
> >     compelling argument for using EVL_VLANOFTAG() everywhere instead,
> >     as long as the use is consistent.
>   
> VLAN tags have only 12 bits.  Anything else is part of the 802.1p
> encapsulation header (either the priority or the ``this packet was
> translated from Token Ring'' bit).
> 
> -GAWollman
> 

Ok, based on this (and what others have said), it looks like the
correct behavior is to mask off all but the VLAN ID bits in all cases
in vlan_input(). This is easy enough to do. The only issue then is
what to do about the fact that the user is currently allowed to set
the non-VLAN ID bits with ifconfig(8). We can do one of two things:

1) Change vlan_input() to apply EVL_VLANOFTAG() to all incoming tags
   (in both hardware and non-hardware tagging cases) and change the
   vlan_ioctl() function to only accept vlan tags from 0 to 4095
   and return EINVAL if the caller tries to set any of the other
   bits. (Prevent user from shooting self in foot.)

2) Let the user set any bits, but always mask off the non-VLAN ID
   bits in vlan_input(), i.e.:

--- if_vlan.c.orig	Sat Jul  5 19:53:51 2003
+++ if_vlan.c	Mon Jul  7 10:44:07 2003
@@ -429,7 +429,8 @@
 
 	for (ifv = LIST_FIRST(&ifv_list); ifv != NULL;
 	    ifv = LIST_NEXT(ifv, ifv_list))
-		if (ifp == ifv->ifv_p && tag == ifv->ifv_tag)
+		if (ifp == ifv->ifv_p && EVL_VLANOFTAG(tag) ==
+		    EVL_VLANOFTAG(ifv->ifv_tag))
 			break;
 
 	if (ifv == NULL || (ifv->ifv_if.if_flags & IFF_UP) == 0) {

I am leaning towards option 1). Am I sane or not?

Er... you know what I mean.

-Bill

--
=============================================================================
-Bill Paul            (510) 749-2329 | Senior Engineer, Master of Unix-Fu
                 wpaul@windriver.com | Wind River Systems
=============================================================================
      "If stupidity were a handicap, you'd have the best parking spot."
=============================================================================



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030707234511.3B0FB37B401>