Re: Broadcom bge and 802.1Q vlan tags

Ruslan Ermilov wrote:
> On Tue, Oct 12, 2004 at 08:54:24AM -0700, Sam Leffler wrote:
> 
>>Gleb Smirnoff wrote:
>>
>>>On Tue, Oct 12, 2004 at 10:36:27AM +0200, Roub?cek Zdenek (T-Systems 
>>>PragoNet) wrote:
>>>R>  I have run into a problem with my Broadcom NIC (Dell LATITUDE D600). I 
>>>am not able to detect 802.1Q tags on incoming interface with ethereal or 
>>>tcpdump. All incoming packets seems like they are not coming through trunk 
>>>but as native ETH frames, ie. the vlan tag is missing, probably removed 
>>>before being passed to tcpdump?
>>>R> 
>>>R> No I have not tested NIC's behaviour on 4.X, but I is working with 
>>>linux (2.6.something kernel probably?)
>>>R> 
>>>R> Any ideas what to modify or set so I can detect vlan_tag would be very 
>>>apreciated.
>>>
>>>As Ruslan already mentioned, it is impossible to turn off hardware VLAN
>>>stripping in bge driver.
>>>
>>>A patch to stop tagged frames to come on trunk interface is like this:
>>>
>>>_at__at_ -701,13 +657,16 _at__at_
>>>        * see if the device performed the decapsulation and
>>>        * provided us with the tag.
>>>        */
>>>-       if (ifp->if_nvlans &&
>>>-           m_tag_locate(m, MTAG_VLAN, MTAG_VLAN_TAG, NULL) != NULL) {
>>>+       if (m_tag_locate(m, MTAG_VLAN, MTAG_VLAN_TAG, NULL) != NULL) {
>>>               /*
>>>                * vlan_input() will either recursively call ether_input()
>>>                * or drop the packet.
>>>                */
>>>-               KASSERT(vlan_input_p != NULL,("ether_input: VLAN not 
>>>loaded!"));
>>>+               if (vlan_input_p == NULL) {
>>>+                       /* vlan(4) is not loaded, discard frame */
>>>+                       m_freem(m);
>>>+                       return;
>>>+               }
>>>               (*vlan_input_p)(ifp, m);
>>>               return;
>>>       }
>>>
>>
>>This pessimizes normal traffic.
>>
> 
> m_tag_locate() doesn't look like a very expensive function.  And
> with the "normal traffic", I don't expect to be more than one tag,
> no?  Also, if if_nvlans > 0, this is already "pessimized".
> 
> 
>>We should look for a solution in the 
>>driver(s) to avoid sending packets up with tags when no vlans are 
>>configured.
>>
> 
> I'd be opposed to such a change in behavior.  The VLAN consumer can
> be not only vlan(4), it can equally be the ng_vlan(4) node, etc.

I'm not sure what you are opposed to or why.  The issue I have is that 
m_tag_locate can be expensive if many packets have tags.  The check for 
the existence of vlans configured on the interface short-circuits this 
work.  That vlan-tagged packets may be generated when no vlans are 
configured seems wrong to me and breaks the assumption used to write the 
code.  Changing the driver to drop the frame if ifp->if_nvlans is zero 
seems straightforward and could probably be hidden in the existing macro.

	Sam