NetBSD Problem Report #8536

Received: (qmail 27007 invoked from network); 2 Oct 1999 18:37:09 -0000
Message-Id: <199910021454.QAA02693@yacht.domestic.de>
Date: Sat, 2 Oct 1999 16:54:06 +0200 (CEST)
From: kuebart@mathematik.uni-ulm.de (Joachim Kuebart)
To: gnats-bugs@gnats.netbsd.org
Subject: SPPP does not support Van Jacobson header compression
X-Send-Pr-Version: 3.95

>Number:         8536
>Category:       kern
>Synopsis:       SPPP does not support Van Jacobson header compression
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    martin
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Oct 02 11:50:01 +0000 1999
>Closed-Date:    
>Last-Modified:  Tue Apr 01 18:55:25 +0000 2003
>Originator:     Joachim Kuebart
>Release:        NetBSD-current, Oct 2, 1999
>Organization:
>Environment:

	i386, NetBSD-current, i4b package 0.81 beta
System: NetBSD yacht 1.4K NetBSD 1.4K (YACHT) #40: Sat Oct 2 16:15:23 CEST 1999 joki@yacht:/usr/src/sys/arch/i386/compile/YACHT i386


>Description:

	During IPCP negotiation, NetBSD's sppp-driver rejects VJ
	header compression and does not suggest to use it from its
	side.
>How-To-Repeat:

	Use i4b or some other driver that relies on
	/sys/net/if_spppsubr.c to connect to a modern router. If the
	device is in debug mode, attempts to configure VJ compression
	and their rejection are logged.
>Fix:

	The following patch is based on my submission to FreeBSD's
	problem database a while ago. Unfortunately, it was never
	commited there.

	The code was tested with i4b-0.81 beta and worked well
	for almost a year under FreeBSD. I did the port to NetBSD
	today, but it looks OK.

	PS: Due to the use of "#ifdef SPPP_VJ", SPPP_VJ might become
	a kernel option if that seems appropriate.

Index: if_sppp.h
===================================================================
RCS file: /home/cvs/netbsd/src/sys/net/if_sppp.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 if_sppp.h
--- if_sppp.h	1999/06/14 20:07:37	1.1.1.1
+++ if_sppp.h	1999/10/02 14:40:02
@@ -23,8 +23,8 @@
  * From: Id: if_sppp.h,v 1.7 1998/12/01 20:20:19 hm Exp
  */

-#ifndef _NET_IF_HDLC_H_
-#define _NET_IF_HDLC_H_ 1
+#ifndef _NET_IF_SPPP_H_
+#define _NET_IF_SPPP_H_ 1

 #define IDX_LCP 0		/* idx into state table */

@@ -50,6 +50,9 @@
 #define IPCP_HISADDR_SEEN 1	/* have seen his address already */
 #define IPCP_MYADDR_DYN   2	/* my address is dynamically assigned */
 #define IPCP_MYADDR_SEEN  4	/* have seen his address already */
+#define IPCP_VJ           8	/* We can use VJ compression */
+	int	max_state;	/* Max-Slot-Id */
+	int	compress_cid;	/* Comp-Slot-Id */
 };

 #define AUTHNAMELEN	32
@@ -105,6 +108,7 @@
 	struct sipcp ipcp;		/* IPCP params */
 	struct sauth myauth;		/* auth params, i'm peer */
 	struct sauth hisauth;		/* auth params, i'm authenticator */
+	struct slcompress *pp_comp;	/* for VJ compression */
 	/*
 	 * These functions are filled in by sppp_attach(), and are
 	 * expected to be used by the lower layer (hardware) drivers
@@ -134,8 +138,8 @@
 	void	(*pp_chg)(struct sppp *sp, int new_state);
 };

-#define PP_KEEPALIVE    0x01    /* use keepalive protocol */
-#define PP_CISCO        0x02    /* use Cisco protocol instead of PPP */
+#define PP_KEEPALIVE	0x01	/* use keepalive protocol */
+#define PP_CISCO	0x02	/* use Cisco protocol instead of PPP */
 				/* 0x04 was PP_TIMO */
 #define PP_CALLIN	0x08	/* we are being called */
 #define PP_NEEDAUTH	0x10	/* remote requested authentication */
@@ -186,4 +190,4 @@
 void sppp_flush (struct ifnet *ifp);
 #endif

-#endif /* _NET_IF_HDLC_H_ */
+#endif /* _NET_IF_SPPP_H_ */
Index: if_spppsubr.c
===================================================================
RCS file: /home/cvs/netbsd/src/sys/net/if_spppsubr.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 if_spppsubr.c
--- if_spppsubr.c	1999/10/01 18:44:06	1.1.1.2
+++ if_spppsubr.c	1999/10/02 14:03:44
@@ -33,6 +33,8 @@

 #include <sys/param.h>

+#define SPPP_VJ
+
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/sockio.h>
@@ -54,6 +56,10 @@
 #include <net/netisr.h>
 #include <net/if_types.h>
 #include <net/route.h>
+#include <netinet/in.h>
+#include <netinet/in_systm.h>
+#include <netinet/ip.h>
+#include <net/slcompress.h>

 #include <machine/stdarg.h>

@@ -130,6 +136,8 @@
 #define PPP_ISO		0x0023		/* ISO OSI Protocol */
 #define PPP_XNS		0x0025		/* Xerox NS Protocol */
 #define PPP_IPX		0x002b		/* Novell IPX Protocol */
+#define PPP_VJ_COMP	0x002d		/* VJ compressed TCP/IP */
+#define PPP_VJ_UCOMP	0x002f		/* VJ uncompressed TCP/IP */
 #define PPP_IPV6	0x0057		/* Internet Protocol version 6 */
 #define PPP_LCP		0xc021		/* Link Control Protocol */
 #define PPP_PAP		0xc023		/* Password Authentication Protocol */
@@ -161,6 +169,8 @@
 #define IPCP_OPT_COMPRESSION	2	/* IP compression protocol (VJ) */
 #define IPCP_OPT_ADDRESS	3	/* local IP address */

+#define IPCP_COMP_VJ		0x2d	/* Code for VJ compression */
+
 #define PAP_REQ			1	/* PAP name/password request */
 #define PAP_ACK			2	/* PAP acknowledge */
 #define PAP_NAK			3	/* PAP fail */
@@ -505,7 +515,35 @@
 				inq = &ipintrq;
 			}
 			break;
+#ifdef SPPP_VJ
+		case PPP_VJ_COMP:
+			if (sp->state[IDX_IPCP] == STATE_OPENED) {
+				int len;
+
+				if ((len = sl_uncompress_tcp(
+				    (u_char **)&m->m_data, m->m_len,
+				    TYPE_COMPRESSED_TCP, sp->pp_comp)) <= 0)
+					goto drop;
+				m->m_len = m->m_pkthdr.len = len;
+				schednetisr (NETISR_IP);
+				inq = &ipintrq;
+			}
+			break;
+		case PPP_VJ_UCOMP:
+			if (sp->state[IDX_IPCP] == STATE_OPENED) {
+				int len;
+
+				if ((len = sl_uncompress_tcp(
+				    (u_char **)&m->m_data, m->m_len,
+				    TYPE_UNCOMPRESSED_TCP, sp->pp_comp)) <= 0)
+					goto drop;
+				m->m_len = m->m_pkthdr.len = len;
+				schednetisr (NETISR_IP);
+				inq = &ipintrq;
+			}
+			break;
 #endif
+#endif
 #ifdef IPX
 		case PPP_IPX:
 			/* IPX IPXCP not implemented yet */
@@ -615,6 +653,9 @@
 	struct ppp_header *h;
 	struct ifqueue *ifq;
 	int s, rv = 0;
+#ifdef SPPP_VJ
+	int ipproto = PPP_IP;
+#endif

 	s = splimp();

@@ -639,8 +680,7 @@
 	ifq = &ifp->if_snd;

 #ifdef INET
-	if (dst->sa_family == AF_INET)
-	{
+	if (dst->sa_family == AF_INET) {
 		/* Check mbuf length here??? */
 		struct ip *ip = mtod (m, struct ip*);
 		struct tcphdr *tcp = (struct tcphdr*) ((long*)ip + ip->ip_hl);
@@ -670,14 +710,38 @@
 		 * Put low delay, telnet, rlogin and ftp control packets
 		 * in front of the queue.
 		 */
-		 
+
 		if (! IF_QFULL (&sp->pp_fastq) &&
 		    ((ip->ip_tos & IPTOS_LOWDELAY) ||
-	    	    ((ip->ip_p == IPPROTO_TCP &&
-	    	    m->m_len >= sizeof (struct ip) + sizeof (struct tcphdr) &&
-	    	    (INTERACTIVE (ntohs (tcp->th_sport)))) ||
-	    	    INTERACTIVE (ntohs (tcp->th_dport)))))
+		    ((ip->ip_p == IPPROTO_TCP &&
+		    m->m_len >= sizeof (struct ip) + sizeof (struct tcphdr) &&
+		    (INTERACTIVE (ntohs (tcp->th_sport)))) ||
+		    INTERACTIVE (ntohs (tcp->th_dport)))))
 			ifq = &sp->pp_fastq;
+#ifdef SPPP_VJ
+		/*
+		 * Do IP Header compression
+		 */
+		if ((sp->pp_flags & PP_CISCO) == 0 &&
+		    (sp->ipcp.flags & IPCP_VJ) != 0 &&
+		    ip->ip_p == IPPROTO_TCP)
+			switch (sl_compress_tcp(m, ip, sp->pp_comp,
+						sp->ipcp.compress_cid)) {
+			case TYPE_COMPRESSED_TCP:
+				ipproto = PPP_VJ_COMP;
+				break;
+			case TYPE_UNCOMPRESSED_TCP:
+				ipproto = PPP_VJ_UCOMP;
+				break;
+			case TYPE_IP:
+				ipproto = PPP_IP;
+				break;
+			default:
+				m_freem(m);
+				splx(s);
+				return (EINVAL);
+			}
+#endif
 	}
 #endif

@@ -727,7 +791,11 @@
 			 * not ready to carry IP packets, and return
 			 * ENETDOWN, as opposed to ENOBUFS.
 			 */
+#ifdef SPPP_VJ
+			h->protocol = htons(ipproto);
+#else
 			h->protocol = htons(PPP_IP);
+#endif
 			if (sp->state[IDX_IPCP] != STATE_OPENED)
 				rv = ENETDOWN;
 		}
@@ -813,7 +881,7 @@
 	/* Initialize keepalive handler. */
 	if (! spppq)
 #if defined(__FreeBSD__) && __FreeBSD__ >= 3
-		keepalive_ch = 
+		keepalive_ch =
 #endif
 		timeout(sppp_keepalive, 0, hz * 10);

@@ -833,6 +901,11 @@
 	sp->pp_up = lcp.Up;
 	sp->pp_down = lcp.Down;

+#ifdef SPPP_VJ
+	sp->pp_comp = malloc (sizeof (*sp->pp_comp), M_DEVBUF, M_NOWAIT);
+	sl_compress_init(sp->pp_comp);
+#endif
+
 	sppp_lcp_init(sp);
 	sppp_ipcp_init(sp);
 	sppp_pap_init(sp);
@@ -871,6 +944,10 @@
 	, sp->pap_my_to_ch
 #endif
 	);
+
+#ifdef SPPP_VJ
+	free(sp->pp_comp, M_DEVBUF);
+#endif
 }

 /*
@@ -1778,7 +1855,7 @@
 		case STATE_ACK_SENT:
 			(cp->scr)(sp);
 #if defined(__FreeBSD__) && __FreeBSD__ >= 3
-			sp->ch[cp->protoidx] = 
+			sp->ch[cp->protoidx] =
 #endif
 			timeout(cp->TO, (void *)sp, sp->lcp.timeout);
 			break;
@@ -2329,10 +2406,8 @@
 		sp->pp_phase = PHASE_NETWORK;

 	if(debug)
-	{
 		log(LOG_INFO, SPP_FMT "phase %s\n", SPP_ARGS(ifp),
 		    sppp_phase_name(sp->pp_phase));
-	}

 	/*
 	 * Open all authentication protocols.  This is even required
@@ -2376,10 +2451,8 @@
 	sp->pp_phase = PHASE_TERMINATE;

 	if(debug)
-	{
 		log(LOG_INFO, SPP_FMT "phase %s\n", SPP_ARGS(ifp),
 			sppp_phase_name(sp->pp_phase));
-	}

 	/*
 	 * Take upper layers down.  We send the Down event first and
@@ -2402,10 +2475,8 @@
 	sp->pp_phase = PHASE_ESTABLISH;

 	if(debug)
-	{
 		log(LOG_INFO, SPP_FMT "phase %s\n", SPP_ARGS(ifp),
 			sppp_phase_name(sp->pp_phase));
-	}

 	/* Notify lower layer if desired. */
 	if (sp->pp_tls)
@@ -2420,10 +2491,8 @@
 	sp->pp_phase = PHASE_DEAD;

 	if(debug)
-	{
 		log(LOG_INFO, SPP_FMT "phase %s\n", SPP_ARGS(ifp),
 			sppp_phase_name(sp->pp_phase));
-	}

 	/* Notify lower layer if desired. */
 	if (sp->pp_tlf)
@@ -2542,7 +2611,8 @@
 	STDDCL;
 	u_long myaddr, hisaddr;

-	sp->ipcp.flags &= ~(IPCP_HISADDR_SEEN|IPCP_MYADDR_SEEN|IPCP_MYADDR_DYN);
+	sp->ipcp.flags &= ~(IPCP_HISADDR_SEEN | IPCP_MYADDR_SEEN |
+			    IPCP_MYADDR_DYN | IPCP_VJ);

 	sppp_get_ip_addrs(sp, &myaddr, &hisaddr, 0);
 	/*
@@ -2559,6 +2629,11 @@
 		return;
 	}

+#ifdef SPPP_VJ
+	sp->ipcp.opts |= (1 << IPCP_OPT_COMPRESSION);
+	sp->ipcp.max_state = MAX_STATES - 1;
+	sp->ipcp.compress_cid = 1;
+#endif
 	if (myaddr == 0L) {
 		/*
 		 * I don't have an assigned address, so i need to
@@ -2602,6 +2677,9 @@
 	int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
 	u_long hisaddr, desiredaddr;
 	int gotmyaddr = 0;
+#ifdef SPPP_VJ
+	int desiredcomp;
+#endif

 	len -= 4;
 	origlen = len;
@@ -2622,7 +2700,7 @@
 		if (debug)
 			addlog(" %s ", sppp_ipcp_opt_name(*p));
 		switch (*p) {
-#ifdef notyet
+#ifdef SPPP_VJ
 		case IPCP_OPT_COMPRESSION:
 			if (len >= 6 && p[1] >= 6) {
 				/* correctly formed compress option */
@@ -2670,27 +2748,42 @@
 		if (debug)
 			addlog(" %s ", sppp_ipcp_opt_name(*p));
 		switch (*p) {
-#ifdef notyet
+#ifdef SPPP_VJ
 		case IPCP_OPT_COMPRESSION:
-			continue;
+			desiredcomp = p[2] << 8 | p[3];
+			/* We only support VJ */
+			if (desiredcomp == IPCP_COMP_VJ) {
+				if (debug)
+					addlog("VJ [ack] ");
+				sp->ipcp.flags |= IPCP_VJ;
+				sl_compress_setup(sp->pp_comp, p[4]);
+				sp->ipcp.max_state = p[4];
+				sp->ipcp.compress_cid = p[5];
+				continue;
+			}
+			if (debug)
+				addlog("%#04x [not supported] ", desiredcomp);
+			p[2] = IPCP_COMP_VJ >> 8;
+			p[3] = IPCP_COMP_VJ;
+			break;
 #endif
 		case IPCP_OPT_ADDRESS:
 			desiredaddr = p[2] << 24 | p[3] << 16 |
 				p[4] << 8 | p[5];
 			if (!(sp->ipcp.flags & IPCP_MYADDR_SEEN) &&
-			        (sp->ipcp.flags & IPCP_MYADDR_DYN)) {
+				(sp->ipcp.flags & IPCP_MYADDR_DYN)) {
 				/*
 				 * hopefully this is our address !!
 				 */
-			 	if (debug)
+				if (debug)
 					addlog("[wantmyaddr %s] ",
 						sppp_dotted_quad(desiredaddr));
 				/*
 				 * When doing dynamic address assignment,
-			   	 * we accept his offer.  Otherwise, we
-			    	 * ignore it and thus continue to negotiate
-			     	 * our already existing value.
-		      		 */
+				 * we accept his offer.  Otherwise, we
+				 * ignore it and thus continue to negotiate
+				 * our already existing value.
+				 */
 				sppp_set_ip_addr(sp, desiredaddr);
 				if (debug)
 					addlog("[agree] ");
@@ -2699,32 +2792,32 @@
 				continue;
 			} else {
 				if (desiredaddr == hisaddr ||
-			    	(hisaddr == 1 && desiredaddr != 0)) {
+				(hisaddr == 1 && desiredaddr != 0)) {
 					/*
-				 	* Peer's address is same as our value,
-				 	* this is agreeable.  Gonna conf-ack
-				 	* it.
-				 	*/
+					* Peer's address is same as our value,
+					* this is agreeable.  Gonna conf-ack
+					* it.
+					*/
 					if (debug)
 						addlog("%s [ack] ",
-					       		sppp_dotted_quad(hisaddr));
+							sppp_dotted_quad(hisaddr));
 					/* record that we've seen it already */
 					sp->ipcp.flags |= IPCP_HISADDR_SEEN;
 					continue;
 				}
 				/*
-			 	* The address wasn't agreeable.  This is either
-			 	* he sent us 0.0.0.0, asking to assign him an
-			 	* address, or he send us another address not
-			 	* matching our value.  Either case, we gonna
-			 	* conf-nak it with our value.
-			 	*/
+				* The address wasn't agreeable.  This is either
+				* he sent us 0.0.0.0, asking to assign him an
+				* address, or he send us another address not
+				* matching our value.  Either case, we gonna
+				* conf-nak it with our value.
+				*/
 				if (debug) {
 					if (desiredaddr == 0)
 						addlog("[addr requested] ");
 					else
 						addlog("%s [not agreed] ",
-					       		sppp_dotted_quad(desiredaddr));
+							sppp_dotted_quad(desiredaddr));
 				}

 				p[2] = hisaddr >> 24;
@@ -2809,9 +2902,9 @@
 			 */
 			sp->ipcp.opts &= ~(1 << IPCP_OPT_ADDRESS);
 			break;
-#ifdef notyet
-		case IPCP_OPT_COMPRESS:
-			sp->ipcp.opts &= ~(1 << IPCP_OPT_COMPRESS);
+#ifdef SPPP_VJ
+		case IPCP_OPT_COMPRESSION:
+			sp->ipcp.opts &= ~(1 << IPCP_OPT_COMPRESSION);
 			break;
 #endif
 		}
@@ -2832,6 +2925,9 @@
 	u_char *buf, *p;
 	struct ifnet *ifp = &sp->pp_if;
 	int debug = ifp->if_flags & IFF_DEBUG;
+#ifdef SPPP_VJ
+	int desiredcomp;
+#endif
 	u_long wantaddr;

 	len -= 4;
@@ -2875,11 +2971,25 @@
 				}
 			}
 			break;
-#ifdef notyet
-		case IPCP_OPT_COMPRESS:
+#ifdef SPPP_VJ
+		case IPCP_OPT_COMPRESSION:
 			/*
 			 * Peer wants different compression parameters.
 			 */
+			if (len >= 6 && p[1] == 6) {
+				desiredcomp = p[2] << 8 | p[3];
+				if (debug)
+					addlog("[wantcomp %#04x] ",
+						desiredcomp);
+				if (desiredcomp == IPCP_COMP_VJ) {
+					sl_compress_setup(sp->pp_comp, p[4]);
+					sp->ipcp.max_state = p[4];
+					sp->ipcp.compress_cid = p[5];
+					addlog("[agree] ");
+				} else
+					sp->ipcp.opts &=
+						~(1 << IPCP_OPT_COMPRESSION);
+			}
 			break;
 #endif
 		}
@@ -2925,14 +3035,14 @@
 	u_long ouraddr;
 	int i = 0;

-#ifdef notyet
+#ifdef SPPP_VJ
 	if (sp->ipcp.opts & (1 << IPCP_OPT_COMPRESSION)) {
 		opt[i++] = IPCP_OPT_COMPRESSION;
 		opt[i++] = 6;
-		opt[i++] = 0;	/* VJ header compression */
-		opt[i++] = 0x2d; /* VJ header compression */
-		opt[i++] = max_slot_id;
-		opt[i++] = comp_slot_id;
+		opt[i++] = IPCP_COMP_VJ >> 8;
+		opt[i++] = IPCP_COMP_VJ;
+		opt[i++] = sp->ipcp.max_state;
+		opt[i++] = sp->ipcp.compress_cid;
 	}
 #endif

@@ -4168,10 +4278,8 @@
 	sp->pp_phase = PHASE_NETWORK;

 	if(debug)
-	{
 		log(LOG_INFO, SPP_FMT "phase %s\n", SPP_ARGS(ifp),
 			sppp_phase_name(sp->pp_phase));
-	}

 	/* Notify NCPs now. */
 	for (i = 0; i < IDX_COUNT; i++)
@@ -4360,10 +4468,3 @@
 {
 	/* do just nothing */
 }
-/*
- * This file is large.  Tell emacs to highlight it nevertheless.
- *
- * Local Variables:
- * hilit-auto-highlight-maxout: 120000
- * End:
- */
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: kern-bug-people->martin 
Responsible-Changed-By: perry 
Responsible-Changed-When: Tue Apr 1 10:55:02 PST 2003 
Responsible-Changed-Why:  
Martin handles sync PPP issues 
>Unformatted:

Home	PR Database Search