[PATCH 00/11] Add PPP server support

[PATCH 00/11] Add PPP server support

[Zhenhua Zhang]

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

The patches extend GAtPPP to support PPP server role, so that you use gsmdial or wvdial to talk to test-server (Patch 6/11 adds PPP server support in test-server).

If you're interesting to try it, connect two machines by BlueZ serial proxy.

Server side:
1. Run ./test-server by 'root'. It will tell you the server port.
2. Update /etc/bluetooth/servial.conf. Create one if it doesn't exist.

	[Proxy DUN] # UUID for DUN proxy service
	UUID=00001103-0000-1000-8000-00805F9B34FB
	# Address for device node
	Address=/dev/pts/xxx

3. Restart bluetoothd.

Client side:
1. rfcomm bind -r <server bdaddr> 1
2. Run './gsmdial -n /dev/rfcomm0' by 'root'. 

You may update client's /etc/resolve.conf with server DNS info.

[PATCH 01/11] gatserver: Suspend/resume GAtServer with GAtIO

[Zhenhua Zhang]

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

Support g_at_server_suspend and g_at_server_resume operation by using
GAtIO to handle IO related function.
---
 gatchat/gatserver.c |  259 +++++++++++++++++++++-----------------------------
 gatchat/gatserver.h |    5 +
 2 files changed, 114 insertions(+), 150 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 148754a..6bb9244 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -31,6 +31,7 @@
 
 #include "ringbuffer.h"
 #include "gatserver.h"
+#include "gatio.h"
 
 #define BUF_SIZE 4096
 /* <cr><lf> + the max length of information text + <cr><lf> */
@@ -100,16 +101,13 @@ struct at_command {
 struct _GAtServer {
 	gint ref_count;				/* Ref count */
 	struct v250_settings v250;		/* V.250 command setting */
-	GIOChannel *channel;			/* Server IO */
-	guint read_watch;			/* GSource read id, 0 if none */
-	guint write_watch;			/* GSource write id, 0 if none */
+	GAtIO *io;				/* Server IO */
 	guint read_so_far;			/* Number of bytes processed */
 	GAtDisconnectFunc user_disconnect;	/* User disconnect func */
 	gpointer user_disconnect_data;		/* User disconnect data */
 	GAtDebugFunc debugf;			/* Debugging output function */
 	gpointer debug_data;			/* Data to pass to debug func */
 	GHashTable *command_list;		/* List of AT commands */
-	struct ring_buffer *read_buf;		/* Current read buffer */
 	GQueue *write_queue;			/* Write buffer queue */
 	guint max_read_attempts;		/* Max reads per select */
 	enum ParserState parser_state;
@@ -117,12 +115,13 @@ struct _GAtServer {
 	char *last_line;			/* Last read line */
 	unsigned int cur_pos;			/* Where we are on the line */
 	GAtServerResult last_result;
-	gboolean processing_cmdline;
+	gboolean suspended;
 	gboolean final_sent;
 	gboolean final_async;
+	gboolean in_read_handler;
 };
 
-static void g_at_server_wakeup_writer(GAtServer *server);
+static void server_wakeup_writer(GAtServer *server);
 static void server_parse_line(GAtServer *server);
 
 static struct ring_buffer *allocate_next(GAtServer *server)
@@ -162,7 +161,7 @@ static void send_common(GAtServer *server, const char *buf, unsigned int len)
 			write_buf = allocate_next(server);
 	}
 
-	g_at_server_wakeup_writer(server);
+	server_wakeup_writer(server);
 }
 
 static void send_result_common(GAtServer *server, const char *result)
@@ -198,14 +197,14 @@ void g_at_server_send_final(GAtServer *server, GAtServerResult result)
 	server->final_sent = TRUE;
 	server->last_result = result;
 
-	if (result == G_AT_SERVER_RESULT_OK && server->processing_cmdline) {
+	if (result == G_AT_SERVER_RESULT_OK && server->suspended) {
 		if (server->final_async)
 			server_parse_line(server);
 
 		return;
 	}
 
-	server->processing_cmdline = FALSE;
+	g_at_server_resume(server);
 
 	if (server->v250.is_v1)
 		sprintf(buf, "%s", server_result_to_string(result));
@@ -219,7 +218,7 @@ void g_at_server_send_ext_final(GAtServer *server, const char *result)
 {
 	server->final_sent = TRUE;
 	server->last_result = G_AT_SERVER_RESULT_EXT_ERROR;
-	server->processing_cmdline = FALSE;
+	g_at_server_resume(server);
 
 	send_result_common(server, result);
 }
@@ -668,7 +667,7 @@ static void server_parse_line(GAtServer *server)
 	server->final_async = FALSE;
 
 	if (pos == 0)
-		server->processing_cmdline = TRUE;
+		g_at_server_suspend(server);
 
 	while (pos < len) {
 		unsigned int consumed;
@@ -702,7 +701,7 @@ static void server_parse_line(GAtServer *server)
 			return;
 	}
 
-	server->processing_cmdline = FALSE;
+	g_at_server_resume(server);
 	g_at_server_send_final(server, G_AT_SERVER_RESULT_OK);
 }
 
@@ -780,11 +779,11 @@ out:
 	return res;
 }
 
-static char *extract_line(GAtServer *p)
+static char *extract_line(GAtServer *p, struct ring_buffer *rbuf)
 {
-	unsigned int wrap = ring_buffer_len_no_wrap(p->read_buf);
+	unsigned int wrap = ring_buffer_len_no_wrap(rbuf);
 	unsigned int pos = 0;
-	unsigned char *buf = ring_buffer_read_ptr(p->read_buf, pos);
+	unsigned char *buf = ring_buffer_read_ptr(rbuf, pos);
 	int strip_front = 0;
 	int line_length = 0;
 	gboolean in_string = FALSE;
@@ -806,7 +805,7 @@ static char *extract_line(GAtServer *p)
 		pos += 1;
 
 		if (pos == wrap)
-			buf = ring_buffer_read_ptr(p->read_buf, pos);
+			buf = ring_buffer_read_ptr(rbuf, pos);
 	}
 
 	/* We will strip AT and S3 */
@@ -814,17 +813,17 @@ static char *extract_line(GAtServer *p)
 
 	line = g_try_new(char, line_length + 1);
 	if (!line) {
-		ring_buffer_drain(p->read_buf, p->read_so_far);
+		ring_buffer_drain(rbuf, p->read_so_far);
 		return NULL;
 	}
 
 	/* Strip leading whitespace + AT */
-	ring_buffer_drain(p->read_buf, strip_front + 2);
+	ring_buffer_drain(rbuf, strip_front + 2);
 
 	pos = 0;
 	i = 0;
-	wrap = ring_buffer_len_no_wrap(p->read_buf);
-	buf = ring_buffer_read_ptr(p->read_buf, pos);
+	wrap = ring_buffer_len_no_wrap(rbuf);
+	buf = ring_buffer_read_ptr(rbuf, pos);
 
 	while (pos < (p->read_so_far - strip_front - 2)) {
 		if (*buf == '"')
@@ -839,33 +838,39 @@ static char *extract_line(GAtServer *p)
 		pos += 1;
 
 		if (pos == wrap)
-			buf = ring_buffer_read_ptr(p->read_buf, pos);
+			buf = ring_buffer_read_ptr(rbuf, pos);
 	}
 
 	/* Strip S3 */
-	ring_buffer_drain(p->read_buf, p->read_so_far - strip_front - 2);
+	ring_buffer_drain(rbuf, p->read_so_far - strip_front - 2);
 
 	line[i] = '\0';
 
 	return line;
 }
 
-static void new_bytes(GAtServer *p)
+static void new_bytes(struct ring_buffer *rbuf, gpointer user_data)
 {
-	unsigned int len = ring_buffer_len(p->read_buf);
-	unsigned int wrap = ring_buffer_len_no_wrap(p->read_buf);
-	unsigned char *buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
+	GAtServer *p = user_data;
+	unsigned int len = ring_buffer_len(rbuf);
+	unsigned int wrap = ring_buffer_len_no_wrap(rbuf);
+	unsigned char *buf = ring_buffer_read_ptr(rbuf, p->read_so_far);
 	enum ParserResult result;
 
-	while (p->channel && (p->read_so_far < len)) {
+	p->in_read_handler = TRUE;
+
+	while (p->io && (p->read_so_far < len)) {
 		gsize rbytes = MIN(len - p->read_so_far, wrap - p->read_so_far);
 		result = server_feed(p, (char *)buf, &rbytes);
 
+		if (p->v250.echo)
+			send_common(p, (char *)buf, rbytes);
+
 		buf += rbytes;
 		p->read_so_far += rbytes;
 
 		if (p->read_so_far == wrap) {
-			buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
+			buf = ring_buffer_read_ptr(rbuf, p->read_so_far);
 			wrap = len;
 		}
 
@@ -879,14 +884,14 @@ static void new_bytes(GAtServer *p)
 			 * Empty commands must be OK by the DCE
 			 */
 			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
-			ring_buffer_drain(p->read_buf, p->read_so_far);
+			ring_buffer_drain(rbuf, p->read_so_far);
 			break;
 
 		case PARSER_RESULT_COMMAND:
 		{
 			g_free(p->last_line);
 
-			p->last_line = extract_line(p);
+			p->last_line = extract_line(p, rbuf);
 			p->cur_pos = 0;
 
 			if (p->last_line)
@@ -905,11 +910,11 @@ static void new_bytes(GAtServer *p)
 			else
 				g_at_server_send_final(p,
 						G_AT_SERVER_RESULT_OK);
-			ring_buffer_drain(p->read_buf, p->read_so_far);
+			ring_buffer_drain(rbuf, p->read_so_far);
 			break;
 
 		default:
-			ring_buffer_drain(p->read_buf, p->read_so_far);
+			ring_buffer_drain(rbuf, p->read_so_far);
 			break;
 		}
 
@@ -918,72 +923,15 @@ static void new_bytes(GAtServer *p)
 		p->read_so_far = 0;
 	}
 
-	/* We're overflowing the buffer, shutdown the socket */
-	if (p->read_buf && ring_buffer_avail(p->read_buf) == 0)
-		g_source_remove(p->read_watch);
-}
-
-static gboolean received_data(GIOChannel *channel, GIOCondition cond,
-				gpointer data)
-{
-	unsigned char *buf;
-	GAtServer *server = data;
-	GIOError err;
-	gsize rbytes;
-	gsize toread;
-	guint total_read = 0;
-	guint read_count = 0;
-
-	if (cond & G_IO_NVAL)
-		return FALSE;
-
-	do {
-		toread = ring_buffer_avail_no_wrap(server->read_buf);
-
-		if (toread == 0)
-			break;
-
-		rbytes = 0;
-		buf = ring_buffer_write_ptr(server->read_buf, 0);
-
-		err = g_io_channel_read(channel, (char *) buf, toread, &rbytes);
-		g_at_util_debug_chat(TRUE, (char *)buf, rbytes,
-					server->debugf, server->debug_data);
-
-		read_count++;
-
-		if (rbytes == 0)
-			break;
-
-		if (server->v250.echo)
-			send_common(server, (char *)buf, rbytes);
-
-		/* Ignore incoming bytes when processing a command line */
-		if (server->processing_cmdline)
-			continue;
+	p->in_read_handler = FALSE;
 
-		total_read += rbytes;
-		ring_buffer_write_advance(server->read_buf, rbytes);
-	} while (err == G_IO_ERROR_NONE &&
-					read_count < server->max_read_attempts);
-
-	if (total_read > 0)
-		new_bytes(server);
-
-	if (cond & (G_IO_HUP | G_IO_ERR))
-		return FALSE;
-
-	if (read_count > 0 && rbytes == 0 && err != G_IO_ERROR_AGAIN)
-		return FALSE;
-
-	return TRUE;
+	if (p->destroyed)
+		g_free(p);
 }
 
-static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
-				gpointer data)
+static gboolean can_write_data(gpointer data)
 {
 	GAtServer *server = data;
-	GIOError err;
 	gsize bytes_written;
 	gsize towrite;
 	struct ring_buffer *write_buf;
@@ -992,9 +940,6 @@ static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
 	int limiter;
 #endif
 
-	if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
-		return FALSE;
-
 	if (!server->write_queue)
 		return FALSE;
 
@@ -1012,22 +957,17 @@ static gboolean can_write_data(GIOChannel *channel, GIOCondition cond,
 		limiter = 5;
 #endif
 
-	err = g_io_channel_write(server->channel,
+	bytes_written = g_at_io_write(server->io,
 			(char *)buf,
 #ifdef WRITE_SCHEDULER_DEBUG
-			limiter,
+			limiter
 #else
-			towrite,
+			towrite
 #endif
-			&bytes_written);
+			);
 
-	if (err != G_IO_ERROR_NONE) {
-		g_source_remove(server->read_watch);
+	if (bytes_written == 0)
 		return FALSE;
-	}
-
-	g_at_util_debug_chat(FALSE, (char *)buf, bytes_written, server->debugf,
-				server->debug_data);
 
 	ring_buffer_drain(write_buf, bytes_written);
 
@@ -1059,10 +999,6 @@ static void write_queue_free(GQueue *write_queue)
 
 static void g_at_server_cleanup(GAtServer *server)
 {
-	/* Cleanup all received data */
-	ring_buffer_free(server->read_buf);
-	server->read_buf = NULL;
-
 	/* Cleanup pending data to write */
 	write_queue_free(server->write_queue);
 
@@ -1071,15 +1007,15 @@ static void g_at_server_cleanup(GAtServer *server)
 
 	g_free(server->last_line);
 
-	server->channel = NULL;
+	g_at_io_unref(server->io);
+	server->io = NULL;
 }
 
-static void read_watcher_destroy_notify(gpointer user_data)
+static void io_disconnect(gpointer user_data)
 {
 	GAtServer *server = user_data;
 
 	g_at_server_cleanup(server);
-	server->read_watch = 0;
 
 	if (server->user_disconnect)
 		server->user_disconnect(server->user_disconnect_data);
@@ -1088,23 +1024,9 @@ static void read_watcher_destroy_notify(gpointer user_data)
 		g_free(server);
 }
 
-static void write_watcher_destroy_notify(gpointer user_data)
+static void server_wakeup_writer(GAtServer *server)
 {
-	GAtServer *server = user_data;
-
-	server->write_watch = 0;
-}
-
-static void g_at_server_wakeup_writer(GAtServer *server)
-{
-	if (server->write_watch != 0)
-		return;
-
-	server->write_watch = g_io_add_watch_full(server->channel,
-			G_PRIORITY_DEFAULT,
-			G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-			can_write_data, server,
-			write_watcher_destroy_notify);
+	g_at_io_set_write_handler(server->io, can_write_data, server);
 }
 
 static void v250_settings_create(struct v250_settings *v250)
@@ -1156,13 +1078,15 @@ GAtServer *g_at_server_new(GIOChannel *io)
 
 	server->ref_count = 1;
 	v250_settings_create(&server->v250);
-	server->channel = io;
+	server->io = g_at_io_new(io);
+	if (!server->io)
+		goto error;
+
+	g_at_io_set_disconnect_function(server->io, io_disconnect, server);
+
 	server->command_list = g_hash_table_new_full(g_str_hash, g_str_equal,
 							g_free,
 							at_notify_node_destroy);
-	server->read_buf = ring_buffer_new(BUF_SIZE);
-	if (!server->read_buf)
-		goto error;
 
 	server->write_queue = g_queue_new();
 	if (!server->write_queue)
@@ -1173,25 +1097,18 @@ GAtServer *g_at_server_new(GIOChannel *io)
 
 	server->max_read_attempts = 3;
 
-	if (!g_at_util_setup_io(server->channel, G_IO_FLAG_NONBLOCK))
-		goto error;
-
-	server->read_watch = g_io_add_watch_full(io, G_PRIORITY_DEFAULT,
-				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-				received_data, server,
-				read_watcher_destroy_notify);
+	g_at_io_set_read_handler(server->io, new_bytes, server);
 
 	basic_command_register(server);
 
 	return server;
 
 error:
+	g_at_io_unref(server->io);
+
 	if (server->command_list)
 		g_hash_table_destroy(server->command_list);
 
-	if (server->read_buf)
-		ring_buffer_free(server->read_buf);
-
 	if (server->write_queue)
 		write_queue_free(server->write_queue);
 
@@ -1201,6 +1118,22 @@ error:
 	return NULL;
 }
 
+GIOChannel *g_at_server_get_channel(GAtServer *server)
+{
+	if (server == NULL || server->io == NULL)
+		return NULL;
+
+	return g_at_io_get_channel(server->io);
+}
+
+GAtIO *g_at_server_get_io(GAtServer *server)
+{
+	if (server == NULL)
+		return NULL;
+
+	return server->io;
+}
+
 GAtServer *g_at_server_ref(GAtServer *server)
 {
 	if (server == NULL)
@@ -1211,6 +1144,33 @@ GAtServer *g_at_server_ref(GAtServer *server)
 	return server;
 }
 
+void g_at_server_suspend(GAtServer *server)
+{
+	if (server == NULL)
+		return;
+
+	server->suspended = TRUE;
+
+	g_at_io_set_write_handler(server->io, NULL, NULL);
+	g_at_io_set_read_handler(server->io, NULL, NULL);
+
+	g_at_io_set_debug(server->io, NULL, NULL);
+}
+
+void g_at_server_resume(GAtServer *server)
+{
+	if (server == NULL)
+		return;
+
+	server->suspended = FALSE;
+
+	g_at_io_set_debug(server->io, server->debugf, server->debug_data);
+	g_at_io_set_read_handler(server->io, new_bytes, server);
+
+	if (g_queue_get_length(server->write_queue) > 0)
+		server_wakeup_writer(server);
+}
+
 void g_at_server_unref(GAtServer *server)
 {
 	gboolean is_zero;
@@ -1223,6 +1183,11 @@ void g_at_server_unref(GAtServer *server)
 	if (is_zero == FALSE)
 		return;
 
+	if (server->io) {
+		g_at_server_suspend(server);
+		g_at_server_cleanup(server);
+	}
+
 	g_at_server_shutdown(server);
 
 	/* glib delays the destruction of the watcher until it exits, this
@@ -1230,7 +1195,7 @@ void g_at_server_unref(GAtServer *server)
 	 * destroyed already.  We have to wait until the read_watcher
 	 * destroy function gets called
 	 */
-	if (server->read_watch != 0)
+	if (server->in_read_handler)
 		server->destroyed = TRUE;
 	else
 		g_free(server);
@@ -1245,12 +1210,6 @@ gboolean g_at_server_shutdown(GAtServer *server)
 	server->user_disconnect = NULL;
 	server->user_disconnect_data = NULL;
 
-	if (server->write_watch)
-		g_source_remove(server->write_watch);
-
-	if (server->read_watch)
-		g_source_remove(server->read_watch);
-
 	return TRUE;
 }
 
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index b604c37..f0c19da 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -28,6 +28,7 @@ extern "C" {
 
 #include "gatresult.h"
 #include "gatutil.h"
+#include "gatio.h"
 
 struct _GAtServer;
 
@@ -68,8 +69,12 @@ typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type,
 					GAtResult *result, gpointer user_data);
 
 GAtServer *g_at_server_new(GIOChannel *io);
+GIOChannel *g_at_server_get_channel(GAtServer *server);
+GAtIO *g_at_server_get_io(GAtServer *server);
 
 GAtServer *g_at_server_ref(GAtServer *server);
+void g_at_server_suspend(GAtServer *server);
+void g_at_server_resume(GAtServer *server);
 void g_at_server_unref(GAtServer *server);
 
 gboolean g_at_server_shutdown(GAtServer *server);
-- 
1.6.3.3

[PATCH 02/11] gatserver: Check for disconnection when resuming

[Zhenhua Zhang]

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

If the internal GAtIO is no longer valid, treat it as if our channel was
disconnected.
---
 gatchat/gatserver.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 6bb9244..3fa26a0 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -1164,6 +1164,11 @@ void g_at_server_resume(GAtServer *server)
 
 	server->suspended = FALSE;
 
+	if (g_at_io_get_channel(server->io) == NULL) {
+		io_disconnect(server);
+		return;
+	}
+
 	g_at_io_set_debug(server->io, server->debugf, server->debug_data);
 	g_at_io_set_read_handler(server->io, new_bytes, server);
 
-- 
1.6.3.3

[PATCH 03/11] gatppp: Add PPP server extension

[Zhenhua Zhang]

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

1. Add interface to set PPP server info by g_at_ppp_set_server_info.
2. Pass local and peer address through IPCP handshaking.
---
 gatchat/gatppp.c   |   13 +++++-
 gatchat/gatppp.h   |    7 +++-
 gatchat/ppp.h      |    6 ++-
 gatchat/ppp_ipcp.c |  114 ++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 124 insertions(+), 16 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index e92fe5d..05136e0 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -246,7 +246,7 @@ void ppp_auth_notify(GAtPPP *ppp, gboolean success)
 	pppcp_signal_up(ppp->ipcp);
 }
 
-void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
+void ppp_ipcp_up_notify(GAtPPP *ppp, const char *local, const char *peer,
 					const char *dns1, const char *dns2)
 {
 	ppp->net = ppp_net_new(ppp);
@@ -264,7 +264,8 @@ void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
 
 	if (ppp->connect_cb)
 		ppp->connect_cb(ppp_net_get_interface(ppp->net),
-					ip, dns1, dns2, ppp->connect_data);
+					local, peer, dns1, dns2,
+					ppp->connect_data);
 }
 
 void ppp_ipcp_down_notify(GAtPPP *ppp)
@@ -464,6 +465,14 @@ void g_at_ppp_unref(GAtPPP *ppp)
 	g_free(ppp);
 }
 
+void g_at_ppp_set_server_info(GAtPPP *ppp, guint32 local, guint32 peer,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2)
+{
+	ipcp_set_server_info(ppp->ipcp, local, peer, dns1, dns2,
+				nbns1, nbns2);
+}
+
 static GAtPPP *ppp_init_common(GAtHDLC *hdlc)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index 438b952..86b3081 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -43,7 +43,8 @@ typedef enum _GAtPPPDisconnectReason {
 	G_AT_PPP_REASON_LOCAL_CLOSE,	/* Normal user close */
 } GAtPPPDisconnectReason;
 
-typedef void (*GAtPPPConnectFunc)(const char *iface, const char *ip,
+typedef void (*GAtPPPConnectFunc)(const char *iface, const char *local,
+					const char *peer,
 					const char *dns1, const char *dns2,
 					gpointer user_data);
 typedef void (*GAtPPPDisconnectFunc)(GAtPPPDisconnectReason reason,
@@ -68,6 +69,10 @@ const char *g_at_ppp_get_password(GAtPPP *ppp);
 
 void g_at_ppp_set_recording(GAtPPP *ppp, const char *filename);
 
+void g_at_ppp_set_server_info(GAtPPP *ppp, guint32 local, guint32 peer,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index b6c5f4a..56da8a9 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -86,6 +86,10 @@ void lcp_protocol_reject(struct pppcp_data *lcp, guint8 *packet, gsize len);
 /* IPCP related functions */
 struct pppcp_data *ipcp_new(GAtPPP *ppp);
 void ipcp_free(struct pppcp_data *data);
+void ipcp_set_server_info(struct pppcp_data *ipcp, guint32 local_addr,
+				guint32 peer_addr,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2);
 
 /* CHAP related functions */
 struct ppp_chap *ppp_chap_new(GAtPPP *ppp, guint8 method);
@@ -104,7 +108,7 @@ void ppp_debug(GAtPPP *ppp, const char *str);
 void ppp_transmit(GAtPPP *ppp, guint8 *packet, guint infolen);
 void ppp_set_auth(GAtPPP *ppp, const guint8 *auth_data);
 void ppp_auth_notify(GAtPPP *ppp, gboolean success);
-void ppp_ipcp_up_notify(GAtPPP *ppp, const char *ip,
+void ppp_ipcp_up_notify(GAtPPP *ppp, const char *local, const char *peer,
 					const char *dns1, const char *dns2);
 void ppp_ipcp_down_notify(GAtPPP *ppp);
 void ppp_ipcp_finished_notify(GAtPPP *ppp);
diff --git a/gatchat/ppp_ipcp.c b/gatchat/ppp_ipcp.c
index a1eacdf..8f13d85 100644
--- a/gatchat/ppp_ipcp.c
+++ b/gatchat/ppp_ipcp.c
@@ -67,7 +67,8 @@ struct ipcp_data {
 	guint8 options[MAX_CONFIG_OPTION_SIZE];
 	guint16 options_len;
 	guint8 req_options;
-	guint32 ipaddr;
+	guint32 local_addr;
+	guint32 peer_addr;
 	guint32 dns1;
 	guint32 dns2;
 	guint32 nbns1;
@@ -88,7 +89,7 @@ static void ipcp_generate_config_options(struct ipcp_data *ipcp)
 	guint16 len = 0;
 
 	FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
-					IP_ADDRESS, &ipcp->ipaddr);
+					IP_ADDRESS, &ipcp->local_addr);
 	FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
 					PRIMARY_DNS_SERVER, &ipcp->dns1);
 	FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
@@ -101,12 +102,52 @@ static void ipcp_generate_config_options(struct ipcp_data *ipcp)
 	ipcp->options_len = len;
 }
 
+static void ipcp_generate_peer_config_options(struct ipcp_data *ipcp)
+{
+	guint16 len = 0;
+
+	FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
+					IP_ADDRESS, &ipcp->peer_addr);
+	FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
+					PRIMARY_DNS_SERVER, &ipcp->dns1);
+	FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
+					SECONDARY_DNS_SERVER, &ipcp->dns2);
+	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
+					PRIMARY_NBNS_SERVER, &ipcp->nbns1);
+	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
+					SECONDARY_NBNS_SERVER, &ipcp->nbns2);
+
+	ipcp->options_len = len;
+}
+
+void ipcp_set_server_info(struct pppcp_data *pppcp, guint32 local_addr,
+				guint32 peer_addr,
+				guint32 dns1, guint32 dns2,
+				guint32 nbns1, guint32 nbns2)
+{
+	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+
+	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
+				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
+				REQ_OPTION_NBNS2;
+
+	ipcp->local_addr = local_addr;
+	ipcp->peer_addr = peer_addr;
+	ipcp->dns1 = dns1;
+	ipcp->dns2 = dns2;
+	ipcp->nbns1 = nbns1;
+	ipcp->nbns2 = nbns2;
+
+	ipcp_generate_config_options(ipcp);
+}
+
 static void ipcp_reset_config_options(struct ipcp_data *ipcp)
 {
 	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
 				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
 				REQ_OPTION_NBNS2;
-	ipcp->ipaddr = 0;
+	ipcp->local_addr = 0;
+	ipcp->peer_addr = 0;
 	ipcp->dns1 = 0;
 	ipcp->dns2 = 0;
 	ipcp->nbns1 = 0;
@@ -118,14 +159,19 @@ static void ipcp_reset_config_options(struct ipcp_data *ipcp)
 static void ipcp_up(struct pppcp_data *pppcp)
 {
 	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
-	char ip[INET_ADDRSTRLEN];
+	char local[INET_ADDRSTRLEN];
+	char peer[INET_ADDRSTRLEN];
 	char dns1[INET_ADDRSTRLEN];
 	char dns2[INET_ADDRSTRLEN];
 	struct in_addr addr;
 
-	memset(ip, 0, sizeof(ip));
-	addr.s_addr = ipcp->ipaddr;
-	inet_ntop(AF_INET, &addr, ip, INET_ADDRSTRLEN);
+	memset(local, 0, sizeof(local));
+	addr.s_addr = ipcp->local_addr;
+	inet_ntop(AF_INET, &addr, local, INET_ADDRSTRLEN);
+
+	memset(peer, 0, sizeof(peer));
+	addr.s_addr = ipcp->peer_addr;
+	inet_ntop(AF_INET, &addr, peer, INET_ADDRSTRLEN);
 
 	memset(dns1, 0, sizeof(dns1));
 	addr.s_addr = ipcp->dns1;
@@ -135,7 +181,8 @@ static void ipcp_up(struct pppcp_data *pppcp)
 	addr.s_addr = ipcp->dns2;
 	inet_ntop(AF_INET, &addr, dns2, INET_ADDRSTRLEN);
 
-	ppp_ipcp_up_notify(pppcp_get_ppp(pppcp), ip[0] ? ip : NULL,
+	ppp_ipcp_up_notify(pppcp_get_ppp(pppcp), local[0] ? local : NULL,
+					peer[0] ? peer : NULL,
 					dns1[0] ? dns1 : NULL,
 					dns2[0] ? dns2 : NULL);
 }
@@ -167,7 +214,7 @@ static void ipcp_rca(struct pppcp_data *pppcp,
 
 		switch (ppp_option_iter_get_type(&iter)) {
 		case IP_ADDRESS:
-			memcpy(&ipcp->ipaddr, data, 4);
+			memcpy(&ipcp->local_addr, data, 4);
 			break;
 		case PRIMARY_DNS_SERVER:
 			memcpy(&ipcp->dns1, data, 4);
@@ -204,7 +251,7 @@ static void ipcp_rcn_nak(struct pppcp_data *pppcp,
 		case IP_ADDRESS:
 			g_print("Setting suggested ip addr\n");
 			ipcp->req_options |= REQ_OPTION_IPADDR;
-			memcpy(&ipcp->ipaddr, data, 4);
+			memcpy(&ipcp->local_addr, data, 4);
 			break;
 		case PRIMARY_DNS_SERVER:
 			g_print("Setting suggested dns1\n");
@@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct pppcp_data *pppcp,
 					guint8 **new_options, guint16 *new_len)
 {
 	struct ppp_option_iter iter;
+	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
+	guint32 peer_addr;
+	guint32 dns1;
+	guint32 dns2;
 
 	ppp_option_iter_init(&iter, packet);
 
-	if (ppp_option_iter_next(&iter) == FALSE)
-		return RCR_ACCEPT;
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		const guint8 *data = ppp_option_iter_get_data(&iter);
+
+		switch (ppp_option_iter_get_type(&iter)) {
+		case IP_ADDRESS:
+			memcpy(&peer_addr, data, 4);
+			break;
+		case PRIMARY_DNS_SERVER:
+			memcpy(&dns1, data, 4);
+			break;
+		case SECONDARY_DNS_SERVER:
+			memcpy(&dns2, data, 4);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (peer_addr && dns1 && dns2) {
+		if (ipcp->peer_addr == 0) {
+			/* RFC 1332 Section 3.3
+			 * Accept the value of IP address as peer IP address
+			 */
+			ipcp->peer_addr = peer_addr;
+
+			return RCR_ACCEPT;
+		} else if (ipcp->peer_addr == peer_addr)
+			return RCR_ACCEPT;
+	} else {
+		/* Peer requests us to send ip address in config options */
+		if (ipcp->peer_addr) {
+			struct ipcp_data *peer;
+
+			peer = g_memdup(ipcp, sizeof(struct ipcp_data));
+			ipcp_generate_peer_config_options(peer);
+
+			*new_len = peer->options_len;
+			*new_options = peer->options;
+			return RCR_NAK;
+		}
+	}
 
 	/* Reject all options */
 	*new_len = packet->length - sizeof(*packet);
-- 
1.6.3.3

[PATCH 04/11] atmodem: Fix GAtPPPConnectFunc interface change

[Zhenhua Zhang]

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

---
 drivers/atmodem/gprs-context.c |    5 +++--
 gatchat/gsmdial.c              |    5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 4ddf88e..fea80b0 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -65,7 +65,8 @@ struct gprs_context_data {
 	void *cb_data;                                  /* Callback data */
 };
 
-static void ppp_connect(const char *interface, const char *ip,
+static void ppp_connect(const char *interface, const char *local,
+			const char *remote,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
@@ -78,7 +79,7 @@ static void ppp_connect(const char *interface, const char *ip,
 	dns[2] = 0;
 
 	gcd->state = STATE_ACTIVE;
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, ip,
+	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, local,
 					STATIC_IP_NETMASK, NULL,
 					dns, gcd->cb_data);
 }
diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index a7d38c9..35267e1 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -221,13 +221,14 @@ out:
 	return FALSE;
 }
 
-static void ppp_connect(const char *iface, const char *ip,
+static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
 	/* print out the negotiated address and dns server */
 	g_print("Network Device: %s\n", iface);
-	g_print("IP Address: %s\n", ip);
+	g_print("IP Address: %s\n", local);
+	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
 }
-- 
1.6.3.3

[PATCH 05/11] test-server: Fix GIOChannel leak in create_tty

[Zhenhua Zhang]

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

Remove client_io and unref server_io after creating GAtServer.
---
 gatchat/test-server.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 74ddd4d..5c1cfa4 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -722,7 +722,6 @@ static gboolean create_tty(const char *modem_path)
 	int master, slave;
 	char pty_name[256];
 	GIOChannel *server_io;
-	GIOChannel *client_io;
 
 	if (!modem_path)
 		return FALSE;
@@ -732,9 +731,6 @@ static gboolean create_tty(const char *modem_path)
 
 	set_raw_mode(slave);
 
-	client_io = g_io_channel_unix_new(slave);
-	g_io_channel_set_close_on_unref(client_io, TRUE);
-
 	g_print("new pty is created at %s\n", pty_name);
 
 	server_io = g_io_channel_unix_new(master);
@@ -747,6 +743,8 @@ static gboolean create_tty(const char *modem_path)
 		return FALSE;
 	}
 
+	g_io_channel_unref(server_io);
+
 	return TRUE;
 }
 
-- 
1.6.3.3

[PATCH 06/11] test-server: Add PPP server support

[Zhenhua Zhang]

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

So that gsmdial and wvdial could talk to test-server and establish PPP
connection.
---
 gatchat/test-server.c |  131 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 5c1cfa4..68371c2 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -35,6 +35,7 @@
 #include <arpa/inet.h>
 #include <signal.h>
 #include <sys/signalfd.h>
+#include <errno.h>
 
 #include <glib.h>
 #include <utmp.h>
@@ -44,6 +45,7 @@
 #include <sys/stat.h>
 
 #include "gatserver.h"
+#include "gatppp.h"
 #include "ringbuffer.h"
 
 #define DEFAULT_TCP_PORT 12346
@@ -61,6 +63,7 @@ struct sock_server{
 
 static GMainLoop *mainloop;
 static GAtServer *server;
+static GAtPPP *ppp;
 unsigned int server_watch;
 
 static gboolean server_cleanup()
@@ -68,6 +71,11 @@ static gboolean server_cleanup()
 	if (server_watch)
 		g_source_remove(server_watch);
 
+	if (ppp) {
+		g_at_ppp_unref(ppp);
+		ppp = NULL;
+	}
+
 	g_at_server_unref(server);
 	server = NULL;
 
@@ -83,6 +91,81 @@ static void server_debug(const char *str, void *data)
 	g_print("%s: %s\n", (char *) data, str);
 }
 
+static void ppp_connect(const char *iface, const char *local, const char *peer,
+			const char *dns1, const char *dns2,
+			gpointer user)
+{
+	g_print("Network Device: %s\n", iface);
+	g_print("IP Address: %s\n", local);
+	g_print("Peer IP Address: %s\n", peer);
+	g_print("Primary DNS Server: %s\n", dns1);
+	g_print("Secondary DNS Server: %s\n", dns2);
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+{
+	GAtServer *server = user;
+
+	g_print("PPP Link down: %d\n", reason);
+
+	g_at_ppp_unref(ppp);
+	ppp = NULL;
+
+	g_at_server_resume(server);
+	g_at_server_set_debug(server, server_debug, "Server");
+}
+
+static gboolean update_ppp(gpointer user)
+{
+	GAtPPP *ppp = user;
+	char local_ip[INET_ADDRSTRLEN] = "192.168.1.1";
+	char remote_ip[INET_ADDRSTRLEN] = "192.168.1.2";
+	char dns1[INET_ADDRSTRLEN] = "10.10.10.0";
+	char dns2[INET_ADDRSTRLEN] = "127.0.0.1";
+	guint32 l, r, d1, d2;
+
+	inet_pton(AF_INET, local_ip, &l);
+	inet_pton(AF_INET, remote_ip, &r);
+	inet_pton(AF_INET, dns1, &d1);
+	inet_pton(AF_INET, dns2, &d2);
+
+	g_at_ppp_set_server_info(ppp, l, r, d1, d2, 0, 0);
+
+	return FALSE;
+}
+
+static gboolean setup_ppp(gpointer user)
+{
+	GAtServer *server = user;
+	GAtIO *io;
+
+	io = g_at_server_get_io(server);
+
+	g_at_server_suspend(server);
+
+	/* open ppp */
+	ppp = g_at_ppp_new_from_io(io);
+	if (ppp == NULL) {
+		g_at_server_resume(server);
+		return FALSE;
+	}
+
+	g_at_ppp_set_debug(ppp, server_debug, "PPP");
+
+	g_at_ppp_set_credentials(ppp, "", "");
+
+	/* set connect and disconnect callbacks */
+	g_at_ppp_set_connect_function(ppp, ppp_connect, server);
+	g_at_ppp_set_disconnect_function(ppp, ppp_disconnect, server);
+
+	/* open the ppp connection */
+	g_at_ppp_open(ppp);
+
+	g_idle_add(update_ppp, ppp);
+
+	return FALSE;
+}
+
 static void cgmi_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 {
 	GAtServer *server = user;
@@ -468,6 +551,7 @@ static void cgdata_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 		break;
 	case G_AT_SERVER_REQUEST_TYPE_SET:
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_CONNECT);
+		g_idle_add(setup_ppp, server);
 		break;
 	default:
 		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
@@ -671,6 +755,40 @@ static void cpbs_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
 	}
 }
 
+static void dial_cb(GAtServerRequestType type, GAtResult *cmd, gpointer user)
+{
+	GAtServer *server = user;
+	GAtServerResult res = G_AT_SERVER_RESULT_ERROR;
+	GAtResultIter iter;
+	const char *dial_str;
+	char c;
+
+	if (type != G_AT_SERVER_REQUEST_TYPE_SET)
+		goto error;
+
+	g_at_result_iter_init(&iter, cmd);
+
+	if (!g_at_result_iter_next(&iter, "D"))
+		goto error;
+
+	dial_str = g_at_result_iter_raw_line(&iter);
+	if (!dial_str)
+		goto error;
+
+	g_print("dial call %s\n", dial_str);
+
+	c = *dial_str;
+	if (c == '*' || c == '#' || c == 'T' || c == 't') {
+		g_at_server_send_final(server, G_AT_SERVER_RESULT_CONNECT);
+		g_idle_add(setup_ppp, server);
+	}
+
+	return;
+
+error:
+	g_at_server_send_final(server, res);
+}
+
 static void add_handler(GAtServer *server)
 {
 	g_at_server_set_debug(server, server_debug, "Server");
@@ -695,6 +813,7 @@ static void add_handler(GAtServer *server)
 	g_at_server_register(server, "+CSCS",    cscs_cb,    server, NULL);
 	g_at_server_register(server, "+CMGL",    cmgl_cb,    server, NULL);
 	g_at_server_register(server, "+CPBS",    cpbs_cb,    server, NULL);
+	g_at_server_register(server, "D",        dial_cb,    server, NULL);
 }
 
 static void server_destroy(gpointer user)
@@ -706,15 +825,11 @@ static void server_destroy(gpointer user)
 
 static void set_raw_mode(int fd)
 {
-	struct termios options;
-
-	tcgetattr(fd, &options);
-
-	/* Set TTY as raw mode to disable echo back of input characters
-	 * when they are received from Modem to avoid feedback loop */
-	options.c_lflag &= ~(ICANON | ECHO | ECHOE | ISIG);
+	struct termios ti;
 
-	tcsetattr(fd, TCSANOW, &options);
+	tcflush(fd, TCIOFLUSH);
+	cfmakeraw(&ti);
+	tcsetattr(fd, TCSANOW, &ti);
 }
 
 static gboolean create_tty(const char *modem_path)
-- 
1.6.3.3

[PATCH 07/11] test-server: Configure network interface

[Zhenhua Zhang]

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

Require ROOT priviledge to:
1. Run external command to configure and bring up network interface.
2. Enable kernel IP forwarding.
---
 gatchat/test-server.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 68371c2..df0bab8 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -50,6 +50,7 @@
 
 #define DEFAULT_TCP_PORT 12346
 #define DEFAULT_SOCK_PATH "./server_sock"
+#define IFCONFIG_PATH "/sbin/ifconfig"
 
 static int modem_mode = 0;
 static int modem_creg = 0;
@@ -91,15 +92,40 @@ static void server_debug(const char *str, void *data)
 	g_print("%s: %s\n", (char *) data, str);
 }
 
+static gboolean execute(const char *cmd)
+{
+	int status;
+
+	status = system(cmd);
+	if (status < 0) {
+		g_print("Failed to execute command: %s\n", strerror(errno));
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user)
 {
+	char buf[512];
+
 	g_print("Network Device: %s\n", iface);
 	g_print("IP Address: %s\n", local);
 	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
+
+	snprintf(buf, sizeof(buf), "%s %s up", IFCONFIG_PATH, iface);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "%s %s %s pointopoint %s", IFCONFIG_PATH,
+				iface, local, peer);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "echo 1 > /proc/sys/net/ipv4/ip_forward");
+	execute(buf);
 }
 
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
@@ -139,6 +165,11 @@ static gboolean setup_ppp(gpointer user)
 	GAtServer *server = user;
 	GAtIO *io;
 
+	if (getuid() != 0) {
+		g_print("Need root priviledge for PPP connection\n");
+		return FALSE;
+	}
+
 	io = g_at_server_get_io(server);
 
 	g_at_server_suspend(server);
-- 
1.6.3.3

[PATCH 08/11] gsmdial: Configure network interface for PPP

[Zhenhua Zhang]

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

---
 gatchat/gsmdial.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 35267e1..29e1d22 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -38,6 +38,8 @@
 #include <gattty.h>
 #include <gatppp.h>
 
+#define IFCONFIG_PATH "/sbin/ifconfig"
+
 static const char *none_prefix[] = { NULL };
 static const char *cfun_prefix[] = { "+CFUN:", NULL };
 static const char *creg_prefix[] = { "+CREG:", NULL };
@@ -221,16 +223,43 @@ out:
 	return FALSE;
 }
 
+static gboolean execute(const char *cmd)
+{
+	int status;
+
+	status = system(cmd);
+	if (status < 0) {
+		g_print("Failed to execute command: %s\n", strerror(errno));
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void ppp_connect(const char *iface, const char *local, const char *peer,
 			const char *dns1, const char *dns2,
 			gpointer user_data)
 {
+	char buf[512];
+
 	/* print out the negotiated address and dns server */
 	g_print("Network Device: %s\n", iface);
 	g_print("IP Address: %s\n", local);
 	g_print("Peer IP Address: %s\n", peer);
 	g_print("Primary DNS Server: %s\n", dns1);
 	g_print("Secondary DNS Server: %s\n", dns2);
+
+	if (getuid() != 0) {
+		g_print("Need root priviledge to config PPP interface\n");
+		return;
+	}
+
+	snprintf(buf, sizeof(buf), "%s %s up", IFCONFIG_PATH, iface);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "%s %s %s pointopoint %s", IFCONFIG_PATH,
+				iface, local, peer);
+	execute(buf);
 }
 
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
-- 
1.6.3.3

[PATCH 09/11] gsmdial: Unref ppp when we get disconnected

[Zhenhua Zhang]

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

---
 gatchat/gsmdial.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 29e1d22..59f1a5c 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -265,7 +265,12 @@ static void ppp_connect(const char *iface, const char *local, const char *peer,
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
 {
 	char buf[64];
+
 	g_print("PPP Link down: %d\n", reason);
+
+	g_at_ppp_unref(ppp);
+	ppp = NULL;
+
 	g_at_chat_resume(modem);
 
 	sprintf(buf, "AT+CFUN=%u", option_offmode);
-- 
1.6.3.3

[PATCH 10/11] test-server: Delay ppp_unref after IO processing

[Zhenhua Zhang]

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

The ppp_disconnect is invoked during ppp receives LCP packet and enter
PPP dead phase. It's not good to stop the IO processing in disconnect
function.  Delay the ppp_unref after packet is processed.
---
 gatchat/test-server.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index df0bab8..dac7234 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -57,6 +57,7 @@ static int modem_creg = 0;
 static int modem_cgreg = 0;
 static int network_status = 4;
 static int network_attach = 0;
+static int resume_source = 0;
 
 struct sock_server{
 	int server_sock;
@@ -72,6 +73,9 @@ static gboolean server_cleanup()
 	if (server_watch)
 		g_source_remove(server_watch);
 
+	if (resume_source)
+		g_source_remove(resume_source);
+
 	if (ppp) {
 		g_at_ppp_unref(ppp);
 		ppp = NULL;
@@ -128,17 +132,28 @@ static void ppp_connect(const char *iface, const char *local, const char *peer,
 	execute(buf);
 }
 
-static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+static gboolean resume_server(gpointer user)
 {
 	GAtServer *server = user;
 
-	g_print("PPP Link down: %d\n", reason);
+	resume_source = 0;
 
 	g_at_ppp_unref(ppp);
 	ppp = NULL;
 
 	g_at_server_resume(server);
 	g_at_server_set_debug(server, server_debug, "Server");
+
+	modem_mode = 0;
+
+	return FALSE;
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user)
+{
+	g_print("PPP Link down: %d\n", reason);
+
+	resume_source = g_idle_add(resume_server, user);
 }
 
 static gboolean update_ppp(gpointer user)
-- 
1.6.3.3

[PATCH 11/11] gsmdial: Delay ppp_unref after IO processing

[Zhenhua Zhang]

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

---
 gatchat/gsmdial.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index 59f1a5c..d6cacc5 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -71,6 +71,7 @@ enum state {
 
 static int state = 0;
 static int oldmode = 0;
+static int resume_source = 0;
 
 static void gsmdial_debug(const char *str, void *data)
 {
@@ -262,11 +263,11 @@ static void ppp_connect(const char *iface, const char *local, const char *peer,
 	execute(buf);
 }
 
-static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
+static gboolean resume_modem(gpointer user_data)
 {
 	char buf[64];
 
-	g_print("PPP Link down: %d\n", reason);
+	resume_source = 0;
 
 	g_at_ppp_unref(ppp);
 	ppp = NULL;
@@ -275,6 +276,15 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
 
 	sprintf(buf, "AT+CFUN=%u", option_offmode);
 	g_at_chat_send(control, buf, none_prefix, power_down, NULL, NULL);
+
+	return FALSE;
+}
+
+static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
+{
+	g_print("PPP Link down: %d\n", reason);
+
+	resume_source = g_idle_add(resume_modem, NULL);
 }
 
 static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -703,6 +713,9 @@ int main(int argc, char **argv)
 	g_main_loop_unref(event_loop);
 
 out:
+	if (resume_source)
+		g_source_remove(resume_source);
+
 	if (ppp == NULL) {
 		g_at_chat_unref(control);
 		g_at_chat_unref(modem);
-- 
1.6.3.3

RE: [PATCH 11/11] gsmdial: Delay ppp_unref after IO processing

[Zhang, Zhenhua]

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

Hi,

Zhenhua Zhang wrote:
> ---
> gatchat/gsmdial.c |   17 +++++++++++++++--
> 1 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
> index 59f1a5c..d6cacc5 100644
> --- a/gatchat/gsmdial.c
> +++ b/gatchat/gsmdial.c

I think we should delay the ppp shutdown stage until the LCP packet was well processed. I got valgrind result as below before I have patches 10/11 and 11/11.

Entering new phase: 0
PPP Link down: 6
==5332== Invalid write of size 2
==5332==    at 0x805520D: new_bytes (gathdlc.c:154)
==5332==    by 0x804F2E6: received_data (gatio.c:119)
==5332==    by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332==    by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332==    by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332==    by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332==    by 0x804BE5D: main (gsmdial.c:702)
==5332==  Address 0x4382130 is 24 bytes inside a block of size 88 free'd
==5332==    at 0x4024886: free (vg_replace_malloc.c:325)
==5332==    by 0x4081195: g_free (gmem.c:190)
==5332==    by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332==    by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332==    by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332==    by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332==    by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332==    by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332==    by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332==    by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332==    by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332==    by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332== 
==5332== Invalid write of size 4
==5332==    at 0x8055216: new_bytes (gathdlc.c:155)
==5332==    by 0x804F2E6: received_data (gatio.c:119)
==5332==    by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332==    by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332==    by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332==    by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332==    by 0x804BE5D: main (gsmdial.c:702)
==5332==  Address 0x438212c is 20 bytes inside a block of size 88 free'd
==5332==    at 0x4024886: free (vg_replace_malloc.c:325)
==5332==    by 0x4081195: g_free (gmem.c:190)
==5332==    by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332==    by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332==    by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332==    by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332==    by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332==    by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332==    by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332==    by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332==    by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332==    by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332== 
==5332== Invalid read of size 4
==5332==    at 0x80552CB: new_bytes (gathdlc.c:167)
==5332==    by 0x804F2E6: received_data (gatio.c:119)
==5332==    by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
==5332==    by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
==5332==    by 0x407C72F: g_main_context_iterate (gmain.c:2591)
==5332==    by 0x407CB9E: g_main_loop_run (gmain.c:2799)
==5332==    by 0x804BE5D: main (gsmdial.c:702)
==5332==  Address 0x438216c is 84 bytes inside a block of size 88 free'd
==5332==    at 0x4024886: free (vg_replace_malloc.c:325)
==5332==    by 0x4081195: g_free (gmem.c:190)
==5332==    by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
==5332==    by 0x8056316: g_at_ppp_unref (gatppp.c:463)
==5332==    by 0x804AF20: ppp_disconnect (gsmdial.c:272)
==5332==    by 0x8055CE9: ppp_dead (gatppp.c:204)
==5332==    by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
==5332==    by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
==5332==    by 0x8057E27: lcp_finished (ppp_lcp.c:140)
==5332==    by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
==5332==    by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
==5332==    by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)
==5332== 
: > AT+CFUN=0\r
==5332== 
==5332== HEAP SUMMARY:
==5332==     in use at exit: 8,854 bytes in 31 blocks
==5332==   total heap usage: 860 allocs, 829 frees, 3,139,527 bytes allocated
==5332== 
==5332== 248 bytes in 1 blocks are possibly lost in loss record 13 of 23
==5332==    at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332==    by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332==    by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332==    by 0x4097580: g_slice_alloc (gslice.c:661)
==5332==    by 0x4076D75: g_list_prepend (glist.c:169)
==5332==    by 0x4085A82: add_pending_null (goption.c:1119)
==5332==    by 0x40868C6: parse_short_option (goption.c:1397)
==5332==    by 0x4087EC1: g_option_context_parse (goption.c:1858)
==5332==    by 0x804BB35: main (gsmdial.c:631)
==5332== 
==5332== 360 bytes in 3 blocks are possibly lost in loss record 14 of 23
==5332==    at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332==    by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332==    by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332==    by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332==    by 0x40987B1: g_slist_prepend (gslist.c:160)
==5332==    by 0x4079859: g_source_add_poll (gmain.c:972)
==5332==    by 0x40AFF04: g_io_unix_create_watch (giounix.c:357)
==5332==    by 0x406D828: g_io_create_watch (giochannel.c:516)
==5332==    by 0x4070712: g_io_add_watch_full (giochannel.c:550)
==5332==    by 0x804F503: create_io (gatio.c:207)
==5332==    by 0x804F555: g_at_io_new (gatio.c:225)
==5332==    by 0x804D1E6: create_chat (gatchat.c:825)
==5332== 
==5332== 496 bytes in 2 blocks are possibly lost in loss record 15 of 23
==5332==    at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332==    by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332==    by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332==    by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332==    by 0x4076D75: g_list_prepend (glist.c:169)
==5332==    by 0x4085A82: add_pending_null (goption.c:1119)
==5332==    by 0x40868C6: parse_short_option (goption.c:1397)
==5332==    by 0x4087EC1: g_option_context_parse (goption.c:1858)
==5332==    by 0x804BB35: main (gsmdial.c:631)
==5332== 
==5332== 504 bytes in 1 blocks are possibly lost in loss record 17 of 23
==5332==    at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332==    by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332==    by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332==    by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332==    by 0x406A628: g_hash_table_new_full (ghash.c:512)
==5332==    by 0x804D263: create_chat (gatchat.c:839)
==5332==    by 0x804D310: g_at_chat_new (gatchat.c:863)
==5332==    by 0x804B837: open_serial (gsmdial.c:517)
==5332==    by 0x804BBDD: main (gsmdial.c:651)
==5332== 
==5332== 1,240 bytes in 5 blocks are possibly lost in loss record 22 of 23
==5332==    at 0x4023E7E: memalign (vg_replace_malloc.c:532)
==5332==    by 0x4023EDB: posix_memalign (vg_replace_malloc.c:660)
==5332==    by 0x409661D: slab_allocator_alloc_chunk (gslice.c:1136)
==5332==    by 0x40975AC: g_slice_alloc (gslice.c:666)
==5332==    by 0x40532D8: g_array_sized_new (garray.c:87)
==5332==    by 0x40533F6: g_array_new (garray.c:79)
==5332==    by 0x40A361E: g_static_private_set (gthread.c:451)
==5332==    by 0x40AA883: g_get_charset (gutf8.c:545)
==5332==    by 0x40834CC: g_print (gmessages.c:1043)
==5332==    by 0x804BBBA: main (gsmdial.c:647)
==5332== 
==5332== LEAK SUMMARY:
==5332==    definitely lost: 0 bytes in 0 blocks
==5332==    indirectly lost: 0 bytes in 0 blocks
==5332==      possibly lost: 2,848 bytes in 12 blocks
==5332==    still reachable: 6,006 bytes in 19 blocks
==5332==         suppressed: 0 bytes in 0 blocks
==5332== Reachable blocks (those to which a pointer was found) are not shown.
==5332== To see them, rerun with: --leak-check=full --show-reachable=yes
==5332== 
==5332== For counts of detected and suppressed errors, rerun with: -v
==5332== ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 15 from 6)

Regards,
Zhenhua

Re: [PATCH 01/11] gatserver: Suspend/resume GAtServer with GAtIO

[Denis Kenzior]

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

Hi Zhenhua,

> Support g_at_server_suspend and g_at_server_resume operation by using
> GAtIO to handle IO related function.
> ---

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 02/11] gatserver: Check for disconnection when resuming

[Denis Kenzior]

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

Hi Zhenhua,

> If the internal GAtIO is no longer valid, treat it as if our channel was
> disconnected.
> ---

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 03/11] gatppp: Add PPP server extension

[Denis Kenzior]

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

Hi Zhenhua,

> 1. Add interface to set PPP server info by g_at_ppp_set_server_info.
> 2. Pass local and peer address through IPCP handshaking.
> ---

Its great that you're working on the PPP Server extensions.  Can I get you to 
submit a patch taking ownership of this task?  See ofono/TODO 'PPP Server 
support'.

> +static void ipcp_generate_peer_config_options(struct ipcp_data *ipcp)
> +{
> +	guint16 len = 0;
> +
> +	FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
> +					IP_ADDRESS, &ipcp->peer_addr);
> +	FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
> +					PRIMARY_DNS_SERVER, &ipcp->dns1);
> +	FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
> +					SECONDARY_DNS_SERVER, &ipcp->dns2);
> +	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
> +					PRIMARY_NBNS_SERVER, &ipcp->nbns1);
> +	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
> +					SECONDARY_NBNS_SERVER, &ipcp->nbns2);
> +

Using ipcp->req_options is the wrong thing here.  That is used for options 
sent in a Configure-Req.  Here you're filling in options to be sent in a 
Configure-Nak, Configure-Reject or a Configure-Ack.  You should simply be filling 
these in ipcp_rcr.  Feel free to modify the FILL_IP macro to take an 
additional destination parameter.

> +	ipcp->options_len = len;
> +}
> +
> +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32 local_addr,
> +				guint32 peer_addr,
> +				guint32 dns1, guint32 dns2,
> +				guint32 nbns1, guint32 nbns2)
> +{
> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
> +
> +	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
> +				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
> +				REQ_OPTION_NBNS2;
> +
> +	ipcp->local_addr = local_addr;
> +	ipcp->peer_addr = peer_addr;
> +	ipcp->dns1 = dns1;
> +	ipcp->dns2 = dns2;
> +	ipcp->nbns1 = nbns1;
> +	ipcp->nbns2 = nbns2;
> +
> +	ipcp_generate_config_options(ipcp);

Please note that this function is currently having no effect, you're missing a 
call to pppcp_set_local_options.  However, in the case of a server, we 
actually do not want to request DNS or NBNS, and we should only send our local 
address to the peer if it is non-zero.  Please note that all PPP modems we 
have encountered so far, do not send their local IP address in a Configure-
Request.  Thus calling ipcp_set_server_info should end up generating an empty 
Configure-Request or with only the local IP address present.

> +}
> +
>  static void ipcp_reset_config_options(struct ipcp_data *ipcp)
>  {
>  	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>  				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>  				REQ_OPTION_NBNS2;
> -	ipcp->ipaddr = 0;
> +	ipcp->local_addr = 0;

In the case of a server, the local_address should not be reset.

> +	ipcp->peer_addr = 0;
>  	ipcp->dns1 = 0;
>  	ipcp->dns2 = 0;
>  	ipcp->nbns1 = 0;
> @@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct pppcp_data
>  *pppcp, guint8 **new_options, guint16 *new_len)
>  {
>  	struct ppp_option_iter iter;
> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
> +	guint32 peer_addr;
> +	guint32 dns1;
> +	guint32 dns2;
> 
>  	ppp_option_iter_init(&iter, packet);
> 
> -	if (ppp_option_iter_next(&iter) == FALSE)
> -		return RCR_ACCEPT;

Again, be careful here.  In the case of a client we should only accept the 
Server's IP-Address, nothing else.

> +	while (ppp_option_iter_next(&iter) == TRUE) {
> +		const guint8 *data = ppp_option_iter_get_data(&iter);
> +
> +		switch (ppp_option_iter_get_type(&iter)) {
> +		case IP_ADDRESS:
> +			memcpy(&peer_addr, data, 4);
> +			break;
> +		case PRIMARY_DNS_SERVER:
> +			memcpy(&dns1, data, 4);
> +			break;
> +		case SECONDARY_DNS_SERVER:
> +			memcpy(&dns2, data, 4);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	if (peer_addr && dns1 && dns2) {
> +		if (ipcp->peer_addr == 0) {
> +			/* RFC 1332 Section 3.3
> +			 * Accept the value of IP address as peer IP address
> +			 */
> +			ipcp->peer_addr = peer_addr;
> +

This part doesn't really make any sense, if peer is sending 0, he's asking us 
for an IP.

> +			return RCR_ACCEPT;
> +		} else if (ipcp->peer_addr == peer_addr)
> +			return RCR_ACCEPT;

You need to ensure that not only the peer_addr matches, but also the dns / 
nbns addresses.  In our case we should be Configure-Rejecting the NBNS options 
and Configure-Naking DNS ones if they're set to zero or don't match what the 
peer is sending us.

> +	} else {
> +		/* Peer requests us to send ip address in config options */
> +		if (ipcp->peer_addr) {
> +			struct ipcp_data *peer;
> +
> +			peer = g_memdup(ipcp, sizeof(struct ipcp_data));
> +			ipcp_generate_peer_config_options(peer);

Yuck, please don't do this.  The main PPPCP layer will take care of freeing 
new_options, however you're now leaking the rest of the ipcp_data structure.

> +
> +			*new_len = peer->options_len;
> +			*new_options = peer->options;
> +			return RCR_NAK;
> +		}
> +	}
> 
>  	/* Reject all options */
>  	*new_len = packet->length - sizeof(*packet);
> 

Regards,
-Denis

Re: [PATCH 05/11] test-server: Fix GIOChannel leak in create_tty

[Denis Kenzior]

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

Hi Zhenhua,

> Remove client_io and unref server_io after creating GAtServer.
> ---
>  gatchat/test-server.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 

This patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 09/11] gsmdial: Unref ppp when we get disconnected

[Denis Kenzior]

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

Hi Zhenhua,

> ---
>  gatchat/gsmdial.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 

This patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 11/11] gsmdial: Delay ppp_unref after IO processing

[Denis Kenzior]

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

Hi Zhenhua,

> I think we should delay the ppp shutdown stage until the LCP packet was
>  well processed. I got valgrind result as below before I have patches 10/11
>  and 11/11.
> 
> Entering new phase: 0
> PPP Link down: 6
> ==5332== Invalid write of size 2
> ==5332==    at 0x805520D: new_bytes (gathdlc.c:154)
> ==5332==    by 0x804F2E6: received_data (gatio.c:119)
> ==5332==    by 0x40AFDAA: g_io_unix_dispatch (giounix.c:162)
> ==5332==    by 0x4078E87: g_main_context_dispatch (gmain.c:1960)
> ==5332==    by 0x407C72F: g_main_context_iterate (gmain.c:2591)
> ==5332==    by 0x407CB9E: g_main_loop_run (gmain.c:2799)
> ==5332==    by 0x804BE5D: main (gsmdial.c:702)
> ==5332==  Address 0x4382130 is 24 bytes inside a block of size 88 free'd
> ==5332==    at 0x4024886: free (vg_replace_malloc.c:325)
> ==5332==    by 0x4081195: g_free (gmem.c:190)
> ==5332==    by 0x805557D: g_at_hdlc_unref (gathdlc.c:270)
> ==5332==    by 0x8056316: g_at_ppp_unref (gatppp.c:463)
> ==5332==    by 0x804AF20: ppp_disconnect (gsmdial.c:272)
> ==5332==    by 0x8055CE9: ppp_dead (gatppp.c:204)
> ==5332==    by 0x8055D1E: ppp_enter_phase (gatppp.c:214)
> ==5332==    by 0x8055FE0: ppp_lcp_finished_notify (gatppp.c:317)
> ==5332==    by 0x8057E27: lcp_finished (ppp_lcp.c:140)
> ==5332==    by 0x80568DF: pppcp_this_layer_finished (ppp_cp.c:340)
> ==5332==    by 0x8057446: pppcp_generate_event (ppp_cp.c:688)
> ==5332==    by 0x8057B5B: pppcp_process_packet (ppp_cp.c:965)

Good catch, however I fixed it slightly differently in GAtHDLC itself.  Can you 
test and make sure my fix works?

Thanks,
-Denis

RE: [PATCH 03/11] gatppp: Add PPP server extension

[Zhang, Zhenhua]

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

Hi Denis,

Denis Kenzior wrote:
> Hi Zhenhua,
> 
>> 1. Add interface to set PPP server info by g_at_ppp_set_server_info.
>> 2. Pass local and peer address through IPCP handshaking.
>> ---
> 
> Its great that you're working on the PPP Server extensions.
> Can I get you to
> submit a patch taking ownership of this task?  See ofono/TODO 'PPP
> Server support'.
> 
>> +static void ipcp_generate_peer_config_options(struct ipcp_data
>> *ipcp) +{ +	guint16 len = 0;
>> +
>> +	FILL_IP(ipcp->req_options & REQ_OPTION_IPADDR,
>> +					IP_ADDRESS, &ipcp->peer_addr);
>> +	FILL_IP(ipcp->req_options & REQ_OPTION_DNS1,
>> +					PRIMARY_DNS_SERVER,
> &ipcp->dns1);
>> +	FILL_IP(ipcp->req_options & REQ_OPTION_DNS2,
>> +					SECONDARY_DNS_SERVER,
> &ipcp->dns2);
>> +	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS1,
>> +					PRIMARY_NBNS_SERVER,
> &ipcp->nbns1);
>> +	FILL_IP(ipcp->req_options & REQ_OPTION_NBNS2,
>> +					SECONDARY_NBNS_SERVER,
> &ipcp->nbns2);
>> +
> 
> Using ipcp->req_options is the wrong thing here.  That is used
> for options
> sent in a Configure-Req.  Here you're filling in options to be
> sent in a
> Configure-Nak, Configure-Reject or a Configure-Ack.  You
> should simply be filling
> these in ipcp_rcr.  Feel free to modify the FILL_IP macro to take an
> additional destination parameter.

Okay. I will do that.

>> +	ipcp->options_len = len;
>> +}
>> +
>> +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32
>> local_addr, +				guint32 peer_addr, +				guint32 dns1, guint32 dns2,
>> +				guint32 nbns1, guint32 nbns2)
>> +{
>> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
>> +
>> +	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>> +				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>> +				REQ_OPTION_NBNS2;
>> +
>> +	ipcp->local_addr = local_addr;
>> +	ipcp->peer_addr = peer_addr;
>> +	ipcp->dns1 = dns1;
>> +	ipcp->dns2 = dns2;
>> +	ipcp->nbns1 = nbns1;
>> +	ipcp->nbns2 = nbns2;
>> +
>> +	ipcp_generate_config_options(ipcp);
> 
> Please note that this function is currently having no effect,
> you're missing a
> call to pppcp_set_local_options.  However, in the case of a server, we
> actually do not want to request DNS or NBNS, and we should
> only send our local
> address to the peer if it is non-zero.  Please note that all
> PPP modems we
> have encountered so far, do not send their local IP address in
> a Configure-
> Request.  Thus calling ipcp_set_server_info should end up
> generating an empty
> Configure-Request or with only the local IP address present.

It's confused me a little. Please correct me if I am wrong. I think PPP modem should send their local IP address to peer in a Configure-Request. So client could accept the peer address in ipcp_rcr.

In the case of a server, ipcp_set_server_info() should set local, peer, DNS and NBNS info and call pppcp_set_local_options(). And server only send our local address to peer in a Configure-Request.

>> +}
>> +
>>  static void ipcp_reset_config_options(struct ipcp_data *ipcp)  {
>>  	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>>  				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>>  				REQ_OPTION_NBNS2;
>> -	ipcp->ipaddr = 0;
>> +	ipcp->local_addr = 0;
> 
> In the case of a server, the local_address should not be reset.

It seems we need to add a flag in ipcp_data to indicate whether it's server or not. Does it make sense?

>> +	ipcp->peer_addr = 0;
>>  	ipcp->dns1 = 0;
>>  	ipcp->dns2 = 0;
>>  	ipcp->nbns1 = 0;
>> @@ -274,11 +321,54 @@ static enum rcr_result ipcp_rcr(struct
>>  pppcp_data *pppcp, guint8 **new_options, guint16 *new_len)
>>  {
>>  	struct ppp_option_iter iter;
>> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
>> +	guint32 peer_addr;
>> +	guint32 dns1;
>> +	guint32 dns2;
>> 
>>  	ppp_option_iter_init(&iter, packet);
>> 
>> -	if (ppp_option_iter_next(&iter) == FALSE)
>> -		return RCR_ACCEPT;
> 
> Again, be careful here.  In the case of a client we should
> only accept the
> Server's IP-Address, nothing else.
> 
>> +	while (ppp_option_iter_next(&iter) == TRUE) {
>> +		const guint8 *data = ppp_option_iter_get_data(&iter); +
>> +		switch (ppp_option_iter_get_type(&iter)) {
>> +		case IP_ADDRESS:
>> +			memcpy(&peer_addr, data, 4);
>> +			break;
>> +		case PRIMARY_DNS_SERVER:
>> +			memcpy(&dns1, data, 4);
>> +			break;
>> +		case SECONDARY_DNS_SERVER:
>> +			memcpy(&dns2, data, 4);
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (peer_addr && dns1 && dns2) {
>> +		if (ipcp->peer_addr == 0) {
>> +			/* RFC 1332 Section 3.3
>> +			 * Accept the value of IP address as
> peer IP address
>> +			 */
>> +			ipcp->peer_addr = peer_addr;
>> +
> 
> This part doesn't really make any sense, if peer is sending 0,
> he's asking us
> for an IP.

For client side, the peer server address is zero initially. So it need to accept the server address immediately.

>> +			return RCR_ACCEPT;
>> +		} else if (ipcp->peer_addr == peer_addr)
>> +			return RCR_ACCEPT;
> 
> You need to ensure that not only the peer_addr matches, but
> also the dns /
> nbns addresses.  In our case we should be Configure-Rejecting
> the NBNS options
> and Configure-Naking DNS ones if they're set to zero or don't
> match what the
> peer is sending us.

Agree. Will update that part.

>> +	} else {
>> +		/* Peer requests us to send ip address in config options */ +		if
>> (ipcp->peer_addr) { +			struct ipcp_data *peer;
>> +
>> +			peer = g_memdup(ipcp, sizeof(struct ipcp_data));
>> +			ipcp_generate_peer_config_options(peer);
> 
> Yuck, please don't do this.  The main PPPCP layer will take
> care of freeing
> new_options, however you're now leaking the rest of the
> ipcp_data structure.

Ok. I will take care about it and resend the patch later.

>> +
>> +			*new_len = peer->options_len;
>> +			*new_options = peer->options;
>> +			return RCR_NAK;
>> +		}
>> +	}
>> 
>>  	/* Reject all options */
>>  	*new_len = packet->length - sizeof(*packet);
>> 
> 
> Regards,
> -Denis

Regards,
Zhenhua

Re: [PATCH 03/11] gatppp: Add PPP server extension

[Denis Kenzior]

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

Hi Zhenhua,

> >> +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32
> >> local_addr, +				guint32 peer_addr, +				guint32 dns1, guint32 dns2,
> >> +				guint32 nbns1, guint32 nbns2)
> >> +{
> >> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
> >> +
> >> +	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
> >> +				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
> >> +				REQ_OPTION_NBNS2;
> >> +
> >> +	ipcp->local_addr = local_addr;
> >> +	ipcp->peer_addr = peer_addr;
> >> +	ipcp->dns1 = dns1;
> >> +	ipcp->dns2 = dns2;
> >> +	ipcp->nbns1 = nbns1;
> >> +	ipcp->nbns2 = nbns2;
> >> +
> >> +	ipcp_generate_config_options(ipcp);
> >
> > Please note that this function is currently having no effect,
> > you're missing a
> > call to pppcp_set_local_options.  However, in the case of a server, we
> > actually do not want to request DNS or NBNS, and we should
> > only send our local
> > address to the peer if it is non-zero.  Please note that all
> > PPP modems we
> > have encountered so far, do not send their local IP address in
> > a Configure-
> > Request.  Thus calling ipcp_set_server_info should end up
> > generating an empty
> > Configure-Request or with only the local IP address present.
> 
> It's confused me a little. Please correct me if I am wrong. I think PPP
>  modem should send their local IP address to peer in a Configure-Request.
>  So client could accept the peer address in ipcp_rcr.

This is correct if the PPP server wants to do so, it is perfectly OK for the 
server not to send its IP address (e.g. if it doesn't have one.)  This is what 
we observe on most PPP modems.

> 
> In the case of a server, ipcp_set_server_info() should set local, peer, DNS
>  and NBNS info and call pppcp_set_local_options(). And server only send our
>  local address to peer in a Configure-Request.

I'd modify this slightly.  If ipcp_set_server_info is called with server's 
local address as 0, then don't even send IP-Address in a configure Request.  
This wouldn't make sense, in effect the server is asking the client to provide 
its IP address. Otherwise, go ahead and send it (as the client will simply 
ack).

> 
> >> +}
> >> +
> >>  static void ipcp_reset_config_options(struct ipcp_data *ipcp)  {
> >>  	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
> >>  				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
> >>  				REQ_OPTION_NBNS2;
> >> -	ipcp->ipaddr = 0;
> >> +	ipcp->local_addr = 0;
> >
> > In the case of a server, the local_address should not be reset.
> 
> It seems we need to add a flag in ipcp_data to indicate whether it's server
>  or not. Does it make sense?

Might be a good idea.

Regards,
-Denis

RE: [PATCH 03/11] gatppp: Add PPP server extension

[Zhang, Zhenhua]

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

Hi Denis,

Denis Kenzior wrote:
> Hi Zhenhua,
> 
>>>> +void ipcp_set_server_info(struct pppcp_data *pppcp, guint32
>>>> local_addr, +				guint32
> peer_addr, +				guint32 dns1, guint32 dns2,
>>>> +				guint32 nbns1, guint32 nbns2)
>>>> +{
>>>> +	struct ipcp_data *ipcp = pppcp_get_data(pppcp);
>>>> +
>>>> +	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>>>> +				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>>>> +				REQ_OPTION_NBNS2;
>>>> +
>>>> +	ipcp->local_addr = local_addr;
>>>> +	ipcp->peer_addr = peer_addr;
>>>> +	ipcp->dns1 = dns1;
>>>> +	ipcp->dns2 = dns2;
>>>> +	ipcp->nbns1 = nbns1;
>>>> +	ipcp->nbns2 = nbns2;
>>>> +
>>>> +	ipcp_generate_config_options(ipcp);
>>> 
>>> Please note that this function is currently having no effect,
>>> you're missing a call to pppcp_set_local_options.  However, in the
>>> case of a server, we actually do not want to request DNS or NBNS,
>>> and we should only send our local address to the peer if it is
>>> non-zero.  Please note that all PPP modems we have encountered so
>>> far, do not send their local IP address in a Configure- Request. 
>>> Thus calling ipcp_set_server_info should end up generating an empty
>>> Configure-Request or with only the local IP address present. 
>> 
>> It's confused me a little. Please correct me if I am wrong. I think
>>  PPP modem should send their local IP address to peer in a
>>  Configure-Request. So client could accept the peer address in
>> ipcp_rcr. 
> 
> This is correct if the PPP server wants to do so, it is perfectly OK
> for the server not to send its IP address (e.g. if it doesn't have
> one.)  This is what we observe on most PPP modems.

Okay. That's fine. At least, PPP server need to send peer_addr in Conf-Nak reply. Client 'thinks' it's server address.

>> 
>> In the case of a server, ipcp_set_server_info() should set local,
>>  peer, DNS and NBNS info and call pppcp_set_local_options(). And
>>  server only send our local address to peer in a Configure-Request.
> 
> I'd modify this slightly.  If ipcp_set_server_info is called with
> server's local address as 0, then don't even send IP-Address in a
> configure Request. This wouldn't make sense, in effect the server is
> asking the client to provide its IP address. Otherwise, go ahead and
> send it (as the client will simply ack).

No problems. If server local address is zero, we don't send IP in a configure request.

>> 
>>>> +}
>>>> +
>>>>  static void ipcp_reset_config_options(struct ipcp_data *ipcp)  {
>>>>  	ipcp->req_options = REQ_OPTION_IPADDR | REQ_OPTION_DNS1 |
>>>>  				REQ_OPTION_DNS2 | REQ_OPTION_NBNS1 |
>>>>  				REQ_OPTION_NBNS2;
>>>> -	ipcp->ipaddr = 0;
>>>> +	ipcp->local_addr = 0;
>>> 
>>> In the case of a server, the local_address should not be reset.
>> 
>> It seems we need to add a flag in ipcp_data to indicate whether it's
>>  server or not. Does it make sense?
> 
> Might be a good idea.
> 
> Regards,
> -Denis



Regards,
Zhenhua