[PATCH 0/4] add PPP_DEAD and PPP_TERMINATE support

[PATCH 0/4] add PPP_DEAD and PPP_TERMINATE support

[Kristen Carlson Accardi]

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

These patches complete implementation of the PPP_DEAD and PPP_TERMINATE 
phases, as well as allow this to be tested via gsmdial.

Kristen Carlson Accardi (4):
  remove unneeded debug statement
  add tracing for PPP terminate path
  separate memory cleanup from PPP shutdown
  gsmdial: shutdown ppp link if we have one.

 gatchat/gatppp.c  |   25 +++++++------------------
 gatchat/gsmdial.c |    4 +++-
 gatchat/ppp.c     |   27 +++++++++++++++++++++++----
 gatchat/ppp_cp.c  |   10 ++++++++++
 4 files changed, 43 insertions(+), 23 deletions(-)

[PATCH 1/4] remove unneeded debug statement

[Kristen Carlson Accardi]

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

cleanup old debug print statement
---
 gatchat/ppp.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/gatchat/ppp.c b/gatchat/ppp.c
index 4612976..f3bfba7 100644
--- a/gatchat/ppp.c
+++ b/gatchat/ppp.c
@@ -328,10 +328,8 @@ gboolean ppp_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
 	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");
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
 		return FALSE;
-	}
 
 	if (cond & G_IO_IN) {
 		status = g_io_channel_read_chars(channel, buf, 256,
-- 
1.6.6.1

[PATCH 2/4] add tracing for PPP terminate path

[Kristen Carlson Accardi]

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

Insert some calls to pppcp_trace() for future debugging needs.
---
 gatchat/ppp_cp.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/gatchat/ppp_cp.c b/gatchat/ppp_cp.c
index 4cc6c13..df14a33 100644
--- a/gatchat/ppp_cp.c
+++ b/gatchat/ppp_cp.c
@@ -399,6 +399,8 @@ static void pppcp_send_terminate_request(struct pppcp_data *data)
 	struct pppcp_packet *packet;
 	struct pppcp_timer_data *timer_data = &data->terminate_timer_data;
 
+	pppcp_trace(data);
+
 	/*
 	 * the data field can be used by the sender (us).
 	 * leave this empty for now.
@@ -430,6 +432,8 @@ static void pppcp_send_terminate_ack(struct pppcp_data *data,
 	struct pppcp_packet *packet;
 	struct pppcp_packet *pppcp_header = (struct pppcp_packet *) request;
 
+	pppcp_trace(data);
+
 	packet = pppcp_packet_new(data, TERMINATE_ACK, 0);
 
 	/* match identifier of the request */
@@ -621,6 +625,8 @@ static void pppcp_open_event(struct pppcp_data *data, guint8 *packet, guint len)
 
 static void pppcp_close_event(struct pppcp_data *data, guint8* packet, guint len)
 {
+	pppcp_trace(data);
+
 	switch (data->state) {
 	case INITIAL:
 		pppcp_transition_state(INITIAL, data);
@@ -1366,6 +1372,8 @@ static guint8 pppcp_process_configure_reject(struct pppcp_data *data,
 static guint8 pppcp_process_terminate_request(struct pppcp_data *data,
 					struct pppcp_packet *packet)
 {
+	pppcp_trace(data);
+
 	return RTR;
 }
 
@@ -1378,6 +1386,8 @@ static guint8 pppcp_process_terminate_ack(struct pppcp_data *data,
 	 * even if the identifiers don't match, we still handle
 	 * a terminate ack, as it is allowed to be unelicited
 	 */
+	pppcp_trace(data);
+
 	return RTA;
 }
 
-- 
1.6.6.1

[PATCH 3/4] separate memory cleanup from PPP shutdown

[Kristen Carlson Accardi]

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

don't free memory at PPP shutdown, because we need to be able to
send terminate traffic.  Free memory when we have reached the
PPP_DEAD phase instead.
---
 gatchat/gatppp.c |   25 +++++++------------------
 gatchat/ppp.c    |   23 ++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 68c4dd1..8f19636 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -66,22 +66,8 @@ void g_at_ppp_set_disconnect_function(GAtPPP *ppp,
 
 void g_at_ppp_shutdown(GAtPPP *ppp)
 {
-	/* close the ppp */
+	/* close the ppp link */
 	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);
-
-	/* remove lcp */
-	lcp_free(ppp->lcp);
-
-	/* remove auth */
-	auth_free(ppp->auth);
 }
 
 void g_at_ppp_ref(GAtPPP *ppp)
@@ -91,10 +77,13 @@ void g_at_ppp_ref(GAtPPP *ppp)
 
 void g_at_ppp_unref(GAtPPP *ppp)
 {
-	if (g_atomic_int_dec_and_test(&ppp->ref_count)) {
+	if (g_atomic_int_dec_and_test(&ppp->ref_count))
 		g_at_ppp_shutdown(ppp);
-		g_free(ppp);
-	}
+
+	/*
+	 * we can't free the link yet, because we need to terminate
+	 * the link first.
+	 */
 }
 
 GAtPPP *g_at_ppp_new(GIOChannel *modem)
diff --git a/gatchat/ppp.c b/gatchat/ppp.c
index f3bfba7..d771c3f 100644
--- a/gatchat/ppp.c
+++ b/gatchat/ppp.c
@@ -372,7 +372,28 @@ static void ppp_authenticate(GAtPPP *ppp)
 
 static void ppp_dead(GAtPPP *ppp)
 {
-	/* re-initialize everything */
+	/* notify interested parties */
+	if (ppp->disconnect_cb)
+		ppp->disconnect_cb(ppp, ppp->disconnect_data);
+
+	if (g_atomic_int_get(&ppp->ref_count))
+		return;
+
+	/* 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);
+
+	/* remove lcp */
+	lcp_free(ppp->lcp);
+
+	/* remove auth */
+	auth_free(ppp->auth);
+
+	g_free(ppp);
 }
 
 static void ppp_network(GAtPPP *ppp)
-- 
1.6.6.1

[PATCH 4/4] gsmdial: shutdown ppp link if we have one.

[Kristen Carlson Accardi]

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

If we have created a ppp link, shut it down when the signal handler
is called.
---
 gatchat/gsmdial.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index aee9eea..a18f486 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -101,6 +101,8 @@ static gboolean signal_cb(GIOChannel *channel, GIOCondition cond, gpointer data)
 	case SIGTERM:
 		if (terminated == 0) {
 			char buf[64];
+			if (ppp)
+				g_at_ppp_shutdown(ppp);
 
 			g_timeout_add_seconds(10, quit_eventloop, NULL);
 			sprintf(buf, "AT+CFUN=%u", option_offmode);
@@ -285,7 +287,7 @@ static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ppp = g_at_ppp_new(channel);
 	if (!ppp) {
 		g_print("Unable to create PPP object\n");
-		return;
+		exit(1);
 	}
 	g_at_ppp_set_credentials(ppp, option_username,
 				option_password);
-- 
1.6.6.1

Re: [PATCH 0/4] add PPP_DEAD and PPP_TERMINATE support

[Marcel Holtmann]

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

Hi Kristen,

> These patches complete implementation of the PPP_DEAD and PPP_TERMINATE 
> phases, as well as allow this to be tested via gsmdial.
> 
> Kristen Carlson Accardi (4):
>   remove unneeded debug statement
>   add tracing for PPP terminate path
>   separate memory cleanup from PPP shutdown
>   gsmdial: shutdown ppp link if we have one.
> 
>  gatchat/gatppp.c  |   25 +++++++------------------
>  gatchat/gsmdial.c |    4 +++-
>  gatchat/ppp.c     |   27 +++++++++++++++++++++++----
>  gatchat/ppp_cp.c  |   10 ++++++++++
>  4 files changed, 43 insertions(+), 23 deletions(-)

all four patches have been applied. Thanks.

Regards

Marcel