[patch 1/6] Add PPP protocol support with HDLC framing

[patch 1/6] Add PPP protocol support with HDLC framing

[kristen]

[-- Attachment #1: Type: text/plain, Size: 21668 bytes --]

This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in a
different patch.

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-10 16:58:09.915955860 -0800
+++ ofono/Makefile.am	2010-03-10 16:58:12.039961039 -0800
@@ -55,7 +55,9 @@
 				gatchat/gattty.h gatchat/gattty.c \
 				gatchat/gatutil.h gatchat/gatutil.c \
 				gatchat/gat.h \
-				gatchat/gatserver.h gatchat/gatserver.c
+				gatchat/gatserver.h gatchat/gatserver.c \
+				gatchat/gatppp.c gatchat/gatppp.h \
+				gatchat/gatppp_internal.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp_internal.h	2010-03-10 16:58:12.039961039 -0800
@@ -0,0 +1,97 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __G_AT_PPP_INTERNAL_H
+#define __G_AT_PPP_INTERNAL_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define DEFAULT_MRU	1500
+#define BUFFERSZ	DEFAULT_MRU*2
+#define DEFAULT_ACCM	0x00000000
+#define PPP_ESC		0x7d
+#define PPP_FLAG_SEQ 	0x7e
+#define PPP_ADDR_FIELD	0xff
+#define PPP_CTRL	0x03
+#define LCP_PROTOCOL	0xc021
+#define CHAP_PROTOCOL	0xc223
+#define PPP_HEADROOM	2
+#define HDLC_HEADROOM	3
+#define HDLC_TAIL	3
+#define MD5		5
+
+enum PPP_PHASE {
+	DEAD = 0,
+	ESTABLISHMENT,
+	AUTHENTICATION,
+	NETWORK,
+	TERMINATION,
+};
+
+enum PPP_EVENTS {
+	PPP_UP = 1,
+	PPP_OPENED,
+	PPP_SUCCESS,
+	PPP_NONE,
+	PPP_CLOSING,
+	PPP_FAIL,
+	PPP_DOWN
+};
+
+struct ppp_packet_handler {
+	guint16 proto;
+	void (*handler)(gpointer priv, guint8 *packet);
+	gpointer priv;
+};
+
+#define ppp_generate_event(l, e) \
+	__ppp_generate_event(l, e)
+
+#define ppp_transmit(l, p, i) \
+	__ppp_transmit(l, p, i)
+
+#define ppp_register_packet_handler(h) \
+	__ppp_register_packet_handler(h)
+
+#define ppp_info(packet) \
+	(packet+4)
+
+#define ppp_proto(packet) \
+	(ntohs(*((guint16*)(packet+2))))
+
+void __ppp_generate_event(struct ppp_link *link, guint event);
+void __ppp_register_packet_handler(struct ppp_packet_handler *handler);
+void __ppp_transmit(struct ppp_link *link, guint8 *packet, guint infolen);
+void __ppp_set_auth(struct ppp_link *link, guint8 *auth_data);
+void __ppp_set_recv_accm(struct ppp_link *link, guint32 accm);
+guint32 __ppp_get_xmit_accm(struct ppp_link *link);
+void __ppp_set_pfc(struct ppp_link *link, gboolean pfc);
+gboolean __ppp_get_pfc(struct ppp_link *link);
+void __ppp_set_acfc(struct ppp_link *link, gboolean acfc);
+gboolean __ppp_get_acfc(struct ppp_link *link);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __G_AT_PPP_INTERNAL_H */
+
Index: ofono/gatchat/gatppp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.c	2010-03-10 16:58:12.039961039 -0800
@@ -0,0 +1,557 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+
+#include <glib.h>
+
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+#define PPPINITFCS16    0xffff  /* Initial FCS value */
+#define PPPGOODFCS16    0xf0b8  /* Good final FCS value */
+
+static GList *packet_handlers = NULL;
+
+void __ppp_register_packet_handler(struct ppp_packet_handler *handler)
+{
+	packet_handlers = g_list_append(packet_handlers, handler);
+}
+
+/*
+ * FCS lookup table copied from rfc1662.
+ */
+static guint16 fcstab[256] = {
+	0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+	0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+	0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
+	0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
+	0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
+	0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
+	0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
+	0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
+	0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
+	0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
+	0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
+	0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
+	0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
+	0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
+	0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
+	0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
+	0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
+	0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
+	0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
+	0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
+	0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
+	0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
+	0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
+	0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
+	0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
+	0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
+	0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
+	0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
+	0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
+	0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+	0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+	0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};
+
+/*
+ * Calculate a new fcs given the current fcs and the new data.
+ * copied from rfc1662
+ *
+ *     The FCS field is calculated over all bits of the Address, Control,
+ *     Protocol, Information and Padding fields, not including any start
+ *     and stop bits (asynchronous) nor any bits (synchronous) or octets
+ *     (asynchronous or synchronous) inserted for transparency.  This
+ *     also does not include the Flag Sequences nor the FCS field itself.
+ */
+static guint16 ppp_fcs(guint16 fcs, guint8 c)
+{
+	guint16 new_fcs;
+
+	new_fcs = (fcs >> 8) ^ fcstab[(fcs ^ c) & 0xff];
+	return new_fcs;
+}
+
+/*
+ * escape any chars less than 0x20, and check the transmit accm table to
+ * see if this character should be escaped.
+ */
+static gboolean ppp_escape(struct ppp_link *link, guint8 c, gboolean lcp)
+{
+	if ((lcp && c < 0x20) || (link->xmit_accm[c >> 5] & (1 << (c & 0x1f))))
+		return TRUE;
+	return FALSE;
+}
+
+static void ppp_put(struct ppp_link *link, guint8 *buf, int *pos,
+			guint8 c, gboolean lcp)
+{
+	int i = *pos;
+
+	/* escape characters if needed,  copy into buf, increment pos */
+	if (ppp_escape(link, c, lcp)) {
+		buf[i++] = PPP_ESC;
+		buf[i++] = c ^ 0x20;
+	} else
+		buf[i++] = c;
+	*pos = i;
+}
+
+/* XXX implement PFC and ACFC */
+static guint8 *ppp_encode(struct ppp_link *link, guint8 *data, int len,
+				guint *newlen)
+{
+	guint8 *frame = g_malloc0(BUFFERSZ);
+	int pos = 0;
+	int i = 0;
+	guint16 fcs = PPPINITFCS16;
+	guint16 proto = ntohs(*(guint16 *)data);
+	gboolean lcp = (proto == LCP_PROTOCOL);
+	if (!frame)
+		return NULL;
+
+	/* copy in the HDLC framing */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	/* from here till end flag, calculate FCS over each character */
+	fcs = ppp_fcs(fcs, PPP_ADDR_FIELD);
+	ppp_put(link, frame, &pos, PPP_ADDR_FIELD, lcp);
+	fcs = ppp_fcs(fcs, PPP_CTRL);
+	ppp_put(link, frame, &pos, PPP_CTRL, lcp);
+
+	/*
+	 * for each byte, first calculate FCS, then do escaping if
+	 * neccessary
+	 */
+	while (len--) {
+		fcs = ppp_fcs(fcs, data[i]);
+		ppp_put(link, frame, &pos, data[i++], lcp);
+	}
+
+	/* add FCS */
+	fcs ^= 0xffff;                 /* complement */
+	ppp_put(link, frame, &pos, (guint8)(fcs & 0x00ff), lcp);
+	ppp_put(link, frame, &pos, (guint8)((fcs >> 8) & 0x00ff), lcp);
+
+	/* add flag */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	*newlen = pos;
+	return frame;
+}
+
+static gint is_proto_handler(struct ppp_packet_handler *h, gpointer user)
+{
+	guint16 proto = (guint16) GPOINTER_TO_UINT(user);
+
+	if (h->proto == proto)
+		return 0;
+	else
+		return -1;
+}
+
+/* called when we have received a complete ppp frame */
+static void ppp_recv(struct ppp_link *link)
+{
+	guint16 protocol;
+	guint8 *frame, *packet;
+	GList *list;
+	struct ppp_packet_handler *h;
+
+	/* pop frames off of receive queue */
+	while ((frame = g_queue_pop_head(link->recv_queue))) {
+		protocol = ppp_proto(frame);
+		packet = ppp_info(frame);
+
+		/*
+		 * check to see if we have a protocol handler
+		 * registered for this packet
+		 */
+		list = g_list_find_custom(packet_handlers,
+				GUINT_TO_POINTER(protocol),
+				(GCompareFunc) is_proto_handler);
+		if (list) {
+			h = list->data;
+			h->handler(h->priv, packet);
+		}
+		g_free(frame);
+	}
+}
+
+/* XXX - Implement PFC and ACFC */
+static guint8 * ppp_decode(struct ppp_link *link, guint8 *frame)
+{
+	guint8 *data;
+	guint pos = 0;
+	int i = 0;
+	int len;
+	guint16 fcs;
+
+	data = g_malloc0(link->mru + 10);
+
+	/* skip the first flag char */
+	pos++;
+
+	/* TBD - how to deal with recv_accm */
+	while (frame[pos] != PPP_FLAG_SEQ) {
+		/* scan for escape character */
+		if (frame[pos] == PPP_ESC) {
+			/* skip that char */
+			pos++;
+			data[i] = frame[pos] ^ 0x20;
+		} else
+			data[i] = frame[pos];
+		i++; pos++;
+	}
+
+	len = i;
+
+	/* see if we have a good FCS */
+	fcs = PPPINITFCS16;
+	for (i = 0; i < len; i++)
+		fcs = ppp_fcs(fcs, data[i]);
+
+	if (fcs != PPPGOODFCS16) {
+		g_free(data);
+		return NULL;
+	}
+	return data;
+}
+
+static void ppp_feed(struct ppp_link *link, guint8 *data, gsize len)
+{
+	guint pos = 0;
+	guint8 *frame;
+
+	/* collect bytes until we detect we have received a complete frame */
+	/* examine the data.  If we are@the beginning of a new frame,
+	 * allocate memory to buffer the frame.
+	 */
+	for (pos = 0; pos < len; pos++) {
+		if (data[pos] == PPP_FLAG_SEQ) {
+			if (link->buffer == NULL) {
+				link->buffer = g_malloc0(BUFFERSZ);
+				link->index = 0;
+			} else {
+				/* store last flag character */
+				link->buffer[link->index++] = data[pos];
+				frame = ppp_decode(link, link->buffer);
+				/* push decoded frame onto receive queue */
+				if (frame)
+					g_queue_push_tail(link->recv_queue,
+								frame);
+
+				/* free and reintialize buffer */
+				g_free(link->buffer);
+				link->buffer = NULL;
+				link->index = 0;
+				continue;
+			}
+		}
+		/* copy byte to buffer */
+		if (link->buffer)
+			link->buffer[link->index++] = data[pos];
+		else
+			g_printerr("oops, no buffer, no start of frame\n");
+
+	}
+	/* process receive queue */
+	ppp_recv(link);
+}
+
+/*
+ * transmit out through the lower layer interface
+ *
+ * infolen - length of the information part of the packet
+ */
+void __ppp_transmit(struct ppp_link *link, guint8 *packet, guint infolen)
+{
+	guint8 *frame;
+	guint framelen;
+        GError *error = NULL;
+        GIOStatus status;
+        gsize bytes_written;
+
+	/*
+	 * do the octet stuffing.  Add 2 bytes to the infolen to
+	 * include the protocol field.
+	 */
+	frame = ppp_encode(link, packet, infolen+2, &framelen);
+	if (!frame) {
+		g_printerr("Failed to encode packet to transmit\n");
+		return;
+	}
+
+	/* transmit through the lower layer interface */
+	/*
+	 * TBD - should we just put this on a queue and transmit when
+	 * we won't block, or allow ourselves to block here?
+	 */
+	status = g_io_channel_write_chars(link->modem, (gchar *) frame,
+                                          framelen, &bytes_written, &error);
+
+	g_free(frame);
+
+}
+
+static gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
+{
+	struct ppp_link *ppp = (struct ppp_link *) data;
+	GIOStatus status;
+	gchar buf[256];
+	gsize bytes_read;
+	GError *error = NULL;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		g_print("G_IO_NVAL | G_IO_ERR");
+		return FALSE;
+	}
+
+	if (cond & G_IO_IN) {
+		status = g_io_channel_read_chars(channel, buf, 256,
+				&bytes_read, &error);
+		if (bytes_read > 0)
+			ppp_feed(ppp, (guint8 *)buf, bytes_read);
+        	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+                	return FALSE;
+	}
+
+	return TRUE;
+}
+
+/* Administrative Close */
+static void ppp_close(struct ppp_link *link)
+{
+	/* send a CLOSE event to the lcp layer */
+}
+
+/* Administrative Open */
+void g_at_ppp_open(struct ppp_link *link)
+{
+	/* send an OPEN event to the lcp layer */
+}
+
+static void ppp_link_establishment(struct ppp_link *link)
+{
+	/* signal UP event to LCP */
+}
+
+static void ppp_terminate(struct ppp_link *link)
+{
+	/* signal DOWN event to LCP */
+}
+
+static void ppp_authenticate(struct ppp_link *link)
+{
+	/* we don't do authentication right now, so send NONE */
+	if (!link->auth_proto)
+		ppp_generate_event(link, PPP_NONE);
+	/* otherwise we need to wait for the peer to send us a challenge */
+}
+
+static void ppp_dead(struct ppp_link *link)
+{
+	/* re-initialize everything */
+}
+
+static void ppp_network(struct ppp_link *link)
+{
+	/* bring network phase up */
+}
+
+static void ppp_transition_phase(struct ppp_link *link, guint phase)
+{
+	/* don't do anything if we're already there */
+	if (link->phase == phase)
+		return;
+
+	/* set new phase */
+	link->phase = phase;
+
+	switch(phase) {
+	case ESTABLISHMENT:
+		ppp_link_establishment(link);
+		break;
+	case AUTHENTICATION:
+		ppp_authenticate(link);
+		break;
+	case TERMINATION:
+		ppp_terminate(link);
+		break;
+	case DEAD:
+		ppp_dead(link);
+		break;
+	case NETWORK:
+		ppp_network(link);
+		break;
+	default:
+		break;
+	}
+
+}
+
+static void ppp_handle_event(struct ppp_link *link)
+{
+	guint event;
+
+	while ((event = GPOINTER_TO_UINT(g_queue_pop_head(link->event_queue)))){
+		switch(event) {
+		case PPP_UP:
+			/* causes transition to link establishment */
+			ppp_transition_phase(link, ESTABLISHMENT);
+			break;
+		case PPP_OPENED:
+			ppp_transition_phase(link, AUTHENTICATION);
+			break;
+		case PPP_CLOSING:
+			/* causes transition to termination phase */
+			ppp_transition_phase(link, TERMINATION);
+			break;
+		case PPP_DOWN:
+			/* cases transition to dead phase */
+			ppp_transition_phase(link, DEAD);
+			break;
+		case PPP_NONE:
+		case PPP_SUCCESS:
+			/* causes transition to network phase */
+			ppp_transition_phase(link, NETWORK);
+			break;
+		}
+	}
+}
+
+/*
+ * send the event handler a new event to process
+ */
+void __ppp_generate_event(struct ppp_link *link, guint event)
+{
+	g_queue_push_tail(link->event_queue, GUINT_TO_POINTER(event));
+	ppp_handle_event(link);
+}
+
+void g_at_ppp_set_connect_function(struct ppp_link *link,
+			       GAtPPPConnectFunc callback, gpointer user_data)
+{
+	link->connect_cb = callback;
+	link->connect_priv = user_data;
+}
+
+void g_at_ppp_set_disconnect_function(struct ppp_link *link,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data)
+{
+	link->disconnect_cb = callback;
+	link->disconnect_priv = user_data;
+}
+
+void g_at_ppp_shutdown(struct ppp_link *link)
+{
+	/* close the link */
+	ppp_close(link);
+
+	/* clean up all the queues */
+	g_queue_free(link->event_queue);
+	g_queue_free(link->recv_queue);
+
+	/* cleanup modem channel */
+	g_source_remove(link->modem_watch);
+	g_io_channel_unref(link->modem);
+}
+
+void g_at_ppp_ref(struct ppp_link *link)
+{
+	g_atomic_int_inc(&link->ref_count);
+}
+
+void g_at_ppp_unref(struct ppp_link *link)
+{
+	gboolean last;
+
+	last = g_atomic_int_dec_and_test(&link->ref_count);
+	if (last) {
+		g_at_ppp_shutdown(link);
+		g_free(link);
+	}
+}
+
+struct ppp_link * g_at_ppp_new(GIOChannel *modem)
+{
+	struct ppp_link *link;
+	GIOFlags flags;
+	int signal_source;
+
+	link = g_malloc0(sizeof(struct ppp_link));
+	if (!link)
+		return NULL;
+
+	link->ref_count = 1;
+
+	/* set options to defaults */
+	link->mru = DEFAULT_MRU;
+	link->recv_accm = DEFAULT_ACCM;
+	link->xmit_accm[0] = DEFAULT_ACCM;
+	link->xmit_accm[3] = 0x60000000; /* 0x7d, 0x7e */
+	link->pfc = FALSE;
+	link->acfc = FALSE;
+
+	/* allocate the queues */
+	link->event_queue = g_queue_new();
+	link->recv_queue = g_queue_new();
+
+	link->buffer = NULL;
+	link->index = 0;
+
+	/* setup the watch functions for the modem */
+	link->modem = g_io_channel_ref(modem);
+
+	flags = g_io_channel_get_flags(modem) | G_IO_FLAG_NONBLOCK;
+	g_io_channel_set_flags(modem, flags, NULL);
+
+	g_io_channel_set_encoding(modem, NULL, NULL);
+	g_io_channel_set_buffered(modem, FALSE);
+
+	/* initialize the lcp state */
+
+
+	/* initialize the autentication state */
+
+
+	/* intialize the network state */
+
+	/* start listening for packets from the modem */
+	signal_source = g_io_add_watch(modem,
+			G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+			ppp_cb, link);
+	link->modem_watch = signal_source;
+
+	return link;
+}
Index: ofono/gatchat/gatppp.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.h	2010-03-10 16:58:12.040979395 -0800
@@ -0,0 +1,81 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __G_AT_PPP_H
+#define __G_AT_PPP_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct ppp_link;
+
+enum PPP_CONNECT_STATUS {
+	PPP_CONNECT_SUCCESS,
+	PPP_CONNECT_FAIL
+};
+
+typedef void (*GAtPPPConnectFunc)(struct ppp_link *link, gint success,
+				 guint32 ip_address,
+				 guint32 dns1, guint32 dns2,
+				 gpointer user_data);
+
+typedef void (*GAtPPPDisconnectFunc)(struct ppp_link *link, gpointer user_data);
+
+struct ppp_link {
+	gint ref_count;
+	guint phase;
+	guint8 *buffer;
+	int index;
+	gint mru;
+	guint16 auth_proto;
+	char user_name[256];
+	char passwd[256];
+	gboolean pfc;
+	gboolean acfc;
+	guint32 xmit_accm[8];
+	guint32 recv_accm;
+	GIOChannel *modem;
+	GQueue *event_queue;
+	GQueue *recv_queue;
+	GAtPPPConnectFunc connect_cb;
+	gpointer connect_priv;
+	GAtPPPDisconnectFunc disconnect_cb;
+	gpointer disconnect_priv;
+	gint modem_watch;
+};
+
+struct ppp_link * g_at_ppp_new(GIOChannel *modem);
+void g_at_ppp_open(struct ppp_link *link);
+void g_at_ppp_set_connect_function(struct ppp_link *link,
+			       GAtPPPConnectFunc callback, gpointer user_data);
+void g_at_ppp_set_disconnect_function(struct ppp_link *link,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data);
+void g_at_ppp_shutdown(struct ppp_link *link);
+void g_at_ppp_ref(struct ppp_link *link);
+void g_at_ppp_unref(struct ppp_link *link);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __G_AT_PPP_H */
+

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 25002 bytes --]

Hi Kristen,

> This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in
>  a different patch.
> 
> Index: ofono/Makefile.am
> 
===================================================================
> --- ofono.orig/Makefile.am	2010-03-10 16:58:09.915955860 -0800
> +++ ofono/Makefile.am	2010-03-10 16:58:12.039961039 -0800
> @@ -55,7 +55,9 @@
>  				gatchat/gattty.h gatchat/gattty.c \
>  				gatchat/gatutil.h gatchat/gatutil.c \
>  				gatchat/gat.h \
> -				gatchat/gatserver.h gatchat/gatserver.c
> +				gatchat/gatserver.h gatchat/gatserver.c \
> +				gatchat/gatppp.c gatchat/gatppp.h \
> +				gatchat/gatppp_internal.h
> 
>  udev_files = plugins/ofono.rules
> 
> Index: ofono/gatchat/gatppp_internal.h
> 
===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ofono/gatchat/gatppp_internal.h	2010-03-10 16:58:12.039961039 -0800
> @@ -0,0 +1,97 @@
> +/*
> + *
> + *  PPP library with GLib integration
> + *
> + *  Copyright (C) 2010  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
>  USA + *
> + */
> +
> +#ifndef __G_AT_PPP_INTERNAL_H
> +#define __G_AT_PPP_INTERNAL_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define DEFAULT_MRU	1500
> +#define BUFFERSZ	DEFAULT_MRU*2
> +#define DEFAULT_ACCM	0x00000000
> +#define PPP_ESC		0x7d
> +#define PPP_FLAG_SEQ 	0x7e
> +#define PPP_ADDR_FIELD	0xff
> +#define PPP_CTRL	0x03
> +#define LCP_PROTOCOL	0xc021
> +#define CHAP_PROTOCOL	0xc223
> +#define PPP_HEADROOM	2
> +#define HDLC_HEADROOM	3
> +#define HDLC_TAIL	3
> +#define MD5		5
> +
> +enum PPP_PHASE {
> +	DEAD = 0,
> +	ESTABLISHMENT,
> +	AUTHENTICATION,
> +	NETWORK,
> +	TERMINATION,
> +};
> +
> +enum PPP_EVENTS {
> +	PPP_UP = 1,
> +	PPP_OPENED,
> +	PPP_SUCCESS,
> +	PPP_NONE,
> +	PPP_CLOSING,
> +	PPP_FAIL,
> +	PPP_DOWN
> +};
> +
> +struct ppp_packet_handler {
> +	guint16 proto;
> +	void (*handler)(gpointer priv, guint8 *packet);
> +	gpointer priv;
> +};
> +
> +#define ppp_generate_event(l, e) \
> +	__ppp_generate_event(l, e)

Why do we need this define?

> +
> +#define ppp_transmit(l, p, i) \
> +	__ppp_transmit(l, p, i)
> +

and this?

> +#define ppp_register_packet_handler(h) \
> +	__ppp_register_packet_handler(h)

or this?

> +
> +#define ppp_info(packet) \
> +	(packet+4)
> +
> +#define ppp_proto(packet) \
> +	(ntohs(*((guint16*)(packet+2))))
> +
> +void __ppp_generate_event(struct ppp_link *link, guint event);
> +void __ppp_register_packet_handler(struct ppp_packet_handler *handler);
> +void __ppp_transmit(struct ppp_link *link, guint8 *packet, guint infolen);
> +void __ppp_set_auth(struct ppp_link *link, guint8 *auth_data);
> +void __ppp_set_recv_accm(struct ppp_link *link, guint32 accm);
> +guint32 __ppp_get_xmit_accm(struct ppp_link *link);
> +void __ppp_set_pfc(struct ppp_link *link, gboolean pfc);
> +gboolean __ppp_get_pfc(struct ppp_link *link);
> +void __ppp_set_acfc(struct ppp_link *link, gboolean acfc);
> +gboolean __ppp_get_acfc(struct ppp_link *link);
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __G_AT_PPP_INTERNAL_H */
> +
> Index: ofono/gatchat/gatppp.c
> 
===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ofono/gatchat/gatppp.c	2010-03-10 16:58:12.039961039 -0800
> @@ -0,0 +1,557 @@
> +/*
> + *
> + *  PPP library with GLib integration
> + *
> + *  Copyright (C) 2010  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
>  USA + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <termios.h>
> +#include <arpa/inet.h>
> +
> +#include <glib.h>
> +
> +#include "gatppp.h"
> +#include "gatppp_internal.h"
> +
> +#define PPPINITFCS16    0xffff  /* Initial FCS value */
> +#define PPPGOODFCS16    0xf0b8  /* Good final FCS value */
> +
> +static GList *packet_handlers = NULL;
> +
> +void __ppp_register_packet_handler(struct ppp_packet_handler *handler)
> +{
> +	packet_handlers = g_list_append(packet_handlers, handler);
> +}
> +
> +/*
> + * FCS lookup table copied from rfc1662.
> + */
> +static guint16 fcstab[256] = {
> +	0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
> +	0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
> +	0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
> +	0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
> +	0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
> +	0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
> +	0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
> +	0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
> +	0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
> +	0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
> +	0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
> +	0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
> +	0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
> +	0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
> +	0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
> +	0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
> +	0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
> +	0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
> +	0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
> +	0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
> +	0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
> +	0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
> +	0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
> +	0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
> +	0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
> +	0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
> +	0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
> +	0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
> +	0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
> +	0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
> +	0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
> +	0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
> +};
> +
> +/*
> + * Calculate a new fcs given the current fcs and the new data.
> + * copied from rfc1662
> + *
> + *     The FCS field is calculated over all bits of the Address, Control,
> + *     Protocol, Information and Padding fields, not including any start
> + *     and stop bits (asynchronous) nor any bits (synchronous) or octets
> + *     (asynchronous or synchronous) inserted for transparency.  This
> + *     also does not include the Flag Sequences nor the FCS field itself.
> + */
> +static guint16 ppp_fcs(guint16 fcs, guint8 c)
> +{
> +	guint16 new_fcs;
> +
> +	new_fcs = (fcs >> 8) ^ fcstab[(fcs ^ c) & 0xff];
> +	return new_fcs;
> +}
> +
> +/*
> + * escape any chars less than 0x20, and check the transmit accm table to
> + * see if this character should be escaped.
> + */
> +static gboolean ppp_escape(struct ppp_link *link, guint8 c, gboolean lcp)
> +{
> +	if ((lcp && c < 0x20) || (link->xmit_accm[c >> 5] & (1 << (c & 0x1f))))
> +		return TRUE;
> +	return FALSE;
> +}
> +
> +static void ppp_put(struct ppp_link *link, guint8 *buf, int *pos,
> +			guint8 c, gboolean lcp)
> +{
> +	int i = *pos;
> +
> +	/* escape characters if needed,  copy into buf, increment pos */
> +	if (ppp_escape(link, c, lcp)) {
> +		buf[i++] = PPP_ESC;
> +		buf[i++] = c ^ 0x20;
> +	} else
> +		buf[i++] = c;
> +	*pos = i;
> +}
> +
> +/* XXX implement PFC and ACFC */
> +static guint8 *ppp_encode(struct ppp_link *link, guint8 *data, int len,
> +				guint *newlen)
> +{
> +	guint8 *frame = g_malloc0(BUFFERSZ);
> +	int pos = 0;
> +	int i = 0;
> +	guint16 fcs = PPPINITFCS16;
> +	guint16 proto = ntohs(*(guint16 *)data);
> +	gboolean lcp = (proto == LCP_PROTOCOL);
> +	if (!frame)
> +		return NULL;
> +
> +	/* copy in the HDLC framing */
> +	frame[pos++] = PPP_FLAG_SEQ;
> +
> +	/* from here till end flag, calculate FCS over each character */
> +	fcs = ppp_fcs(fcs, PPP_ADDR_FIELD);
> +	ppp_put(link, frame, &pos, PPP_ADDR_FIELD, lcp);
> +	fcs = ppp_fcs(fcs, PPP_CTRL);
> +	ppp_put(link, frame, &pos, PPP_CTRL, lcp);
> +
> +	/*
> +	 * for each byte, first calculate FCS, then do escaping if
> +	 * neccessary
> +	 */
> +	while (len--) {
> +		fcs = ppp_fcs(fcs, data[i]);
> +		ppp_put(link, frame, &pos, data[i++], lcp);
> +	}
> +
> +	/* add FCS */
> +	fcs ^= 0xffff;                 /* complement */
> +	ppp_put(link, frame, &pos, (guint8)(fcs & 0x00ff), lcp);
> +	ppp_put(link, frame, &pos, (guint8)((fcs >> 8) & 0x00ff), lcp);
> +
> +	/* add flag */
> +	frame[pos++] = PPP_FLAG_SEQ;
> +
> +	*newlen = pos;
> +	return frame;
> +}
> +
> +static gint is_proto_handler(struct ppp_packet_handler *h, gpointer user)
> +{
> +	guint16 proto = (guint16) GPOINTER_TO_UINT(user);
> +
> +	if (h->proto == proto)
> +		return 0;
> +	else
> +		return -1;
> +}
> +
> +/* called when we have received a complete ppp frame */
> +static void ppp_recv(struct ppp_link *link)
> +{
> +	guint16 protocol;
> +	guint8 *frame, *packet;
> +	GList *list;
> +	struct ppp_packet_handler *h;
> +
> +	/* pop frames off of receive queue */
> +	while ((frame = g_queue_pop_head(link->recv_queue))) {
> +		protocol = ppp_proto(frame);
> +		packet = ppp_info(frame);
> +
> +		/*
> +		 * check to see if we have a protocol handler
> +		 * registered for this packet
> +		 */
> +		list = g_list_find_custom(packet_handlers,
> +				GUINT_TO_POINTER(protocol),
> +				(GCompareFunc) is_proto_handler);
> +		if (list) {
> +			h = list->data;
> +			h->handler(h->priv, packet);
> +		}
> +		g_free(frame);
> +	}
> +}
> +
> +/* XXX - Implement PFC and ACFC */
> +static guint8 * ppp_decode(struct ppp_link *link, guint8 *frame)
> +{
> +	guint8 *data;
> +	guint pos = 0;
> +	int i = 0;
> +	int len;
> +	guint16 fcs;
> +
> +	data = g_malloc0(link->mru + 10);
> +
> +	/* skip the first flag char */
> +	pos++;
> +
> +	/* TBD - how to deal with recv_accm */
> +	while (frame[pos] != PPP_FLAG_SEQ) {
> +		/* scan for escape character */
> +		if (frame[pos] == PPP_ESC) {
> +			/* skip that char */
> +			pos++;
> +			data[i] = frame[pos] ^ 0x20;
> +		} else
> +			data[i] = frame[pos];
> +		i++; pos++;
> +	}
> +
> +	len = i;
> +
> +	/* see if we have a good FCS */
> +	fcs = PPPINITFCS16;
> +	for (i = 0; i < len; i++)
> +		fcs = ppp_fcs(fcs, data[i]);
> +
> +	if (fcs != PPPGOODFCS16) {
> +		g_free(data);
> +		return NULL;
> +	}
> +	return data;
> +}
> +
> +static void ppp_feed(struct ppp_link *link, guint8 *data, gsize len)
> +{
> +	guint pos = 0;
> +	guint8 *frame;
> +
> +	/* collect bytes until we detect we have received a complete frame */
> +	/* examine the data.  If we are at the beginning of a new frame,
> +	 * allocate memory to buffer the frame.
> +	 */
> +	for (pos = 0; pos < len; pos++) {
> +		if (data[pos] == PPP_FLAG_SEQ) {
> +			if (link->buffer == NULL) {
> +				link->buffer = g_malloc0(BUFFERSZ);
> +				link->index = 0;

So my question is why do we keep allocating / freeing this buffer.  Seems like 
a waste of time..?

> +			} else {
> +				/* store last flag character */
> +				link->buffer[link->index++] = data[pos];
> +				frame = ppp_decode(link, link->buffer);

This function along with ppp_decode do almost exactly the same thing as 
gsm0710_advanced_extract_frame in gsm0710.c.  They both do HDLC frame 
decoding, and the only difference I can see is in the fcs table.  Can we 
combine these somehow?

> +				/* push decoded frame onto receive queue */
> +				if (frame)
> +					g_queue_push_tail(link->recv_queue,
> +								frame);
> +
> +				/* free and reintialize buffer */
> +				g_free(link->buffer);
> +				link->buffer = NULL;
> +				link->index = 0;
> +				continue;
> +			}
> +		}
> +		/* copy byte to buffer */
> +		if (link->buffer)
> +			link->buffer[link->index++] = data[pos];

What concerns me here is that we don't check for buffer overflow here.

> +		else
> +			g_printerr("oops, no buffer, no start of frame\n");
> +
> +	}
> +	/* process receive queue */
> +	ppp_recv(link);
> +}
> +
> +/*
> + * transmit out through the lower layer interface
> + *
> + * infolen - length of the information part of the packet
> + */
> +void __ppp_transmit(struct ppp_link *link, guint8 *packet, guint infolen)
> +{
> +	guint8 *frame;
> +	guint framelen;
> +        GError *error = NULL;
> +        GIOStatus status;
> +        gsize bytes_written;
> +
> +	/*
> +	 * do the octet stuffing.  Add 2 bytes to the infolen to
> +	 * include the protocol field.
> +	 */
> +	frame = ppp_encode(link, packet, infolen+2, &framelen);
> +	if (!frame) {
> +		g_printerr("Failed to encode packet to transmit\n");
> +		return;
> +	}
> +
> +	/* transmit through the lower layer interface */
> +	/*
> +	 * TBD - should we just put this on a queue and transmit when
> +	 * we won't block, or allow ourselves to block here?
> +	 */
> +	status = g_io_channel_write_chars(link->modem, (gchar *) frame,
> +                                          framelen, &bytes_written,
>  &error); +
> +	g_free(frame);

You set the underlying IO channel to be non-blocking below.  Strictly speaking 
what you have here will not work out, you cannot assume that the entire frame 
will be written.

You will need to add some sort of tx_queue and start a G_IO_OUT channel watch 
to handle this properly.  However I'm fine fixing this later...

> +
> +}
> +
> +static gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer
>  data) +{
> +	struct ppp_link *ppp = (struct ppp_link *) data;
> +	GIOStatus status;
> +	gchar buf[256];
> +	gsize bytes_read;
> +	GError *error = NULL;
> +
> +	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
> +		g_print("G_IO_NVAL | G_IO_ERR");
> +		return FALSE;
> +	}
> +
> +	if (cond & G_IO_IN) {
> +		status = g_io_channel_read_chars(channel, buf, 256,
> +				&bytes_read, &error);
> +		if (bytes_read > 0)
> +			ppp_feed(ppp, (guint8 *)buf, bytes_read);
> +        	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
> +                	return FALSE;
> +	}
> +
> +	return TRUE;
> +}
> +
> +/* Administrative Close */
> +static void ppp_close(struct ppp_link *link)
> +{
> +	/* send a CLOSE event to the lcp layer */
> +}
> +
> +/* Administrative Open */
> +void g_at_ppp_open(struct ppp_link *link)
> +{
> +	/* send an OPEN event to the lcp layer */
> +}
> +
> +static void ppp_link_establishment(struct ppp_link *link)
> +{
> +	/* signal UP event to LCP */
> +}
> +
> +static void ppp_terminate(struct ppp_link *link)
> +{
> +	/* signal DOWN event to LCP */
> +}
> +
> +static void ppp_authenticate(struct ppp_link *link)
> +{
> +	/* we don't do authentication right now, so send NONE */
> +	if (!link->auth_proto)
> +		ppp_generate_event(link, PPP_NONE);
> +	/* otherwise we need to wait for the peer to send us a challenge */
> +}
> +
> +static void ppp_dead(struct ppp_link *link)
> +{
> +	/* re-initialize everything */
> +}
> +
> +static void ppp_network(struct ppp_link *link)
> +{
> +	/* bring network phase up */
> +}
> +
> +static void ppp_transition_phase(struct ppp_link *link, guint phase)
> +{
> +	/* don't do anything if we're already there */
> +	if (link->phase == phase)
> +		return;
> +
> +	/* set new phase */
> +	link->phase = phase;
> +
> +	switch(phase) {
> +	case ESTABLISHMENT:
> +		ppp_link_establishment(link);
> +		break;
> +	case AUTHENTICATION:
> +		ppp_authenticate(link);
> +		break;
> +	case TERMINATION:
> +		ppp_terminate(link);
> +		break;
> +	case DEAD:
> +		ppp_dead(link);
> +		break;
> +	case NETWORK:
> +		ppp_network(link);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +}
> +
> +static void ppp_handle_event(struct ppp_link *link)
> +{
> +	guint event;
> +
> +	while ((event = GPOINTER_TO_UINT(g_queue_pop_head(link->event_queue)))){
> +		switch(event) {
> +		case PPP_UP:
> +			/* causes transition to link establishment */
> +			ppp_transition_phase(link, ESTABLISHMENT);
> +			break;
> +		case PPP_OPENED:
> +			ppp_transition_phase(link, AUTHENTICATION);
> +			break;
> +		case PPP_CLOSING:
> +			/* causes transition to termination phase */
> +			ppp_transition_phase(link, TERMINATION);
> +			break;
> +		case PPP_DOWN:
> +			/* cases transition to dead phase */
> +			ppp_transition_phase(link, DEAD);
> +			break;
> +		case PPP_NONE:
> +		case PPP_SUCCESS:
> +			/* causes transition to network phase */
> +			ppp_transition_phase(link, NETWORK);
> +			break;
> +		}
> +	}
> +}
> +
> +/*
> + * send the event handler a new event to process
> + */
> +void __ppp_generate_event(struct ppp_link *link, guint event)
> +{
> +	g_queue_push_tail(link->event_queue, GUINT_TO_POINTER(event));
> +	ppp_handle_event(link);
> +}
> +
> +void g_at_ppp_set_connect_function(struct ppp_link *link,
> +			       GAtPPPConnectFunc callback, gpointer user_data)
> +{
> +	link->connect_cb = callback;
> +	link->connect_priv = user_data;
> +}
> +
> +void g_at_ppp_set_disconnect_function(struct ppp_link *link,
> +				  GAtPPPDisconnectFunc callback,
> +				  gpointer user_data)
> +{
> +	link->disconnect_cb = callback;
> +	link->disconnect_priv = user_data;
> +}
> +
> +void g_at_ppp_shutdown(struct ppp_link *link)
> +{
> +	/* close the link */
> +	ppp_close(link);
> +
> +	/* clean up all the queues */
> +	g_queue_free(link->event_queue);
> +	g_queue_free(link->recv_queue);
> +
> +	/* cleanup modem channel */
> +	g_source_remove(link->modem_watch);
> +	g_io_channel_unref(link->modem);
> +}
> +
> +void g_at_ppp_ref(struct ppp_link *link)
> +{
> +	g_atomic_int_inc(&link->ref_count);
> +}
> +
> +void g_at_ppp_unref(struct ppp_link *link)
> +{
> +	gboolean last;
> +
> +	last = g_atomic_int_dec_and_test(&link->ref_count);
> +	if (last) {
> +		g_at_ppp_shutdown(link);
> +		g_free(link);
> +	}
> +}
> +
> +struct ppp_link * g_at_ppp_new(GIOChannel *modem)
> +{
> +	struct ppp_link *link;
> +	GIOFlags flags;
> +	int signal_source;
> +
> +	link = g_malloc0(sizeof(struct ppp_link));

I suggest using g_try_new0 instead of g_malloc0

> +	if (!link)
> +		return NULL;
> +
> +	link->ref_count = 1;
> +
> +	/* set options to defaults */
> +	link->mru = DEFAULT_MRU;
> +	link->recv_accm = DEFAULT_ACCM;
> +	link->xmit_accm[0] = DEFAULT_ACCM;
> +	link->xmit_accm[3] = 0x60000000; /* 0x7d, 0x7e */
> +	link->pfc = FALSE;
> +	link->acfc = FALSE;
> +
> +	/* allocate the queues */
> +	link->event_queue = g_queue_new();
> +	link->recv_queue = g_queue_new();
> +
> +	link->buffer = NULL;
> +	link->index = 0;
> +
> +	/* setup the watch functions for the modem */
> +	link->modem = g_io_channel_ref(modem);
> +
> +	flags = g_io_channel_get_flags(modem) | G_IO_FLAG_NONBLOCK;
> +	g_io_channel_set_flags(modem, flags, NULL);
> +
> +	g_io_channel_set_encoding(modem, NULL, NULL);
> +	g_io_channel_set_buffered(modem, FALSE);

g_at_util_setup_io does the above four operations for you.  Might be a good 
idea to use that instead.

> +
> +	/* initialize the lcp state */
> +
> +
> +	/* initialize the autentication state */
> +
> +
> +	/* intialize the network state */
> +
> +	/* start listening for packets from the modem */
> +	signal_source = g_io_add_watch(modem,
> +			G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +			ppp_cb, link);
> +	link->modem_watch = signal_source;

Shouldn't we assign this directly and avoid the signal_source variable 
altogether?

> +
> +	return link;
> +}
> Index: ofono/gatchat/gatppp.h
> 
===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ofono/gatchat/gatppp.h	2010-03-10 16:58:12.040979395 -0800
> @@ -0,0 +1,81 @@
> +/*
> + *
> + *  PPP library with GLib integration
> + *
> + *  Copyright (C) 2010  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
>  USA + *
> + */
> +
> +#ifndef __G_AT_PPP_H
> +#define __G_AT_PPP_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +struct ppp_link;

For consistency this should be typedefed to GAtPpp, similar to how GAtChat is 
typedefed.

> +
> +enum PPP_CONNECT_STATUS {
> +	PPP_CONNECT_SUCCESS,
> +	PPP_CONNECT_FAIL
> +};
> +

The enum names should be prefixed by G_AT.  e.g. G_AT_PPP_RESULT_OK, 
G_AT_PPP_RESULT_UNSPECIFIED_ERROR, G_AT_PPP_RESULT_AUTH_ERROR, etc.  Keep the 
enum definition itself camel-case, e.g. enum _GAtPppResult, typedef to 
GAtPppResult

> +typedef void (*GAtPPPConnectFunc)(struct ppp_link *link, gint success,
> +				 guint32 ip_address,
> +				 guint32 dns1, guint32 dns2,
> +				 gpointer user_data);
> 

I suggest that the gint success be changed to use the enum defined above.

> +
> +typedef void (*GAtPPPDisconnectFunc)(struct ppp_link *link, gpointer
>  user_data); +
> +struct ppp_link {
> +	gint ref_count;
> +	guint phase;

I suggest you use the enum directly here instead of the guint.

> +	guint8 *buffer;
> +	int index;
> +	gint mru;
> +	guint16 auth_proto;
> +	char user_name[256];
> +	char passwd[256];
> +	gboolean pfc;
> +	gboolean acfc;
> +	guint32 xmit_accm[8];
> +	guint32 recv_accm;
> +	GIOChannel *modem;
> +	GQueue *event_queue;
> +	GQueue *recv_queue;
> +	GAtPPPConnectFunc connect_cb;
> +	gpointer connect_priv;

The suffix 'data' is preferred over 'priv'

> +	GAtPPPDisconnectFunc disconnect_cb;
> +	gpointer disconnect_priv;

As above

> +	gint modem_watch;
> +};
> +

If possible, I would suggest that you hide the struct implementation in the .c 
file instead of exposing it here.

> +struct ppp_link * g_at_ppp_new(GIOChannel *modem);
> +void g_at_ppp_open(struct ppp_link *link);
> +void g_at_ppp_set_connect_function(struct ppp_link *link,
> +			       GAtPPPConnectFunc callback, gpointer user_data);
> +void g_at_ppp_set_disconnect_function(struct ppp_link *link,
> +				  GAtPPPDisconnectFunc callback,
> +				  gpointer user_data);
> +void g_at_ppp_shutdown(struct ppp_link *link);
> +void g_at_ppp_ref(struct ppp_link *link);
> +void g_at_ppp_unref(struct ppp_link *link);
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __G_AT_PPP_H */
> +
> 

Regards,
-Denis

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On Thu, 11 Mar 2010 20:17:49 -0600
Denis Kenzior <denkenz@gmail.com> wrote:


> 
> > +			} else {
> > +				/* store last flag character */
> > +				link->buffer[link->index++] = data[pos];
> > +				frame = ppp_decode(link, link->buffer);
> 
> This function along with ppp_decode do almost exactly the same thing as 
> gsm0710_advanced_extract_frame in gsm0710.c.  They both do HDLC frame 
> decoding, and the only difference I can see is in the fcs table.  Can we 
> combine these somehow?

Possibly - although in theory in addition to the escaping that you do
in the gsm0710 code, we have to support a negotiated accm (which you see
I've not yet implemented here).  We also in theory should support PFC and
ACFC (which the one modem I tested with required, otherwise it refused to
ack my Configure-Request).  I think there may eventually be enough 
differences to keep these separate.

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

Hi Kristen,

> On Thu, 11 Mar 2010 20:17:49 -0600
> 
> Denis Kenzior <denkenz@gmail.com> wrote:
> > > +			} else {
> > > +				/* store last flag character */
> > > +				link->buffer[link->index++] = data[pos];
> > > +				frame = ppp_decode(link, link->buffer);
> >
> > This function along with ppp_decode do almost exactly the same thing as
> > gsm0710_advanced_extract_frame in gsm0710.c.  They both do HDLC frame
> > decoding, and the only difference I can see is in the fcs table.  Can we
> > combine these somehow?
> 
> Possibly - although in theory in addition to the escaping that you do
> in the gsm0710 code, we have to support a negotiated accm (which you see
> I've not yet implemented here).  We also in theory should support PFC and
> ACFC (which the one modem I tested with required, otherwise it refused to
> ack my Configure-Request).  I think there may eventually be enough
> differences to keep these separate.
> 

You will have to explain to me what that all means ;)  However, it would be 
ideal if we can create a set of utilities that can be shared between ppp and 
mux code (maybe with extra configuration parameters turning on/off or passing in 
parameters required for pfc/acfc/accm support.)

No sense writing, testing and debugging (and more importantly maintaining) the 
same code twice.

Regards,
-Denis

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

Hi Kristen,

> +struct ppp_link * g_at_ppp_new(GIOChannel *modem);
> +void g_at_ppp_open(struct ppp_link *link);
> +void g_at_ppp_set_connect_function(struct ppp_link *link,
> +			       GAtPPPConnectFunc callback, gpointer user_data);
> +void g_at_ppp_set_disconnect_function(struct ppp_link *link,
> +				  GAtPPPDisconnectFunc callback,
> +				  gpointer user_data);
> +void g_at_ppp_shutdown(struct ppp_link *link);
> +void g_at_ppp_ref(struct ppp_link *link);
> +void g_at_ppp_unref(struct ppp_link *link);

Almost forgot, let us not use struct ppp_link here, but instead use GAtPpp 
*ppp as the parameter.  Again, mostly for consistency with how glib / gatchat 
does this.

Regards,
-Denis

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

Hi Denis,

> > +struct ppp_link * g_at_ppp_new(GIOChannel *modem);
> > +void g_at_ppp_open(struct ppp_link *link);
> > +void g_at_ppp_set_connect_function(struct ppp_link *link,
> > +			       GAtPPPConnectFunc callback, gpointer user_data);
> > +void g_at_ppp_set_disconnect_function(struct ppp_link *link,
> > +				  GAtPPPDisconnectFunc callback,
> > +				  gpointer user_data);
> > +void g_at_ppp_shutdown(struct ppp_link *link);
> > +void g_at_ppp_ref(struct ppp_link *link);
> > +void g_at_ppp_unref(struct ppp_link *link);
> 
> Almost forgot, let us not use struct ppp_link here, but instead use GAtPpp 
> *ppp as the parameter.  Again, mostly for consistency with how glib / gatchat 
> does this.

I am actually okay with GAtPPP since it looks much nicer. True camel
case is making it harder to read.

Regards

Marcel

[patch 2/6] Generic PPP control protocol

[kristen]

[-- Attachment #1: Type: text/plain, Size: 45048 bytes --]

Implement a generic control protocol that can be shared by both LCP and NCP
implementations.

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-10 16:58:12.039961039 -0800
+++ ofono/Makefile.am	2010-03-10 17:00:34.037087986 -0800
@@ -57,7 +57,8 @@
 				gatchat/gat.h \
 				gatchat/gatserver.h gatchat/gatserver.c \
 				gatchat/gatppp.c gatchat/gatppp.h \
-				gatchat/gatppp_internal.h
+				gatchat/gatppp_internal.h gatchat/gatpppcp.c
+				gatchat/gatpppcp.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatpppcp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatpppcp.c	2010-03-10 17:00:34.039103187 -0800
@@ -0,0 +1,1492 @@
+/*
+ *
+ *  AT chat library with GLib integration
+ *
+ *  Copyright (C) 2008-2009  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <glib.h>
+#include <arpa/inet.h>
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+static gboolean pppcp_timeout(gpointer user);
+static void pppcp_handle_event(gpointer user);
+static gint is_option(struct ppp_option *o, gpointer user);
+
+const char *pppcp_state_strings[] =
+	{"INITIAL", "STARTING", "CLOSED", "STOPPED", "CLOSING", "STOPPING",
+	"REQSENT", "ACKRCVD", "ACKSENT", "OPENED" };
+
+#ifdef DEBUG
+#define pppcp_trace(p) \
+	(g_print("%s: current state %d:%s\n", __FUNCTION__, \
+		p->state, pppcp_state_strings[p->state]))
+#else
+#define pppcp_trace(p)
+#endif
+
+#define pppcp_to_ppp_packet(p) \
+	(p-PPP_HEADROOM)
+
+/* CP packet header */
+struct pppcp_header {
+	guint8 code;
+	guint8 identifier;
+	guint16 length;
+};
+
+struct pppcp_event {
+	guint type;
+	gint len;
+	guint8 data[0];
+};
+
+enum {
+	INITIAL_RESTART_TIMEOUT	= 3000,
+	MAX_TERMINATE		= 2,
+	MAX_CONFIGURE		= 10,
+	MAX_FAILURE		= 5,
+	CP_HEADER_SZ		= 4,
+
+	/* states */
+	INITIAL = 0,
+	STARTING	= 1,
+	CLOSED		= 2,
+	STOPPED		= 3,
+	CLOSING		= 4,
+	STOPPING	= 5,
+	REQSENT		= 6,
+	ACKRCVD		= 7,
+	ACKSENT		= 8,
+	OPENED		= 9,
+};
+
+static struct pppcp_packet * pppcp_packet_new(struct pppcp_data *data,
+						guint type, guint bufferlen)
+{
+	struct pppcp_header *packet;
+	guint8 *ppp_packet;
+
+	ppp_packet = g_malloc0(bufferlen + sizeof(*packet) + 2);
+	if (!ppp_packet)
+		return NULL;
+
+	/* add our protocol information */
+	*((guint16 *)ppp_packet) = htons(data->proto);
+
+	/* advance past protocol to add CP header information */
+	packet = (struct pppcp_header *) (ppp_packet + 2);
+
+	packet->length = htons((guint16)(bufferlen + sizeof(*packet)));
+	packet->code = type;
+	return ((struct pppcp_packet *)packet);
+}
+
+static void pppcp_start_timer(struct pppcp_data *data)
+{
+	data->restart_timer = g_timeout_add(data->restart_interval,
+				pppcp_timeout, data);
+}
+
+static void pppcp_stop_timer(struct pppcp_data *data)
+{
+	if (data->restart_timer) {
+		g_source_remove(data->restart_timer);
+		data->restart_timer = 0;
+	}
+}
+
+static gboolean pppcp_timer_is_running(struct pppcp_data *data)
+{
+	/* determine if the restart timer is running */
+	if (data->restart_timer)
+		return TRUE;
+	return FALSE;
+}
+
+
+static gboolean pppcp_timeout(gpointer user)
+{
+	struct pppcp_data *data = user;
+
+	pppcp_trace(data);
+
+	data->restart_timer = 0;
+
+	if (data->restart_counter)
+		pppcp_generate_event(data, TO_PLUS, NULL, 0);
+	else
+		pppcp_generate_event(data, TO_MINUS, NULL, 0);
+	return FALSE;
+}
+
+struct pppcp_event * pppcp_event_new(guint type, gpointer event_data, guint len)
+{
+	struct pppcp_event *event;
+	guint8 *data = (guint8 *)event_data;
+
+	event = g_malloc(sizeof(struct pppcp_event) + len);
+	if (!event)
+		return NULL;
+
+	event->type = type;
+	memcpy(event->data, data, len);
+	event->len = len;
+	return event;
+}
+
+/*
+ * send the event handler a new event to process
+ */
+void pppcp_generate_event(struct pppcp_data *data, guint event_type,
+				gpointer event_data, guint data_len)
+{
+	struct pppcp_event *event;
+
+	event = pppcp_event_new(event_type, event_data, data_len);
+	if (event)
+		g_queue_push_tail(data->event_queue, event);
+	pppcp_handle_event(data);
+}
+
+static struct pppcp_event * pppcp_get_event(struct pppcp_data *data)
+{
+	return g_queue_pop_head(data->event_queue);
+}
+
+/* actions */
+/* log an illegal event, but otherwise do nothing */
+static void pppcp_illegal_event(guint8 state, guint8 type)
+{
+	g_printerr("Illegal event %d while in state %d\n", type, state);
+}
+
+static void pppcp_this_layer_up(struct pppcp_data *data)
+{
+	struct pppcp_action *action = data->action;
+
+	if (action->this_layer_up)
+		action->this_layer_up(data);
+}
+
+static void pppcp_this_layer_down(struct pppcp_data *data)
+{
+	struct pppcp_action *action = data->action;
+
+	if (action->this_layer_up)
+		action->this_layer_down(data);
+}
+
+static void pppcp_this_layer_started(struct pppcp_data *data)
+{
+	struct pppcp_action *action = data->action;
+
+	if (action->this_layer_up)
+		action->this_layer_started(data);
+}
+
+static void pppcp_this_layer_finished(struct pppcp_data *data)
+{
+	struct pppcp_action *action = data->action;
+
+	if (action->this_layer_up)
+		action->this_layer_finished(data);
+}
+
+static void pppcp_free_option(gpointer data, gpointer user)
+{
+	struct ppp_option *option = (struct ppp_option *)data;
+	g_free(option);
+}
+
+static void pppcp_clear_options(struct pppcp_data *data)
+{
+	g_list_foreach(data->acceptable_options, pppcp_free_option, NULL);
+	g_list_foreach(data->unacceptable_options, pppcp_free_option, NULL);
+	g_list_foreach(data->rejected_options, pppcp_free_option, NULL);
+	g_list_free(data->acceptable_options);
+	g_list_free(data->unacceptable_options);
+	g_list_free(data->rejected_options);
+	data->acceptable_options = NULL;
+	data->unacceptable_options = NULL;
+	data->rejected_options = NULL;
+}
+
+/*
+ * set the restart counter to either max-terminate
+ * or max-configure.  The counter is decremented for
+ * each transmission, including the first.
+ */
+static void pppcp_initialize_restart_count(struct pppcp_data *data, guint value)
+{
+	pppcp_trace(data);
+	pppcp_clear_options(data);
+	data->restart_counter = value;
+}
+
+/*
+ * set restart counter to zero
+ */
+static void pppcp_zero_restart_count(struct pppcp_data *data)
+{
+	data->restart_counter = 0;
+}
+
+/*
+ * TBD - generate new identifier for packet
+ */
+static guint8 new_identity(struct pppcp_data *data, guint prev_identifier)
+{
+	return prev_identifier+1;
+}
+
+static void get_option_length(gpointer data, gpointer user)
+{
+	struct ppp_option *option = (struct ppp_option *)data;
+	guint8 *length = (guint8 *)user;
+
+	*length += option->length;
+}
+
+static void copy_option(gpointer data, gpointer user)
+{
+	struct ppp_option *option = (struct ppp_option *)data;
+	guint8 **location = (guint8 **)user;
+	memcpy(*location, (guint8 *)option, option->length);
+	*location += option->length;
+}
+
+void pppcp_add_config_option(struct pppcp_data *data, struct ppp_option *option)
+{
+	data->config_options = g_list_append(data->config_options, option);
+}
+
+/*
+ * transmit a Configure-Request packet
+ * start the restart timer
+ * decrement the restart counter
+ */
+static void pppcp_send_configure_request(struct pppcp_data *data)
+{
+	struct pppcp_packet *packet;
+	guint8 olength = 0;
+	guint8 *odata;
+
+	pppcp_trace(data);
+
+	/* figure out how much space to allocate for options */
+	g_list_foreach(data->config_options, get_option_length, &olength);
+
+	packet = pppcp_packet_new(data, CONFIGURE_REQUEST, olength);
+
+	/* copy config options into packet data */
+	odata = packet->data;
+	g_list_foreach(data->config_options, copy_option, &odata);
+
+	/*
+	 * if this is the first request, we need a new identifier.
+	 * if this is a retransmission, leave the identifier alone.
+	 */
+	if (data->restart_counter == data->max_configure)
+		data->config_identifier =
+			new_identity(data, data->config_identifier);
+	packet->identifier = data->config_identifier;
+
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+
+	/* XXX don't retransmit right now */
+#if 0
+	data->restart_counter--;
+	pppcp_start_timer(data);
+#endif
+}
+
+/*
+ * transmit a Configure-Ack packet
+ */
+static void pppcp_send_configure_ack(struct pppcp_data *data,
+					guint8 *request)
+{
+	struct pppcp_packet *packet;
+	struct pppcp_packet *pppcp_header = (struct pppcp_packet *)request;
+	guint len;
+	guint8 *odata;
+
+	pppcp_trace(data);
+
+	/* subtract for header. */
+	len = ntohs(pppcp_header->length) - sizeof(*packet);
+
+	packet = pppcp_packet_new(data, CONFIGURE_ACK, len);
+
+	/* copy the applied options in. */
+	odata = packet->data;
+
+	if (g_list_length(data->acceptable_options))
+		g_list_foreach(data->acceptable_options, copy_option, &odata);
+
+	/* match identifier of the request */
+	packet->identifier = pppcp_header->identifier;
+
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+}
+
+/*
+ * transmit a Configure-Nak or Configure-Reject packet
+ */
+static void pppcp_send_configure_nak(struct pppcp_data *data,
+					guint8 *configure_packet)
+{
+	struct pppcp_packet *packet;
+	struct pppcp_packet *pppcp_header =
+			(struct pppcp_packet *)configure_packet;
+	guint8 olength = 0;
+	guint8 *odata;
+
+	/* if we have any rejected options, send a config-reject */
+	if (g_list_length(data->rejected_options)) {
+		/* figure out how much space to allocate for options */
+		g_list_foreach(data->rejected_options, get_option_length,
+				&olength);
+
+		packet = pppcp_packet_new(data, CONFIGURE_REJECT, olength);
+
+		/* copy the rejected options in. */
+		odata = packet->data;
+		g_list_foreach(data->rejected_options, copy_option,
+				&odata);
+
+		packet->identifier = pppcp_header->identifier;
+		ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+
+	}
+	/* if we have any unacceptable options, send a config-nak */
+	if (g_list_length(data->unacceptable_options)) {
+		olength = 0;
+
+		/* figure out how much space to allocate for options */
+		g_list_foreach(data->unacceptable_options, get_option_length,
+				&olength);
+
+		packet = pppcp_packet_new(data, CONFIGURE_NAK, olength);
+
+		/* copy the unacceptable options in. */
+		odata = packet->data;
+		g_list_foreach(data->unacceptable_options, copy_option,
+				&odata);
+
+		packet->identifier = pppcp_header->identifier;
+		ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+				ntohs(packet->length));
+	}
+}
+
+/*
+ * transmit a Terminate-Request packet.
+ * start the restart timer.
+ * decrement the restart counter
+ */
+static void pppcp_send_terminate_request(struct pppcp_data *data)
+{
+	struct pppcp_packet *packet;
+
+	/*
+	 * the data field can be used by the sender (us).
+	 * leave this empty for now.
+	 */
+	packet = pppcp_packet_new(data, TERMINATE_REQUEST, 0);
+
+	/*
+	 * Is this a retransmission?  If so, do not change
+	 * the identifier.  If not, we need a fresh identity.
+	 */
+	if (data->restart_counter == data->max_terminate)
+		data->terminate_identifier =
+			new_identity(data, data->terminate_identifier);
+	packet->identifier = data->terminate_identifier;
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+	data->restart_counter--;
+	pppcp_start_timer(data);
+}
+
+/*
+ * transmit a Terminate-Ack packet
+ */
+static void pppcp_send_terminate_ack(struct pppcp_data *data,
+					guint8 *request)
+{
+	struct pppcp_packet *packet;
+	struct pppcp_packet *pppcp_header = (struct pppcp_packet *)request;
+
+	packet = pppcp_packet_new(data, TERMINATE_ACK, 0);
+
+	/* match identifier of the request */
+	packet->identifier = pppcp_header->identifier;
+
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(pppcp_header->length));
+}
+
+/*
+ * transmit a Code-Reject packet
+ *
+ * XXX this seg faults.
+ */
+static void pppcp_send_code_reject(struct pppcp_data *data,
+					guint8 *rejected_packet)
+{
+	struct pppcp_packet *packet;
+
+	packet = pppcp_packet_new(data, CODE_REJECT,
+			ntohs(((struct pppcp_packet *)rejected_packet)->length));
+
+	/*
+	 * Identifier must be changed for each Code-Reject sent
+	 */
+	packet->identifier = new_identity(data, data->reject_identifier);
+
+	/*
+	 * rejected packet should be copied in, but it should be
+	 * truncated if it needs to be to comply with mtu requirement
+	 */
+	memcpy(packet->data, rejected_packet,
+			ntohs(packet->length - CP_HEADER_SZ));
+
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+}
+
+/*
+ * transmit an Echo-Reply packet
+ */
+static void pppcp_send_echo_reply(struct pppcp_data *data,
+				guint8 *request)
+{
+	struct pppcp_packet *packet;
+	struct pppcp_packet *header = (struct pppcp_packet *)request;
+
+	/*
+	 * 0 bytes for data, 4 bytes for magic number
+	 */
+	packet = pppcp_packet_new(data, ECHO_REPLY, 4);
+
+	/*
+	 * match identifier of request
+	 */
+	packet->identifier = header->identifier;
+
+	/* magic number? */
+	ppp_transmit(data->link, pppcp_to_ppp_packet((guint8 *)packet),
+			ntohs(packet->length));
+
+}
+
+/* what about locking, do we need to worry about that here */
+static void pppcp_transition_state(int new_state, struct pppcp_data *data)
+{
+	/*
+	 * if switching from a state where
+	 * TO events occur, to one where they
+	 * may not, shut off the timer
+	 */
+	switch(new_state) {
+	case INITIAL:
+	case STARTING:
+	case CLOSED:
+	case STOPPED:
+	case OPENED:
+		/* if timer is running, stop it */
+		if (pppcp_timer_is_running(data))
+			pppcp_stop_timer(data);
+		break;
+	default:
+		break;
+		/* do nothing */
+	}
+	data->state = new_state;
+}
+
+static void pppcp_up_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+	switch(data->state) {
+	case INITIAL:
+		/* switch state to CLOSED */
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case STARTING:
+		/* irc, scr/6 */
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	default:
+		/* all other are illegal */
+		pppcp_illegal_event(data->state, UP);
+	}
+
+}
+
+static void pppcp_down_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	switch(data->state) {
+	case CLOSED:
+		pppcp_transition_state(INITIAL, data);
+		break;
+	case STOPPED:
+		/* tls/1 */
+		pppcp_transition_state(STARTING, data);
+		pppcp_this_layer_started(data);
+		break;
+	case CLOSING:
+		pppcp_transition_state(INITIAL, data);
+		break;
+	case STOPPING:
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_transition_state(STARTING, data);
+		break;
+	case OPENED:
+		pppcp_transition_state(STARTING, data);
+		pppcp_this_layer_down(data);
+	default:
+		pppcp_illegal_event(data->state, DOWN);
+		/* illegal */
+	}
+}
+
+static void pppcp_open_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+	switch(data->state) {
+	case INITIAL:
+		/* tls/1 */
+		pppcp_transition_state(STARTING, data);
+		pppcp_this_layer_started(data);
+		break;
+	case STARTING:
+		pppcp_transition_state(STARTING, data);
+		break;
+	case CLOSED:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case STOPPED:
+		/* 3r */
+		pppcp_transition_state(STOPPED, data);
+		break;
+	case CLOSING:
+	case STOPPING:
+		/* 5r */
+		pppcp_transition_state(STOPPING, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_transition_state(data->state, data);
+		break;
+	case OPENED:
+		/* 9r */
+		pppcp_transition_state(data->state, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, OPEN);
+	}
+}
+
+static void pppcp_close_event(struct pppcp_data *data, guint8* packet, guint len)
+{
+	switch(data->state) {
+	case INITIAL:
+		pppcp_transition_state(INITIAL, data);
+		break;
+	case STARTING:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(INITIAL, data);
+		break;
+	case CLOSED:
+	case STOPPED:
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(CLOSING, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_initialize_restart_count(data, data->max_terminate);
+		pppcp_send_terminate_request(data);
+		pppcp_transition_state(CLOSING, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, CLOSE);
+		/* illegal */
+	}
+}
+
+static void pppcp_to_plus_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSING:
+		pppcp_send_terminate_request(data);
+		pppcp_transition_state(CLOSING, data);
+		break;
+	case STOPPING:
+		pppcp_send_terminate_request(data);
+		pppcp_transition_state(STOPPING, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case ACKSENT:
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, TO_PLUS);
+	}
+}
+
+static void pppcp_to_minus_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+	switch(data->state) {
+	case CLOSING:
+		pppcp_transition_state(CLOSED, data);
+		pppcp_this_layer_finished(data);
+		break;
+	case STOPPING:
+		pppcp_transition_state(STOPPED, data);
+		pppcp_this_layer_finished(data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		/* tlf/3p */
+		pppcp_transition_state(STOPPED, data);
+		pppcp_this_layer_finished(data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, TO_MINUS);
+	}
+}
+
+static void pppcp_rcr_plus_event(struct pppcp_data *data,
+				guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+	switch(data->state) {
+	case CLOSED:
+		pppcp_send_terminate_ack(data, packet);
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case STOPPED:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_send_configure_ack(data, packet);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(data->state, data);
+		break;
+	case REQSENT:
+		pppcp_send_configure_ack(data, packet);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	case ACKRCVD:
+		pppcp_send_configure_ack(data, packet);
+		pppcp_this_layer_up(data);
+		pppcp_transition_state(OPENED, data);
+		break;
+	case ACKSENT:
+		pppcp_send_configure_ack(data, packet);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_send_configure_request(data);
+		pppcp_send_configure_ack(data, packet);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RCR_PLUS);
+	}
+}
+
+static void pppcp_rcr_minus_event(struct pppcp_data *data,
+				guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+		pppcp_send_terminate_ack(data, packet);
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case STOPPED:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_send_configure_nak(data, packet);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(data->state, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+		pppcp_send_configure_nak(data, packet);
+		pppcp_transition_state(data->state, data);
+		break;
+	case ACKSENT:
+		pppcp_send_configure_nak(data, packet);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_send_configure_request(data);
+		pppcp_send_configure_nak(data, packet);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RCR_MINUS);
+	}
+}
+
+static void pppcp_rca_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+		pppcp_send_terminate_ack(data, packet);
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(data->state, data);
+		break;
+	case REQSENT:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_transition_state(ACKRCVD, data);
+		break;
+	case ACKRCVD:
+		/* scr/6x */
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+	case ACKSENT:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_this_layer_up(data);
+		pppcp_transition_state(OPENED, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RCA);
+	}
+}
+
+static void pppcp_rcn_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+		pppcp_send_terminate_ack(data, packet);
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(data->state, data);
+	case REQSENT:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case ACKRCVD:
+		/* scr/6x */
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case ACKSENT:
+		pppcp_initialize_restart_count(data, data->max_configure);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RCN);
+	}
+}
+
+static void pppcp_rtr_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+		pppcp_send_terminate_ack(data, packet);
+	case CLOSING:
+	case STOPPING:
+		break;
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_send_terminate_ack(data, packet);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_zero_restart_count(data);
+		pppcp_send_terminate_ack(data, packet);
+		pppcp_transition_state(STOPPING, data);
+		break;
+
+	default:
+		pppcp_illegal_event(data->state, RTR);
+	}
+}
+
+static void pppcp_rta_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+		pppcp_transition_state(data->state, data);
+		break;
+	case CLOSING:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case STOPPING:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(STOPPED, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case ACKSENT:
+		pppcp_transition_state(ACKSENT, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_send_configure_request(data);
+		pppcp_transition_state(REQSENT, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RTA);
+	}
+}
+
+static void pppcp_ruc_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+	case CLOSING:
+	case STOPPING:
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+	case OPENED:
+		pppcp_send_code_reject(data, packet);
+		pppcp_transition_state(data->state, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RUC);
+	}
+}
+
+static void pppcp_rxj_plus_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+	case CLOSING:
+	case STOPPING:
+		pppcp_transition_state(data->state, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+		pppcp_transition_state(REQSENT, data);
+		break;
+	case ACKSENT:
+	case OPENED:
+		pppcp_transition_state(data->state, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RXJ_PLUS);
+	}
+}
+
+static void pppcp_rxj_minus_event(struct pppcp_data *data,
+				guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(data->state, data);
+		break;
+	case CLOSING:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(CLOSED, data);
+		break;
+	case STOPPING:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(STOPPED, data);
+		break;
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_this_layer_finished(data);
+		pppcp_transition_state(STOPPED, data);
+		break;
+	case OPENED:
+		pppcp_this_layer_down(data);
+		pppcp_initialize_restart_count(data, data->max_terminate);
+		pppcp_send_terminate_request(data);
+		pppcp_transition_state(STOPPING, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RXJ_MINUS);
+	}
+}
+
+static void pppcp_rxr_event(struct pppcp_data *data, guint8 *packet, guint len)
+{
+	pppcp_trace(data);
+
+	switch(data->state) {
+	case CLOSED:
+	case STOPPED:
+	case CLOSING:
+	case STOPPING:
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		pppcp_transition_state(data->state, data);
+		break;
+	case OPENED:
+		pppcp_send_echo_reply(data, packet);
+		pppcp_transition_state(OPENED, data);
+		break;
+	default:
+		pppcp_illegal_event(data->state, RXR);
+	}
+}
+
+/*
+ * This was implemented as a separate thread which
+ * gets event messages off of an asynchronous queue.
+ *
+ * it is now just a synchronous queue to avoid the
+ * use of threads.
+ */
+static void pppcp_handle_event(gpointer user)
+{
+	struct pppcp_event *event;
+	struct pppcp_data *data = (struct pppcp_data *) user;
+
+	while ((event = pppcp_get_event(data))) {
+		if (event->type > RXR)
+			pppcp_illegal_event(data->state, event->type);
+		else
+			data->event_ops[event->type](data, event->data,
+							event->len);
+		g_free(event);
+	}
+}
+
+static void verify_config_option(gpointer elem, gpointer user)
+{
+	struct ppp_option *config = (struct ppp_option *)elem;
+	struct pppcp_data *data = (struct pppcp_data *)user;
+	GList *list = NULL;
+	struct ppp_option *option;
+
+	/*
+	 * determine whether this config option is in the
+	 * acceptable options list
+	 */
+	if (g_list_length(data->acceptable_options))
+		list = g_list_find_custom(data->acceptable_options,
+				GUINT_TO_POINTER(config->type),
+				(GCompareFunc) is_option);
+	if (!list) {
+		/*
+		 * if the option did not exist, we need to store a copy
+		 * of the option in the unacceptable_options list so it
+		 * can be nak'ed.
+		 */
+		option = g_malloc0(config->length);
+		option->type = config->type;
+		option->length = config->length;
+		data->unacceptable_options =
+			g_list_append(data->unacceptable_options, option);
+	}
+
+}
+
+static void remove_config_option(gpointer elem, gpointer user)
+{
+	struct ppp_option *config = (struct ppp_option *)elem;
+	struct pppcp_data *data = (struct pppcp_data *)user;
+	GList *list = NULL;
+
+	/*
+	 * determine whether this config option is in the
+	 * applied options list
+	 */
+	if (g_list_length(data->config_options)) {
+		list = g_list_find_custom(data->config_options,
+				GUINT_TO_POINTER(config->type),
+				(GCompareFunc) is_option);
+		if (list)
+			data->config_options =
+				g_list_delete_link(data->config_options, list);
+	}
+}
+
+static guint8 pppcp_process_configure_request(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	gint len;
+	int i= 0;
+	struct ppp_option *option;
+	guint rval = -1;
+	struct pppcp_action *action = data->action;
+
+	pppcp_trace(data);
+
+	len = ntohs(packet->length) - CP_HEADER_SZ;
+
+	/*
+	 * check the options.
+	 */
+	while (i < len) {
+		guint8 otype = packet->data[i];
+		guint8 olen = packet->data[i+1];
+		option = g_malloc0(olen);
+		option->type = otype;
+		option->length = olen;
+		memcpy(option->data, &packet->data[i+2], olen-2);
+		if (action->option_scan)
+			rval = action->option_scan(option, data);
+		switch(rval) {
+		case OPTION_ACCEPT:
+			data->acceptable_options =
+				g_list_append(data->acceptable_options, option);
+			break;
+		case OPTION_REJECT:
+			data->rejected_options =
+				g_list_append(data->rejected_options, option);
+			break;
+		case OPTION_NAK:
+			data->unacceptable_options =
+				g_list_append(data->unacceptable_options,
+						option);
+			break;
+		default:
+			g_printerr("unexpected scan_option rval %d\n", rval);
+		}
+		/* skip ahead to the next option */
+		i += olen;
+	}
+
+	/* make sure all required config options were included */
+	g_list_foreach(data->config_options, verify_config_option, data);
+
+	if (g_list_length(data->unacceptable_options) ||
+		g_list_length(data->rejected_options))
+		return RCR_MINUS;
+
+	/*
+	 * all options were acceptable, so we should apply them before
+	 * sending a configure-ack
+	 *
+	 * Remove all applied options from the config_option list.  The
+	 * protocol will have to re-add them if they want them renegotiated
+	 * when the link goes down.
+	 */
+	if (action->option_process) {
+		g_list_foreach(data->acceptable_options,
+				action->option_process, data->priv);
+		g_list_foreach(data->acceptable_options, remove_config_option,
+				data);
+	}
+
+	return RCR_PLUS;
+}
+
+static guint8 pppcp_process_configure_ack(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	guint len;
+	GList *list;
+	struct ppp_option *acked_option;
+	guint i = 0;
+	struct pppcp_action *action = data->action;
+
+	pppcp_trace(data);
+
+	len = ntohs(packet->length) - CP_HEADER_SZ;
+
+	/* if identifiers don't match, we should silently discard */
+	if (packet->identifier != data->config_identifier) {
+		g_printerr("received an ack id %d, but config id is %d\n",
+			packet->identifier, data->config_identifier);
+		return 0;
+	}
+
+	/*
+	 * check each acked option.  If it is what we requested,
+	 * then we can apply these option values.
+	 *
+	 * XXX what if it isn't?  Do this correctly -- for now
+	 * we are just going to assume that all options matched
+	 * and apply them.
+	 */
+	while (i < len) {
+		guint8 otype = packet->data[i];
+		guint8 olen = packet->data[i+1];
+		acked_option = g_malloc0(olen);
+		acked_option->type = otype;
+		acked_option->length = olen;
+		memcpy(acked_option->data, &packet->data[i+2], olen-2);
+		list = g_list_find_custom(data->config_options,
+				GUINT_TO_POINTER(acked_option->type),
+				(GCompareFunc) is_option);
+		if (list) {
+			/*
+			 * once we've applied the option, delete it from
+		 	 * the config_options list.
+			 */
+			if (action->option_process)
+				action->option_process(acked_option,
+							data->priv);
+			data->config_options =
+				g_list_delete_link(data->config_options, list);
+		} else
+			g_printerr("oops -- found acked option %d we didn't request\n", acked_option->type);
+		g_free(acked_option);
+
+		/* skip ahead to the next option */
+		i += olen;
+	}
+	return RCA;
+}
+
+static gint is_option(struct ppp_option *o, gpointer user)
+{
+	guint8 otype = (guint8) GPOINTER_TO_UINT(user);
+
+	if (o->type == otype)
+		return 0;
+	else
+		return -1;
+}
+
+static guint8 pppcp_process_configure_nak(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	guint len;
+	GList *list;
+	struct ppp_option *naked_option;
+	struct ppp_option *config_option;
+	guint i = 0;
+	gint rval = -1;
+	struct pppcp_action *action = data->action;
+
+	pppcp_trace(data);
+
+	len = ntohs(packet->length) - CP_HEADER_SZ;
+
+	/* if identifiers don't match, we should silently discard */
+	if (packet->identifier != data->config_identifier)
+		return 0;
+
+	/*
+	 * check each unacceptable option.  If it is acceptable, then
+	 * we can resend the configure request with this value. we need
+	 * to check the current config options to see if we need to
+	 * modify a value there, or add a new option.
+	 */
+	while (i < len) {
+		guint8 otype = packet->data[i];
+		guint8 olen = packet->data[i+1];
+		naked_option = g_malloc0(olen);
+		naked_option->type = otype;
+		naked_option->length = olen;
+		memcpy(naked_option->data, &packet->data[i+2], olen-2);
+		if (action->option_scan)
+			rval = action->option_scan(naked_option, data);
+		if (rval == OPTION_ACCEPT) {
+			/*
+			 * check the current config options to see if they
+			 * match.
+			 */
+			list = g_list_find_custom(data->config_options,
+				GUINT_TO_POINTER(otype),
+				(GCompareFunc) is_option);
+			if (list) {
+				/* modify current option value to match */
+				config_option = list->data;
+
+				/*
+				 * option values should match, otherwise
+				 * we need to reallocate
+				 */
+				if ((config_option->length ==
+					naked_option->length) && (olen-2)) {
+						memcpy(config_option->data,
+							naked_option->data,
+							olen-2);
+				} else {
+					/* XXX implement this */
+					g_printerr("uh oh, option value doesn't match\n");
+				}
+				g_free(naked_option);
+			} else {
+				/* add to list of config options */
+				pppcp_add_config_option(data, naked_option);
+			}
+		} else {
+			/* XXX handle this correctly */
+			g_printerr("oops, option wasn't acceptable\n");
+			g_free(naked_option);
+		}
+
+		/* skip ahead to the next option */
+		i += olen;
+	}
+	return RCN;
+}
+
+static guint8 pppcp_process_configure_reject(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	/*
+	 * make sure identifier matches that of last sent configure
+	 * request
+	 */
+	if (packet->identifier == data->config_identifier) {
+		/*
+		 * check to see which options were rejected
+		 * Rejected options must be a subset of requested
+		 * options.
+		 *
+		 * when a new configure-request is sent, we may
+		 * not request any of these options be negotiated
+		 */
+		return RCN;
+	}
+	return 0;
+}
+
+static guint8 pppcp_process_terminate_request(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	return RTR;
+}
+
+static guint8 pppcp_process_terminate_ack(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	/*
+	 * if we wind up using the data field for anything, then
+	 * we'd want to check the identifier.
+	 * even if the identifiers don't match, we still handle
+	 * a terminate ack, as it is allowed to be unelicited
+	 */
+	return RTA;
+}
+
+static guint8 pppcp_process_code_reject(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	/*
+	 * determine if the code reject is catastrophic or not.
+	 * return RXJ_PLUS if this reject is acceptable, RXJ_MINUS if
+	 * it is catastrophic.
+	 */
+	return RXJ_MINUS;
+}
+
+static guint8 pppcp_process_protocol_reject(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	/*
+	 * determine if the protocol reject is catastrophic or not.
+	 * return RXJ_PLUS if this reject is acceptable, RXJ_MINUS if
+	 * it is catastrophic.
+	 */
+	return RXJ_MINUS;
+}
+
+static guint8 pppcp_process_echo_request(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	return RXR;
+}
+
+static guint8 pppcp_process_echo_reply(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	return 0;
+}
+
+static guint8 pppcp_process_discard_request(struct pppcp_data *data,
+					struct pppcp_packet *packet)
+{
+	return 0;
+}
+
+/*
+ * parse the packet and determine which event this packet caused
+ */
+void pppcp_process_packet(gpointer priv, guint8 *new_packet)
+{
+	struct pppcp_data *data = (struct pppcp_data *)priv;
+	struct pppcp_packet *packet = (struct pppcp_packet *)new_packet;
+	guint8 event_type;
+	gpointer event_data = NULL;
+	guint data_len = 0;
+
+	/* check flags to see if we support this code */
+	if (!(data->valid_codes & (1 << packet->code))) {
+		event_type = RUC;
+	} else
+		event_type = data->packet_ops[packet->code-1](data, packet);
+	if (event_type) {
+		data_len = ntohs(packet->length);
+		event_data = packet;
+		pppcp_generate_event(data, event_type, event_data, data_len);
+	}
+}
+
+void pppcp_set_valid_codes(struct pppcp_data *data, guint16 codes)
+{
+	data->valid_codes = codes;
+}
+
+void pppcp_free(struct pppcp_data *data)
+{
+	/* free event queue */
+	if (!g_queue_is_empty(data->event_queue))
+		g_queue_foreach(data->event_queue, (GFunc) g_free, NULL);
+	g_queue_free(data->event_queue);
+
+	/* remove all config options */
+	pppcp_clear_options(data);
+
+	/* free self */
+	g_free(data);
+}
+
+struct pppcp_data * pppcp_new(struct ppp_link *link, guint16 proto,
+				gpointer priv)
+{
+	struct pppcp_data *data;
+
+	data = g_malloc0(sizeof(struct pppcp_data));
+	if (!data)
+		return NULL;
+
+	data->state = INITIAL;
+	data->restart_interval = INITIAL_RESTART_TIMEOUT;
+	data->max_terminate = MAX_TERMINATE;
+	data->max_configure = MAX_CONFIGURE;
+	data->max_failure = MAX_FAILURE;
+	data->event_queue = g_queue_new();
+	data->identifier = 0;
+	data->link = link;
+	data->proto = proto;
+	data->priv = priv;
+
+	/* setup func ptrs for processing packet by pppcp code */
+	data->packet_ops[CONFIGURE_REQUEST-1] = pppcp_process_configure_request;
+	data->packet_ops[CONFIGURE_ACK-1] = pppcp_process_configure_ack;
+	data->packet_ops[CONFIGURE_NAK-1] = pppcp_process_configure_nak;
+	data->packet_ops[CONFIGURE_REJECT-1] = pppcp_process_configure_reject;
+	data->packet_ops[TERMINATE_REQUEST-1]= pppcp_process_terminate_request;
+	data->packet_ops[TERMINATE_ACK-1] = pppcp_process_terminate_ack;
+	data->packet_ops[CODE_REJECT-1] = pppcp_process_code_reject;
+	data->packet_ops[PROTOCOL_REJECT-1] = pppcp_process_protocol_reject;
+	data->packet_ops[ECHO_REQUEST-1] = pppcp_process_echo_request;
+	data->packet_ops[ECHO_REPLY-1] = pppcp_process_echo_reply;
+	data->packet_ops[DISCARD_REQUEST-1] = pppcp_process_discard_request;
+
+	/* setup func ptrs for handling events by event type */
+	data->event_ops[UP] = pppcp_up_event;
+	data->event_ops[DOWN] = pppcp_down_event;
+	data->event_ops[OPEN] = pppcp_open_event;
+	data->event_ops[CLOSE] = pppcp_close_event;
+	data->event_ops[TO_PLUS] = pppcp_to_plus_event;
+	data->event_ops[TO_MINUS] = pppcp_to_minus_event;
+	data->event_ops[RCR_PLUS] = pppcp_rcr_plus_event;
+	data->event_ops[RCR_MINUS] = pppcp_rcr_minus_event;
+	data->event_ops[RCA] = pppcp_rca_event;
+	data->event_ops[RCN] = pppcp_rcn_event;
+	data->event_ops[RTR] = pppcp_rtr_event;
+	data->event_ops[RTA] = pppcp_rta_event;
+	data->event_ops[RUC] = pppcp_ruc_event;
+	data->event_ops[RXJ_PLUS] = pppcp_rxj_plus_event;
+	data->event_ops[RXJ_MINUS] = pppcp_rxj_minus_event;
+	data->event_ops[RXR] = pppcp_rxr_event;
+
+	return data;
+}
Index: ofono/gatchat/gatpppcp.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatpppcp.h	2010-03-10 17:00:34.039103187 -0800
@@ -0,0 +1,133 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __PPPCP_H
+#define __PPPCP_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct pppcp_data;
+
+/* PPPCP CODE FIELD */
+enum PPPCP_CODE {
+	CONFIGURE_REQUEST = 1,
+	CONFIGURE_ACK,
+	CONFIGURE_NAK,
+	CONFIGURE_REJECT,
+	TERMINATE_REQUEST,
+	TERMINATE_ACK,
+	CODE_REJECT,
+	PROTOCOL_REJECT,
+	ECHO_REQUEST,
+	ECHO_REPLY,
+	DISCARD_REQUEST
+};
+
+enum PPPCP_EVENTS {
+	UP,
+	DOWN,
+	OPEN,
+	CLOSE,
+	TO_PLUS,
+	TO_MINUS,
+	RCR_PLUS,
+	RCR_MINUS,
+	RCA,
+	RCN,
+	RTR,
+	RTA,
+	RUC,
+	RXJ_PLUS,
+	RXJ_MINUS,
+	RXR,
+};
+
+/* option format */
+struct ppp_option {
+	guint8 type;
+	guint8 length;
+	guint8 data[0];
+};
+
+enum OPTION_RVAL {
+	OPTION_ACCEPT,
+	OPTION_REJECT,
+	OPTION_NAK,
+};
+
+struct pppcp_action {
+	void (*this_layer_up)(struct pppcp_data *data);
+	void (*this_layer_down)(struct pppcp_data *data);
+	void (*this_layer_started)(struct pppcp_data *data);
+	void (*this_layer_finished)(struct pppcp_data *data);
+	guint (*option_scan)(struct ppp_option *option, gpointer user);
+	void (*option_process)(gpointer option, gpointer user);
+};
+
+struct pppcp_packet {
+	guint8 code;
+	guint8 identifier;
+	guint16 length;
+	guint8 data[0];
+};
+
+struct pppcp_data {
+	gint state;
+	guint restart_timer;
+	guint restart_counter;
+	guint restart_interval;
+	guint max_terminate;
+	guint max_configure;
+	guint max_failure;
+	guint32 magic_number;
+	GQueue *event_queue;
+	GList *config_options;
+	GList *acceptable_options;
+	GList *unacceptable_options;
+	GList *rejected_options;
+	GList *applied_options;
+	struct ppp_link *link;
+	guint8 identifier;  /* don't think I need this now */
+	guint8 config_identifier;
+	guint8 terminate_identifier;
+	guint8 reject_identifier;
+	struct pppcp_action *action;
+	guint16 valid_codes;
+	guint8 (*packet_ops[11])(struct pppcp_data *data, struct pppcp_packet *packet);
+	void (*event_ops[16])(struct pppcp_data *data, guint8 *packet,
+				guint length);
+	gpointer priv;
+	guint16 proto;
+};
+
+struct pppcp_data * pppcp_new(struct ppp_link *link, guint16 proto, gpointer priv);
+void pppcp_free(struct pppcp_data *data);
+void pppcp_add_config_option(struct pppcp_data *data, struct ppp_option *option);
+void pppcp_set_valid_codes(struct pppcp_data *data, guint16 codes);
+void pppcp_generate_event(struct pppcp_data *data, guint event_type,
+				gpointer event_data, guint data_len);
+struct pppcp_event * pppcp_event_new(guint type, gpointer event_data, guint len);
+void pppcp_process_packet(gpointer priv, guint8 *new_packet);
+
+
+#endif
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- ofono.orig/gatchat/gatppp_internal.h	2010-03-10 16:58:12.039961039 -0800
+++ ofono/gatchat/gatppp_internal.h	2010-03-10 17:00:34.040101010 -0800
@@ -26,6 +26,8 @@
 extern "C" {
 #endif
 
+#include "gatpppcp.h"
+
 #define DEFAULT_MRU	1500
 #define BUFFERSZ	DEFAULT_MRU*2
 #define DEFAULT_ACCM	0x00000000

[patch 3/6] LCP support

[kristen]

[-- Attachment #1: Type: text/plain, Size: 9863 bytes --]

Implement LCP support for the PPP protocol

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-10 17:00:34.037087986 -0800
+++ ofono/Makefile.am	2010-03-10 17:01:56.687955124 -0800
@@ -56,9 +56,9 @@
 				gatchat/gatutil.h gatchat/gatutil.c \
 				gatchat/gat.h \
 				gatchat/gatserver.h gatchat/gatserver.c \
-				gatchat/gatppp.c gatchat/gatppp.h \
-				gatchat/gatppp_internal.h gatchat/gatpppcp.c
-				gatchat/gatpppcp.h
+				gatchat/gatppp_internal.h gatchat/gatpppcp.c \
+				gatchat/gatpppcp.h gatchat/gatppp.c \
+				gatchat/gatppplcp.c gatchat/gatppp.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp.c
===================================================================
--- ofono.orig/gatchat/gatppp.c	2010-03-10 16:58:12.039961039 -0800
+++ ofono/gatchat/gatppp.c	2010-03-10 17:01:56.687955124 -0800
@@ -352,22 +352,26 @@
 static void ppp_close(struct ppp_link *link)
 {
 	/* send a CLOSE event to the lcp layer */
+	lcp_close(link->lcp);
 }
 
 /* Administrative Open */
 void g_at_ppp_open(struct ppp_link *link)
 {
 	/* send an OPEN event to the lcp layer */
+	lcp_open(link->lcp);
 }
 
 static void ppp_link_establishment(struct ppp_link *link)
 {
 	/* signal UP event to LCP */
+	lcp_establish(link->lcp);
 }
 
 static void ppp_terminate(struct ppp_link *link)
 {
 	/* signal DOWN event to LCP */
+	lcp_terminate(link->lcp);
 }
 
 static void ppp_authenticate(struct ppp_link *link)
@@ -485,6 +489,9 @@
 	/* cleanup modem channel */
 	g_source_remove(link->modem_watch);
 	g_io_channel_unref(link->modem);
+
+	/* remove lcp */
+	lcp_free(link->lcp);
 }
 
 void g_at_ppp_ref(struct ppp_link *link)
@@ -503,6 +510,50 @@
 	}
 }
 
+void __ppp_set_auth(struct ppp_link *link, guint8* auth_data)
+{
+	guint16 proto = ntohs(*(guint16 *)auth_data);
+
+	switch(proto) {
+	case CHAP_PROTOCOL:
+		/* get the algorithm */
+		break;
+	default:
+		g_printerr("unknown authentication proto\n");
+		break;
+	}
+}
+
+void __ppp_set_recv_accm(struct ppp_link *link, guint32 accm)
+{
+	link->recv_accm = accm;
+}
+
+guint32 __ppp_get_xmit_accm(struct ppp_link *link)
+{
+	return link->xmit_accm[0];
+}
+
+void __ppp_set_pfc(struct ppp_link *link, gboolean pfc)
+{
+	link->pfc = pfc;
+}
+
+gboolean __ppp_get_pfc(struct ppp_link *link)
+{
+	return link->pfc;
+}
+
+void __ppp_set_acfc(struct ppp_link *link, gboolean acfc)
+{
+	link->acfc = acfc;
+}
+
+gboolean __ppp_get_acfc(struct ppp_link *link)
+{
+	return link->acfc;
+}
+
 struct ppp_link * g_at_ppp_new(GIOChannel *modem)
 {
 	struct ppp_link *link;
@@ -540,7 +591,7 @@
 	g_io_channel_set_buffered(modem, FALSE);
 
 	/* initialize the lcp state */
-
+	link->lcp = lcp_new(link);
 
 	/* initialize the autentication state */
 
Index: ofono/gatchat/gatppp.h
===================================================================
--- ofono.orig/gatchat/gatppp.h	2010-03-10 16:58:12.040979395 -0800
+++ ofono/gatchat/gatppp.h	2010-03-10 17:01:56.688955112 -0800
@@ -43,6 +43,7 @@
 struct ppp_link {
 	gint ref_count;
 	guint phase;
+	struct pppcp_data *lcp;
 	guint8 *buffer;
 	int index;
 	gint mru;
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- ofono.orig/gatchat/gatppp_internal.h	2010-03-10 17:00:34.040101010 -0800
+++ ofono/gatchat/gatppp_internal.h	2010-03-10 17:01:56.688955112 -0800
@@ -91,6 +91,12 @@
 gboolean __ppp_get_pfc(struct ppp_link *link);
 void __ppp_set_acfc(struct ppp_link *link, gboolean acfc);
 gboolean __ppp_get_acfc(struct ppp_link *link);
+struct pppcp_data * lcp_new(struct ppp_link *link);
+void lcp_free(struct pppcp_data *lcp);
+void lcp_open(struct pppcp_data *data);
+void lcp_close(struct pppcp_data *data);
+void lcp_establish(struct pppcp_data *data);
+void lcp_terminate(struct pppcp_data *data);
 #ifdef __cplusplus
 }
 #endif
Index: ofono/gatchat/gatppplcp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppplcp.c	2010-03-10 17:01:56.688955112 -0800
@@ -0,0 +1,225 @@
+/*
+ *
+ *  AT chat library with GLib integration
+ *
+ *  Copyright (C) 2008-2009  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <glib.h>
+#include <arpa/inet.h>
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+enum {
+	/* options */
+	RESERVED 	= 0,
+	MRU		= 1,
+	ACCM		= 2,
+	AUTH_PROTO	= 3,
+	QUAL_PROTO	= 4,
+	MAGIC_NUMBER	= 5,
+	DEPRECATED_QUAL_PROTO	= 6,
+	PFC			= 7,
+	ACFC			= 8,
+
+	LCP_SUPPORTED_CODES	= (1 << CONFIGURE_REQUEST) |
+				(1 << CONFIGURE_ACK) |
+				(1 << CONFIGURE_NAK) |
+				(1 << CONFIGURE_REJECT) |
+				(1 << TERMINATE_REQUEST) |
+				(1 << TERMINATE_ACK) |
+				(1 << CODE_REJECT) |
+				(1 << PROTOCOL_REJECT) |
+				(1 << ECHO_REQUEST) |
+				(1 << ECHO_REPLY) |
+				(1 << DISCARD_REQUEST),
+};
+
+/*
+ * signal the Up event to the NCP
+ */
+static void lcp_up(struct pppcp_data *pppcp)
+{
+	ppp_generate_event(pppcp->link, PPP_OPENED);
+}
+
+/*
+ * signal the Down event to the NCP
+ */
+static void lcp_down(struct pppcp_data *pppcp)
+{
+	ppp_generate_event(pppcp->link, PPP_DOWN);
+}
+
+/*
+ * Indicate that the lower layer is now needed
+ * Should trigger Up event
+ */
+static void lcp_started(struct pppcp_data *pppcp)
+{
+	ppp_generate_event(pppcp->link, PPP_UP);
+}
+
+/*
+ * Indicate that the lower layer is not needed
+ * Should trigger Down event
+ */
+static void lcp_finished(struct pppcp_data *pppcp)
+{
+	ppp_generate_event(pppcp->link, PPP_CLOSING);
+}
+
+/*
+ * Scan the option to see if it is acceptable, unacceptable, or rejected
+ */
+static guint lcp_option_scan(struct ppp_option *option, gpointer user)
+{
+	switch(option->type) {
+	case ACCM:
+	case AUTH_PROTO:
+		/* XXX check to make sure it's a proto we recognize */
+	case MAGIC_NUMBER:
+	case PFC:
+	case ACFC:
+		return OPTION_ACCEPT;
+	default:
+		g_print("Unknown lcp option type %d\n", option->type);
+		return OPTION_REJECT;
+	}
+}
+
+/*
+ * act on an acceptable option
+ */
+static void lcp_option_process(gpointer data, gpointer user)
+{
+	struct ppp_option *option = (struct ppp_option *)data;
+	struct pppcp_data *pppcp = (struct pppcp_data *)user;
+	struct ppp_link *link = pppcp->link;
+	guint32 *val32;
+	guint32 magic;
+
+	switch(option->type) {
+	case ACCM:
+		val32 = (guint32 *)option->data;
+		__ppp_set_recv_accm(link, ntohl(*val32));
+		break;
+	case AUTH_PROTO:
+		__ppp_set_auth(link, option->data);
+		break;
+	case MAGIC_NUMBER:
+		/* XXX handle loopback */
+		val32 = (guint32 *)option->data;
+		magic = ntohl(*val32);
+		if (magic != pppcp->magic_number)
+			pppcp->magic_number = magic;
+		else
+			g_print("looped back? I should do something\n");
+		break;
+	case PFC:
+		__ppp_set_pfc(link, TRUE);
+		break;
+	case ACFC:
+		__ppp_set_acfc(link, TRUE);
+		break;
+	default:
+		g_print("unhandled option %d\n", option->type);
+	}
+}
+
+struct ppp_packet_handler lcp_packet_handler = {
+	.proto = LCP_PROTOCOL,
+	.handler = pppcp_process_packet,
+};
+
+struct pppcp_action lcp_action = {
+	.this_layer_up =	lcp_up,
+	.this_layer_down = 	lcp_down,
+	.this_layer_started = 	lcp_started,
+	.this_layer_finished =	lcp_finished,
+	.option_scan = 		lcp_option_scan,
+	.option_process = 	lcp_option_process,
+};
+
+void lcp_open(struct pppcp_data *data)
+{
+	/* send an open event to the lcp layer */
+	pppcp_generate_event(data, OPEN, NULL, 0);
+}
+
+void lcp_close(struct pppcp_data *data)
+{
+	/* send a CLOSE  event to the lcp layer */
+	pppcp_generate_event(data, CLOSE, NULL, 0);
+}
+
+void lcp_establish(struct pppcp_data *data)
+{
+	/* send an UP event to the lcp layer */
+	pppcp_generate_event(data, UP, NULL, 0);
+}
+
+void lcp_terminate(struct pppcp_data *data)
+{
+	/* send a DOWN event to the lcp layer */
+	pppcp_generate_event(data, DOWN, NULL, 0);
+}
+
+void lcp_free(struct pppcp_data *lcp)
+{
+	/* TBD unregister packet handler */
+
+	pppcp_free(lcp);
+}
+
+struct pppcp_data * lcp_new(struct ppp_link *link)
+{
+	struct pppcp_data *pppcp;
+	struct ppp_option *option;
+	guint16 codes = LCP_SUPPORTED_CODES;
+
+	pppcp = pppcp_new(link, LCP_PROTOCOL, NULL);
+	if (!pppcp) {
+		g_print("Failed to allocate PPPCP struct\n");
+		return NULL;
+	}
+	pppcp_set_valid_codes(pppcp, codes);
+	pppcp->priv = pppcp;
+
+	/* set the actions */
+	pppcp->action = &lcp_action;
+
+	/* add the default config options */
+	option = g_malloc0(6);
+	option->type = ACCM;
+	option->length= 6;
+	pppcp_add_config_option(pppcp, option);
+
+	/* register packet handler for LCP protocol */
+	lcp_packet_handler.priv = pppcp;
+	ppp_register_packet_handler(&lcp_packet_handler);
+	return pppcp;
+}

[patch 4/6] CHAP with MD5 authentication

[kristen]

[-- Attachment #1: Type: text/plain, Size: 9623 bytes --]

Implement authentication support with CHAP and MD5 digest
Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-10 17:01:56.687955124 -0800
+++ ofono/Makefile.am	2010-03-10 17:03:01.695955140 -0800
@@ -58,7 +58,8 @@
 				gatchat/gatserver.h gatchat/gatserver.c \
 				gatchat/gatppp_internal.h gatchat/gatpppcp.c \
 				gatchat/gatpppcp.h gatchat/gatppp.c \
-				gatchat/gatppplcp.c gatchat/gatppp.h
+				gatchat/gatppplcp.c gatchat/gatpppauth.c \
+				gatchat/gatppp.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp.c
===================================================================
--- ofono.orig/gatchat/gatppp.c	2010-03-10 17:01:56.687955124 -0800
+++ ofono/gatchat/gatppp.c	2010-03-10 17:03:01.696955547 -0800
@@ -390,6 +390,7 @@
 static void ppp_network(struct ppp_link *link)
 {
 	/* bring network phase up */
+	net_open(link->net);
 }
 
 static void ppp_transition_phase(struct ppp_link *link, guint phase)
@@ -492,6 +493,9 @@
 
 	/* remove lcp */
 	lcp_free(link->lcp);
+
+	/* remove auth */
+	auth_free(link->auth);
 }
 
 void g_at_ppp_ref(struct ppp_link *link)
@@ -510,6 +514,12 @@
 	}
 }
 
+void g_at_ppp_set_credentials(struct ppp_link *link, gchar *username,
+				gchar *passwd)
+{
+	auth_set_credentials(link->auth, username, passwd);
+}
+
 void __ppp_set_auth(struct ppp_link *link, guint8* auth_data)
 {
 	guint16 proto = ntohs(*(guint16 *)auth_data);
@@ -517,6 +527,7 @@
 	switch(proto) {
 	case CHAP_PROTOCOL:
 		/* get the algorithm */
+		auth_set_proto(link->auth, proto, auth_data[2]);
 		break;
 	default:
 		g_printerr("unknown authentication proto\n");
@@ -594,9 +605,10 @@
 	link->lcp = lcp_new(link);
 
 	/* initialize the autentication state */
-
+	link->auth = auth_new(link);
 
 	/* intialize the network state */
+	link->net = net_new(link);
 
 	/* start listening for packets from the modem */
 	signal_source = g_io_add_watch(modem,
Index: ofono/gatchat/gatppp.h
===================================================================
--- ofono.orig/gatchat/gatppp.h	2010-03-10 17:01:56.688955112 -0800
+++ ofono/gatchat/gatppp.h	2010-03-10 17:03:01.696955547 -0800
@@ -27,6 +27,7 @@
 #endif
 
 struct ppp_link;
+struct auth_data;
 
 enum PPP_CONNECT_STATUS {
 	PPP_CONNECT_SUCCESS,
@@ -44,6 +45,7 @@
 	gint ref_count;
 	guint phase;
 	struct pppcp_data *lcp;
+	struct auth_data *auth;
 	guint8 *buffer;
 	int index;
 	gint mru;
@@ -74,6 +76,8 @@
 void g_at_ppp_shutdown(struct ppp_link *link);
 void g_at_ppp_ref(struct ppp_link *link);
 void g_at_ppp_unref(struct ppp_link *link);
+void g_at_ppp_set_credentials(struct ppp_link *link, gchar *username,
+				gchar *passwd);
 #ifdef __cplusplus
 }
 #endif
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- ofono.orig/gatchat/gatppp_internal.h	2010-03-10 17:01:56.688955112 -0800
+++ ofono/gatchat/gatppp_internal.h	2010-03-10 17:03:01.697955395 -0800
@@ -66,6 +66,15 @@
 	gpointer priv;
 };
 
+struct auth_data {
+	guint16 proto;
+	gpointer proto_data;
+	void (*process_packet)(struct auth_data *data, guint8 *packet);
+	struct ppp_link *link;
+	gchar *username;
+	gchar *passwd;
+};
+
 #define ppp_generate_event(l, e) \
 	__ppp_generate_event(l, e)
 
@@ -97,6 +106,11 @@
 void lcp_close(struct pppcp_data *data);
 void lcp_establish(struct pppcp_data *data);
 void lcp_terminate(struct pppcp_data *data);
+void auth_set_credentials(struct auth_data *data, gchar *username,
+				gchar *passwd);
+void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);
+struct auth_data *auth_new(struct ppp_link *link);
+void auth_free(struct auth_data *auth);
 #ifdef __cplusplus
 }
 #endif
Index: ofono/gatchat/gatpppauth.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatpppauth.c	2010-03-10 17:03:01.697955395 -0800
@@ -0,0 +1,219 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+
+#include <glib.h>
+
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+struct chap_header {
+	guint8 code;
+	guint8 identifier;
+	guint16 length;
+	guint8 data[0];
+};
+
+struct chap_data {
+	guint8 method;
+	struct auth_data *auth;
+};
+
+enum CHAP_CODE {
+	CHALLENGE=1,
+	RESPONSE,
+	SUCCESS,
+	FAILURE
+};
+
+void auth_set_credentials(struct auth_data *data, gchar *username,
+				gchar *passwd)
+{
+	if (data->username)
+		g_free(data->username);
+	if (data->passwd)
+		g_free(data->passwd);
+
+	data->username = g_strdup(username);
+	data->passwd = g_strdup(passwd);
+}
+
+static void chap_process_challenge(struct auth_data *auth, guint8 *packet)
+{
+	struct chap_header *header = (struct chap_header *)packet;
+	struct chap_header *response;
+	struct chap_data *data = auth->proto_data;
+	GChecksum *checksum;
+	gchar *secret = data->auth->passwd;
+	guint16 response_length;
+	guint8 *ppp_packet;
+	gsize digest_len;
+
+	/* create a checksum over id, secret, and challenge */
+	checksum = g_checksum_new(data->method);
+	if (!checksum)
+		return;
+	g_checksum_update(checksum, &header->identifier, 1);
+	g_checksum_update(checksum, (guchar *)secret, strlen(secret));
+	g_checksum_update(checksum, &header->data[1], header->data[0]);
+
+	/* transmit a response packet */
+	/*
+	 * allocate space for the header, the checksum, and the ppp header,
+	 * and the value size byte
+	 */
+	digest_len = g_checksum_type_get_length(data->method);
+	response_length = digest_len + sizeof(*header) + 1;
+	ppp_packet = g_malloc0(response_length + 2);
+	if (!ppp_packet)
+		goto challenge_out;
+
+	/* add our protocol information */
+	*((guint16 *)ppp_packet) = htons(CHAP_PROTOCOL);
+	response = (struct chap_header *)(ppp_packet+2);
+	if (response) {
+		response->code = RESPONSE;
+		response->identifier = header->identifier;
+		response->length = htons(response_length);
+		response->data[0] = digest_len;
+		g_checksum_get_digest(checksum, &response->data[1],
+					(gsize *)&response->data[0]);
+		/* leave the name empty? */
+	}
+
+	/* transmit the packet */
+	ppp_transmit(auth->link, ppp_packet, response_length);
+
+challenge_out:
+	g_checksum_free(checksum);
+}
+
+static void chap_process_success(struct auth_data *data, guint8 *packet)
+{
+	ppp_generate_event(data->link, PPP_SUCCESS);
+}
+
+static void chap_process_failure(struct auth_data *data, guint8 *packet)
+{
+	struct chap_header *header = (struct chap_header *)packet;
+
+	g_print("Failed to authenticate, message %s\n", header->data);
+}
+
+/*
+ * parse the packet
+ */
+static void chap_process_packet(gpointer priv, guint8 *new_packet)
+{
+	struct auth_data *data = (struct auth_data *)priv;
+	guint8 code = new_packet[0];
+
+	switch(code) {
+	case CHALLENGE:
+		chap_process_challenge(data, new_packet);
+		break;
+	case RESPONSE:
+		g_print("Oops, received RESPONSE, but I've not implemented\n");
+		break;
+	case SUCCESS:
+		chap_process_success(data, new_packet);
+		break;
+	case FAILURE:
+		chap_process_failure(data, new_packet);
+		break;
+	default:
+		g_print("unknown auth code\n");
+		break;
+	}
+}
+
+struct ppp_packet_handler chap_packet_handler = {
+	.proto = CHAP_PROTOCOL,
+	.handler = chap_process_packet,
+};
+
+static void chap_free(struct auth_data *auth)
+{
+	/* TBD unregister protocol handler */
+
+	g_free(auth->proto_data);
+}
+
+static struct chap_data *chap_new(struct auth_data *auth, guint8 method)
+{
+	struct chap_data *data;
+
+	data = g_malloc0(sizeof(*data));
+
+	if (data) {
+		data->auth = auth;
+		switch(method) {
+		case MD5:
+			data->method = G_CHECKSUM_MD5;
+			break;
+		default:
+			g_print("Unknown method\n");
+		}
+
+		/* register packet handler for CHAP protocol */
+		chap_packet_handler.priv = auth;
+		ppp_register_packet_handler(&chap_packet_handler);
+	}
+	return data;
+}
+
+void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method)
+{
+	switch(proto) {
+	case CHAP_PROTOCOL:
+		data->proto_data = (gpointer) chap_new(data, method);
+		break;
+	default:
+		g_print("Unknown auth protocol 0x%x\n", proto);
+	}
+}
+
+void auth_free(struct auth_data *data)
+{
+	chap_free(data);
+	g_free(data);
+}
+
+struct auth_data *auth_new(struct ppp_link *link)
+{
+	struct auth_data *data;
+
+	data = g_malloc0(sizeof(*data));
+	if (data)
+		data->link = link;
+	return data;
+}
+

[patch 5/6] IP support for PPP

[kristen]

[-- Attachment #1: Type: text/plain, Size: 11497 bytes --]

Implement IPCP support.  Creates a tun interface to pass IP traffic.

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-10 17:03:01.695955140 -0800
+++ ofono/Makefile.am	2010-03-10 17:04:30.034088637 -0800
@@ -59,7 +59,7 @@
 				gatchat/gatppp_internal.h gatchat/gatpppcp.c \
 				gatchat/gatpppcp.h gatchat/gatppp.c \
 				gatchat/gatppplcp.c gatchat/gatpppauth.c \
-				gatchat/gatppp.h
+				gatchat/gatpppnet.c gatchat/gatppp.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- ofono.orig/gatchat/gatppp_internal.h	2010-03-10 17:03:01.697955395 -0800
+++ ofono/gatchat/gatppp_internal.h	2010-03-10 17:04:30.034088637 -0800
@@ -66,6 +66,13 @@
 	gpointer priv;
 };
 
+struct net_data {
+	struct ppp_link *link;
+	char *if_name;
+	GIOChannel *channel;
+	struct pppcp_data *ipcp;
+};
+
 struct auth_data {
 	guint16 proto;
 	gpointer proto_data;
@@ -111,6 +118,10 @@
 void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);
 struct auth_data *auth_new(struct ppp_link *link);
 void auth_free(struct auth_data *auth);
+struct net_data *net_new(struct ppp_link *link);
+void net_open(struct net_data *data);
+void net_free(struct net_data *data);
+void net_close(struct net_data *data);
 #ifdef __cplusplus
 }
 #endif
Index: ofono/gatchat/gatpppnet.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatpppnet.c	2010-03-10 17:04:30.034088637 -0800
@@ -0,0 +1,355 @@
+/*
+ *
+ *  AT chat library with GLib integration
+ *
+ *  Copyright (C) 2008-2009  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <linux/if_tun.h>
+#include <sys/ioctl.h>
+#include <glib.h>
+
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+static void ipcp_free(struct pppcp_data *data);
+
+/* XXX should be maximum IP Packet size */
+#define MAX_PACKET 1500
+#define PPP_IP_PROTO	0x0021
+
+struct ipcp_data {
+	guint8 ip_address[4];
+	guint8 primary_dns[4];
+	guint8 secondary_dns[4];
+	struct pppcp_data *pppcp;
+};
+
+static struct pppcp_data * ipcp_new(struct ppp_link *link);
+static void ipcp_option_process(gpointer data, gpointer user);
+static guint ipcp_option_scan(struct ppp_option *option, gpointer user);
+
+static void ip_process_packet(gpointer priv, guint8 *packet)
+{
+	struct net_data *data = (struct net_data *)priv;
+	GError *error = NULL;
+        GIOStatus status;
+        gsize bytes_written;
+	guint16 len;
+
+	/*
+	 * since net_open can fail, we need to make sure
+	 * channel is valid
+	 */
+	if (data->channel == NULL)
+		return;
+
+	/* find the length of the packet to transmit */
+	len = ntohs(*(guint16 *)&packet[2]);
+	status = g_io_channel_write_chars(data->channel, (gchar *) packet,
+                                          len, &bytes_written, &error);
+}
+
+/*
+ * packets received by the tun interface need to be written to
+ * the modem.  So, just read a packet, write out to the modem
+ *
+ * TBD - how do we know we have a full packet?  Do we care?
+ */
+static gboolean net_callback(GIOChannel *channel, GIOCondition cond,
+				gpointer userdata)
+{
+	GIOStatus status;
+	gchar buf[MAX_PACKET+2];
+	gsize bytes_read;
+	GError *error = NULL;
+	guint16 *proto = (guint16 *)buf;
+	struct net_data *data = (struct net_data *) userdata;
+
+	if (cond & G_IO_IN) {
+		/* leave space to add PPP protocol field */
+		status = g_io_channel_read_chars(channel, buf+2, MAX_PACKET,
+				&bytes_read, &error);
+		if (bytes_read > 0) {
+			/* I assume we are pointing to the protocol? */
+			*proto = htons(PPP_IP_PROTO);
+			ppp_transmit(data->link, (guint8 *)buf, bytes_read);
+		}
+		if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+			return FALSE;
+	}
+	return TRUE;
+}
+
+void net_close(struct net_data *data)
+{
+	/* Not Implemented Yet */
+}
+
+void net_open(struct net_data *data)
+{
+	int fd;
+	struct ifreq ifr;
+	GIOChannel *channel;
+	GIOFlags flags;
+	int signal_source;
+	int err;
+
+	/* open a tun interface */
+	fd = open("/dev/net/tun", O_RDWR);
+	if (fd < 0) {
+		g_printerr("error opening tun\n");
+		return;
+	}
+
+	memset(&ifr, 0, sizeof(ifr));
+	ifr.ifr_flags = IFF_TUN | IFF_NO_PI;
+	err = ioctl(fd, TUNSETIFF, (void *)&ifr);
+	if (err < 0) {
+		g_printerr("error %d setting ifr\n", err);
+		close(fd);
+		return;
+	}
+	data->if_name = strdup(ifr.ifr_name);
+
+	/* create a channel for reading and writing to this interface */
+	channel = g_io_channel_unix_new(fd);
+	if (!channel) {
+		g_printerr("Error creating I/O Channel to TUN device\n");
+		close(fd);
+		return;
+	}
+	data->channel = channel;
+	flags = g_io_channel_get_flags(channel) | G_IO_FLAG_NONBLOCK;
+	g_io_channel_set_flags(channel, flags, NULL);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+        signal_source = g_io_add_watch(channel,
+                        G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+                        net_callback, (gpointer) data);
+
+	pppcp_generate_event(data->ipcp, OPEN, NULL, 0);
+
+}
+
+struct ppp_packet_handler ip_packet_handler = {
+	.proto = PPP_IP_PROTO,
+	.handler = ip_process_packet,
+};
+
+void net_free(struct net_data *data)
+{
+	/* TBD unregister packet handler */
+
+	/* cleanup tun interface */
+	net_close(data);
+
+	/* free ipcp data */
+	ipcp_free(data->ipcp);
+
+	/* free self */
+	g_free(data);
+}
+
+struct net_data *net_new(struct ppp_link *link)
+{
+	struct net_data *data;
+
+	data = g_malloc0(sizeof(*data));
+	if (data) {
+		data->link = link;
+		data->ipcp = ipcp_new(link);
+
+		/* register packet handler for IP protocol */
+		ip_packet_handler.priv = data;
+		ppp_register_packet_handler(&ip_packet_handler);
+	}
+	return data;
+}
+
+/****** IPCP support ****************/
+enum {
+	/* supported codes */
+	IPCP_SUPPORTED_CODES	= (1 << CONFIGURE_REQUEST) |
+				  (1 << CONFIGURE_ACK) |
+				  (1 << CONFIGURE_NAK) |
+				  (1 << CONFIGURE_REJECT) |
+				  (1 << TERMINATE_REQUEST) |
+				  (1 << TERMINATE_ACK) |
+				  (1 << CODE_REJECT),
+
+	IPCP_PROTO		= 0x8021,
+
+	/* option types */
+	IP_ADDRESSES		= 1,
+	IP_COMPRESSION_PROTO	= 2,
+	IP_ADDRESS		= 3,
+	PRIMARY_DNS_SERVER	= 129,
+	SECONDARY_DNS_SERVER	= 131,
+};
+
+static guint32 bytes_to_32(guint8 *bytes)
+{
+	return (bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) |
+			(bytes[3] << 24));
+}
+
+static void ipcp_up(struct pppcp_data *pppcp)
+{
+	struct ipcp_data *data = (struct ipcp_data *)pppcp->priv;
+	struct ppp_link *link = pppcp->link;
+
+	/* call the connect function */
+	if (link->connect_cb)
+		link->connect_cb(link, PPP_CONNECT_SUCCESS,
+				bytes_to_32(data->ip_address),
+				bytes_to_32(data->primary_dns),
+				bytes_to_32(data->secondary_dns),
+				link->connect_priv);
+}
+
+static void ipcp_down(struct pppcp_data *data)
+{
+	g_print("ipcp down\n");
+
+	/* re-add what default config options we want negotiated */
+}
+
+/*
+ * Tell the protocol to start the handshake
+ */
+static void ipcp_started(struct pppcp_data *data)
+{
+	pppcp_generate_event(data, UP, NULL, 0);
+}
+
+static void ipcp_finished(struct pppcp_data *data)
+{
+	g_print("ipcp finished\n");
+}
+
+struct pppcp_action ipcp_action = {
+	.this_layer_up =	ipcp_up,
+	.this_layer_down = 	ipcp_down,
+	.this_layer_started = 	ipcp_started,
+	.this_layer_finished =	ipcp_finished,
+	.option_scan = 		ipcp_option_scan,
+	.option_process = 	ipcp_option_process,
+};
+
+struct ppp_packet_handler ipcp_packet_handler = {
+	.proto = IPCP_PROTO,
+	.handler = pppcp_process_packet,
+};
+
+/*
+ * Scan the option to see if it is acceptable, unacceptable, or rejected
+ */
+static guint ipcp_option_scan(struct ppp_option *option, gpointer user)
+{
+	switch(option->type) {
+	case IP_ADDRESS:
+	case PRIMARY_DNS_SERVER:
+	case SECONDARY_DNS_SERVER:
+		return OPTION_ACCEPT;
+	default:
+		g_printerr("Unknown ipcp option type %d\n", option->type);
+		return OPTION_REJECT;
+	}
+}
+
+/*
+ * act on an acceptable option
+ */
+static void ipcp_option_process(gpointer data, gpointer user)
+{
+	struct ppp_option *option = (struct ppp_option *)data;
+	struct ipcp_data *ipcp = (struct ipcp_data *)user;
+
+	switch(option->type) {
+	case IP_ADDRESS:
+		memcpy(ipcp->ip_address, option->data, 4);
+		break;
+	case PRIMARY_DNS_SERVER:
+		memcpy(ipcp->primary_dns, option->data, 4);
+		break;
+	case SECONDARY_DNS_SERVER:
+		memcpy(ipcp->secondary_dns, option->data, 4);
+		break;
+	default:
+		g_printerr("Unable to process unknown option %d\n", option->type);
+		break;
+	}
+}
+
+static void ipcp_free(struct pppcp_data *data)
+{
+	struct ipcp_data *ipcp = data->priv;
+
+	/* TBD unregister IPCP packet handler */
+
+	/* free ipcp */
+	g_free(ipcp);
+
+	/* free pppcp */
+	pppcp_free(data);
+}
+
+static struct pppcp_data * ipcp_new(struct ppp_link *link)
+{
+	struct ipcp_data *data;
+	struct pppcp_data *pppcp;
+	struct ppp_option *ipcp_option;
+
+	data = g_malloc0(sizeof(*data));
+	if (!data)
+		return NULL;
+
+	pppcp = pppcp_new(link, IPCP_PROTO, data);
+	if (!pppcp) {
+		g_printerr("Failed to allocate PPPCP struct\n");
+		g_free(data);
+		return NULL;
+	}
+	pppcp_set_valid_codes(pppcp, IPCP_SUPPORTED_CODES);
+	pppcp->priv = data;
+
+	/* set the actions */
+	pppcp->action = &ipcp_action;
+
+	/* add the default config options */
+	ipcp_option = g_malloc0(6);
+	ipcp_option->type = IP_ADDRESS;
+	ipcp_option->length= 6;
+	pppcp_add_config_option(pppcp, ipcp_option);
+
+	/* register packet handler for IPCP protocol */
+	ipcp_packet_handler.priv = pppcp;
+	ppp_register_packet_handler(&ipcp_packet_handler);
+	return pppcp;
+}
Index: ofono/gatchat/gatppp.h
===================================================================
--- ofono.orig/gatchat/gatppp.h	2010-03-10 17:03:01.696955547 -0800
+++ ofono/gatchat/gatppp.h	2010-03-10 17:04:30.035099659 -0800
@@ -28,6 +28,7 @@
 
 struct ppp_link;
 struct auth_data;
+struct net_data;
 
 enum PPP_CONNECT_STATUS {
 	PPP_CONNECT_SUCCESS,
@@ -46,6 +47,7 @@
 	guint phase;
 	struct pppcp_data *lcp;
 	struct auth_data *auth;
+	struct net_data *net;
 	guint8 *buffer;
 	int index;
 	gint mru;

Re: [patch 5/6] IP support for PPP

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 4878 bytes --]

Hi Kristen,

> Implement IPCP support.  Creates a tun interface to pass IP traffic.

this is not a proper review of the patch, just some general comments I
found by looking through the whole series.

> ===================================================================
> --- ofono.orig/gatchat/gatppp_internal.h	2010-03-10 17:03:01.697955395 -0800
> +++ ofono/gatchat/gatppp_internal.h	2010-03-10 17:04:30.034088637 -0800
> @@ -66,6 +66,13 @@
>  	gpointer priv;
>  };
>  
> +struct net_data {
> +	struct ppp_link *link;
> +	char *if_name;
> +	GIOChannel *channel;
> +	struct pppcp_data *ipcp;
> +};
> +
>  struct auth_data {
>  	guint16 proto;
>  	gpointer proto_data;
> @@ -111,6 +118,10 @@
>  void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);
>  struct auth_data *auth_new(struct ppp_link *link);
>  void auth_free(struct auth_data *auth);
> +struct net_data *net_new(struct ppp_link *link);
> +void net_open(struct net_data *data);
> +void net_free(struct net_data *data);
> +void net_close(struct net_data *data);
>  #ifdef __cplusplus
>  }
>  #endif

If these are not public exported header files, then I don't want the
cplusplus or or circular inclusion protection ifdefs. I want developers
fall flat on their face if they try to do something stupid.

So first we will never compiler internal header files with C++ so that
is just pointless. They are internal and we will compile them with C
compiler.

For the circular inclusion protection, I want the compiler to complain.
Quite frankly if you need that for internal header files to make your
code work. You code is way too complex. I want the compiler to tell us.

> +static void ip_process_packet(gpointer priv, guint8 *packet)
> +{
> +	struct net_data *data = (struct net_data *)priv;

This assignment should not be needed. It is a void pointer and can be
assigned without casting.

As a general rule. Avoid casting as much as possible. Especially for
void pointer and function pointers. I want the compiler to do sanity
checks for us. Once you cast, all this extra protection is gone.

There is a reason for using -Wall -Wextra and -Werror and being pretty
much pedantic about it. Developers should use ./bootstrap-configure
since it enables all of these and more.

> +	GError *error = NULL;
> +        GIOStatus status;
> +        gsize bytes_written;
> +	guint16 len;

Please use an editor that can visualize whitespaces vs tabs.

> +struct net_data *net_new(struct ppp_link *link)
> +{
> +	struct net_data *data;
> +
> +	data = g_malloc0(sizeof(*data));
> +	if (data) {
> +		data->link = link;
> +		data->ipcp = ipcp_new(link);

So here is the tricky part with GLib memory allocations. The default
function will abort when OOM case happens. What you really wanted here
is actually g_try_malloc0.

And then again, please do it like this:

	data = g_try_malloc0(...)
	if (!data)
		return NULL;

	data->link = ...

> +/****** IPCP support ****************/
> +enum {
> +	/* supported codes */
> +	IPCP_SUPPORTED_CODES	= (1 << CONFIGURE_REQUEST) |
> +				  (1 << CONFIGURE_ACK) |
> +				  (1 << CONFIGURE_NAK) |
> +				  (1 << CONFIGURE_REJECT) |
> +				  (1 << TERMINATE_REQUEST) |
> +				  (1 << TERMINATE_ACK) |
> +				  (1 << CODE_REJECT),
> +
> +	IPCP_PROTO		= 0x8021,
> +
> +	/* option types */
> +	IP_ADDRESSES		= 1,
> +	IP_COMPRESSION_PROTO	= 2,
> +	IP_ADDRESS		= 3,
> +	PRIMARY_DNS_SERVER	= 129,
> +	SECONDARY_DNS_SERVER	= 131,
> +};

I don't think enum is the right way for this. I can see it for the
option types, but for IPCP_PROTO and IPCP_SUPPORTED_CODES. I would say
just using simple defines is way better.

Feel free to convince me other way or show me what I have missed here.

> +static guint32 bytes_to_32(guint8 *bytes)
> +{
> +	return (bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) |
> +			(bytes[3] << 24));
> +}

Shouldn't this be done by an endian aware function that can also handle
the unaligned case. I mentioned this before, on a non-x86 platform we
need to worry about the alignment if you just access the memory location
directly.

GLib should have proper functions for this. Just install the GLib
documentation package and devhelp program. That way can easily search
through it. Even when you are offline.

> ===================================================================
> --- ofono.orig/gatchat/gatppp.h	2010-03-10 17:03:01.696955547 -0800
> +++ ofono/gatchat/gatppp.h	2010-03-10 17:04:30.035099659 -0800
> @@ -28,6 +28,7 @@
>  
>  struct ppp_link;
>  struct auth_data;
> +struct net_data;

Denis and I mentioned it already, please keep proper namespacing for
public functions or structures in mind. It is really important.

Also here it might be not needed to expose them at all.

Regards

Marcel

Re: [patch 5/6] IP support for PPP

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On Sun, 14 Mar 2010 13:22:37 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:
> > +/****** IPCP support ****************/
> > +enum {
> > +	/* supported codes */
> > +	IPCP_SUPPORTED_CODES	= (1 << CONFIGURE_REQUEST) |
> > +				  (1 << CONFIGURE_ACK) |
> > +				  (1 << CONFIGURE_NAK) |
> > +				  (1 << CONFIGURE_REJECT) |
> > +				  (1 << TERMINATE_REQUEST) |
> > +				  (1 << TERMINATE_ACK) |
> > +				  (1 << CODE_REJECT),
> > +
> > +	IPCP_PROTO		= 0x8021,
> > +
> > +	/* option types */
> > +	IP_ADDRESSES		= 1,
> > +	IP_COMPRESSION_PROTO	= 2,
> > +	IP_ADDRESS		= 3,
> > +	PRIMARY_DNS_SERVER	= 129,
> > +	SECONDARY_DNS_SERVER	= 131,
> > +};
> 
> I don't think enum is the right way for this. I can see it for the
> option types, but for IPCP_PROTO and IPCP_SUPPORTED_CODES. I would say
> just using simple defines is way better.
> 
> Feel free to convince me other way or show me what I have missed here.

It's mostly just a habit, but in general there are advantages to
using enum vs. a define.  You are assured that the scope is kept local,
even if you are uncreative with your name choices, whereas with a macro 
it is not.  Also it's sometimes easier to debug with an enum vs. a macro.  
I can certainly change it if you really want me too.

Re: [patch 5/6] IP support for PPP

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

Hi Kristen,

> > > +/****** IPCP support ****************/
> > > +enum {
> > > +	/* supported codes */
> > > +	IPCP_SUPPORTED_CODES	= (1 << CONFIGURE_REQUEST) |
> > > +				  (1 << CONFIGURE_ACK) |
> > > +				  (1 << CONFIGURE_NAK) |
> > > +				  (1 << CONFIGURE_REJECT) |
> > > +				  (1 << TERMINATE_REQUEST) |
> > > +				  (1 << TERMINATE_ACK) |
> > > +				  (1 << CODE_REJECT),
> > > +
> > > +	IPCP_PROTO		= 0x8021,
> > > +
> > > +	/* option types */
> > > +	IP_ADDRESSES		= 1,
> > > +	IP_COMPRESSION_PROTO	= 2,
> > > +	IP_ADDRESS		= 3,
> > > +	PRIMARY_DNS_SERVER	= 129,
> > > +	SECONDARY_DNS_SERVER	= 131,
> > > +};
> > 
> > I don't think enum is the right way for this. I can see it for the
> > option types, but for IPCP_PROTO and IPCP_SUPPORTED_CODES. I would say
> > just using simple defines is way better.
> > 
> > Feel free to convince me other way or show me what I have missed here.
> 
> It's mostly just a habit, but in general there are advantages to
> using enum vs. a define.  You are assured that the scope is kept local,
> even if you are uncreative with your name choices, whereas with a macro 
> it is not.  Also it's sometimes easier to debug with an enum vs. a macro.  
> I can certainly change it if you really want me too.

I have nothing against enums. I actually like them a lot if used
correctly, because gcc and gdb does make your life easier.

The thing that I dislike is clashing namespaces and scopes in just one
enum. In the example of both you mix two or more things that should at
least in different enums.

Regards

Marcel

[patch 6/6] Allow gsmdial to use gatchat ppp support

[kristen]

[-- Attachment #1: Type: text/plain, Size: 3730 bytes --]

Add option to use PPP to gsmdial.

Index: ofono/gatchat/gsmdial.c
===================================================================
--- ofono.orig/gatchat/gsmdial.c	2010-03-10 16:58:09.773080389 -0800
+++ ofono/gatchat/gsmdial.c	2010-03-10 17:06:45.071975512 -0800
@@ -33,6 +33,9 @@
 #include <glib.h>
 #include <gatchat.h>
 #include <gattty.h>
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <gatppp.h>
 
 static const char *none_prefix[] = { NULL };
 static const char *cgreg_prefix[] = { "+CGREG:", NULL };
@@ -45,10 +48,14 @@
 static gchar *option_apn = NULL;
 static gint option_offmode = 0;
 static gboolean option_legacy = FALSE;
+static gboolean option_ppp = FALSE;
+static gchar *option_username = NULL;
+static gchar *option_passwd = NULL;
 
 static GAtChat *control;
 static GAtChat *modem;
 static GMainLoop *event_loop;
+static struct ppp_link *ppp;
 
 enum state {
 	STATE_NONE = 0,
@@ -220,6 +227,70 @@
 	g_at_chat_send(modem, buf, none_prefix, NULL, NULL, NULL);
 }
 
+static void print_ip_address(guint32 ip_addr)
+{
+	struct in_addr addr;
+	addr.s_addr = ip_addr;
+	g_print("%s\n", inet_ntoa(addr));
+}
+
+static void ppp_connect(struct ppp_link *link, gint success, guint32 ip_addr,
+		 guint32 dns1, guint32 dns2, gpointer user_data)
+{
+	if (success == PPP_CONNECT_SUCCESS) {
+		/* print out the negotiated address and dns server */
+		g_print("Negotiated IP Address: ");
+		print_ip_address(ip_addr);
+
+		g_print("Primary DNS Server: ");
+		print_ip_address(dns1);
+
+		g_print("Secondary DNS Server: ");
+		print_ip_address(dns2);
+	} else {
+		g_print("Failed to create PPP interface!\n");
+	}
+}
+
+static void ppp_disconnect(struct ppp_link *link, gpointer user_data)
+{
+	g_print("PPP Link down\n");
+}
+
+static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	GIOChannel *channel;
+
+	if (!ok) {
+		g_print("Unable to define context\n");
+		exit(1);
+	}
+
+	/* get the data IO channel */
+	channel = g_at_chat_get_channel(modem);
+	channel = g_io_channel_ref(channel);
+
+	/* shutdown gatchat */
+	g_at_chat_unref(control);
+	g_at_chat_unref(modem);
+
+	/* open ppp */
+	ppp = g_at_ppp_new(channel);
+	if (ppp) {
+		g_print("Setting PPP Credentials to %s, %s\n", option_username,
+				option_passwd);
+		g_at_ppp_set_credentials(ppp, option_username,
+					option_passwd);
+
+		/* set connect and disconnect callbacks */
+		g_at_ppp_set_connect_function(ppp, ppp_connect, NULL);
+		g_at_ppp_set_disconnect_function(ppp, ppp_disconnect, NULL);
+
+		/* open the ppp connection */
+		g_at_ppp_open(ppp);
+	}
+}
+
 static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	char buf[64];
@@ -231,8 +302,14 @@
 
 	if (option_legacy == TRUE) {
 			sprintf(buf, "ATD*99***%u#", option_cid);
-			g_at_chat_send(modem, buf, none_prefix,
-					NULL, NULL, NULL);
+			if (option_ppp == TRUE) {
+				g_print("Option PPP enabled\n");
+				g_at_chat_send(modem, buf, none_prefix,
+						connect_cb, NULL, NULL);
+			}
+			else
+				g_at_chat_send(modem, buf, none_prefix,
+						NULL, NULL, NULL);
 	} else {
 		sprintf(buf, "AT+CGACT=1,%u", option_cid);
 		g_at_chat_send(control, buf, none_prefix,
@@ -406,6 +483,12 @@
 				"Specify CFUN offmode" },
 	{ "legacy", 'l', 0, G_OPTION_ARG_NONE, &option_legacy,
 				"Use ATD*99***<cid>#" },
+	{ "ppp", 'P', 0, G_OPTION_ARG_NONE, &option_ppp,
+				"Connect using PPP" },
+	{ "username", 'u', 0, G_OPTION_ARG_STRING, &option_username,
+				"Specify PPP Username" },
+	{ "password", 'w', 0, G_OPTION_ARG_STRING, &option_passwd,
+				"Specifiy PPP Password" },
 	{ NULL },
 };
 

Re: [patch 6/6] Allow gsmdial to use gatchat ppp support

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 5457 bytes --]

Hi Kristen,

> Add option to use PPP to gsmdial.
> 
> Index: ofono/gatchat/gsmdial.c
> ===================================================================
> --- ofono.orig/gatchat/gsmdial.c	2010-03-10 16:58:09.773080389 -0800
> +++ ofono/gatchat/gsmdial.c	2010-03-10 17:06:45.071975512 -0800
> @@ -33,6 +33,9 @@
>  #include <glib.h>
>  #include <gatchat.h>
>  #include <gattty.h>
> +#include <arpa/inet.h>
> +#include <net/if.h>
> +#include <gatppp.h>

please put system includes before glib or internal includes.
 
>  static const char *none_prefix[] = { NULL };
>  static const char *cgreg_prefix[] = { "+CGREG:", NULL };
> @@ -45,10 +48,14 @@
>  static gchar *option_apn = NULL;
>  static gint option_offmode = 0;
>  static gboolean option_legacy = FALSE;
> +static gboolean option_ppp = FALSE;
> +static gchar *option_username = NULL;
> +static gchar *option_passwd = NULL;

Just use option_password here. Shortening is not really useful in this
case.
 
>  static GAtChat *control;
>  static GAtChat *modem;
>  static GMainLoop *event_loop;
> +static struct ppp_link *ppp;
>  
>  enum state {
>  	STATE_NONE = 0,
> @@ -220,6 +227,70 @@
>  	g_at_chat_send(modem, buf, none_prefix, NULL, NULL, NULL);
>  }
>  
> +static void print_ip_address(guint32 ip_addr)
> +{
> +	struct in_addr addr;
> +	addr.s_addr = ip_addr;
> +	g_print("%s\n", inet_ntoa(addr));

It is just fine to use printf() like everybody else. The g_print
function is pretty much stupid idea from GLib. Same as gchar should not
be used either.

The only case where g_print makes sense is in GLib based unit tests.

And personally I would do

static void print_ip_address(const char *label, guint32 addr)
{
}

That way you can later on just call one function and don't have to
manually print the label first.

> +static void ppp_connect(struct ppp_link *link, gint success, guint32 ip_addr,
> +		 guint32 dns1, guint32 dns2, gpointer user_data)
> +{
> +	if (success == PPP_CONNECT_SUCCESS) {
> +		/* print out the negotiated address and dns server */
> +		g_print("Negotiated IP Address: ");
> +		print_ip_address(ip_addr);
> +
> +		g_print("Primary DNS Server: ");
> +		print_ip_address(dns1);
> +
> +		g_print("Secondary DNS Server: ");
> +		print_ip_address(dns2);
> +	} else {
> +		g_print("Failed to create PPP interface!\n");
> +	}
> +}

So personally I prefer the style more like this:

	if (success != PPP_CONNECT_SUCCESS) {
		printf(... err msg ...);
		return;
	}

	printf(... success ...)

It is a lot better to get the error cases out of your way first and then
just focus on the success case. Instead of handling success and then
btw. we have to take of the error case.

> +static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	GIOChannel *channel;
> +
> +	if (!ok) {
> +		g_print("Unable to define context\n");
> +		exit(1);
> +	}
> +
> +	/* get the data IO channel */
> +	channel = g_at_chat_get_channel(modem);
> +	channel = g_io_channel_ref(channel);

What is this reference for? The g_at_ppp_new() should take a reference
on that channel anyway.

> +	/* shutdown gatchat */
> +	g_at_chat_unref(control);
> +	g_at_chat_unref(modem);

If you try to protect these, then I prefer that you call
g_at_chat_shutdown() here first and not directly unref them.

We should be able to keep the chat object around even if PPP is running
now.

> +	/* open ppp */
> +	ppp = g_at_ppp_new(channel);
> +	if (ppp) {
> +		g_print("Setting PPP Credentials to %s, %s\n", option_username,
> +				option_passwd);
> +		g_at_ppp_set_credentials(ppp, option_username,
> +					option_passwd);
> +
> +		/* set connect and disconnect callbacks */
> +		g_at_ppp_set_connect_function(ppp, ppp_connect, NULL);
> +		g_at_ppp_set_disconnect_function(ppp, ppp_disconnect, NULL);
> +
> +		/* open the ppp connection */
> +		g_at_ppp_open(ppp);
> +	}
> +}

This should be again done like this:

	if (!ppp) {
		printf(... err msg ..)
		return;
	}

	g_at_ppp_set_credentials ....

>  static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  {
>  	char buf[64];
> @@ -231,8 +302,14 @@
>  
>  	if (option_legacy == TRUE) {
>  			sprintf(buf, "ATD*99***%u#", option_cid);
> -			g_at_chat_send(modem, buf, none_prefix,
> -					NULL, NULL, NULL);
> +			if (option_ppp == TRUE) {
> +				g_print("Option PPP enabled\n");
> +				g_at_chat_send(modem, buf, none_prefix,
> +						connect_cb, NULL, NULL);
> +			}
> +			else
> +				g_at_chat_send(modem, buf, none_prefix,
> +						NULL, NULL, NULL);

I don't like this at all. Lets move the option_ppp check into the
connect_cb function. Check for option_ppp in the callback and if not
set, then leave the callback.

>  	} else {
>  		sprintf(buf, "AT+CGACT=1,%u", option_cid);
>  		g_at_chat_send(control, buf, none_prefix,
> @@ -406,6 +483,12 @@
>  				"Specify CFUN offmode" },
>  	{ "legacy", 'l', 0, G_OPTION_ARG_NONE, &option_legacy,
>  				"Use ATD*99***<cid>#" },
> +	{ "ppp", 'P', 0, G_OPTION_ARG_NONE, &option_ppp,
> +				"Connect using PPP" },
> +	{ "username", 'u', 0, G_OPTION_ARG_STRING, &option_username,
> +				"Specify PPP Username" },
> +	{ "password", 'w', 0, G_OPTION_ARG_STRING, &option_passwd,
> +				"Specifiy PPP Password" },

Small nitpick. Username and Password should be both lowercase.

Regards

Marcel

Re: [patch 6/6] Allow gsmdial to use gatchat ppp support

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Sun, 14 Mar 2010 13:02:52 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> >  
> > +static void print_ip_address(guint32 ip_addr)
> > +{
> > +	struct in_addr addr;
> > +	addr.s_addr = ip_addr;
> > +	g_print("%s\n", inet_ntoa(addr));
> 
> It is just fine to use printf() like everybody else. The g_print
> function is pretty much stupid idea from GLib. Same as gchar should not
> be used either.
> 
> The only case where g_print makes sense is in GLib based unit tests.

OK, but I think I should leave this the way it is just because it
is consistent with the rest of gsmdial.

[patch 1/6] Add PPP protocol support with HDLC framing

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 20901 bytes --]

This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in a
different patch.

---
 Makefile.am               |    4 
 gatchat/gatppp.c          |  552 ++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/gatppp.h          |   59 ++++
 gatchat/gatppp_internal.h |   97 ++++++++
 4 files changed, 711 insertions(+), 1 deletion(-)

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-16 15:16:53.536470034 -0700
+++ ofono/Makefile.am	2010-03-16 15:33:46.144468807 -0700
@@ -55,7 +55,9 @@
 				gatchat/gattty.h gatchat/gattty.c \
 				gatchat/gatutil.h gatchat/gatutil.c \
 				gatchat/gat.h \
-				gatchat/gatserver.h gatchat/gatserver.c
+				gatchat/gatserver.h gatchat/gatserver.c \
+				gatchat/gatppp.c gatchat/gatppp.h \
+				gatchat/gatppp_internal.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp_internal.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp_internal.h	2010-03-16 15:33:46.144468807 -0700
@@ -0,0 +1,97 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+#define DEFAULT_MRU	1500
+#define BUFFERSZ	DEFAULT_MRU*2
+#define DEFAULT_ACCM	0x00000000
+#define PPP_ESC		0x7d
+#define PPP_FLAG_SEQ 	0x7e
+#define PPP_ADDR_FIELD	0xff
+#define PPP_CTRL	0x03
+#define LCP_PROTOCOL	0xc021
+#define CHAP_PROTOCOL	0xc223
+#define PPP_HEADROOM	2
+#define HDLC_HEADROOM	3
+#define HDLC_TAIL	3
+#define MD5		5
+
+typedef enum _GAtPPPPhase {
+	DEAD = 0,
+	ESTABLISHMENT,
+	AUTHENTICATION,
+	NETWORK,
+	TERMINATION,
+} GAtPPPPhase;
+
+typedef enum _GAtPPPEvents {
+	PPP_UP = 1,
+	PPP_OPENED,
+	PPP_SUCCESS,
+	PPP_NONE,
+	PPP_CLOSING,
+	PPP_FAIL,
+	PPP_DOWN
+} GAtPPPEvents;
+
+struct ppp_packet_handler {
+	guint16 proto;
+	void (*handler)(gpointer priv, guint8 *packet);
+	gpointer priv;
+};
+
+#define ppp_info(packet) \
+	(packet+4)
+
+#define ppp_proto(packet) \
+	(ntohs(*((guint16*)(packet+2))))
+
+struct _GAtPPP {
+	gint ref_count;
+	GAtPPPPhase phase;
+	guint8 buffer[BUFFERSZ];
+	int index;
+	gint mru;
+	guint16 auth_proto;
+	char user_name[256];
+	char passwd[256];
+	gboolean pfc;
+	gboolean acfc;
+	guint32 xmit_accm[8];
+	guint32 recv_accm;
+	GIOChannel *modem;
+	GQueue *event_queue;
+	GQueue *recv_queue;
+	GAtPPPConnectFunc connect_cb;
+	gpointer connect_data;
+	GAtPPPDisconnectFunc disconnect_cb;
+	gpointer disconnect_data;
+	gint modem_watch;
+};
+
+void __ppp_generate_event(GAtPPP *ppp, GAtPPPEvents event);
+void __ppp_register_packet_handler(struct ppp_packet_handler *handler);
+void __ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen);
+void __ppp_set_auth(GAtPPP *ppp, guint8 *auth_data);
+void __ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
+guint32 __ppp_get_xmit_accm(GAtPPP *ppp);
+void __ppp_set_pfc(GAtPPP *ppp, gboolean pfc);
+gboolean __ppp_get_pfc(GAtPPP *ppp);
+void __ppp_set_acfc(GAtPPP *ppp, gboolean acfc);
+gboolean __ppp_get_acfc(GAtPPP *ppp);
Index: ofono/gatchat/gatppp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.c	2010-03-16 15:34:30.492565338 -0700
@@ -0,0 +1,552 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+
+#include <glib.h>
+
+#include "gatutil.h"
+#include "gatppp.h"
+#include "gatppp_internal.h"
+
+#define PPPINITFCS16    0xffff  /* Initial FCS value */
+#define PPPGOODFCS16    0xf0b8  /* Good final FCS value */
+
+static GList *packet_handlers = NULL;
+
+void __ppp_register_packet_handler(struct ppp_packet_handler *handler)
+{
+	packet_handlers = g_list_append(packet_handlers, handler);
+}
+
+/*
+ * FCS lookup table copied from rfc1662.
+ */
+static guint16 fcstab[256] = {
+	0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+	0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+	0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
+	0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
+	0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
+	0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
+	0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
+	0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
+	0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
+	0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
+	0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
+	0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
+	0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
+	0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
+	0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
+	0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
+	0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
+	0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
+	0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
+	0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
+	0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
+	0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
+	0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
+	0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
+	0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
+	0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
+	0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
+	0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
+	0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
+	0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+	0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+	0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};
+
+/*
+ * Calculate a new fcs given the current fcs and the new data.
+ * copied from rfc1662
+ *
+ *     The FCS field is calculated over all bits of the Address, Control,
+ *     Protocol, Information and Padding fields, not including any start
+ *     and stop bits (asynchronous) nor any bits (synchronous) or octets
+ *     (asynchronous or synchronous) inserted for transparency.  This
+ *     also does not include the Flag Sequences nor the FCS field itself.
+ */
+static guint16 ppp_fcs(guint16 fcs, guint8 c)
+{
+	guint16 new_fcs;
+
+	new_fcs = (fcs >> 8) ^ fcstab[(fcs ^ c) & 0xff];
+	return new_fcs;
+}
+
+/*
+ * escape any chars less than 0x20, and check the transmit accm table to
+ * see if this character should be escaped.
+ */
+static gboolean ppp_escape(GAtPPP *ppp, guint8 c, gboolean lcp)
+{
+	if ((lcp && c < 0x20) || (ppp->xmit_accm[c >> 5] & (1 << (c & 0x1f))))
+		return TRUE;
+	return FALSE;
+}
+
+static void ppp_put(GAtPPP *ppp, guint8 *buf, int *pos,
+			guint8 c, gboolean lcp)
+{
+	int i = *pos;
+
+	/* escape characters if needed,  copy into buf, increment pos */
+	if (ppp_escape(ppp, c, lcp)) {
+		buf[i++] = PPP_ESC;
+		buf[i++] = c ^ 0x20;
+	} else
+		buf[i++] = c;
+	*pos = i;
+}
+
+/* XXX implement PFC and ACFC */
+static guint8 *ppp_encode(GAtPPP *ppp, guint8 *data, int len,
+				guint *newlen)
+{
+	int pos = 0;
+	int i = 0;
+	guint16 fcs = PPPINITFCS16;
+	guint16 proto = ntohs(*(guint16 *)data);
+	gboolean lcp = (proto == LCP_PROTOCOL);
+	guint8 *frame = g_try_malloc0(BUFFERSZ);
+	if (!frame)
+		return NULL;
+
+	/* copy in the HDLC framing */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	/* from here till end flag, calculate FCS over each character */
+	fcs = ppp_fcs(fcs, PPP_ADDR_FIELD);
+	ppp_put(ppp, frame, &pos, PPP_ADDR_FIELD, lcp);
+	fcs = ppp_fcs(fcs, PPP_CTRL);
+	ppp_put(ppp, frame, &pos, PPP_CTRL, lcp);
+
+	/*
+	 * for each byte, first calculate FCS, then do escaping if
+	 * neccessary
+	 */
+	while (len--) {
+		fcs = ppp_fcs(fcs, data[i]);
+		ppp_put(ppp, frame, &pos, data[i++], lcp);
+	}
+
+	/* add FCS */
+	fcs ^= 0xffff;                 /* complement */
+	ppp_put(ppp, frame, &pos, (guint8)(fcs & 0x00ff), lcp);
+	ppp_put(ppp, frame, &pos, (guint8)((fcs >> 8) & 0x00ff), lcp);
+
+	/* add flag */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	*newlen = pos;
+	return frame;
+}
+
+static gint is_proto_handler(struct ppp_packet_handler *h, gpointer user)
+{
+	guint16 proto = (guint16) GPOINTER_TO_UINT(user);
+
+	if (h->proto == proto)
+		return 0;
+	else
+		return -1;
+}
+
+/* called when we have received a complete ppp frame */
+static void ppp_recv(GAtPPP *ppp)
+{
+	guint16 protocol;
+	guint8 *frame, *packet;
+	GList *list;
+	struct ppp_packet_handler *h;
+
+	/* pop frames off of receive queue */
+	while ((frame = g_queue_pop_head(ppp->recv_queue))) {
+		protocol = ppp_proto(frame);
+		packet = ppp_info(frame);
+
+		/*
+		 * check to see if we have a protocol handler
+		 * registered for this packet
+		 */
+		list = g_list_find_custom(packet_handlers,
+				GUINT_TO_POINTER(protocol),
+				(GCompareFunc) is_proto_handler);
+		if (list) {
+			h = list->data;
+			h->handler(h->priv, packet);
+		}
+		g_free(frame);
+	}
+}
+
+/* XXX - Implement PFC and ACFC */
+static guint8 * ppp_decode(GAtPPP *ppp, guint8 *frame)
+{
+	guint8 *data;
+	guint pos = 0;
+	int i = 0;
+	int len;
+	guint16 fcs;
+
+	data = g_try_malloc0(ppp->mru + 10);
+	if (!data)
+		return NULL;
+
+	/* skip the first flag char */
+	pos++;
+
+	/* TBD - how to deal with recv_accm */
+	while (frame[pos] != PPP_FLAG_SEQ) {
+		/* scan for escape character */
+		if (frame[pos] == PPP_ESC) {
+			/* skip that char */
+			pos++;
+			data[i] = frame[pos] ^ 0x20;
+		} else
+			data[i] = frame[pos];
+		i++; pos++;
+	}
+
+	len = i;
+
+	/* see if we have a good FCS */
+	fcs = PPPINITFCS16;
+	for (i = 0; i < len; i++)
+		fcs = ppp_fcs(fcs, data[i]);
+
+	if (fcs != PPPGOODFCS16) {
+		g_free(data);
+		return NULL;
+	}
+	return data;
+}
+
+static void ppp_feed(GAtPPP *ppp, guint8 *data, gsize len)
+{
+	guint pos = 0;
+	guint8 *frame;
+
+	/* collect bytes until we detect we have received a complete frame */
+	/* examine the data.  If we are@the beginning of a new frame,
+	 * allocate memory to buffer the frame.
+	 */
+
+	for (pos = 0; pos < len; pos++) {
+		if (data[pos] == PPP_FLAG_SEQ) {
+			if (ppp->index != 0) {
+				/* store last flag character & decode */
+				ppp->buffer[ppp->index++] = data[pos];
+				frame = ppp_decode(ppp, ppp->buffer);
+
+				/* push decoded frame onto receive queue */
+				if (frame)
+					g_queue_push_tail(ppp->recv_queue,
+								frame);
+
+				/* zero buffer */
+				memset(ppp->buffer, 0, BUFFERSZ);
+				ppp->index = 0;
+				continue;
+			}
+		}
+		/* copy byte to buffer */
+		if (ppp->index < BUFFERSZ)
+			ppp->buffer[ppp->index++] = data[pos];
+	}
+	/* process receive queue */
+	ppp_recv(ppp);
+}
+
+/*
+ * transmit out through the lower layer interface
+ *
+ * infolen - length of the information part of the packet
+ */
+void __ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
+{
+	guint8 *frame;
+	guint framelen;
+	GError *error = NULL;
+	GIOStatus status;
+	gsize bytes_written;
+
+	/*
+	 * do the octet stuffing.  Add 2 bytes to the infolen to
+	 * include the protocol field.
+	 */
+	frame = ppp_encode(ppp, packet, infolen+2, &framelen);
+	if (!frame) {
+		g_printerr("Failed to encode packet to transmit\n");
+		return;
+	}
+
+	/* transmit through the lower layer interface */
+	/*
+	 * TBD - should we just put this on a queue and transmit when
+	 * we won't block, or allow ourselves to block here?
+	 */
+	status = g_io_channel_write_chars(ppp->modem, (gchar *) frame,
+                                          framelen, &bytes_written, &error);
+
+	g_free(frame);
+
+}
+
+static gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
+{
+	GAtPPP *ppp = data;
+	GIOStatus status;
+	gchar buf[256];
+	gsize bytes_read;
+	GError *error = NULL;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		g_print("G_IO_NVAL | G_IO_ERR");
+		return FALSE;
+	}
+
+	if (cond & G_IO_IN) {
+		status = g_io_channel_read_chars(channel, buf, 256,
+				&bytes_read, &error);
+		if (bytes_read > 0)
+			ppp_feed(ppp, (guint8 *)buf, bytes_read);
+        	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+                	return FALSE;
+	}
+
+	return TRUE;
+}
+
+/* Administrative Close */
+static void ppp_close(GAtPPP *ppp)
+{
+	/* send a CLOSE event to the lcp layer */
+}
+
+/* Administrative Open */
+void g_at_ppp_open(GAtPPP *ppp)
+{
+	/* send an OPEN event to the lcp layer */
+}
+
+static void ppp_ppp_establishment(GAtPPP *ppp)
+{
+	/* signal UP event to LCP */
+}
+
+static void ppp_terminate(GAtPPP *ppp)
+{
+	/* signal DOWN event to LCP */
+}
+
+static void ppp_authenticate(GAtPPP *ppp)
+{
+	/* we don't do authentication right now, so send NONE */
+	if (!ppp->auth_proto)
+		__ppp_generate_event(ppp, PPP_NONE);
+	/* otherwise we need to wait for the peer to send us a challenge */
+}
+
+static void ppp_dead(GAtPPP *ppp)
+{
+	/* re-initialize everything */
+}
+
+static void ppp_network(GAtPPP *ppp)
+{
+	/* bring network phase up */
+}
+
+static void ppp_transition_phase(GAtPPP *ppp, guint phase)
+{
+	/* don't do anything if we're already there */
+	if (ppp->phase == phase)
+		return;
+
+	/* set new phase */
+	ppp->phase = phase;
+
+	switch(phase) {
+	case ESTABLISHMENT:
+		ppp_ppp_establishment(ppp);
+		break;
+	case AUTHENTICATION:
+		ppp_authenticate(ppp);
+		break;
+	case TERMINATION:
+		ppp_terminate(ppp);
+		break;
+	case DEAD:
+		ppp_dead(ppp);
+		break;
+	case NETWORK:
+		ppp_network(ppp);
+		break;
+	default:
+		break;
+	}
+
+}
+
+static void ppp_handle_event(GAtPPP *ppp)
+{
+	guint event;
+
+	while ((event = GPOINTER_TO_UINT(g_queue_pop_head(ppp->event_queue)))){
+		switch(event) {
+		case PPP_UP:
+			/* causes transition to ppp establishment */
+			ppp_transition_phase(ppp, ESTABLISHMENT);
+			break;
+		case PPP_OPENED:
+			ppp_transition_phase(ppp, AUTHENTICATION);
+			break;
+		case PPP_CLOSING:
+			/* causes transition to termination phase */
+			ppp_transition_phase(ppp, TERMINATION);
+			break;
+		case PPP_DOWN:
+			/* cases transition to dead phase */
+			ppp_transition_phase(ppp, DEAD);
+			break;
+		case PPP_NONE:
+		case PPP_SUCCESS:
+			/* causes transition to network phase */
+			ppp_transition_phase(ppp, NETWORK);
+			break;
+		}
+	}
+}
+
+/*
+ * send the event handler a new event to process
+ */
+void __ppp_generate_event(GAtPPP *ppp, GAtPPPEvents event)
+{
+	g_queue_push_tail(ppp->event_queue, GUINT_TO_POINTER(event));
+	ppp_handle_event(ppp);
+}
+
+void g_at_ppp_set_connect_function(GAtPPP *ppp,
+			       GAtPPPConnectFunc callback, gpointer user_data)
+{
+	ppp->connect_cb = callback;
+	ppp->connect_data = user_data;
+}
+
+void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data)
+{
+	ppp->disconnect_cb = callback;
+	ppp->disconnect_data = user_data;
+}
+
+void g_at_ppp_shutdown(GAtPPP *ppp)
+{
+	/* close the ppp */
+	ppp_close(ppp);
+
+	/* clean up all the queues */
+	g_queue_free(ppp->event_queue);
+	g_queue_free(ppp->recv_queue);
+
+	/* cleanup modem channel */
+	g_source_remove(ppp->modem_watch);
+	g_io_channel_unref(ppp->modem);
+}
+
+void g_at_ppp_ref(GAtPPP *ppp)
+{
+	g_atomic_int_inc(&ppp->ref_count);
+}
+
+void g_at_ppp_unref(GAtPPP *ppp)
+{
+	gboolean last;
+
+	last = g_atomic_int_dec_and_test(&ppp->ref_count);
+	if (last) {
+		g_at_ppp_shutdown(ppp);
+		g_free(ppp);
+	}
+}
+
+GAtPPP *g_at_ppp_new(GIOChannel *modem)
+{
+	GAtPPP *ppp;
+
+	ppp = g_try_malloc0(sizeof(GAtPPP));
+	if (!ppp)
+		return NULL;
+
+	ppp->modem = g_io_channel_ref(modem);
+	if (!g_at_util_setup_io(ppp->modem, G_IO_FLAG_NONBLOCK)) {
+		g_io_channel_unref(modem);
+		g_free(ppp);
+		return NULL;
+	}
+
+	/* think I still need to do this */
+	g_io_channel_set_buffered(modem, FALSE);
+
+	ppp->ref_count = 1;
+
+	/* set options to defaults */
+	ppp->mru = DEFAULT_MRU;
+	ppp->recv_accm = DEFAULT_ACCM;
+	ppp->xmit_accm[0] = DEFAULT_ACCM;
+	ppp->xmit_accm[3] = 0x60000000; /* 0x7d, 0x7e */
+	ppp->pfc = FALSE;
+	ppp->acfc = FALSE;
+
+	/* allocate the queues */
+	ppp->event_queue = g_queue_new();
+	ppp->recv_queue = g_queue_new();
+
+	ppp->index = 0;
+
+	/* initialize the lcp state */
+
+
+	/* initialize the autentication state */
+
+
+	/* intialize the network state */
+
+	/* start listening for packets from the modem */
+	ppp->modem_watch = g_io_add_watch(modem,
+			G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+			ppp_cb, ppp);
+
+	return ppp;
+}
Index: ofono/gatchat/gatppp.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.h	2010-03-16 15:27:28.906468886 -0700
@@ -0,0 +1,59 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __G_AT_PPP_H
+#define __G_AT_PPP_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct _GAtPPP;
+typedef struct _GAtPPP GAtPPP;
+
+typedef enum _GAtPPPConnectStatus {
+	G_AT_PPP_CONNECT_SUCCESS,
+	G_AT_PPP_CONNECT_FAIL
+} GAtPPPConnectStatus;
+
+typedef void (*GAtPPPConnectFunc)(GAtPPP *ppp, GAtPPPConnectStatus success,
+				 guint32 ip_address,
+				 guint32 dns1, guint32 dns2,
+				 gpointer user_data);
+
+typedef void (*GAtPPPDisconnectFunc)(GAtPPP *ppp, gpointer user_data);
+
+GAtPPP * g_at_ppp_new(GIOChannel *modem);
+void g_at_ppp_open(GAtPPP *ppp);
+void g_at_ppp_set_connect_function(GAtPPP *ppp,
+			       GAtPPPConnectFunc callback, gpointer user_data);
+void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data);
+void g_at_ppp_shutdown(GAtPPP *ppp);
+void g_at_ppp_ref(GAtPPP *ppp);
+void g_at_ppp_unref(GAtPPP *ppp);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __G_AT_PPP_H */
+

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 6742 bytes --]

Hi Kristen,

> This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in a
> different patch.
> 
> ---
>  Makefile.am               |    4 
>  gatchat/gatppp.c          |  552 ++++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/gatppp.h          |   59 ++++
>  gatchat/gatppp_internal.h |   97 ++++++++
>  4 files changed, 711 insertions(+), 1 deletion(-)

so before we merge this patch and then continue development in the oFono
tree, we have some renaming to do.

Denis and I talked about it on IRC. And what we actually want is
something like this:

	gatchat/gatppp.h	Public include file
	gatchat/gatppp.c	Implementation of PPP API
	gatchat/ppp.h		Internal header file
	gatchat/ppp.c		Generic PPP stuff (maybe not needed)
	gatchat/ppp_<proto>.c	Protocol specific extensions

This makes it similar to what we do with gsm7010.[ch] and
ringbuffer.[ch]. So this means for internal structures and functions the
prefix should be ppp_*. Similar to what we do with GSM 70.10 and the
ring buffer implementation.

We might have mislead you with parts of the previous review. I apologize
for that. The more we are looking at your patches it becomes clearer
which direction we wanna take.

> +
> +typedef enum _GAtPPPPhase {
> +	DEAD = 0,
> +	ESTABLISHMENT,
> +	AUTHENTICATION,
> +	NETWORK,
> +	TERMINATION,
> +} GAtPPPPhase;
> +
> +typedef enum _GAtPPPEvents {
> +	PPP_UP = 1,
> +	PPP_OPENED,
> +	PPP_SUCCESS,
> +	PPP_NONE,
> +	PPP_CLOSING,
> +	PPP_FAIL,
> +	PPP_DOWN
> +} GAtPPPEvents;

If we don't expose them in public headers, then typedefs are not needed.
We actually dislike typedefs. The only reason for having them in public
GAtChat headers is to confirm with GLib style.

> +struct ppp_packet_handler {
> +	guint16 proto;
> +	void (*handler)(gpointer priv, guint8 *packet);
> +	gpointer priv;
> +};
> +
> +#define ppp_info(packet) \
> +	(packet+4)
> +
> +#define ppp_proto(packet) \
> +	(ntohs(*((guint16*)(packet+2))))

Arithmetic operations requires spaces. Even in defines. So it packet + 4
etc.

When you cast don't forget whitepaces. So (guint16 *) (packet...

> +struct _GAtPPP {
> +	gint ref_count;
> +	GAtPPPPhase phase;
> +	guint8 buffer[BUFFERSZ];
> +	int index;
> +	gint mru;
> +	guint16 auth_proto;
> +	char user_name[256];
> +	char passwd[256];
> +	gboolean pfc;
> +	gboolean acfc;
> +	guint32 xmit_accm[8];
> +	guint32 recv_accm;
> +	GIOChannel *modem;
> +	GQueue *event_queue;
> +	GQueue *recv_queue;
> +	GAtPPPConnectFunc connect_cb;
> +	gpointer connect_data;
> +	GAtPPPDisconnectFunc disconnect_cb;
> +	gpointer disconnect_data;
> +	gint modem_watch;
> +};
> +
> +void __ppp_generate_event(GAtPPP *ppp, GAtPPPEvents event);
> +void __ppp_register_packet_handler(struct ppp_packet_handler *handler);
> +void __ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen);
> +void __ppp_set_auth(GAtPPP *ppp, guint8 *auth_data);
> +void __ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
> +guint32 __ppp_get_xmit_accm(GAtPPP *ppp);
> +void __ppp_set_pfc(GAtPPP *ppp, gboolean pfc);
> +gboolean __ppp_get_pfc(GAtPPP *ppp);
> +void __ppp_set_acfc(GAtPPP *ppp, gboolean acfc);
> +gboolean __ppp_get_acfc(GAtPPP *ppp);

Just use ppp_* here and forget about __ppp_*. I will add autofoo magic
to ensure we are not accidentally exporting these. It is not your
concern.

> + *  PPP library with GLib integration
> + *
> + *  Copyright (C) 2010  Intel Corporation. All rights reserved.

Nitpick here. Copyright is 2009-2010. Applies to all new files you are
adding.

> +	/*
> +	 * do the octet stuffing.  Add 2 bytes to the infolen to
> +	 * include the protocol field.
> +	 */
> +	frame = ppp_encode(ppp, packet, infolen+2, &framelen);
> +	if (!frame) {
> +		g_printerr("Failed to encode packet to transmit\n");
> +		return;
> +	}
> +
> +	/* transmit through the lower layer interface */
> +	/*
> +	 * TBD - should we just put this on a queue and transmit when
> +	 * we won't block, or allow ourselves to block here?
> +	 */
> +	status = g_io_channel_write_chars(ppp->modem, (gchar *) frame,
> +                                          framelen, &bytes_written, &error);
> +
> +	g_free(frame);
> +
> +}

Look out for unneeded empty lines at the end of functions. We don't want
them.

> +static void ppp_transition_phase(GAtPPP *ppp, guint phase)
> +{
> +	/* don't do anything if we're already there */
> +	if (ppp->phase == phase)
> +		return;
> +
> +	/* set new phase */
> +	ppp->phase = phase;
> +
> +	switch(phase) {
> +	case ESTABLISHMENT:
> +		ppp_ppp_establishment(ppp);
> +		break;
> +	case AUTHENTICATION:
> +		ppp_authenticate(ppp);
> +		break;
> +	case TERMINATION:
> +		ppp_terminate(ppp);
> +		break;
> +	case DEAD:
> +		ppp_dead(ppp);
> +		break;
> +	case NETWORK:
> +		ppp_network(ppp);
> +		break;
> +	default:
> +		break;
> +	}

Since this is an enum anyway. Please skip the default statement and just
ensure you handle all states. For the ones that don't apply, add just
empty statements.

That way the compiler will warn you if you add a new state to the enum
later on. And you accidentally forget to add code for handling that
state.

And I do prefer having a prefix here. So PPP_DEAD, PPP_NETWORK etc.

> +void g_at_ppp_unref(GAtPPP *ppp)
> +{
> +	gboolean last;
> +
> +	last = g_atomic_int_dec_and_test(&ppp->ref_count);
> +	if (last) {
> +		g_at_ppp_shutdown(ppp);
> +		g_free(ppp);
> +	}
> +}

Not using the last variable here is better. You really want this to be
atomic. Just check the return value of the function directly.

> +	/* think I still need to do this */
> +	g_io_channel_set_buffered(modem, FALSE);

I have sounds really picky now ;)

Comment should be either with FIXME if it is something that is unclear.
Or better at least in third person.

> +	ppp->ref_count = 1;
> +
> +	/* set options to defaults */
> +	ppp->mru = DEFAULT_MRU;
> +	ppp->recv_accm = DEFAULT_ACCM;
> +	ppp->xmit_accm[0] = DEFAULT_ACCM;
> +	ppp->xmit_accm[3] = 0x60000000; /* 0x7d, 0x7e */
> +	ppp->pfc = FALSE;
> +	ppp->acfc = FALSE;
> +
> +	/* allocate the queues */
> +	ppp->event_queue = g_queue_new();
> +	ppp->recv_queue = g_queue_new();
> +
> +	ppp->index = 0;
> +
> +	/* initialize the lcp state */
> +
> +
> +	/* initialize the autentication state */
> +
> +
> +	/* intialize the network state */

I know that your other patches are going to add this code later one.
However having these prefixed with FIXME would be better. Not that it is
a big thing if we see the patch series as a whole, but would be better
for future things like this.

Regards

Marcel

[patch 1/6] Add PPP protocol support with HDLC framing

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 23096 bytes --]

This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in a
different patch.

---
 Makefile.am      |    4 
 gatchat/gatppp.c |  133 ++++++++++++++++
 gatchat/gatppp.h |   59 +++++++
 gatchat/ppp.c    |  455 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/ppp.h    |  130 +++++++++++++++
 5 files changed, 780 insertions(+), 1 deletion(-)

Index: ofono/Makefile.am
===================================================================
--- ofono.orig/Makefile.am	2010-03-19 11:49:00.060469174 -0700
+++ ofono/Makefile.am	2010-03-19 11:49:02.652468489 -0700
@@ -55,7 +55,9 @@
 				gatchat/gattty.h gatchat/gattty.c \
 				gatchat/gatutil.h gatchat/gatutil.c \
 				gatchat/gat.h \
-				gatchat/gatserver.h gatchat/gatserver.c
+				gatchat/gatserver.h gatchat/gatserver.c \
+				gatchat/gatppp.c gatchat/gatppp.h \
+				gatchat/ppp.c gatchat/ppp.h
 
 udev_files = plugins/ofono.rules
 
Index: ofono/gatchat/gatppp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.c	2010-03-19 11:49:02.652468489 -0700
@@ -0,0 +1,133 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2009-2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+
+#include <glib.h>
+
+#include "gatutil.h"
+#include "gatppp.h"
+#include "ppp.h"
+
+/* Administrative Open */
+void g_at_ppp_open(GAtPPP *ppp)
+{
+	/* send an OPEN event to the lcp layer */
+}
+
+void g_at_ppp_set_connect_function(GAtPPP *ppp,
+			       GAtPPPConnectFunc callback, gpointer user_data)
+{
+	ppp->connect_cb = callback;
+	ppp->connect_data = user_data;
+}
+
+void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data)
+{
+	ppp->disconnect_cb = callback;
+	ppp->disconnect_data = user_data;
+}
+
+void g_at_ppp_shutdown(GAtPPP *ppp)
+{
+	/* close the ppp */
+	ppp_close(ppp);
+
+	/* clean up all the queues */
+	g_queue_free(ppp->event_queue);
+	g_queue_free(ppp->recv_queue);
+
+	/* cleanup modem channel */
+	g_source_remove(ppp->modem_watch);
+	g_io_channel_unref(ppp->modem);
+}
+
+void g_at_ppp_ref(GAtPPP *ppp)
+{
+	g_atomic_int_inc(&ppp->ref_count);
+}
+
+void g_at_ppp_unref(GAtPPP *ppp)
+{
+	if (g_atomic_int_dec_and_test(&ppp->ref_count)) {
+		g_at_ppp_shutdown(ppp);
+		g_free(ppp);
+	}
+}
+
+GAtPPP *g_at_ppp_new(GIOChannel *modem)
+{
+	GAtPPP *ppp;
+
+	ppp = g_try_malloc0(sizeof(GAtPPP));
+	if (!ppp)
+		return NULL;
+
+	ppp->modem = g_io_channel_ref(modem);
+	if (!g_at_util_setup_io(ppp->modem, G_IO_FLAG_NONBLOCK)) {
+		g_io_channel_unref(modem);
+		g_free(ppp);
+		return NULL;
+	}
+	g_io_channel_set_buffered(modem, FALSE);
+
+	ppp->ref_count = 1;
+
+	/* set options to defaults */
+	ppp->mru = DEFAULT_MRU;
+	ppp->recv_accm = DEFAULT_ACCM;
+	ppp->xmit_accm[0] = DEFAULT_ACCM;
+	ppp->xmit_accm[3] = 0x60000000; /* 0x7d, 0x7e */
+	ppp->pfc = FALSE;
+	ppp->acfc = FALSE;
+
+	/* allocate the queues */
+	ppp->event_queue = g_queue_new();
+	ppp->recv_queue = g_queue_new();
+
+	ppp->index = 0;
+
+	/* initialize the lcp state */
+
+
+	/* initialize the autentication state */
+
+
+	/* intialize the network state */
+
+	/* start listening for packets from the modem */
+	ppp->modem_watch = g_io_add_watch(modem,
+			G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+			ppp_cb, ppp);
+
+	return ppp;
+}
Index: ofono/gatchat/gatppp.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/gatppp.h	2010-03-19 11:49:02.652468489 -0700
@@ -0,0 +1,59 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2009-2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __G_AT_PPP_H
+#define __G_AT_PPP_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct _GAtPPP;
+typedef struct _GAtPPP GAtPPP;
+
+typedef enum _GAtPPPConnectStatus {
+	G_AT_PPP_CONNECT_SUCCESS,
+	G_AT_PPP_CONNECT_FAIL
+} GAtPPPConnectStatus;
+
+typedef void (*GAtPPPConnectFunc)(GAtPPP *ppp, GAtPPPConnectStatus success,
+				 guint32 ip_address,
+				 guint32 dns1, guint32 dns2,
+				 gpointer user_data);
+
+typedef void (*GAtPPPDisconnectFunc)(GAtPPP *ppp, gpointer user_data);
+
+GAtPPP * g_at_ppp_new(GIOChannel *modem);
+void g_at_ppp_open(GAtPPP *ppp);
+void g_at_ppp_set_connect_function(GAtPPP *ppp,
+			       GAtPPPConnectFunc callback, gpointer user_data);
+void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
+				  GAtPPPDisconnectFunc callback,
+				  gpointer user_data);
+void g_at_ppp_shutdown(GAtPPP *ppp);
+void g_at_ppp_ref(GAtPPP *ppp);
+void g_at_ppp_unref(GAtPPP *ppp);
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __G_AT_PPP_H */
+
Index: ofono/gatchat/ppp.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/ppp.c	2010-03-19 11:52:31.011496932 -0700
@@ -0,0 +1,455 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2009-2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <termios.h>
+#include <arpa/inet.h>
+
+#include <glib.h>
+
+#include "gatutil.h"
+#include "gatppp.h"
+#include "ppp.h"
+
+#define PPPINITFCS16    0xffff  /* Initial FCS value */
+#define PPPGOODFCS16    0xf0b8  /* Good final FCS value */
+
+static GList *packet_handlers = NULL;
+
+void ppp_register_packet_handler(struct ppp_packet_handler *handler)
+{
+	packet_handlers = g_list_append(packet_handlers, handler);
+}
+
+/*
+ * FCS lookup table copied from rfc1662.
+ */
+static guint16 fcstab[256] = {
+	0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+	0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+	0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
+	0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
+	0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
+	0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
+	0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
+	0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
+	0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
+	0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
+	0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
+	0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
+	0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
+	0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
+	0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
+	0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
+	0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
+	0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
+	0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
+	0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
+	0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
+	0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
+	0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
+	0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
+	0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
+	0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
+	0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
+	0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
+	0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
+	0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+	0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+	0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};
+
+/*
+ * Calculate a new fcs given the current fcs and the new data.
+ * copied from rfc1662
+ *
+ *     The FCS field is calculated over all bits of the Address, Control,
+ *     Protocol, Information and Padding fields, not including any start
+ *     and stop bits (asynchronous) nor any bits (synchronous) or octets
+ *     (asynchronous or synchronous) inserted for transparency.  This
+ *     also does not include the Flag Sequences nor the FCS field itself.
+ */
+static guint16 ppp_fcs(guint16 fcs, guint8 c)
+{
+	guint16 new_fcs;
+
+	new_fcs = (fcs >> 8) ^ fcstab[(fcs ^ c) & 0xff];
+	return new_fcs;
+}
+
+/*
+ * escape any chars less than 0x20, and check the transmit accm table to
+ * see if this character should be escaped.
+ */
+static gboolean ppp_escape(GAtPPP *ppp, guint8 c, gboolean lcp)
+{
+	if ((lcp && c < 0x20) || (ppp->xmit_accm[c >> 5] & (1 << (c & 0x1f))))
+		return TRUE;
+	return FALSE;
+}
+
+static void ppp_put(GAtPPP *ppp, guint8 *buf, int *pos,
+			guint8 c, gboolean lcp)
+{
+	int i = *pos;
+
+	/* escape characters if needed,  copy into buf, increment pos */
+	if (ppp_escape(ppp, c, lcp)) {
+		buf[i++] = PPP_ESC;
+		buf[i++] = c ^ 0x20;
+	} else
+		buf[i++] = c;
+	*pos = i;
+}
+
+/* XXX implement PFC and ACFC */
+static guint8 *ppp_encode(GAtPPP *ppp, guint8 *data, int len,
+				guint *newlen)
+{
+	int pos = 0;
+	int i = 0;
+	guint16 fcs = PPPINITFCS16;
+	guint16 proto = get_host_short(data);
+	gboolean lcp = (proto == LCP_PROTOCOL);
+	guint8 *frame = g_try_malloc0(BUFFERSZ);
+	if (!frame)
+		return NULL;
+
+	/* copy in the HDLC framing */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	/* from here till end flag, calculate FCS over each character */
+	fcs = ppp_fcs(fcs, PPP_ADDR_FIELD);
+	ppp_put(ppp, frame, &pos, PPP_ADDR_FIELD, lcp);
+	fcs = ppp_fcs(fcs, PPP_CTRL);
+	ppp_put(ppp, frame, &pos, PPP_CTRL, lcp);
+
+	/*
+	 * for each byte, first calculate FCS, then do escaping if
+	 * neccessary
+	 */
+	while (len--) {
+		fcs = ppp_fcs(fcs, data[i]);
+		ppp_put(ppp, frame, &pos, data[i++], lcp);
+	}
+
+	/* add FCS */
+	fcs ^= 0xffff;                 /* complement */
+	ppp_put(ppp, frame, &pos, (guint8)(fcs & 0x00ff), lcp);
+	ppp_put(ppp, frame, &pos, (guint8)((fcs >> 8) & 0x00ff), lcp);
+
+	/* add flag */
+	frame[pos++] = PPP_FLAG_SEQ;
+
+	*newlen = pos;
+	return frame;
+}
+
+static gint is_proto_handler(gconstpointer a, gconstpointer b)
+{
+	const struct ppp_packet_handler *h = a;
+	const guint16 proto = (guint16) GPOINTER_TO_UINT(b);
+
+	if (h->proto == proto)
+		return 0;
+	else
+		return -1;
+}
+
+/* called when we have received a complete ppp frame */
+static void ppp_recv(GAtPPP *ppp)
+{
+	guint16 protocol;
+	guint8 *frame, *packet;
+	GList *list;
+	struct ppp_packet_handler *h;
+
+	/* pop frames off of receive queue */
+	while ((frame = g_queue_pop_head(ppp->recv_queue))) {
+		protocol = ppp_proto(frame);
+		packet = ppp_info(frame);
+
+		/*
+		 * check to see if we have a protocol handler
+		 * registered for this packet
+		 */
+		list = g_list_find_custom(packet_handlers,
+				GUINT_TO_POINTER(protocol),
+				is_proto_handler);
+		if (list) {
+			h = list->data;
+			h->handler(h->priv, packet);
+		}
+		g_free(frame);
+	}
+}
+
+/* XXX - Implement PFC and ACFC */
+static guint8 *ppp_decode(GAtPPP *ppp, guint8 *frame)
+{
+	guint8 *data;
+	guint pos = 0;
+	int i = 0;
+	int len;
+	guint16 fcs;
+
+	data = g_try_malloc0(ppp->mru + 10);
+	if (!data)
+		return NULL;
+
+	/* skip the first flag char */
+	pos++;
+
+	/* TBD - how to deal with recv_accm */
+	while (frame[pos] != PPP_FLAG_SEQ) {
+		/* scan for escape character */
+		if (frame[pos] == PPP_ESC) {
+			/* skip that char */
+			pos++;
+			data[i] = frame[pos] ^ 0x20;
+		} else
+			data[i] = frame[pos];
+		i++; pos++;
+	}
+
+	len = i;
+
+	/* see if we have a good FCS */
+	fcs = PPPINITFCS16;
+	for (i = 0; i < len; i++)
+		fcs = ppp_fcs(fcs, data[i]);
+
+	if (fcs != PPPGOODFCS16) {
+		g_free(data);
+		return NULL;
+	}
+	return data;
+}
+
+static void ppp_feed(GAtPPP *ppp, guint8 *data, gsize len)
+{
+	guint pos = 0;
+	guint8 *frame;
+
+	/* collect bytes until we detect we have received a complete frame */
+	/* examine the data.  If we are@the beginning of a new frame,
+	 * allocate memory to buffer the frame.
+	 */
+
+	for (pos = 0; pos < len; pos++) {
+		if (data[pos] == PPP_FLAG_SEQ) {
+			if (ppp->index != 0) {
+				/* store last flag character & decode */
+				ppp->buffer[ppp->index++] = data[pos];
+				frame = ppp_decode(ppp, ppp->buffer);
+
+				/* push decoded frame onto receive queue */
+				if (frame)
+					g_queue_push_tail(ppp->recv_queue,
+								frame);
+
+				/* zero buffer */
+				memset(ppp->buffer, 0, BUFFERSZ);
+				ppp->index = 0;
+				continue;
+			}
+		}
+		/* copy byte to buffer */
+		if (ppp->index < BUFFERSZ)
+			ppp->buffer[ppp->index++] = data[pos];
+	}
+	/* process receive queue */
+	ppp_recv(ppp);
+}
+
+/*
+ * transmit out through the lower layer interface
+ *
+ * infolen - length of the information part of the packet
+ */
+void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen)
+{
+	guint8 *frame;
+	guint framelen;
+	GError *error = NULL;
+	GIOStatus status;
+	gsize bytes_written;
+
+	/*
+	 * do the octet stuffing.  Add 2 bytes to the infolen to
+	 * include the protocol field.
+	 */
+	frame = ppp_encode(ppp, packet, infolen + 2, &framelen);
+	if (!frame) {
+		g_printerr("Failed to encode packet to transmit\n");
+		return;
+	}
+
+	/* transmit through the lower layer interface */
+	/*
+	 * TBD - should we just put this on a queue and transmit when
+	 * we won't block, or allow ourselves to block here?
+	 */
+	status = g_io_channel_write_chars(ppp->modem, (gchar *) frame,
+                                          framelen, &bytes_written, &error);
+
+	g_free(frame);
+}
+
+gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
+{
+	GAtPPP *ppp = data;
+	GIOStatus status;
+	gchar buf[256];
+	gsize bytes_read;
+	GError *error = NULL;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
+		g_print("G_IO_NVAL | G_IO_ERR");
+		return FALSE;
+	}
+
+	if (cond & G_IO_IN) {
+		status = g_io_channel_read_chars(channel, buf, 256,
+				&bytes_read, &error);
+		if (bytes_read > 0)
+			ppp_feed(ppp, (guint8 *)buf, bytes_read);
+        	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+                	return FALSE;
+	}
+
+	return TRUE;
+}
+
+/* Administrative Close */
+void ppp_close(GAtPPP *ppp)
+{
+	/* send a CLOSE event to the lcp layer */
+}
+
+static void ppp_link_establishment(GAtPPP *ppp)
+{
+	/* signal UP event to LCP */
+}
+
+static void ppp_terminate(GAtPPP *ppp)
+{
+	/* signal DOWN event to LCP */
+}
+
+static void ppp_authenticate(GAtPPP *ppp)
+{
+	/* we don't do authentication right now, so send NONE */
+	if (!ppp->auth_proto)
+		ppp_generate_event(ppp, PPP_NONE);
+	/* otherwise we need to wait for the peer to send us a challenge */
+}
+
+static void ppp_dead(GAtPPP *ppp)
+{
+	/* re-initialize everything */
+}
+
+static void ppp_network(GAtPPP *ppp)
+{
+	/* bring network phase up */
+}
+
+static void ppp_transition_phase(GAtPPP *ppp, enum ppp_phase phase)
+{
+	/* don't do anything if we're already there */
+	if (ppp->phase == phase)
+		return;
+
+	/* set new phase */
+	ppp->phase = phase;
+
+	switch (phase) {
+	case PPP_ESTABLISHMENT:
+		ppp_link_establishment(ppp);
+		break;
+	case PPP_AUTHENTICATION:
+		ppp_authenticate(ppp);
+		break;
+	case PPP_TERMINATION:
+		ppp_terminate(ppp);
+		break;
+	case PPP_DEAD:
+		ppp_dead(ppp);
+		break;
+	case PPP_NETWORK:
+		ppp_network(ppp);
+		break;
+	}
+
+}
+
+static void ppp_handle_event(GAtPPP *ppp)
+{
+	enum ppp_event event;
+
+	while ((event = GPOINTER_TO_UINT(g_queue_pop_head(ppp->event_queue)))){
+		switch (event) {
+		case PPP_UP:
+			/* causes transition to ppp establishment */
+			ppp_transition_phase(ppp, PPP_ESTABLISHMENT);
+			break;
+		case PPP_OPENED:
+			ppp_transition_phase(ppp, PPP_AUTHENTICATION);
+			break;
+		case PPP_CLOSING:
+			/* causes transition to termination phase */
+			ppp_transition_phase(ppp, PPP_TERMINATION);
+			break;
+		case PPP_DOWN:
+			/* cases transition to dead phase */
+			ppp_transition_phase(ppp, PPP_DEAD);
+			break;
+		case PPP_NONE:
+		case PPP_SUCCESS:
+			/* causes transition to network phase */
+			ppp_transition_phase(ppp, PPP_NETWORK);
+			break;
+		case PPP_FAIL:
+			if (ppp->phase == PPP_ESTABLISHMENT)
+				ppp_transition_phase(ppp, PPP_DEAD);
+			else if (ppp->phase == PPP_AUTHENTICATION)
+				ppp_transition_phase(ppp, PPP_TERMINATION);
+		}
+	}
+}
+
+/*
+ * send the event handler a new event to process
+ */
+void ppp_generate_event(GAtPPP *ppp, enum ppp_event event)
+{
+	g_queue_push_tail(ppp->event_queue, GUINT_TO_POINTER(event));
+	ppp_handle_event(ppp);
+}
+
Index: ofono/gatchat/ppp.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ ofono/gatchat/ppp.h	2010-03-19 11:51:37.553076071 -0700
@@ -0,0 +1,130 @@
+/*
+ *
+ *  PPP library with GLib integration
+ *
+ *  Copyright (C) 2009-2010  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+#define DEFAULT_MRU	1500
+#define BUFFERSZ	DEFAULT_MRU*2
+#define DEFAULT_ACCM	0x00000000
+#define PPP_ESC		0x7d
+#define PPP_FLAG_SEQ 	0x7e
+#define PPP_ADDR_FIELD	0xff
+#define PPP_CTRL	0x03
+#define LCP_PROTOCOL	0xc021
+#define CHAP_PROTOCOL	0xc223
+#define PPP_HEADROOM	2
+#define HDLC_HEADROOM	3
+#define HDLC_TAIL	3
+#define MD5		5
+
+enum ppp_phase {
+	PPP_DEAD = 0,
+	PPP_ESTABLISHMENT,
+	PPP_AUTHENTICATION,
+	PPP_NETWORK,
+	PPP_TERMINATION,
+};
+
+enum ppp_event {
+	PPP_UP = 1,
+	PPP_OPENED,
+	PPP_SUCCESS,
+	PPP_NONE,
+	PPP_CLOSING,
+	PPP_FAIL,
+	PPP_DOWN
+};
+
+struct ppp_packet_handler {
+	guint16 proto;
+	void (*handler)(gpointer priv, guint8 *packet);
+	gpointer priv;
+};
+
+struct ppp_header {
+	guint16 proto;
+	guint8 info[0];
+} __attribute__((packed));
+
+struct packed_short {
+	guint16 s;
+} __attribute__((packed));
+
+struct packed_long {
+	guint32 l;
+} __attribute__((packed));
+
+static inline guint32 __get_unaligned_long(const gpointer p)
+{
+	const struct packed_long *ptr = p;
+	return ptr->l;
+}
+
+static inline guint16 __get_unaligned_short(const gpointer p)
+{
+	const struct packed_short *ptr = p;
+	return ptr->s;
+}
+
+#define get_host_long(p) \
+	(ntohl(__get_unaligned_long(p)))
+
+#define get_host_short(p) \
+	(ntohs(__get_unaligned_short(p)))
+
+#define ppp_info(packet) \
+	(packet + 4)
+
+#define ppp_proto(packet) \
+	(get_host_short(packet + 2))
+
+struct _GAtPPP {
+	gint ref_count;
+	enum ppp_phase phase;
+	guint8 buffer[BUFFERSZ];
+	int index;
+	gint mru;
+	guint16 auth_proto;
+	char user_name[256];
+	char passwd[256];
+	gboolean pfc;
+	gboolean acfc;
+	guint32 xmit_accm[8];
+	guint32 recv_accm;
+	GIOChannel *modem;
+	GQueue *event_queue;
+	GQueue *recv_queue;
+	GAtPPPConnectFunc connect_cb;
+	gpointer connect_data;
+	GAtPPPDisconnectFunc disconnect_cb;
+	gpointer disconnect_data;
+	gint modem_watch;
+};
+
+gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data);
+void ppp_close(GAtPPP *ppp);
+void ppp_generate_event(GAtPPP *ppp, enum ppp_event event);
+void ppp_register_packet_handler(struct ppp_packet_handler *handler);
+void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen);
+void ppp_set_auth(GAtPPP *ppp, guint8 *auth_data);
+void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm);
+guint32 ppp_get_xmit_accm(GAtPPP *ppp);
+void ppp_set_pfc(GAtPPP *ppp, gboolean pfc);
+gboolean ppp_get_pfc(GAtPPP *ppp);
+void ppp_set_acfc(GAtPPP *ppp, gboolean acfc);
+gboolean ppp_get_acfc(GAtPPP *ppp);

Re: [patch 1/6] Add PPP protocol support with HDLC framing

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

Hi Kristen,

> This patch implements the basic PPP protocol.  LCP, NCP etc. are handled in a
> different patch.
> 
> ---
>  Makefile.am      |    4 
>  gatchat/gatppp.c |  133 ++++++++++++++++
>  gatchat/gatppp.h |   59 +++++++
>  gatchat/ppp.c    |  455 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gatchat/ppp.h    |  130 +++++++++++++++
>  5 files changed, 780 insertions(+), 1 deletion(-)

please keep the patches whitespace clean. Otherwise you will make it
really hard and complicated for everybody if we have to do whitespace
cleanup in between patches.

Applying: Add PPP protocol support with HDLC framing
/data/devel/ofono/.git/rebase-apply/patch:572: space before tab in indent.
        	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
/data/devel/ofono/.git/rebase-apply/patch:573: space before tab in indent.
                	return FALSE;
fatal: 2 lines add whitespace errors.

Using an editor that visualizes whitespaces and tabs might help here.

And empty lines at the end of a file. Please remove them and if your
editor is too stupid, then please pick a different editor.

Applying: Add PPP protocol support with HDLC framing
/data/devel/ofono/.git/rebase-apply/patch:166: new blank line at EOF.
+
/data/devel/ofono/.git/rebase-apply/patch:230: new blank line at EOF.
+
fatal: 2 lines add whitespace errors.

Before you submit any patches, you might wanna quickly check with git am
that they still apply. My .gitconfig uses a strict apply policy:

[diff]
	color = auto
	check = true
[apply]
	whitespace = error
[format-patch]
	check = true

Having diff and format-patch color your whitespace damages is always a
good idea. Then you see what is wrong.

Regards

Marcel