From 79a58745415c25708194d7d8be0dbb03cc02605f Mon Sep 17 00:00:00 2001 From: Bill Paul Date: Sun, 6 Jul 2003 03:24:25 +0000 Subject: [PATCH] Testing VLANs with the new 8139C+ chip (which does hardware tag insertion and extraction) has revealed two bugs: - In vlan_start(), we're supposed to check the underlying interface to see if it has the IFCAP_VLAN_HWTAGGING cabability set and, if so, set things up for the VLAN_OUTPUT_TAG() routine. However the code checks ifp->if_capabilities, which is the vlan pseudo-interface's capabilities when it should be checking p->if_capabilities, which relates to the underlying physical interface. Change ifp->if_capabilities to p->if_capabilities so this works. - In vlan_input(), we have to extract the 16-bit tag value from the received frame and use it to figure out which vlan interface gets the frame. The code that we use to track down the desired vlan pseudo-interface is: for (ifv = LIST_FIRST(&ifv_list); ifv != NULL; ifv = LIST_NEXT(ifv, ifv_list)) if (ifp == ifv->ifv_p && tag == ifv->ifv_tag) break; The problem is that 'tag' is not computed consistently. In the case where the interface supports hardware VLAN tag extraction and calls VLAN_INPUT_TAG(), we do this: tag = *(u_int*)(mtag+1); But in the software emulation case, we do this tag = EVL_VLANOFTAG(ntohs(evl->evl_tag)); The problem here is the EVL_VLANOFTAG() macro is only ever applied in this one case. It's never applied to ifv->ifv_tag or anwhere else. We must be consistent: either it's applied everywhere or nowhere. To see how this can be a problem, do something like ifconfig vlan0 vlan 12345 vlandev foo0 and observe the results. 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. --- sys/net/if_vlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index 5fe0a0ab14f2..99b1dcfb8f1a 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -319,7 +319,7 @@ vlan_start(struct ifnet *ifp) * knows how to find the VLAN tag to use, so we attach a * packet tag that holds it. */ - if (ifp->if_capabilities & IFCAP_VLAN_HWTAGGING) { + if (p->if_capabilities & IFCAP_VLAN_HWTAGGING) { struct m_tag *mtag = m_tag_alloc(MTAG_VLAN, MTAG_VLAN_TAG, sizeof (u_int), @@ -409,7 +409,7 @@ vlan_input(struct ifnet *ifp, struct mbuf *m) ("vlan_input: bad encapsulated protocols (%u)", ntohs(evl->evl_encap_proto))); - tag = EVL_VLANOFTAG(ntohs(evl->evl_tag)); + tag = ntohs(evl->evl_tag); /* * Restore the original ethertype. We'll remove