All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] PPP Patches v3
@ 2010-03-23  0:05 Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 1/6] Basic PPP protocol support Kristen Carlson Accardi
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:05 UTC (permalink / raw)
  To: ofono

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

Changes since v2 - corrected whitespace. 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/6] Basic PPP protocol support
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
@ 2010-03-23  0:05 ` Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 2/6] Generic PPP control " Kristen Carlson Accardi
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:05 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 22852 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 |   58 +++++++
 gatchat/ppp.c    |  454 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/ppp.h    |  130 ++++++++++++++++
 5 files changed, 778 insertions(+), 1 deletions(-)
 create mode 100644 gatchat/gatppp.c
 create mode 100644 gatchat/gatppp.h
 create mode 100644 gatchat/ppp.c
 create mode 100644 gatchat/ppp.h

diff --git a/Makefile.am b/Makefile.am
index eca8eee..6891d16 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -55,7 +55,9 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				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
 
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
new file mode 100644
index 0000000..7b68ff3
--- /dev/null
+++ b/gatchat/gatppp.c
@@ -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;
+}
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
new file mode 100644
index 0000000..0d5d5cc
--- /dev/null
+++ b/gatchat/gatppp.h
@@ -0,0 +1,58 @@
+/*
+ *
+ *  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 */
diff --git a/gatchat/ppp.c b/gatchat/ppp.c
new file mode 100644
index 0000000..db20d51
--- /dev/null
+++ b/gatchat/ppp.c
@@ -0,0 +1,454 @@
+/*
+ *
+ *  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);
+}
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
new file mode 100644
index 0000000..573c967
--- /dev/null
+++ b/gatchat/ppp.h
@@ -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);
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/6] Generic PPP control protocol support
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 1/6] Basic PPP protocol support Kristen Carlson Accardi
@ 2010-03-23  0:05 ` Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 3/6] PPP LCP support Kristen Carlson Accardi
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:05 UTC (permalink / raw)
  To: ofono

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

Implement a generic protocol that can be shared by both the LCP and the 
NCP implementation.
---
 Makefile.am      |    3 +-
 gatchat/ppp.h    |    2 +
 gatchat/ppp_cp.c | 1503 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/ppp_cp.h |  139 +++++
 4 files changed, 1646 insertions(+), 1 deletions(-)
 create mode 100644 gatchat/ppp_cp.c
 create mode 100644 gatchat/ppp_cp.h

diff --git a/Makefile.am b/Makefile.am
index 6891d16..a58f10e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -57,7 +57,8 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				gatchat/gat.h \
 				gatchat/gatserver.h gatchat/gatserver.c \
 				gatchat/gatppp.c gatchat/gatppp.h \
-				gatchat/ppp.c gatchat/ppp.h
+				gatchat/ppp.c gatchat/ppp.h gatchat/ppp_cp.h \
+				gatchat/ppp_cp.c
 
 udev_files = plugins/ofono.rules
 
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 573c967..0f39440 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -18,6 +18,8 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  *
  */
+#include "ppp_cp.h"
+
 #define DEFAULT_MRU	1500
 #define BUFFERSZ	DEFAULT_MRU*2
 #define DEFAULT_ACCM	0x00000000
diff --git a/gatchat/ppp_cp.c b/gatchat/ppp_cp.c
new file mode 100644
index 0000000..6967f9d
--- /dev/null
+++ b/gatchat/ppp_cp.c
@@ -0,0 +1,1503 @@
+/*
+ *
+ *  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 <glib.h>
+#include <arpa/inet.h>
+#include "gatppp.h"
+#include "ppp.h"
+
+#ifdef DEBUG
+const char *pppcp_state_strings[] =
+	{"INITIAL", "STARTING", "CLOSED", "STOPPED", "CLOSING", "STOPPING",
+	"REQSENT", "ACKRCVD", "ACKSENT", "OPENED" };
+
+#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)
+
+struct pppcp_event {
+	enum pppcp_event_type type;
+	gint len;
+	guint8 data[0];
+};
+
+#define INITIAL_RESTART_TIMEOUT	3000
+#define MAX_TERMINATE		2
+#define MAX_CONFIGURE		10
+#define MAX_FAILURE		5
+#define CP_HEADER_SZ		4
+
+static struct pppcp_packet *pppcp_packet_new(struct pppcp_data *data,
+						guint type, guint bufferlen)
+{
+	struct pppcp_packet *packet;
+	struct ppp_header *ppp_packet;
+	guint16 packet_length = bufferlen + sizeof(*packet);
+
+	ppp_packet = g_try_malloc0(packet_length + 2);
+	if (!ppp_packet)
+		return NULL;
+
+	/* add our protocol information */
+	ppp_packet->proto = htons(data->proto);
+
+	/* advance past protocol to add CP header information */
+	packet = (struct pppcp_packet *) (ppp_packet->info);
+
+	packet->length = htons(packet_length);
+	packet->code = type;
+	return packet;
+}
+
+static gboolean pppcp_timeout(gpointer user_data)
+{
+	struct pppcp_data *data = user_data;
+
+	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;
+}
+
+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 struct pppcp_event *pppcp_event_new(enum pppcp_event_type type,
+					gpointer event_data, guint len)
+{
+	struct pppcp_event *event;
+	guint8 *data = event_data;
+
+	event = g_try_malloc0(sizeof(struct pppcp_event) + len);
+	if (!event)
+		return NULL;
+
+	event->type = type;
+	memcpy(event->data, data, len);
+	event->len = len;
+	return event;
+}
+
+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_data)
+{
+	struct ppp_option *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_data)
+{
+	struct ppp_option *option = data;
+	guint8 *length = user_data;
+
+	*length += option->length;
+}
+
+static void copy_option(gpointer data, gpointer user_data)
+{
+	struct ppp_option *option = data;
+	guint8 **location = user_data;
+	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->ppp, 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->ppp, 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->ppp, 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->ppp, 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->ppp, 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->ppp, 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->ppp, 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->ppp, pppcp_to_ppp_packet((guint8 *) packet),
+			ntohs(packet->length));
+
+}
+
+static void pppcp_transition_state(enum pppcp_state 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;
+	case CLOSING:
+	case STOPPING:
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		break;
+	}
+	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;
+	case CLOSED:
+	case STOPPED:
+	case OPENED:
+	case CLOSING:
+	case STOPPING:
+	case REQSENT:
+	case ACKRCVD:
+	case ACKSENT:
+		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);
+	case INITIAL:
+	case STARTING:
+		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;
+	}
+}
+
+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;
+	}
+}
+
+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;
+	case INITIAL:
+	case STARTING:
+	case CLOSED:
+	case STOPPED:
+	case OPENED:
+		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;
+	case INITIAL:
+	case STARTING:
+	case CLOSED:
+	case STOPPED:
+	case OPENED:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		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;
+	case INITIAL:
+	case STARTING:
+		pppcp_illegal_event(data->state, RXR);
+	}
+}
+
+static void pppcp_handle_event(gpointer user_data)
+{
+	struct pppcp_event *event;
+	struct pppcp_data *data = user_data;
+
+	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);
+	}
+}
+
+/*
+ * send the event handler a new event to process
+ */
+void pppcp_generate_event(struct pppcp_data *data,
+				enum pppcp_event_type 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 gint is_option(gconstpointer a, gconstpointer b)
+{
+	const struct ppp_option *o = a;
+	guint8 otype = (guint8) GPOINTER_TO_UINT(b);
+
+	if (o->type == otype)
+		return 0;
+	else
+		return -1;
+}
+
+static void verify_config_option(gpointer elem, gpointer user_data)
+{
+	struct ppp_option *config = elem;
+	struct pppcp_data *data = user_data;
+	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),
+				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_try_malloc0(config->length);
+		if (option == NULL)
+			return;
+		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_data)
+{
+	struct ppp_option *config = elem;
+	struct pppcp_data *data = user_data;
+	GList *list;
+
+	/*
+	 * 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),
+				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;
+	enum option_rval rval = OPTION_ERR;
+	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_try_malloc0(olen);
+		if (option == NULL)
+			break;
+		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;
+		case OPTION_ERR:
+			g_printerr("unhandled option type %d\n", otype);
+		}
+		/* 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 ppp 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_try_malloc0(olen);
+		if (acked_option == NULL)
+			break;
+		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),
+				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 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;
+	enum option_rval rval = OPTION_ERR;
+	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_try_malloc0(olen);
+		if (naked_option == NULL)
+			break;
+		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),
+				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 = priv;
+	struct pppcp_packet *packet = (struct pppcp_packet *) new_packet;
+	guint8 event_type;
+	gpointer event_data = NULL;
+	guint data_len = 0;
+
+	if (data == NULL)
+		return;
+
+	/* 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)
+{
+	if (data == NULL)
+		return;
+
+	data->valid_codes = codes;
+}
+
+void pppcp_free(struct pppcp_data *data)
+{
+	if (data == NULL)
+		return;
+
+	/* 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(GAtPPP *ppp, guint16 proto,
+				gpointer priv)
+{
+	struct pppcp_data *data;
+
+	data = g_try_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->ppp = ppp;
+	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;
+}
diff --git a/gatchat/ppp_cp.h b/gatchat/ppp_cp.h
new file mode 100644
index 0000000..875d02f
--- /dev/null
+++ b/gatchat/ppp_cp.h
@@ -0,0 +1,139 @@
+/*
+ *
+ *  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
+ *
+ */
+
+struct pppcp_data;
+
+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_event_type {
+	UP,
+	DOWN,
+	OPEN,
+	CLOSE,
+	TO_PLUS,
+	TO_MINUS,
+	RCR_PLUS,
+	RCR_MINUS,
+	RCA,
+	RCN,
+	RTR,
+	RTA,
+	RUC,
+	RXJ_PLUS,
+	RXJ_MINUS,
+	RXR,
+};
+
+enum pppcp_state {
+	INITIAL,
+	STARTING,
+	CLOSED,
+	STOPPED,
+	CLOSING,
+	STOPPING,
+	REQSENT,
+	ACKRCVD,
+	ACKSENT,
+	OPENED,
+};
+
+/* option format */
+struct ppp_option {
+	guint8 type;
+	guint8 length;
+	guint8 data[0];
+};
+
+enum option_rval {
+	OPTION_ACCEPT,
+	OPTION_REJECT,
+	OPTION_NAK,
+	OPTION_ERR,
+};
+
+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);
+	enum option_rval (*option_scan)(struct ppp_option *option,
+						gpointer user_data);
+	void (*option_process)(gpointer option, gpointer user_data);
+};
+
+struct pppcp_packet {
+	guint8 code;
+	guint8 identifier;
+	guint16 length;
+	guint8 data[0];
+} __attribute__((packed));
+
+struct pppcp_data {
+	enum pppcp_state 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;
+	GAtPPP *ppp;
+	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(GAtPPP *ppp, 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,
+				enum pppcp_event_type event_type,
+				gpointer event_data, guint data_len);
+void pppcp_process_packet(gpointer priv, guint8 *new_packet);
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/6] PPP LCP support
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 1/6] Basic PPP protocol support Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 2/6] Generic PPP control " Kristen Carlson Accardi
@ 2010-03-23  0:05 ` Kristen Carlson Accardi
  2010-03-23  0:05 ` [PATCH 4/6] CHAP with MD5 authentication support Kristen Carlson Accardi
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:05 UTC (permalink / raw)
  To: ofono

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

Implement LCP support for the PPP protocol.
---
 Makefile.am       |    2 +-
 gatchat/gatppp.c  |    6 +-
 gatchat/ppp.c     |   47 ++++++++++
 gatchat/ppp.h     |    7 ++
 gatchat/ppp_lcp.c |  246 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 306 insertions(+), 2 deletions(-)
 create mode 100644 gatchat/ppp_lcp.c

diff --git a/Makefile.am b/Makefile.am
index a58f10e..1a444ca 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -58,7 +58,7 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				gatchat/gatserver.h gatchat/gatserver.c \
 				gatchat/gatppp.c gatchat/gatppp.h \
 				gatchat/ppp.c gatchat/ppp.h gatchat/ppp_cp.h \
-				gatchat/ppp_cp.c
+				gatchat/ppp_cp.c gatchat/ppp_lcp.c
 
 udev_files = plugins/ofono.rules
 
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 7b68ff3..f083842 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -40,6 +40,7 @@
 void g_at_ppp_open(GAtPPP *ppp)
 {
 	/* send an OPEN event to the lcp layer */
+	lcp_open(ppp->lcp);
 }
 
 void g_at_ppp_set_connect_function(GAtPPP *ppp,
@@ -69,6 +70,9 @@ void g_at_ppp_shutdown(GAtPPP *ppp)
 	/* cleanup modem channel */
 	g_source_remove(ppp->modem_watch);
 	g_io_channel_unref(ppp->modem);
+
+	/* remove lcp */
+	lcp_free(ppp->lcp);
 }
 
 void g_at_ppp_ref(GAtPPP *ppp)
@@ -117,7 +121,7 @@ GAtPPP *g_at_ppp_new(GIOChannel *modem)
 	ppp->index = 0;
 
 	/* initialize the lcp state */
-
+	ppp->lcp = lcp_new(ppp);
 
 	/* initialize the autentication state */
 
diff --git a/gatchat/ppp.c b/gatchat/ppp.c
index db20d51..2399ed4 100644
--- a/gatchat/ppp.c
+++ b/gatchat/ppp.c
@@ -350,16 +350,19 @@ gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
 void ppp_close(GAtPPP *ppp)
 {
 	/* send a CLOSE event to the lcp layer */
+	lcp_close(ppp->lcp);
 }
 
 static void ppp_link_establishment(GAtPPP *ppp)
 {
 	/* signal UP event to LCP */
+	lcp_establish(ppp->lcp);
 }
 
 static void ppp_terminate(GAtPPP *ppp)
 {
 	/* signal DOWN event to LCP */
+	lcp_terminate(ppp->lcp);
 }
 
 static void ppp_authenticate(GAtPPP *ppp)
@@ -452,3 +455,47 @@ 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);
 }
+
+void ppp_set_auth(GAtPPP *ppp, guint8* auth_data)
+{
+	guint16 proto = get_host_short(auth_data);
+
+	switch (proto) {
+	case CHAP_PROTOCOL:
+		/* get the algorithm */
+		break;
+	default:
+		g_printerr("unknown authentication proto\n");
+		break;
+	}
+}
+
+void ppp_set_recv_accm(GAtPPP *ppp, guint32 accm)
+{
+	ppp->recv_accm = accm;
+}
+
+guint32 ppp_get_xmit_accm(GAtPPP *ppp)
+{
+	return ppp->xmit_accm[0];
+}
+
+void ppp_set_pfc(GAtPPP *ppp, gboolean pfc)
+{
+	ppp->pfc = pfc;
+}
+
+gboolean ppp_get_pfc(GAtPPP *ppp)
+{
+	return ppp->pfc;
+}
+
+void ppp_set_acfc(GAtPPP *ppp, gboolean acfc)
+{
+	ppp->acfc = acfc;
+}
+
+gboolean ppp_get_acfc(GAtPPP *ppp)
+{
+	return ppp->acfc;
+}
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 0f39440..7753a39 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -98,6 +98,7 @@ static inline guint16 __get_unaligned_short(const gpointer p)
 struct _GAtPPP {
 	gint ref_count;
 	enum ppp_phase phase;
+	struct pppcp_data *lcp;
 	guint8 buffer[BUFFERSZ];
 	int index;
 	gint mru;
@@ -130,3 +131,9 @@ 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);
+struct pppcp_data * lcp_new(GAtPPP *ppp);
+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);
diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
new file mode 100644
index 0000000..644842a
--- /dev/null
+++ b/gatchat/ppp_lcp.c
@@ -0,0 +1,246 @@
+/*
+ *
+ *  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 <glib.h>
+#include <arpa/inet.h>
+#include "gatppp.h"
+#include "ppp.h"
+
+enum lcp_options {
+	RESERVED 	= 0,
+	MRU		= 1,
+	ACCM		= 2,
+	AUTH_PROTO	= 3,
+	QUAL_PROTO	= 4,
+	MAGIC_NUMBER	= 5,
+	DEPRECATED_QUAL_PROTO	= 6,
+	PFC			= 7,
+	ACFC			= 8,
+};
+
+#define 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->ppp, PPP_OPENED);
+}
+
+/*
+ * signal the Down event to the NCP
+ */
+static void lcp_down(struct pppcp_data *pppcp)
+{
+	ppp_generate_event(pppcp->ppp, 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->ppp, 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->ppp, PPP_CLOSING);
+}
+
+/*
+ * Scan the option to see if it is acceptable, unacceptable, or rejected
+ *
+ * We need to use a default case here because this option type value
+ * could be anything.
+ */
+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;
+		break;
+	default:
+		return OPTION_REJECT;
+	}
+}
+
+/*
+ * act on an acceptable option
+ *
+ * We need to use a default case here because this option type value
+ * could be anything.
+ */
+static void lcp_option_process(gpointer data, gpointer user)
+{
+	struct ppp_option *option = data;
+	struct pppcp_data *pppcp = user;
+	GAtPPP *ppp = pppcp->ppp;
+	guint32 magic;
+
+	switch (option->type) {
+	case ACCM:
+		ppp_set_recv_accm(ppp, get_host_long(option->data));
+		break;
+	case AUTH_PROTO:
+		ppp_set_auth(ppp, option->data);
+		break;
+	case MAGIC_NUMBER:
+		/* XXX handle loopback */
+		magic = get_host_long(option->data);
+		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(ppp, TRUE);
+		break;
+	case ACFC:
+		ppp_set_acfc(ppp, TRUE);
+		break;
+	default:
+		g_printerr("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)
+{
+	if (data == NULL)
+		return;
+
+	/* send an open event to the lcp layer */
+	pppcp_generate_event(data, OPEN, NULL, 0);
+}
+
+void lcp_close(struct pppcp_data *data)
+{
+	if (data == NULL)
+		return;
+
+	/* send a CLOSE  event to the lcp layer */
+	pppcp_generate_event(data, CLOSE, NULL, 0);
+}
+
+void lcp_establish(struct pppcp_data *data)
+{
+	if (data == NULL)
+		return;
+
+	/* send an UP event to the lcp layer */
+	pppcp_generate_event(data, UP, NULL, 0);
+}
+
+void lcp_terminate(struct pppcp_data *data)
+{
+	if (data == NULL)
+		return;
+
+	/* send a DOWN event to the lcp layer */
+	pppcp_generate_event(data, DOWN, NULL, 0);
+}
+
+void lcp_free(struct pppcp_data *lcp)
+{
+	if (lcp == NULL)
+		return;
+
+	/* TBD unregister packet handler */
+
+	pppcp_free(lcp);
+}
+
+struct pppcp_data * lcp_new(GAtPPP *ppp)
+{
+	struct pppcp_data *pppcp;
+	struct ppp_option *option;
+	guint16 codes = LCP_SUPPORTED_CODES;
+
+	pppcp = pppcp_new(ppp, 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_try_malloc0(6);
+	if (option == NULL) {
+		pppcp_free(pppcp);
+		return NULL;
+	}
+	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;
+}
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/6] CHAP with MD5 authentication support
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
                   ` (2 preceding siblings ...)
  2010-03-23  0:05 ` [PATCH 3/6] PPP LCP support Kristen Carlson Accardi
@ 2010-03-23  0:05 ` Kristen Carlson Accardi
  2010-03-23  0:06 ` [PATCH 5/6] IP support for PPP Kristen Carlson Accardi
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:05 UTC (permalink / raw)
  To: ofono

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

Authentication support with CHAP and MD5
---
 Makefile.am        |    3 +-
 gatchat/gatppp.c   |   11 +++-
 gatchat/gatppp.h   |    2 +
 gatchat/ppp.c      |    1 +
 gatchat/ppp.h      |   15 ++++
 gatchat/ppp_auth.c |  229 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 259 insertions(+), 2 deletions(-)
 create mode 100644 gatchat/ppp_auth.c

diff --git a/Makefile.am b/Makefile.am
index 1a444ca..df89ef5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -58,7 +58,8 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				gatchat/gatserver.h gatchat/gatserver.c \
 				gatchat/gatppp.c gatchat/gatppp.h \
 				gatchat/ppp.c gatchat/ppp.h gatchat/ppp_cp.h \
-				gatchat/ppp_cp.c gatchat/ppp_lcp.c
+				gatchat/ppp_cp.c gatchat/ppp_lcp.c \
+				gatchat/ppp_auth.c
 
 udev_files = plugins/ofono.rules
 
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index f083842..2b682f8 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -43,6 +43,12 @@ void g_at_ppp_open(GAtPPP *ppp)
 	lcp_open(ppp->lcp);
 }
 
+void g_at_ppp_set_credentials(GAtPPP *ppp, const char *username,
+				const char *passwd)
+{
+	auth_set_credentials(ppp->auth, username, passwd);
+}
+
 void g_at_ppp_set_connect_function(GAtPPP *ppp,
 			       GAtPPPConnectFunc callback, gpointer user_data)
 {
@@ -73,6 +79,9 @@ void g_at_ppp_shutdown(GAtPPP *ppp)
 
 	/* remove lcp */
 	lcp_free(ppp->lcp);
+
+	/* remove auth */
+	auth_free(ppp->auth);
 }
 
 void g_at_ppp_ref(GAtPPP *ppp)
@@ -124,7 +133,7 @@ GAtPPP *g_at_ppp_new(GIOChannel *modem)
 	ppp->lcp = lcp_new(ppp);
 
 	/* initialize the autentication state */
-
+	ppp->auth = auth_new(ppp);
 
 	/* intialize the network state */
 
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index 0d5d5cc..8db26c9 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -51,6 +51,8 @@ void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
 void g_at_ppp_shutdown(GAtPPP *ppp);
 void g_at_ppp_ref(GAtPPP *ppp);
 void g_at_ppp_unref(GAtPPP *ppp);
+void g_at_ppp_set_credentials(GAtPPP *ppp, const char *username,
+				const char *passwd);
 #ifdef __cplusplus
 }
 #endif
diff --git a/gatchat/ppp.c b/gatchat/ppp.c
index 2399ed4..0b3221b 100644
--- a/gatchat/ppp.c
+++ b/gatchat/ppp.c
@@ -463,6 +463,7 @@ void ppp_set_auth(GAtPPP *ppp, guint8* auth_data)
 	switch (proto) {
 	case CHAP_PROTOCOL:
 		/* get the algorithm */
+		auth_set_proto(ppp->auth, proto, auth_data[2]);
 		break;
 	default:
 		g_printerr("unknown authentication proto\n");
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 7753a39..53d5274 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -95,10 +95,20 @@ static inline guint16 __get_unaligned_short(const gpointer p)
 #define ppp_proto(packet) \
 	(get_host_short(packet + 2))
 
+struct auth_data {
+	guint16 proto;
+	gpointer proto_data;
+	void (*process_packet)(struct auth_data *data, guint8 *packet);
+	char *username;
+	char *passwd;
+	GAtPPP *ppp;
+};
+
 struct _GAtPPP {
 	gint ref_count;
 	enum ppp_phase phase;
 	struct pppcp_data *lcp;
+	struct auth_data *auth;
 	guint8 buffer[BUFFERSZ];
 	int index;
 	gint mru;
@@ -137,3 +147,8 @@ 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);
+void auth_set_credentials(struct auth_data *data, const char *username,
+				const char *passwd);
+void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);
+struct auth_data *auth_new(GAtPPP *ppp);
+void auth_free(struct auth_data *auth);
diff --git a/gatchat/ppp_auth.c b/gatchat/ppp_auth.c
new file mode 100644
index 0000000..c23d9ad
--- /dev/null
+++ b/gatchat/ppp_auth.c
@@ -0,0 +1,229 @@
+/*
+ *
+ *  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 "gatppp.h"
+#include "ppp.h"
+
+struct chap_header {
+	guint8 code;
+	guint8 identifier;
+	guint16 length;
+	guint8 data[0];
+} __attribute__((packed));
+
+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, const char *username,
+				const char *passwd)
+{
+	if (data == NULL)
+		return;
+
+	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;
+	struct ppp_header *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_try_malloc0(response_length + 2);
+	if (!ppp_packet)
+		goto challenge_out;
+
+	/* add our protocol information */
+	ppp_packet->proto = htons(CHAP_PROTOCOL);
+	response = (struct chap_header *) &ppp_packet->info;
+	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->ppp, (guint8 *) 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->ppp, 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 = 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_try_malloc0(sizeof(*data));
+	if (!data)
+		return NULL;
+
+	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)
+{
+	if (data == NULL)
+		return;
+
+	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)
+{
+	if (data == NULL)
+		return;
+
+	chap_free(data);
+	g_free(data);
+}
+
+struct auth_data *auth_new(GAtPPP *ppp)
+{
+	struct auth_data *data;
+
+	data = g_try_malloc0(sizeof(*data));
+	if (!data)
+		return NULL;
+
+	data->ppp = ppp;
+	return data;
+}
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/6] IP support for PPP.
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
                   ` (3 preceding siblings ...)
  2010-03-23  0:05 ` [PATCH 4/6] CHAP with MD5 authentication support Kristen Carlson Accardi
@ 2010-03-23  0:06 ` Kristen Carlson Accardi
  2010-03-23  0:06 ` [PATCH 6/6] Add PPP option to gsmdial Kristen Carlson Accardi
  2010-03-23  0:32 ` [PATCH 0/6] PPP Patches v3 Marcel Holtmann
  6 siblings, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:06 UTC (permalink / raw)
  To: ofono

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

Adds IPCP support, and creates a TUN interface for sending/receiving IP
packets.
---
 Makefile.am       |    2 +-
 gatchat/gatppp.c  |    1 +
 gatchat/ppp.c     |    1 +
 gatchat/ppp.h     |   12 ++
 gatchat/ppp_net.c |  356 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 371 insertions(+), 1 deletions(-)
 create mode 100644 gatchat/ppp_net.c

diff --git a/Makefile.am b/Makefile.am
index df89ef5..0eaadda 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -59,7 +59,7 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				gatchat/gatppp.c gatchat/gatppp.h \
 				gatchat/ppp.c gatchat/ppp.h gatchat/ppp_cp.h \
 				gatchat/ppp_cp.c gatchat/ppp_lcp.c \
-				gatchat/ppp_auth.c
+				gatchat/ppp_auth.c gatchat/ppp_net.c
 
 udev_files = plugins/ofono.rules
 
diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 2b682f8..68c4dd1 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -136,6 +136,7 @@ GAtPPP *g_at_ppp_new(GIOChannel *modem)
 	ppp->auth = auth_new(ppp);
 
 	/* intialize the network state */
+	ppp->net = ppp_net_new(ppp);
 
 	/* start listening for packets from the modem */
 	ppp->modem_watch = g_io_add_watch(modem,
diff --git a/gatchat/ppp.c b/gatchat/ppp.c
index 0b3221b..de0d5ab 100644
--- a/gatchat/ppp.c
+++ b/gatchat/ppp.c
@@ -381,6 +381,7 @@ static void ppp_dead(GAtPPP *ppp)
 static void ppp_network(GAtPPP *ppp)
 {
 	/* bring network phase up */
+	ppp_net_open(ppp->net);
 }
 
 static void ppp_transition_phase(GAtPPP *ppp, enum ppp_phase phase)
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 53d5274..b6797f5 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -104,11 +104,19 @@ struct auth_data {
 	GAtPPP *ppp;
 };
 
+struct ppp_net_data {
+	GAtPPP *ppp;
+	char *if_name;
+	GIOChannel *channel;
+	struct pppcp_data *ipcp;
+};
+
 struct _GAtPPP {
 	gint ref_count;
 	enum ppp_phase phase;
 	struct pppcp_data *lcp;
 	struct auth_data *auth;
+	struct ppp_net_data *net;
 	guint8 buffer[BUFFERSZ];
 	int index;
 	gint mru;
@@ -152,3 +160,7 @@ void auth_set_credentials(struct auth_data *data, const char *username,
 void auth_set_proto(struct auth_data *data, guint16 proto, guint8 method);
 struct auth_data *auth_new(GAtPPP *ppp);
 void auth_free(struct auth_data *auth);
+struct ppp_net_data *ppp_net_new(GAtPPP *ppp);
+void ppp_net_open(struct ppp_net_data *data);
+void ppp_net_free(struct ppp_net_data *data);
+void ppp_net_close(struct ppp_net_data *data);
diff --git a/gatchat/ppp_net.c b/gatchat/ppp_net.c
new file mode 100644
index 0000000..a873c00
--- /dev/null
+++ b/gatchat/ppp_net.c
@@ -0,0 +1,356 @@
+/*
+ *
+ *  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 <net/if.h>
+#include <linux/if_tun.h>
+#include <sys/ioctl.h>
+#include <glib.h>
+
+#include "gatutil.h"
+#include "gatppp.h"
+#include "ppp.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(GAtPPP *ppp);
+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 ppp_net_data *data = priv;
+	GError *error = NULL;
+	GIOStatus status;
+	gsize bytes_written;
+	guint16 len;
+
+	/*
+	 * since ppp_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 = get_host_short(&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 ppp_net_callback(GIOChannel *channel, GIOCondition cond,
+				gpointer userdata)
+{
+	GIOStatus status;
+	gchar buf[MAX_PACKET + 2];
+	gsize bytes_read;
+	GError *error = NULL;
+	struct ppp_header *ppp = (struct ppp_header *) buf;
+	struct ppp_net_data *data = (struct ppp_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) {
+			ppp->proto = htons(PPP_IP_PROTO);
+			ppp_transmit(data->ppp, (guint8 *) buf, bytes_read);
+		}
+		if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+			return FALSE;
+	}
+	return TRUE;
+}
+
+void ppp_net_close(struct ppp_net_data *data)
+{
+	/* Not Implemented Yet */
+}
+
+void ppp_net_open(struct ppp_net_data *data)
+{
+	int fd;
+	struct ifreq ifr;
+	GIOChannel *channel;
+	int signal_source;
+	int err;
+
+	if (data == NULL)
+		return;
+
+	/* 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;
+	}
+	if (!g_at_util_setup_io(channel, G_IO_FLAG_NONBLOCK)) {
+		g_io_channel_unref(channel);
+		return;
+	}
+	data->channel = channel;
+	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,
+			ppp_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 ppp_net_free(struct ppp_net_data *data)
+{
+	/* TBD unregister packet handler */
+
+	/* cleanup tun interface */
+	ppp_net_close(data);
+
+	/* free ipcp data */
+	ipcp_free(data->ipcp);
+
+	/* free self */
+	g_free(data);
+}
+
+struct ppp_net_data *ppp_net_new(GAtPPP *ppp)
+{
+	struct ppp_net_data *data;
+
+	data = g_try_malloc0(sizeof(*data));
+	if (!data)
+		return NULL;
+
+	data->ppp = ppp;
+	data->ipcp = ipcp_new(ppp);
+
+	/* register packet handler for IP protocol */
+	ip_packet_handler.priv = data;
+	ppp_register_packet_handler(&ip_packet_handler);
+	return data;
+}
+
+/****** IPCP support ****************/
+#define IPCP_SUPPORTED_CODES	  ((1 << CONFIGURE_REQUEST) | \
+				  (1 << CONFIGURE_ACK) | \
+				  (1 << CONFIGURE_NAK) | \
+				  (1 << CONFIGURE_REJECT) | \
+				  (1 << TERMINATE_REQUEST) | \
+				  (1 << TERMINATE_ACK) | \
+				  (1 << CODE_REJECT))
+
+#define IPCP_PROTO		0x8021
+
+enum ipcp_option_types {
+	IP_ADDRESSES		= 1,
+	IP_COMPRESSION_PROTO	= 2,
+	IP_ADDRESS		= 3,
+	PRIMARY_DNS_SERVER	= 129,
+	SECONDARY_DNS_SERVER	= 131,
+};
+
+static void ipcp_up(struct pppcp_data *pppcp)
+{
+	struct ipcp_data *data = pppcp->priv;
+	GAtPPP *ppp = pppcp->ppp;
+
+	/* call the connect function */
+	if (ppp->connect_cb)
+		ppp->connect_cb(ppp, G_AT_PPP_CONNECT_SUCCESS,
+				__get_unaligned_long(data->ip_address),
+				__get_unaligned_long(data->primary_dns),
+				__get_unaligned_long(data->secondary_dns),
+				ppp->connect_data);
+}
+
+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 = data;
+	struct ipcp_data *ipcp = 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(GAtPPP *ppp)
+{
+	struct ipcp_data *data;
+	struct pppcp_data *pppcp;
+	struct ppp_option *ipcp_option;
+
+	data = g_try_malloc0(sizeof(*data));
+	if (!data)
+		return NULL;
+
+	pppcp = pppcp_new(ppp, 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_try_malloc0(6);
+	if (!ipcp_option) {
+		pppcp_free(pppcp);
+		g_free(data);
+		return NULL;
+	}
+	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;
+}
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 6/6] Add PPP option to gsmdial
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
                   ` (4 preceding siblings ...)
  2010-03-23  0:06 ` [PATCH 5/6] IP support for PPP Kristen Carlson Accardi
@ 2010-03-23  0:06 ` Kristen Carlson Accardi
  2010-03-23  5:30   ` Gustavo F. Padovan
  2010-03-23  0:32 ` [PATCH 0/6] PPP Patches v3 Marcel Holtmann
  6 siblings, 1 reply; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  0:06 UTC (permalink / raw)
  To: ofono

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

Implement new options for gsmdial to use PPP and set the user name and
password for authentication if needed.
---
 gatchat/gsmdial.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 2087e70..aee9eea 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -36,6 +36,7 @@
 #include <glib.h>
 #include <gatchat.h>
 #include <gattty.h>
+#include <gatppp.h>
 
 static const char *none_prefix[] = { NULL };
 static const char *cgreg_prefix[] = { "+CGREG:", NULL };
@@ -48,7 +49,11 @@ static gint option_cid = 0;
 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_password = NULL;
 
+static GAtPPP *ppp;
 static GAtChat *control;
 static GAtChat *modem;
 static GMainLoop *event_loop;
@@ -223,6 +228,76 @@ static void at_cgact_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	g_at_chat_send(modem, buf, none_prefix, NULL, NULL, NULL);
 }
 
+static void print_ip_address(const char *label, guint32 ip_addr)
+{
+	struct in_addr addr;
+	char buf[INET_ADDRSTRLEN];
+
+	addr.s_addr = ip_addr;
+
+	if (inet_ntop(AF_INET, &addr, buf, INET_ADDRSTRLEN))
+		g_print("%s: %s\n", label, buf);
+}
+
+static void ppp_connect(GAtPPP *ppp, GAtPPPConnectStatus success,
+			guint32 ip_addr, guint32 dns1, guint32 dns2,
+			gpointer user_data)
+{
+	if (success != G_AT_PPP_CONNECT_SUCCESS) {
+		g_print("Failed to create PPP interface!\n");
+		return;
+	}
+
+	/* print out the negotiated address and dns server */
+	print_ip_address("IP Address", ip_addr);
+	print_ip_address("Primary DNS Server", dns1);
+	print_ip_address("Secondary DNS Server", dns2);
+}
+
+static void ppp_disconnect(GAtPPP *ppp, 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);
+	}
+
+	if (option_ppp == FALSE)
+		return;
+
+	/* get the data IO channel */
+	channel = g_at_chat_get_channel(modem);
+
+	/*
+	 * shutdown gatchat or else it tries to take all the input
+	 * from the modem and does not let PPP get it.
+	 */
+	g_at_chat_shutdown(control);
+	g_at_chat_shutdown(modem);
+
+	/* open ppp */
+	ppp = g_at_ppp_new(channel);
+	if (!ppp) {
+		g_print("Unable to create PPP object\n");
+		return;
+	}
+	g_at_ppp_set_credentials(ppp, option_username,
+				option_password);
+
+	/* 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];
@@ -233,9 +308,9 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	}
 
 	if (option_legacy == TRUE) {
-			sprintf(buf, "ATD*99***%u#", option_cid);
-			g_at_chat_send(modem, buf, none_prefix,
-					NULL, NULL, NULL);
+		sprintf(buf, "ATD*99***%u#", option_cid);
+		g_at_chat_send(modem, buf, none_prefix,
+				connect_cb, NULL, NULL);
 	} else {
 		sprintf(buf, "AT+CGACT=1,%u", option_cid);
 		g_at_chat_send(control, buf, none_prefix,
@@ -452,6 +527,12 @@ static GOptionEntry options[] = {
 				"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_password,
+				"Specifiy PPP password" },
 	{ NULL },
 };
 
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
                   ` (5 preceding siblings ...)
  2010-03-23  0:06 ` [PATCH 6/6] Add PPP option to gsmdial Kristen Carlson Accardi
@ 2010-03-23  0:32 ` Marcel Holtmann
  2010-03-23  2:22   ` Marcel Holtmann
  6 siblings, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  0:32 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> Changes since v2 - corrected whitespace. 

all the patches have been applied.

So form now on please send smaller patches as soon as you have them
ready and tested. If you need comments or have questions, you can use
the mailing list as well.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  0:32 ` [PATCH 0/6] PPP Patches v3 Marcel Holtmann
@ 2010-03-23  2:22   ` Marcel Holtmann
  2010-03-23  3:22     ` Kristen Carlson Accardi
  2010-03-23 12:34     ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  0 siblings, 2 replies; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  2:22 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > Changes since v2 - corrected whitespace. 
> 
> all the patches have been applied.
> 
> So form now on please send smaller patches as soon as you have them
> ready and tested. If you need comments or have questions, you can use
> the mailing list as well.

some additional comments from my side.

If you have variables that are only valid inside the scope of a
statement, then I prefer if you only declare them there. This makes the
code a bit simpler to read. Check the patch I just pushed to give you an
example.

Also please find access to a 32-bit system if you are running 64-bit or
vice versa. Our build system is strict and will turn turn all casting
mistakes in an error.

Please be present on #ofono IRC as much as possible so people can report
such things. Remember that your code is now part of the tree and it can
break the build.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  2:22   ` Marcel Holtmann
@ 2010-03-23  3:22     ` Kristen Carlson Accardi
  2010-03-23  3:47       ` Marcel Holtmann
  2010-03-23  3:56       ` Kristen Carlson Accardi
  2010-03-23 12:34     ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  1 sibling, 2 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  3:22 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 19:22:00 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Kristen,
> 
> > > Changes since v2 - corrected whitespace. 
> > 
> > all the patches have been applied.
> > 
> > So form now on please send smaller patches as soon as you have them
> > ready and tested. If you need comments or have questions, you can use
> > the mailing list as well.
> 
> some additional comments from my side.
> 
> If you have variables that are only valid inside the scope of a
> statement, then I prefer if you only declare them there. This makes the
> code a bit simpler to read. Check the patch I just pushed to give you an
> example.
> 
> Also please find access to a 32-bit system if you are running 64-bit or
> vice versa. Our build system is strict and will turn turn all casting
> mistakes in an error.
> 
> Please be present on #ofono IRC as much as possible so people can report
> such things. Remember that your code is now part of the tree and it can
> break the build.
> 
> Regards
> 
> Marcel
> 
> 

Marcel - I just started reviewing the changes you made and the first 
patch I hit I discovered that you broke something.  Would you like to
revert the patches and let me review them first, or should I just send 
a patch to the mailing list to fix what you broke?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  3:22     ` Kristen Carlson Accardi
@ 2010-03-23  3:47       ` Marcel Holtmann
  2010-03-23  4:07         ` Kristen Carlson Accardi
  2010-03-23  3:56       ` Kristen Carlson Accardi
  1 sibling, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  3:47 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > > > Changes since v2 - corrected whitespace. 
> > > 
> > > all the patches have been applied.
> > > 
> > > So form now on please send smaller patches as soon as you have them
> > > ready and tested. If you need comments or have questions, you can use
> > > the mailing list as well.
> > 
> > some additional comments from my side.
> > 
> > If you have variables that are only valid inside the scope of a
> > statement, then I prefer if you only declare them there. This makes the
> > code a bit simpler to read. Check the patch I just pushed to give you an
> > example.
> > 
> > Also please find access to a 32-bit system if you are running 64-bit or
> > vice versa. Our build system is strict and will turn turn all casting
> > mistakes in an error.
> > 
> > Please be present on #ofono IRC as much as possible so people can report
> > such things. Remember that your code is now part of the tree and it can
> > break the build.
>
> I just started reviewing the changes you made and the first 
> patch I hit I discovered that you broke something.  Would you like to
> revert the patches and let me review them first, or should I just send 
> a patch to the mailing list to fix what you broke?

send patches to the mailing list to fix them. The PPP code is in
development so it is just fine. I prefer to keep reverts for code that
is suppose to be stable.

If I did break something, then I am worried that the code gets too
complex. Such little changes should not have broken anything. Except I
made a stupid typo or thinking mistake (which happens of course). If it
fundamentally changes the code flow, then we do have a bigger problem.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  3:22     ` Kristen Carlson Accardi
  2010-03-23  3:47       ` Marcel Holtmann
@ 2010-03-23  3:56       ` Kristen Carlson Accardi
  1 sibling, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  3:56 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 20:22:02 -0700
Kristen Carlson Accardi <kristen@linux.intel.com> wrote:

> On Mon, 22 Mar 2010 19:22:00 -0700
> Marcel Holtmann <marcel@holtmann.org> wrote:
> 
> > Hi Kristen,
> > 
> > > > Changes since v2 - corrected whitespace. 
> > > 
> > > all the patches have been applied.
> > > 
> > > So form now on please send smaller patches as soon as you have them
> > > ready and tested. If you need comments or have questions, you can use
> > > the mailing list as well.
> > 
> > some additional comments from my side.
> > 
> > If you have variables that are only valid inside the scope of a
> > statement, then I prefer if you only declare them there. This makes the
> > code a bit simpler to read. Check the patch I just pushed to give you an
> > example.
> > 
> > Also please find access to a 32-bit system if you are running 64-bit or
> > vice versa. Our build system is strict and will turn turn all casting
> > mistakes in an error.
> > 
> > Please be present on #ofono IRC as much as possible so people can report
> > such things. Remember that your code is now part of the tree and it can
> > break the build.
> > 
> > Regards
> > 
> > Marcel
> > 
> > 
> 
> Marcel - I just started reviewing the changes you made and the first 
> patch I hit I discovered that you broke something.  Would you like to
> revert the patches and let me review them first, or should I just send 
> a patch to the mailing list to fix what you broke?

Here's the commits to revert:

a09d38643b8da50c8703fd843be26553f294f0bb
e396b7d5afcce6d90d5836d57f7b800a1b6b7b52

I'd be glad to clean them up for you later if you like.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  3:47       ` Marcel Holtmann
@ 2010-03-23  4:07         ` Kristen Carlson Accardi
  2010-03-23  4:51           ` Marcel Holtmann
  0 siblings, 1 reply; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  4:07 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 20:47:16 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Kristen,
> 
> > > > > Changes since v2 - corrected whitespace. 
> > > > 
> > > > all the patches have been applied.
> > > > 
> > > > So form now on please send smaller patches as soon as you have them
> > > > ready and tested. If you need comments or have questions, you can use
> > > > the mailing list as well.
> > > 
> > > some additional comments from my side.
> > > 
> > > If you have variables that are only valid inside the scope of a
> > > statement, then I prefer if you only declare them there. This makes the
> > > code a bit simpler to read. Check the patch I just pushed to give you an
> > > example.
> > > 
> > > Also please find access to a 32-bit system if you are running 64-bit or
> > > vice versa. Our build system is strict and will turn turn all casting
> > > mistakes in an error.
> > > 
> > > Please be present on #ofono IRC as much as possible so people can report
> > > such things. Remember that your code is now part of the tree and it can
> > > break the build.
> >
> > I just started reviewing the changes you made and the first 
> > patch I hit I discovered that you broke something.  Would you like to
> > revert the patches and let me review them first, or should I just send 
> > a patch to the mailing list to fix what you broke?
> 
> send patches to the mailing list to fix them. The PPP code is in
> development so it is just fine. I prefer to keep reverts for code that
> is suppose to be stable.
> 
> If I did break something, then I am worried that the code gets too
> complex. Such little changes should not have broken anything. Except I
> made a stupid typo or thinking mistake (which happens of course). If it
> fundamentally changes the code flow, then we do have a bigger problem.
> 
> Regards
> 
> Marcel
> 
> 

I wanted this code to at least be able to send a packet over the wire,
and with your changes we can't do that anymore.  The problem is that
your little cosmetic change really did change the flow, simply because
you didn't think through what you were doing.  Of course it is up to
you if you want to have this code be completely non-functional rather
than simply doing the revert.  I'll be glad to submit a patch later
which fixes your fix and adds some comments so you know what is 
actually happening.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  4:07         ` Kristen Carlson Accardi
@ 2010-03-23  4:51           ` Marcel Holtmann
  2010-03-23  6:14             ` Kristen Carlson Accardi
  0 siblings, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  4:51 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > > > > > Changes since v2 - corrected whitespace. 
> > > > > 
> > > > > all the patches have been applied.
> > > > > 
> > > > > So form now on please send smaller patches as soon as you have them
> > > > > ready and tested. If you need comments or have questions, you can use
> > > > > the mailing list as well.
> > > > 
> > > > some additional comments from my side.
> > > > 
> > > > If you have variables that are only valid inside the scope of a
> > > > statement, then I prefer if you only declare them there. This makes the
> > > > code a bit simpler to read. Check the patch I just pushed to give you an
> > > > example.
> > > > 
> > > > Also please find access to a 32-bit system if you are running 64-bit or
> > > > vice versa. Our build system is strict and will turn turn all casting
> > > > mistakes in an error.
> > > > 
> > > > Please be present on #ofono IRC as much as possible so people can report
> > > > such things. Remember that your code is now part of the tree and it can
> > > > break the build.
> > >
> > > I just started reviewing the changes you made and the first 
> > > patch I hit I discovered that you broke something.  Would you like to
> > > revert the patches and let me review them first, or should I just send 
> > > a patch to the mailing list to fix what you broke?
> > 
> > send patches to the mailing list to fix them. The PPP code is in
> > development so it is just fine. I prefer to keep reverts for code that
> > is suppose to be stable.
> > 
> > If I did break something, then I am worried that the code gets too
> > complex. Such little changes should not have broken anything. Except I
> > made a stupid typo or thinking mistake (which happens of course). If it
> > fundamentally changes the code flow, then we do have a bigger problem.
>
> I wanted this code to at least be able to send a packet over the wire,
> and with your changes we can't do that anymore.  The problem is that
> your little cosmetic change really did change the flow, simply because
> you didn't think through what you were doing.  Of course it is up to
> you if you want to have this code be completely non-functional rather
> than simply doing the revert.  I'll be glad to submit a patch later
> which fixes your fix and adds some comments so you know what is 
> actually happening.

I am not doing the revert since I do wanna figure out what broke it. And
actually I wanna have that recorded in the tree.

So Denis and I looked through my changes and he figured out what might
have broken it. So your attempt at doing code flow optimization with
g_list_length() throw me on the wrong error path. Please don't do this
since all the GLib functions will do proper empty list checking.

I fixed this now in the tree and it should restore it to a working code
base. I prefer if you don't do things like GList *list = NULL. These
initialization of variables lead to some complex code flow. And if you
do something wrong, the compiler will never warn.

Also in general it is preferred to do list == NULL checking instead of
using g_list_legnth() functions.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/6] Add PPP option to gsmdial
  2010-03-23  0:06 ` [PATCH 6/6] Add PPP option to gsmdial Kristen Carlson Accardi
@ 2010-03-23  5:30   ` Gustavo F. Padovan
  2010-03-23  6:30     ` Marcel Holtmann
  0 siblings, 1 reply; 26+ messages in thread
From: Gustavo F. Padovan @ 2010-03-23  5:30 UTC (permalink / raw)
  To: ofono

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

* Kristen Carlson Accardi <kristen@linux.intel.com> [2010-03-22 17:06:01 -0700]:

> Implement new options for gsmdial to use PPP and set the user name and
> password for authentication if needed.
> ---
>  gatchat/gsmdial.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
> index 2087e70..aee9eea 100644
> --- a/gatchat/gsmdial.c
> +++ b/gatchat/gsmdial.c
> @@ -36,6 +36,7 @@
>  #include <glib.h>
>  #include <gatchat.h>
>  #include <gattty.h>
> +#include <gatppp.h>
>  
>  static const char *none_prefix[] = { NULL };
>  static const char *cgreg_prefix[] = { "+CGREG:", NULL };
> @@ -48,7 +49,11 @@ static gint option_cid = 0;
>  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_password = NULL;
>  
> +static GAtPPP *ppp;
>  static GAtChat *control;
>  static GAtChat *modem;
>  static GMainLoop *event_loop;
> @@ -223,6 +228,76 @@ static void at_cgact_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  	g_at_chat_send(modem, buf, none_prefix, NULL, NULL, NULL);
>  }
>  
> +static void print_ip_address(const char *label, guint32 ip_addr)
> +{
> +	struct in_addr addr;
> +	char buf[INET_ADDRSTRLEN];
> +
> +	addr.s_addr = ip_addr;
> +
> +	if (inet_ntop(AF_INET, &addr, buf, INET_ADDRSTRLEN))
> +		g_print("%s: %s\n", label, buf);
> +}
> +
> +static void ppp_connect(GAtPPP *ppp, GAtPPPConnectStatus success,
> +			guint32 ip_addr, guint32 dns1, guint32 dns2,
> +			gpointer user_data)
> +{
> +	if (success != G_AT_PPP_CONNECT_SUCCESS) {
> +		g_print("Failed to create PPP interface!\n");
> +		return;
> +	}
> +
> +	/* print out the negotiated address and dns server */
> +	print_ip_address("IP Address", ip_addr);
> +	print_ip_address("Primary DNS Server", dns1);
> +	print_ip_address("Secondary DNS Server", dns2);
> +}
> +
> +static void ppp_disconnect(GAtPPP *ppp, 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);

I guess we should not call exit here. Just return.

> +	}
> +
> +	if (option_ppp == FALSE)
> +		return;
> +
> +	/* get the data IO channel */
> +	channel = g_at_chat_get_channel(modem);
> +
> +	/*
> +	 * shutdown gatchat or else it tries to take all the input
> +	 * from the modem and does not let PPP get it.
> +	 */
> +	g_at_chat_shutdown(control);
> +	g_at_chat_shutdown(modem);
> +
> +	/* open ppp */
> +	ppp = g_at_ppp_new(channel);
> +	if (!ppp) {
> +		g_print("Unable to create PPP object\n");
> +		return;
> +	}
> +	g_at_ppp_set_credentials(ppp, option_username,
> +				option_password);
> +
> +	/* 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];
> @@ -233,9 +308,9 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  	}
>  
>  	if (option_legacy == TRUE) {
> -			sprintf(buf, "ATD*99***%u#", option_cid);
> -			g_at_chat_send(modem, buf, none_prefix,
> -					NULL, NULL, NULL);
> +		sprintf(buf, "ATD*99***%u#", option_cid);
> +		g_at_chat_send(modem, buf, none_prefix,
> +				connect_cb, NULL, NULL);
>  	} else {
>  		sprintf(buf, "AT+CGACT=1,%u", option_cid);
>  		g_at_chat_send(control, buf, none_prefix,
> @@ -452,6 +527,12 @@ static GOptionEntry options[] = {
>  				"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_password,
> +				"Specifiy PPP password" },
>  	{ NULL },
>  };
>  
> -- 
> 1.6.6.1
> 
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  4:51           ` Marcel Holtmann
@ 2010-03-23  6:14             ` Kristen Carlson Accardi
  2010-03-23  6:45               ` Marcel Holtmann
  0 siblings, 1 reply; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23  6:14 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 21:51:38 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> So Denis and I looked through my changes and he figured out what might
> have broken it. So your attempt at doing code flow optimization with
> g_list_length() throw me on the wrong error path. Please don't do this
> since all the GLib functions will do proper empty list checking.

I reviewed your patch and it seems that you didn't break it this time :).
If you ever have questions about the code, feel free to ask me.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/6] Add PPP option to gsmdial
  2010-03-23  5:30   ` Gustavo F. Padovan
@ 2010-03-23  6:30     ` Marcel Holtmann
  0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  6:30 UTC (permalink / raw)
  To: ofono

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

Hi Gustavo,

> > +static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
> > +{
> > +	GIOChannel *channel;
> > +
> > +	if (!ok) {
> > +		g_print("Unable to define context\n");
> > +		exit(1);
> 
> I guess we should not call exit here. Just return.

it is a test program. So it doesn't really matter. We will never install
this program. It is just for the developers ;)

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  6:14             ` Kristen Carlson Accardi
@ 2010-03-23  6:45               ` Marcel Holtmann
  2010-03-23 17:16                 ` Kristen Carlson Accardi
  2010-03-23 17:34                 ` Kristen Carlson Accardi
  0 siblings, 2 replies; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23  6:45 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > So Denis and I looked through my changes and he figured out what might
> > have broken it. So your attempt at doing code flow optimization with
> > g_list_length() throw me on the wrong error path. Please don't do this
> > since all the GLib functions will do proper empty list checking.
> 
> I reviewed your patch and it seems that you didn't break it this time :).
> If you ever have questions about the code, feel free to ask me.

please try to keep the code flow inside the functions as simple as
possible. Otherwise things like this happen.

Also in this context I would prefer if you create a helper function like
find_option() instead of manually calling g_list_find_custom() all the
time. From a pure code understanding point of view it is way simpler to
see what the code is meant do be doing. The find_custom + is_option is
not intuitiv.

With a proper helper we didn't need to workaround the casting issue from
guint16/guint8 to guint at every spot. The compiler will inline the
helper anyway if useful. So no real drawback here from a runtime code
point of view. And there was another problem with one variable being
guint16, but the is_option casts it back to guint8. We can't really have
that. Once you start casting on that scale the compiler will not warn
you about type mismatches or if the value of argument is too big for its
type.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  2:22   ` Marcel Holtmann
  2010-03-23  3:22     ` Kristen Carlson Accardi
@ 2010-03-23 12:34     ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  2010-03-23 12:49       ` Marcel Holtmann
  1 sibling, 1 reply; 26+ messages in thread
From: =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont @ 2010-03-23 12:34 UTC (permalink / raw)
  To: ofono

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

On Tuesday 23 March 2010 04:22:00 Marcel Holtmann, you wrote:
> If you have variables that are only valid inside the scope of a
> statement, then I prefer if you only declare them there. This makes the
> code a bit simpler to read. Check the patch I just pushed to give you an
> example.

If you're serious about that, you should turn on C99 support. Only then you 
can use C++-style for-loops.

-- 
Rémi Denis-Courmont

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23 12:34     ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
@ 2010-03-23 12:49       ` Marcel Holtmann
  0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23 12:49 UTC (permalink / raw)
  To: ofono

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

Hi Remi,

> > If you have variables that are only valid inside the scope of a
> > statement, then I prefer if you only declare them there. This makes the
> > code a bit simpler to read. Check the patch I just pushed to give you an
> > example.
> 
> If you're serious about that, you should turn on C99 support. Only then you 
> can use C++-style for-loops.

I wasn't talking about the iterator variable.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  6:45               ` Marcel Holtmann
@ 2010-03-23 17:16                 ` Kristen Carlson Accardi
  2010-03-23 17:34                 ` Kristen Carlson Accardi
  1 sibling, 0 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23 17:16 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 23:45:14 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> please try to keep the code flow inside the functions as simple as
> possible. Otherwise things like this happen.

One suggestion I have for the future is to actually send your patches
to the mailing list for review, even if you've already committed them.
I can understand just committing things without public review for
expediency, but having many eyes looking at things can help as well.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23  6:45               ` Marcel Holtmann
  2010-03-23 17:16                 ` Kristen Carlson Accardi
@ 2010-03-23 17:34                 ` Kristen Carlson Accardi
  2010-03-23 17:56                   ` Marcel Holtmann
  1 sibling, 1 reply; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23 17:34 UTC (permalink / raw)
  To: ofono

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

On Mon, 22 Mar 2010 23:45:14 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> And there was another problem with one variable being
> guint16, but the is_option casts it back to guint8. We can't really have
> that. Once you start casting on that scale the compiler will not warn
> you about type mismatches or if the value of argument is too big for its
> type.

Can you please let me know which git commit this fix you made was?  I 
want to review it because all of the option types should only be a byte
anyway, so I am trying to figure out if there was a mistake somewhere 
where we were using is_option to examine a 16 bit value.  I searched
through the git log and can't figure out where this change was.  I
can see where you changed the option type we are comparing to a regular
guint to avoid compiler problems, but not the other issue you mentioned.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23 17:34                 ` Kristen Carlson Accardi
@ 2010-03-23 17:56                   ` Marcel Holtmann
  2010-03-23 18:26                     ` Kristen Carlson Accardi
  0 siblings, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23 17:56 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > And there was another problem with one variable being
> > guint16, but the is_option casts it back to guint8. We can't really have
> > that. Once you start casting on that scale the compiler will not warn
> > you about type mismatches or if the value of argument is too big for its
> > type.
> 
> Can you please let me know which git commit this fix you made was?  I 
> want to review it because all of the option types should only be a byte
> anyway, so I am trying to figure out if there was a mistake somewhere 
> where we were using is_option to examine a 16 bit value.  I searched
> through the git log and can't figure out where this change was.  I
> can see where you changed the option type we are comparing to a regular
> guint to avoid compiler problems, but not the other issue you mentioned.

I had a second look at these. It is the is_proto_handler actually and
not the is_option. However that thing applies here. A helper function to
find that handle would be better then manually coding g_list_find_custom
in the functions.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23 17:56                   ` Marcel Holtmann
@ 2010-03-23 18:26                     ` Kristen Carlson Accardi
  2010-03-23 18:37                       ` Denis Kenzior
  2010-03-23 19:07                       ` Marcel Holtmann
  0 siblings, 2 replies; 26+ messages in thread
From: Kristen Carlson Accardi @ 2010-03-23 18:26 UTC (permalink / raw)
  To: ofono

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

On Tue, 23 Mar 2010 10:56:00 -0700
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Kristen,
> 
> > > And there was another problem with one variable being
> > > guint16, but the is_option casts it back to guint8. We can't really have
> > > that. Once you start casting on that scale the compiler will not warn
> > > you about type mismatches or if the value of argument is too big for its
> > > type.
> > 
> > Can you please let me know which git commit this fix you made was?  I 
> > want to review it because all of the option types should only be a byte
> > anyway, so I am trying to figure out if there was a mistake somewhere 
> > where we were using is_option to examine a 16 bit value.  I searched
> > through the git log and can't figure out where this change was.  I
> > can see where you changed the option type we are comparing to a regular
> > guint to avoid compiler problems, but not the other issue you mentioned.
> 
> I had a second look at these. It is the is_proto_handler actually and
> not the is_option. However that thing applies here. A helper function to
> find that handle would be better then manually coding g_list_find_custom
> in the functions.
> 
> Regards
> 
> Marcel
> 
> 

so is_proto_handler was casting a 16 bit value to an 8 bit value?  Or are you
just complaining about having g_list_find_custom in the routines.  It would
really be easier on me to understand what you are trying to tell me if you
you posted your patches to the list next time.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23 18:26                     ` Kristen Carlson Accardi
@ 2010-03-23 18:37                       ` Denis Kenzior
  2010-03-23 19:07                       ` Marcel Holtmann
  1 sibling, 0 replies; 26+ messages in thread
From: Denis Kenzior @ 2010-03-23 18:37 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> On Tue, 23 Mar 2010 10:56:00 -0700
> 
> Marcel Holtmann <marcel@holtmann.org> wrote:
> > Hi Kristen,
> >
> > > > And there was another problem with one variable being
> > > > guint16, but the is_option casts it back to guint8. We can't really
> > > > have that. Once you start casting on that scale the compiler will not
> > > > warn you about type mismatches or if the value of argument is too big
> > > > for its type.
> > >
> > > Can you please let me know which git commit this fix you made was?  I
> > > want to review it because all of the option types should only be a byte
> > > anyway, so I am trying to figure out if there was a mistake somewhere
> > > where we were using is_option to examine a 16 bit value.  I searched
> > > through the git log and can't figure out where this change was.  I
> > > can see where you changed the option type we are comparing to a regular
> > > guint to avoid compiler problems, but not the other issue you
> > > mentioned.
> >
> > I had a second look at these. It is the is_proto_handler actually and
> > not the is_option. However that thing applies here. A helper function to
> > find that handle would be better then manually coding g_list_find_custom
> > in the functions.
> >
> > Regards
> >
> > Marcel
> 
> so is_proto_handler was casting a 16 bit value to an 8 bit value?  Or are
>  you just complaining about having g_list_find_custom in the routines.  It
>  would really be easier on me to understand what you are trying to tell me
>  if you you posted your patches to the list next time.

If you have questions or concerns then the best place to discuss this is on 
IRC.  I don't expect maintainers to post re-factoring patches on the mailing 
list.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/6] PPP Patches v3
  2010-03-23 18:26                     ` Kristen Carlson Accardi
  2010-03-23 18:37                       ` Denis Kenzior
@ 2010-03-23 19:07                       ` Marcel Holtmann
  1 sibling, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2010-03-23 19:07 UTC (permalink / raw)
  To: ofono

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

Hi Kristen,

> > > > And there was another problem with one variable being
> > > > guint16, but the is_option casts it back to guint8. We can't really have
> > > > that. Once you start casting on that scale the compiler will not warn
> > > > you about type mismatches or if the value of argument is too big for its
> > > > type.
> > > 
> > > Can you please let me know which git commit this fix you made was?  I 
> > > want to review it because all of the option types should only be a byte
> > > anyway, so I am trying to figure out if there was a mistake somewhere 
> > > where we were using is_option to examine a 16 bit value.  I searched
> > > through the git log and can't figure out where this change was.  I
> > > can see where you changed the option type we are comparing to a regular
> > > guint to avoid compiler problems, but not the other issue you mentioned.
> > 
> > I had a second look at these. It is the is_proto_handler actually and
> > not the is_option. However that thing applies here. A helper function to
> > find that handle would be better then manually coding g_list_find_custom
> > in the functions.
>
> so is_proto_handler was casting a 16 bit value to an 8 bit value?  Or are you
> just complaining about having g_list_find_custom in the routines.  It would
> really be easier on me to understand what you are trying to tell me if you
> you posted your patches to the list next time.

I expect you to ensure that your patches compile on 32-bit systems. And
also that they work on 32-bit and 64-bit systems. In addition I expect
that little endian and big endian works smoothly.

In general I don't wanna have casting in the source since it is a
potential trap for errors. Without casting the compiler will warn us if
we make mistakes, without we run into troubles. Especially with the
different pointer sizes it is important to not cast.

So in this specific cases, I think the usage of g_list_find_custom is
not a good idea. Manually walking this in a helper function and ensuring
that the variable and parameters types are enforced.

In short summary:

1) No casting if not really really needed. Think twice before adding a
casting to the source code.

2) Think about your optimization and understand what you are trying to
optimize. Simple code flow is preferred. Only in hot path we should
attempt to optimize manually and then it needs to be documented in the
source code itself. Otherwise lets GCC do it for us. Walking the GLib
list manually is sometimes a lot simpler to read than trying to use
foreach type functions.

3) No variable initialization on declaration. There are a few exceptions
when it makes sense. However the general rule is not to do it. We want
the compiler to warn us about variables that we might use uninitialized.
In most cases this indicates a mistake in the code flow and that the
code is too complex.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2010-03-23 19:07 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-23  0:05 [PATCH 0/6] PPP Patches v3 Kristen Carlson Accardi
2010-03-23  0:05 ` [PATCH 1/6] Basic PPP protocol support Kristen Carlson Accardi
2010-03-23  0:05 ` [PATCH 2/6] Generic PPP control " Kristen Carlson Accardi
2010-03-23  0:05 ` [PATCH 3/6] PPP LCP support Kristen Carlson Accardi
2010-03-23  0:05 ` [PATCH 4/6] CHAP with MD5 authentication support Kristen Carlson Accardi
2010-03-23  0:06 ` [PATCH 5/6] IP support for PPP Kristen Carlson Accardi
2010-03-23  0:06 ` [PATCH 6/6] Add PPP option to gsmdial Kristen Carlson Accardi
2010-03-23  5:30   ` Gustavo F. Padovan
2010-03-23  6:30     ` Marcel Holtmann
2010-03-23  0:32 ` [PATCH 0/6] PPP Patches v3 Marcel Holtmann
2010-03-23  2:22   ` Marcel Holtmann
2010-03-23  3:22     ` Kristen Carlson Accardi
2010-03-23  3:47       ` Marcel Holtmann
2010-03-23  4:07         ` Kristen Carlson Accardi
2010-03-23  4:51           ` Marcel Holtmann
2010-03-23  6:14             ` Kristen Carlson Accardi
2010-03-23  6:45               ` Marcel Holtmann
2010-03-23 17:16                 ` Kristen Carlson Accardi
2010-03-23 17:34                 ` Kristen Carlson Accardi
2010-03-23 17:56                   ` Marcel Holtmann
2010-03-23 18:26                     ` Kristen Carlson Accardi
2010-03-23 18:37                       ` Denis Kenzior
2010-03-23 19:07                       ` Marcel Holtmann
2010-03-23  3:56       ` Kristen Carlson Accardi
2010-03-23 12:34     ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
2010-03-23 12:49       ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.