Subject: Re: vlan and ethernet "slightly jumbo" frames
To: Yojiro UO <yuo@nui.org>
From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
List: tech-net
Date: 06/04/2002 19:47:21
>> 	at this moment if_vlan.c controls ethernet "slightly jumbo" frames
>> 	(ETHERCAP_VLAN_MTU).  there are other customers which uses "slightly
>> 	jumbo" frames, namely MPLS over ethernet (see www.ayame.org).
>> 	i guess it better to move ETHERCAP_VLAN_MTU handling into
>> 	if_ethersubr.c.  just a thought.
>I completly agree.
>I am a member of ayame project.  We are now implementing an MPLS
>forwarding function on NetBSD network stack.
>The MPLS labeled packet on ethernet is one of "slightly jumbo"
>frames, as itojun said, because the label is represented by the
>shim-header located between the ether header and the IP header.
>Currently, we are using the "ETHERCAP_VLAN_MTU" flag to use
>the shim-header.
>But because this flag is only for VLAN use, some operation which
>changes the vlan configuration causes inconsistensy
>(if ec_nvlans=0, the mpls function becomes disabled, etc..).

	i'm not sure what a couple of things, but here's a starter:
	- what should happen in ether_ifdetach(), when ec_nvlans > 0 after
	  vlan_ifdetach().
	- is ETHERCAP_VLAN_MTU really suitable for MPLS?  MPLS sometimes ask
	  for more than 4 bytes of extension.
	- struct field names and ETHERCAP_xx should be generalized
	  (they are no longer just for vlans)

itojun


Index: if_ether.h
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_ether.h,v
retrieving revision 1.27
diff -u -r1.27 if_ether.h
--- if_ether.h	2002/03/05 04:13:00	1.27
+++ if_ether.h	2002/06/04 10:43:52
@@ -142,17 +142,17 @@
  * begins with this structure.
  */
 struct	ethercom {
-	struct	 ifnet ec_if;			/* network-visible interface */
+	struct	ifnet ec_if;			/* network-visible interface */
 	LIST_HEAD(, ether_multi) ec_multiaddrs;	/* list of ether multicast
 						   addrs */
-	int	 ec_multicnt;			/* length of ec_multiaddrs
+	int	ec_multicnt;			/* length of ec_multiaddrs
 						   list */
-	int	 ec_capabilities;		/* capabilities, provided by
+	int	ec_capabilities;		/* capabilities, provided by
 						   driver */
-	int	 ec_capenable;			/* tells hardware which
+	int	ec_capenable;			/* tells hardware which
 						   capabilities to enable */
 
-	int	 ec_nvlans;			/* # VLANs on this interface */
+	int	ec_nvlans;			/* # of VLANs attached */
 };
 
 #define	ETHERCAP_VLAN_MTU	0x00000001	/* VLAN-compatible MTU */
@@ -243,6 +243,9 @@
 
 u_int32_t ether_crc32_le(const u_int8_t *, size_t);
 u_int32_t ether_crc32_be(const u_int8_t *, size_t);
+
+int ether_capattach(struct ifnet *, int);
+int ether_capdetach(struct ifnet *, int);
 
 #else
 /*
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_ethersubr.c,v
retrieving revision 1.95
diff -u -r1.95 if_ethersubr.c
--- if_ethersubr.c	2002/05/18 22:52:44	1.95
+++ if_ethersubr.c	2002/06/04 10:43:55
@@ -1469,3 +1469,65 @@
 
 	return (error);
 }
+
+int
+ether_capattach(struct ifnet *ifp, int capability)
+{
+	struct ethercom *ec = (void *)ifp;
+	int error;
+
+	switch (capability) {
+	case ETHERCAP_VLAN_MTU:
+		if (ec->ec_nvlans++ == 0 &&
+		    (ec->ec_capabilities & ETHERCAP_VLAN_MTU) != 0) {
+			/*
+			 * Enable Tx/Rx of VLAN-sized frames.
+			 */
+			ec->ec_capenable |= ETHERCAP_VLAN_MTU;
+			if (ifp->if_flags & IFF_UP) {
+				struct ifreq ifr;
+
+				ifr.ifr_flags = ifp->if_flags;
+				error = (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS,
+				    (caddr_t) &ifr);
+				if (error) {
+					if (ec->ec_nvlans-- == 1)
+						ec->ec_capenable &=
+						    ~ETHERCAP_VLAN_MTU;
+					return (error);
+				}
+			}
+		}
+		break;
+	default:
+		return EOPNOTSUPP;
+	}
+	return 0;
+}
+
+int
+ether_capdetach(struct ifnet *ifp, int capability)
+{
+	struct ethercom *ec = (void *)ifp;
+
+	switch (capability) {
+	case ETHERCAP_VLAN_MTU:
+		if (ec->ec_nvlans-- == 1) {
+			/*
+			 * Disable Tx/Rx of VLAN-sized frames.
+			 */
+			ec->ec_capenable &= ~ETHERCAP_VLAN_MTU;
+			if (ifp->if_flags & IFF_UP) {
+				struct ifreq ifr;
+
+				ifr.ifr_flags = ifp->if_flags;
+				(void) (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS,
+				    (caddr_t) &ifr);
+			}
+		}
+		break;
+	default:
+		return EOPNOTSUPP;
+	}
+	return 0;
+}
Index: if_vlan.c
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_vlan.c,v
retrieving revision 1.33
diff -u -r1.33 if_vlan.c
--- if_vlan.c	2001/11/12 23:49:45	1.33
+++ if_vlan.c	2002/06/04 10:43:59
@@ -284,27 +284,15 @@
 		 * i.e. can Tx/Rx larger than ETHER_MAX_LEN frames,
 		 * enable it.
 		 */
-		if (ec->ec_nvlans++ == 0 &&
-		    (ec->ec_capabilities & ETHERCAP_VLAN_MTU) != 0) {
+		error = ether_capattach(p, ETHERCAP_VLAN_MTU);
+		if (error)
+			return error;
+		if ((ec->ec_capenable & ETHERCAP_VLAN_MTU) != 0) {
 			/*
 			 * Enable Tx/Rx of VLAN-sized frames.
 			 */
-			ec->ec_capenable |= ETHERCAP_VLAN_MTU;
-			if (p->if_flags & IFF_UP) {
-				struct ifreq ifr;
-
-				ifr.ifr_flags = p->if_flags;
-				error = (*p->if_ioctl)(p, SIOCSIFFLAGS,
-				    (caddr_t) &ifr);
-				if (error) {
-					if (ec->ec_nvlans-- == 1)
-						ec->ec_capenable &=
-						    ~ETHERCAP_VLAN_MTU;
-					return (error);
-				}
-			}
 			ifv->ifv_mtufudge = 0;
-		} else if ((ec->ec_capabilities & ETHERCAP_VLAN_MTU) == 0) {
+		} else {
 			/*
 			 * Fudge the MTU by the encapsulation size.  This
 			 * makes us incompatible with strictly compliant
@@ -373,27 +361,10 @@
 	/* Disconnect from parent. */
 	switch (ifv->ifv_p->if_type) {
 	case IFT_ETHER:
-	    {
-		struct ethercom *ec = (void *) ifv->ifv_p;
-
-		if (ec->ec_nvlans-- == 1) {
-			/*
-			 * Disable Tx/Rx of VLAN-sized frames.
-			 */
-			ec->ec_capenable &= ~ETHERCAP_VLAN_MTU;
-			if (ifv->ifv_p->if_flags & IFF_UP) {
-				struct ifreq ifr;
-
-				ifr.ifr_flags = ifv->ifv_p->if_flags;
-				(void) (*ifv->ifv_p->if_ioctl)(ifv->ifv_p,
-				    SIOCSIFFLAGS, (caddr_t) &ifr);
-			}
-		}
-
+		ether_capdetach(ifv->ifv_p, ETHERCAP_VLAN_MTU);
 		ether_ifdetach(ifp);
 		vlan_reset_linkname(ifp);
 		break;
-	    }
 
 #ifdef DIAGNOSTIC
 	default: