[PATCH] Part 2 of low level 802.1p priority support

2007-02-10 Thread Bruce M. Simpson
This updated patch moves VLAN tag decapsulation into if_ethersubr.c and 
always uses M_VLANTAG, which is also passed to the upper layer.


Tests with ping:
fxp (no VLAN_HWTAGGING support)  OK
msk (VLAN_HWTAGGING enabled) OK
msk (VLAN_HWTAGGING disanabled) FAIL

I am concerned that this may need review and testing to support 
situations where we do nested VLANs or with bridge(4) before it can be 
committed.


Further testing with drivers is needed (I can't be 100% sure it fails 
with msk(4) because something strange is happening when vlan tagging is 
turned off). Perhaps Pyun knows?


Regards,
BMS


Index: if_ethersubr.c
===
RCS file: /home/ncvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.222
diff -u -p -r1.222 if_ethersubr.c
--- if_ethersubr.c	24 Dec 2006 08:52:13 -	1.222
+++ if_ethersubr.c	10 Feb 2007 18:16:54 -
@@ -701,43 +701,50 @@ post_stats:
 		}
 	}
 #endif
-
 	/*
-	 * Check to see if the device performed the VLAN decapsulation and
-	 * provided us with the tag.
+	 * If the device did not perform decapsulation of the 802.1q VLAN
+	 * header itself, do this now, and tag the mbuf with M_VLANTAG.
+	 * Remove the 802.1q header by copying the Ethernet addresses over
+	 * it and adjusting the beginning of the data in the mbuf.
+	 * Re-inspect the ether_type field so we do the right thing
+	 * for VLAN 0.
 	 */
-	if (m->m_flags & M_VLANTAG) {
-		/*
-		 * If no VLANs are configured, drop.
-		 */
-		if (ifp->if_vlantrunk == NULL) {
-			ifp->if_noproto++;
-			m_freem(m);
+	if ((ether_type == ETHERTYPE_VLAN) && !(m->m_flags & M_VLANTAG)) {
+		struct ether_vlan_header *evl;
+
+		if (m->m_len < sizeof(*evl) &&
+		(m = m_pullup(m, sizeof(*evl))) == NULL) {
+			if_printf(ifp, "cannot pullup VLAN header\n");
 			return;
 		}
-		/*
-		 * vlan_input() will either recursively call ether_input()
-		 * or drop the packet.
-		 */
-		KASSERT(vlan_input_p != NULL,("ether_input: VLAN not loaded!"));
-		(*vlan_input_p)(ifp, m);
-		return;
+
+		evl = mtod(m, struct ether_vlan_header *);
+		m->m_pkthdr.ether_vtag = ntohs(evl->evl_tag);
+		m->m_flags |= M_VLANTAG;
+		bcopy((char *)evl, (char *)evl + ETHER_VLAN_ENCAP_LEN,
+		  ETHER_HDR_LEN - ETHER_TYPE_LEN);
+		m_adj(m, ETHER_VLAN_ENCAP_LEN);
+		/* We need to see the inner type field in case of reentry. */
+		eh = mtod(m, struct ether_header *);
+		ether_type = ntohs(eh->ether_type);
 	}
 
 	/*
-	 * Handle protocols that expect to have the Ethernet header
-	 * (and possibly FCS) intact.
+	 * Deal with numbered 802.1q VLANs, by passing these frames to
+	 * the VLAN input handler. Frames destined for VLAN 0 are for
+	 * the main input path. Otherwise, drop frames with VLAN tags.
 	 */
-	switch (ether_type) {
-	case ETHERTYPE_VLAN:
+	if ((m->m_flags & M_VLANTAG) &&
+	EVL_VLANOFTAG(m->m_pkthdr.ether_vtag) != EVL_VLAN_ZERO) {
 		if (ifp->if_vlantrunk != NULL) {
-			KASSERT(vlan_input_p,("ether_input: VLAN not loaded!"));
+			KASSERT(vlan_input_p,
+			("ether_input: VLAN not loaded!"));
 			(*vlan_input_p)(ifp, m);
 		} else {
 			ifp->if_noproto++;
 			m_freem(m);
+			return;
 		}
-		return;
 	}
 
 	/* Strip off Ethernet header. */
Index: if_vlan.c
===
RCS file: /home/ncvs/src/sys/net/if_vlan.c,v
retrieving revision 1.117
diff -u -p -r1.117 if_vlan.c
--- if_vlan.c	30 Dec 2006 21:10:25 -	1.117
+++ if_vlan.c	10 Feb 2007 18:16:54 -
@@ -911,51 +911,9 @@ vlan_input(struct ifnet *ifp, struct mbu
 	uint16_t tag;
 
 	KASSERT(trunk != NULL, ("%s: no trunk", __func__));
+	KASSERT((m->m_flags & M_VLANTAG),("%s: M_VLANTAG not set", __func__));
 
-	if (m->m_flags & M_VLANTAG) {
-		/*
-		 * Packet is tagged, but m contains a normal
-		 * Ethernet frame; the tag is stored out-of-band.
-		 */
-		tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag);
-		m->m_flags &= ~M_VLANTAG;
-	} else {
-		struct ether_vlan_header *evl;
-
-		/*
-		 * Packet is tagged in-band as specified by 802.1q.
-		 */
-		switch (ifp->if_type) {
-		case IFT_ETHER:
-			if (m->m_len < sizeof(*evl) &&
-			(m = m_pullup(m, sizeof(*evl))) == NULL) {
-if_printf(ifp, "cannot pullup VLAN header\n");
-return;
-			}
-			evl = mtod(m, struct ether_vlan_header *);
-			tag = EVL_VLANOFTAG(ntohs(evl->evl_tag));
-
-			/*
-			 * Remove the 802.1q header by copying the Ethernet
-			 * addresses over it and adjusting the beginning of
-			 * the data in the mbuf.  The encapsulated Ethernet
-			 * type field is already in place.
-			 */
-			bcopy((char *)evl, (char *)evl + ETHER_VLAN_ENCAP_LEN,
-			  ETHER_HDR_LEN - ETHER_TYPE_LEN);
-			m_adj(m, ETHER_VLAN_ENCAP_LEN);
-			break;
-
-		default:
-#ifdef INVARIANTS
-			panic("%s: %s has unsupported if_type %u",
-			  __func__, ifp->if_xname, ifp->if_type);
-#endif
-			m_freem(m);
-			ifp->if_noproto++;
-			return;
-		}
-	}
+	tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag);
 
 	TRUNK_RLOCK(trunk);
 #ifdef VLAN_ARRAY

Re: [PATCH] Part 2 of low level 802.1p priority support

2007-02-10 Thread Andrew Thompson
On Sat, Feb 10, 2007 at 06:28:41PM +, Bruce M. Simpson wrote:
> This updated patch moves VLAN tag decapsulation into if_ethersubr.c and 
> always uses M_VLANTAG, which is also passed to the upper layer.
> 
> Tests with ping:
> fxp (no VLAN_HWTAGGING support)  OK
> msk (VLAN_HWTAGGING enabled) OK
> msk (VLAN_HWTAGGING disanabled) FAIL
> 
> I am concerned that this may need review and testing to support 
> situations where we do nested VLANs or with bridge(4) before it can be 
> committed.

This is great for the bridge, it has needed to take the vlan tag into
account when deciding to forward or not. Having m_pkthdr.ether_vtag
always set makes this much easier to implement.

> Index: if_vlan.c
> ===
> RCS file: /home/ncvs/src/sys/net/if_vlan.c,v
>  
> - if (m->m_flags & M_VLANTAG) {
> - /*
> -  * Packet is tagged, but m contains a normal
> -  * Ethernet frame; the tag is stored out-of-band.
> -  */
> - tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag);
> - m->m_flags &= ~M_VLANTAG;

> - } else {

...

> + tag = EVL_VLANOFTAG(m->m_pkthdr.ether_vtag);

In the deleted code M_VLANTAG is cleared but is not done anymore, is
this right?


Andrew
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: [PATCH] Part 2 of low level 802.1p priority support

2007-02-10 Thread Pyun YongHyeon
On Sat, Feb 10, 2007 at 06:28:41PM +, Bruce M. Simpson wrote:
 > This updated patch moves VLAN tag decapsulation into if_ethersubr.c and 
 > always uses M_VLANTAG, which is also passed to the upper layer.
 > 
 > Tests with ping:
 > fxp (no VLAN_HWTAGGING support)  OK
 > msk (VLAN_HWTAGGING enabled) OK
 > msk (VLAN_HWTAGGING disanabled) FAIL
 > 
 > I am concerned that this may need review and testing to support 
 > situations where we do nested VLANs or with bridge(4) before it can be 
 > committed.
 > 
 > Further testing with drivers is needed (I can't be 100% sure it fails 
 > with msk(4) because something strange is happening when vlan tagging is 
 > turned off). Perhaps Pyun knows?
 > 

I guess I've not merged local changes before committing to HEAD.
How about attached one?

 > Regards,
 > BMS
 > 
 > 

-- 
Regards,
Pyun YongHyeon
Index: if_msk.c
===
RCS file: /home/ncvs/src/sys/dev/msk/if_msk.c,v
retrieving revision 1.8
diff -u -r1.8 if_msk.c
--- if_msk.c9 Jan 2007 01:31:22 -   1.8
+++ if_msk.c11 Feb 2007 07:26:08 -
@@ -3029,7 +3029,8 @@
cons = sc_if->msk_cdata.msk_rx_cons;
do {
rxlen = status >> 16;
-   if ((status & GMR_FS_VLAN) != 0)
+   if ((status & GMR_FS_VLAN) != 0 &&
+   (ifp->if_capenable & IFCAP_VLAN_HWTAGGING) != 0)
rxlen -= ETHER_VLAN_ENCAP_LEN;
if (len > sc_if->msk_framesize ||
((status & GMR_FS_ANY_ERR) != 0) ||
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Re: [PATCH] Part 2 of low level 802.1p priority support

2007-02-11 Thread Bruce M. Simpson

Andrew Thompson wrote:

This is great for the bridge, it has needed to take the vlan tag into
account when deciding to forward or not. Having m_pkthdr.ether_vtag
always set makes this much easier to implement.
  
In the deleted code M_VLANTAG is cleared but is not done anymore, is

this right?
  
Correct. The vlan(4) code as it currently stands consumes M_VLANTAG 
(though as its allocation has been moved into the mbuf header itself, 
there is no additional free needed -- it's not a deep allocation).


The patch moves the M_VLANTAG handling into ether_demux() (it should 
probably move further down to ether_input()) and always sets M_VLANTAG 
if we got raw 802.1q frames from the NIC (!(ifcap & VLAN_HWTAG) 
situation), so that we can deal with VLAN 0 in a clean way in ether_demux().


This may be a slight pessimization, but we always knew that cards which 
don't deal with 802.1q in hardware cause a performance hit anyway -- 
this just means that they will at least be able to process packet 
priorities!


M_VLANTAG then may be passed to upper layers, providing they either 
ignore its presence or do something useful with it. In the case of ALTQ 
we'd examine the priority field in the 802.1q tag to map Layer 2 Quality 
of Service to ALTQ rules.


These changes really need to be considered together with M_PROMISC. 
NetBSD did this presumably to stop the madness we have in ether_demux() 
to deal with the situation of stacked software Ethernet devices (vlan, 
bridge, agr, etc); M_PROMISC gets set for frames coming in off the card 
which we wouldn't have received had we not been in IFF_PROMISC. 'I am a 
hub, not a switch'. It's not smart enough to drop M_MCASTs we didn't ask 
for, it will blindly tag those also.


Both of these changes use mbuf flags, not tags, because tags have a 
higher cost. M_PROMISC is also used in NetBSD to stop the IP forwarding 
paths seeing promiscuous traffic. Throw a wi(4) PRISM2 card into a 
laptop and set up OSPF+DVMRP on the same FreeBSD kernel and you'll see 
what I mean.


We have checks in our code which only deal with that if there's a 
vlan(4) configured on top of your NIC...


BMS
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: [PATCH] Part 2 of low level 802.1p priority support

2007-02-14 Thread Bruce M. Simpson

Pyun YongHyeon wrote:
 > Further testing with drivers is needed (I can't be 100% sure it fails 
 > with msk(4) because something strange is happening when vlan tagging is 
 > turned off). Perhaps Pyun knows?
 > 


I guess I've not merged local changes before committing to HEAD.
How about attached one?
  
I can confirm that the merged VLAN tag code works OK with msk and 
VLAN_HWTAGGING disabled when using this patch.


Regards,
BMS
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: [PATCH] Part 2 of low level 802.1p priority support

2007-02-14 Thread Pyun YongHyeon
On Wed, Feb 14, 2007 at 07:38:21PM +, Bruce M. Simpson wrote:
 > Pyun YongHyeon wrote:
 > > > Further testing with drivers is needed (I can't be 100% sure it fails 
 > > > with msk(4) because something strange is happening when vlan tagging is 
 > > > turned off). Perhaps Pyun knows?
 > > > 
 > >
 > >I guess I've not merged local changes before committing to HEAD.
 > >How about attached one?
 > >  
 > I can confirm that the merged VLAN tag code works OK with msk and 
 > VLAN_HWTAGGING disabled when using this patch.
 > 

Thank you. Patch committed(if_msk.c, rev 1.9).

 > Regards,
 > BMS

-- 
Regards,
Pyun YongHyeon
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"