Re: [PATCHv2 1/6] gatchat: Add IPv6 Control Protocol

Re: [PATCHv2 1/6] gatchat: Add IPv6 Control Protocol

[Denis Kenzior]

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

Hi Oleg,

On 11/08/2011 06:46 AM, Oleg Zhurakivskyy wrote:
> ---
>  Makefile.am          |    2 +-
>  gatchat/ppp.h        |    8 +
>  gatchat/ppp_ipv6cp.c |  377 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 386 insertions(+), 1 deletions(-)
>  create mode 100644 gatchat/ppp_ipv6cp.c

Patch has been applied, however you made one small error here:


<snip>

> +		switch (type) {
> +		case IPV6CP_INTERFACE_ID:
> +			memcpy(&ipv6cp->peer_addr, data,
> +					sizeof(ipv6cp->peer_addr));
> +			if (ipv6cp->peer_addr != 0)
> +				break;
> +			/*
> +			 * Fall through, reject zero Interface ID
> +			 */
> +			break;

The break statement seems to be in error and I fixed it in a follow on
commit.  Let me know if I was wrong.

> +		default:
> +			if (options == NULL) {
> +				guint16 max_len = ntohs(packet->length) - 4;
> +				options = g_new0(guint8, max_len);
> +			}
> +
> +			OPTION_COPY(options, len, options != NULL,
> +					type, data,
> +					ppp_option_iter_get_length(&iter));
> +			break;
> +		}
> +	}
> +

Regards,
-Denis

[PATCHv2 0/6] gatchat IPv6 CP support

[Oleg Zhurakivskyy]

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

Hello,

These patches correct issues found at the initial review.

New functionality:

- IPCP, IPv6 CP can co-exist.
- Add basic IPv6 CP connect callback actions.

I would appreciate your feedback.
 
Regards,
Oleg

Oleg Zhurakivskyy (6):
  gatchat: Add IPv6 Control Protocol
  gatppp: Add IPv6 CP hooks
  gsmdial: Add IPv6 CP option
  test-server: Add IPv6 CP options
  gatppp: Add IPv6 CP connect, disconnect callbacks
  gsmdial: Add IPv6 CP connect hook

 Makefile.am           |    2 +-
 gatchat/gatppp.c      |   99 ++++++++++++-
 gatchat/gatppp.h      |   12 ++-
 gatchat/gsmdial.c     |   48 ++++++
 gatchat/ppp.h         |   11 ++
 gatchat/ppp_ipv6cp.c  |  390 +++++++++++++++++++++++++++++++++++++++++++++++++
 gatchat/test-server.c |   23 +++-
 7 files changed, 580 insertions(+), 5 deletions(-)
 create mode 100644 gatchat/ppp_ipv6cp.c

-- 
1.7.5.4

[PATCHv2 1/6] gatchat: Add IPv6 Control Protocol

[Oleg Zhurakivskyy]

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

---
 Makefile.am          |    2 +-
 gatchat/ppp.h        |    8 +
 gatchat/ppp_ipv6cp.c |  377 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 386 insertions(+), 1 deletions(-)
 create mode 100644 gatchat/ppp_ipv6cp.c

diff --git a/Makefile.am b/Makefile.am
index 83b7737..44fdfff 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -76,7 +76,7 @@ gatchat_sources = gatchat/gatchat.h gatchat/gatchat.c \
 				gatchat/ppp.h gatchat/ppp_cp.h \
 				gatchat/ppp_cp.c gatchat/ppp_lcp.c \
 				gatchat/ppp_auth.c gatchat/ppp_net.c \
-				gatchat/ppp_ipcp.c
+				gatchat/ppp_ipcp.c gatchat/ppp_ipv6cp.c
 
 gisi_sources = gisi/client.c gisi/client.h gisi/common.h \
 				gisi/iter.c gisi/iter.h \
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index a20fe85..718575b 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -24,7 +24,9 @@
 #define LCP_PROTOCOL	0xc021
 #define CHAP_PROTOCOL	0xc223
 #define IPCP_PROTO	0x8021
+#define IPV6CP_PROTO	0x8057
 #define PPP_IP_PROTO	0x0021
+#define PPP_IPV6_PROTO	0x0057
 #define MD5		5
 
 #define DBG(p, fmt, arg...) do {				\
@@ -95,6 +97,12 @@ void ipcp_free(struct pppcp_data *data);
 void ipcp_set_server_info(struct pppcp_data *ipcp, guint32 peer_addr,
 				guint32 dns1, guint32 dns2);
 
+/* IPv6 CP related functions */
+struct pppcp_data *ipv6cp_new(GAtPPP *ppp, gboolean is_server,
+					const char *local, const char *peer,
+					GError **error);
+void ipv6cp_free(struct pppcp_data *data);
+
 /* CHAP related functions */
 struct ppp_chap *ppp_chap_new(GAtPPP *ppp, guint8 method);
 void ppp_chap_free(struct ppp_chap *chap);
diff --git a/gatchat/ppp_ipv6cp.c b/gatchat/ppp_ipv6cp.c
new file mode 100644
index 0000000..ec89fb7
--- /dev/null
+++ b/gatchat/ppp_ipv6cp.c
@@ -0,0 +1,377 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2009-2011  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 <arpa/inet.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include <errno.h>
+#include <string.h>
+
+#include <glib.h>
+
+#include "gatppp.h"
+#include "ppp.h"
+
+#define IPV6CP_SUPPORTED_CODES	((1 << PPPCP_CODE_TYPE_CONFIGURE_REQUEST) | \
+				(1 << PPPCP_CODE_TYPE_CONFIGURE_ACK) | \
+				(1 << PPPCP_CODE_TYPE_CONFIGURE_NAK) | \
+				(1 << PPPCP_CODE_TYPE_CONFIGURE_REJECT) | \
+				(1 << PPPCP_CODE_TYPE_TERMINATE_REQUEST) | \
+				(1 << PPPCP_CODE_TYPE_TERMINATE_ACK) | \
+				(1 << PPPCP_CODE_TYPE_CODE_REJECT))
+
+#define OPTION_COPY(_options, _len, _req, _type, _var, _opt_len)	\
+	if (_req) {							\
+		_options[_len] = _type;					\
+		_options[_len + 1] = _opt_len + 2;			\
+		memcpy(_options + _len + 2, _var, _opt_len);		\
+		_len += _opt_len + 2;					\
+	}
+
+/* We request only IPv6 Interface Id */
+#define IPV6CP_MAX_CONFIG_OPTION_SIZE	10
+#define IPV6CP_MAX_FAILURE		3
+#define IPV6CP_ERROR ipv6cp_error_quark()
+
+enum ipv6cp_option_types {
+	IPV6CP_INTERFACE_ID = 1,
+};
+
+struct ipv6cp_data {
+	guint8 options[IPV6CP_MAX_CONFIG_OPTION_SIZE];
+	guint16 options_len;
+	guint8 req_options;
+	guint64 local_addr;
+	guint64 peer_addr;
+	gboolean is_server;
+};
+
+static GQuark ipv6cp_error_quark(void)
+{
+	return g_quark_from_static_string("ipv6cp");
+}
+
+static void ipv6cp_generate_config_options(struct ipv6cp_data *ipv6cp)
+{
+	guint16 len = 0;
+
+	OPTION_COPY(ipv6cp->options, len,
+			ipv6cp->req_options & IPV6CP_INTERFACE_ID,
+			IPV6CP_INTERFACE_ID, &ipv6cp->local_addr,
+			sizeof(ipv6cp->local_addr));
+
+	ipv6cp->options_len = len;
+}
+
+static void ipv6cp_reset_config_options(struct ipv6cp_data *ipv6cp)
+{
+	ipv6cp->req_options = IPV6CP_INTERFACE_ID;
+
+	ipv6cp_generate_config_options(ipv6cp);
+}
+
+static void ipv6cp_up(struct pppcp_data *pppcp)
+{
+
+}
+
+static void ipv6cp_down(struct pppcp_data *pppcp)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+
+	ipv6cp_reset_config_options(ipv6cp);
+
+	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
+}
+
+static void ipv6cp_finished(struct pppcp_data *pppcp)
+{
+
+}
+
+static enum rcr_result ipv6cp_server_rcr(struct ipv6cp_data *ipv6cp,
+					const struct pppcp_packet *packet,
+					guint8 **new_options, guint16 *new_len)
+{
+	struct ppp_option_iter iter;
+	guint8 nak_options[IPV6CP_MAX_CONFIG_OPTION_SIZE];
+	guint16 len = 0;
+	guint8 *rej_options = NULL;
+	guint16 rej_len = 0;
+	guint64 addr;
+
+	ppp_option_iter_init(&iter, packet);
+
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		guint8 type = ppp_option_iter_get_type(&iter);
+		const void *data = ppp_option_iter_get_data(&iter);
+
+		switch (type) {
+		case IPV6CP_INTERFACE_ID:
+			memcpy(&addr, data, sizeof(addr));
+
+			OPTION_COPY(nak_options, len,
+					addr != ipv6cp->peer_addr || addr == 0,
+					type, &ipv6cp->peer_addr,
+					ppp_option_iter_get_length(&iter));
+			break;
+		default:
+			if (rej_options == NULL) {
+				guint16 max_len = ntohs(packet->length) - 4;
+				rej_options = g_new0(guint8, max_len);
+			}
+
+			OPTION_COPY(rej_options, rej_len, rej_options != NULL,
+					type, data,
+					ppp_option_iter_get_length(&iter));
+			break;
+		}
+	}
+
+	if (rej_len > 0) {
+		*new_len = rej_len;
+		*new_options = rej_options;
+
+		return RCR_REJECT;
+	}
+
+	if (len > 0) {
+		*new_len = len;
+		*new_options = g_memdup(nak_options, len);
+
+		return RCR_NAK;
+	}
+
+	return RCR_ACCEPT;
+}
+
+static enum rcr_result ipv6cp_client_rcr(struct ipv6cp_data *ipv6cp,
+					const struct pppcp_packet *packet,
+					guint8 **new_options, guint16 *new_len)
+{
+	struct ppp_option_iter iter;
+	guint8 *options = NULL;
+	guint8 len = 0;
+
+	ppp_option_iter_init(&iter, packet);
+
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		guint8 type = ppp_option_iter_get_type(&iter);
+		const void *data = ppp_option_iter_get_data(&iter);
+
+		switch (type) {
+		case IPV6CP_INTERFACE_ID:
+			memcpy(&ipv6cp->peer_addr, data,
+					sizeof(ipv6cp->peer_addr));
+			if (ipv6cp->peer_addr != 0)
+				break;
+			/*
+			 * Fall through, reject zero Interface ID
+			 */
+			break;
+		default:
+			if (options == NULL) {
+				guint16 max_len = ntohs(packet->length) - 4;
+				options = g_new0(guint8, max_len);
+			}
+
+			OPTION_COPY(options, len, options != NULL,
+					type, data,
+					ppp_option_iter_get_length(&iter));
+			break;
+		}
+	}
+
+	if (len > 0) {
+		*new_len = len;
+		*new_options = options;
+
+		return RCR_REJECT;
+	}
+
+	return RCR_ACCEPT;
+}
+
+static enum rcr_result ipv6cp_rcr(struct pppcp_data *pppcp,
+					const struct pppcp_packet *packet,
+					guint8 **new_options, guint16 *new_len)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+
+	if (ipv6cp->is_server)
+		return ipv6cp_server_rcr(ipv6cp, packet, new_options, new_len);
+	else
+		return ipv6cp_client_rcr(ipv6cp, packet, new_options, new_len);
+}
+
+static void ipv6cp_rca(struct pppcp_data *pppcp,
+					const struct pppcp_packet *packet)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+	struct ppp_option_iter iter;
+
+	if (ipv6cp->is_server)
+		return;
+
+	ppp_option_iter_init(&iter, packet);
+
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		const guint8 *data = ppp_option_iter_get_data(&iter);
+
+		switch (ppp_option_iter_get_type(&iter)) {
+		case IPV6CP_INTERFACE_ID:
+			memcpy(&ipv6cp->local_addr, data,
+					sizeof(ipv6cp->local_addr));
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+static void ipv6cp_rcn_nak(struct pppcp_data *pppcp,
+				const struct pppcp_packet *packet)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+	struct ppp_option_iter iter;
+
+	if (ipv6cp->is_server)
+		return;
+
+	ppp_option_iter_init(&iter, packet);
+
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		const guint8 *data = ppp_option_iter_get_data(&iter);
+
+		switch (ppp_option_iter_get_type(&iter)) {
+		case IPV6CP_INTERFACE_ID:
+			ipv6cp->req_options |= IPV6CP_INTERFACE_ID;
+			memcpy(&ipv6cp->local_addr, data,
+				sizeof(ipv6cp->local_addr));
+			break;
+		default:
+			break;
+		}
+	}
+
+	ipv6cp_generate_config_options(ipv6cp);
+	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
+}
+
+static void ipv6cp_rcn_rej(struct pppcp_data *pppcp,
+				const struct pppcp_packet *packet)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+	struct ppp_option_iter iter;
+
+	ppp_option_iter_init(&iter, packet);
+
+	while (ppp_option_iter_next(&iter) == TRUE) {
+		switch (ppp_option_iter_get_type(&iter)) {
+		case IPV6CP_INTERFACE_ID:
+			ipv6cp->req_options &= ~IPV6CP_INTERFACE_ID;
+			break;
+		default:
+			break;
+		}
+	}
+
+	ipv6cp_generate_config_options(ipv6cp);
+	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
+}
+
+struct pppcp_proto ipv6cp_proto = {
+	.proto			= IPV6CP_PROTO,
+	.name			= "ipv6cp",
+	.supported_codes	= IPV6CP_SUPPORTED_CODES,
+	.this_layer_up		= ipv6cp_up,
+	.this_layer_down	= ipv6cp_down,
+	.this_layer_finished	= ipv6cp_finished,
+	.rca			= ipv6cp_rca,
+	.rcn_nak		= ipv6cp_rcn_nak,
+	.rcn_rej		= ipv6cp_rcn_rej,
+	.rcr			= ipv6cp_rcr,
+};
+
+struct pppcp_data *ipv6cp_new(GAtPPP *ppp, gboolean is_server,
+					const char *local, const char *peer,
+					GError **error)
+{
+	struct ipv6cp_data *ipv6cp;
+	struct pppcp_data *pppcp;
+	struct in6_addr local_addr;
+	struct in6_addr peer_addr;
+
+	if (local == NULL)
+		memset(&local_addr, 0, sizeof(local_addr));
+	else if (inet_pton(AF_INET6, local, &local_addr) != 1) {
+		g_set_error(error, IPV6CP_ERROR, errno,
+				"Unable to set local Interface ID: %s",
+				strerror(errno));
+		return NULL;
+	}
+
+	if (peer == NULL)
+		memset(&peer_addr, 0, sizeof(peer_addr));
+	else if (inet_pton(AF_INET6, peer, &peer_addr) != 1) {
+		g_set_error(error, IPV6CP_ERROR, errno,
+				"Unable to set peer Interface ID: %s",
+				g_strerror(errno));
+		return NULL;
+	}
+
+	ipv6cp = g_try_new0(struct ipv6cp_data, 1);
+	if (ipv6cp == NULL)
+		return NULL;
+
+	pppcp = pppcp_new(ppp, &ipv6cp_proto, FALSE, IPV6CP_MAX_FAILURE);
+	if (pppcp == NULL) {
+		g_free(ipv6cp);
+		return NULL;
+	}
+
+	memcpy(&ipv6cp->local_addr, &local_addr.s6_addr[8],
+						sizeof(ipv6cp->local_addr));
+	memcpy(&ipv6cp->peer_addr, &peer_addr.s6_addr[8],
+						sizeof(ipv6cp->peer_addr));
+	ipv6cp->is_server = is_server;
+
+	pppcp_set_data(pppcp, ipv6cp);
+
+	ipv6cp_reset_config_options(ipv6cp);
+
+	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
+
+	return pppcp;
+}
+
+void ipv6cp_free(struct pppcp_data *data)
+{
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(data);
+
+	g_free(ipv6cp);
+	pppcp_free(data);
+}
-- 
1.7.5.4

[PATCHv2 2/6] gatppp: Add IPv6 CP hooks

[Oleg Zhurakivskyy]

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

---
 gatchat/gatppp.c     |   61 +++++++++++++++++++++++++++++++++++++++++++++++++-
 gatchat/gatppp.h     |    3 ++
 gatchat/ppp.h        |    3 ++
 gatchat/ppp_ipv6cp.c |   17 ++++++++++++-
 4 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index f767f4a..c425eae 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -62,6 +62,7 @@ struct _GAtPPP {
 	enum ppp_phase phase;
 	struct pppcp_data *lcp;
 	struct pppcp_data *ipcp;
+	struct pppcp_data *ipv6cp;
 	struct ppp_net *net;
 	struct ppp_chap *chap;
 	GAtHDLC *hdlc;
@@ -157,7 +158,8 @@ static inline gboolean ppp_drop_packet(GAtPPP *ppp, guint16 protocol)
 		return TRUE;
 	case PPP_PHASE_NETWORK:
 		if (protocol != LCP_PROTOCOL && protocol != CHAP_PROTOCOL &&
-					protocol != IPCP_PROTO)
+						protocol != IPCP_PROTO &&
+						protocol != IPV6CP_PROTO)
 			return TRUE;
 		break;
 	case PPP_PHASE_LINK_UP:
@@ -222,6 +224,10 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
 	case IPCP_PROTO:
 		pppcp_process_packet(ppp->ipcp, packet, len - offset);
 		break;
+	case IPV6CP_PROTO:
+		if (ppp->ipv6cp)
+			pppcp_process_packet(ppp->ipv6cp, packet, len - offset);
+		break;
 	case CHAP_PROTOCOL:
 		if (ppp->chap) {
 			ppp_chap_process_packet(ppp->chap, packet,
@@ -381,6 +387,12 @@ void ppp_auth_notify(GAtPPP *ppp, gboolean success)
 
 	ppp_enter_phase(ppp, PPP_PHASE_NETWORK);
 
+	/* Send UP & OPEN events to the IPv6 CP */
+	if (ppp->ipv6cp) {
+		pppcp_signal_open(ppp->ipv6cp);
+		pppcp_signal_up(ppp->ipv6cp);
+	}
+
 	/* Send UP & OPEN events to the IPCP layer */
 	pppcp_signal_open(ppp->ipcp);
 	pppcp_signal_up(ppp->ipcp);
@@ -431,6 +443,34 @@ void ppp_ipcp_finished_notify(GAtPPP *ppp)
 
 	/* Our IPCP parameter negotiation failed */
 	ppp->disconnect_reason = G_AT_PPP_REASON_IPCP_FAIL;
+
+	if (ppp->ipv6cp)
+		pppcp_signal_close(ppp->ipv6cp);
+	pppcp_signal_close(ppp->ipcp);
+	pppcp_signal_close(ppp->lcp);
+}
+
+void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer)
+{
+	DBG(ppp, "local: %s, peer: %s", local, peer);
+
+	ppp_enter_phase(ppp, PPP_PHASE_LINK_UP);
+}
+
+void ppp_ipv6cp_down_notify(GAtPPP *ppp)
+{
+	DBG(ppp, "");
+}
+
+void ppp_ipv6cp_finished_notify(GAtPPP *ppp)
+{
+	DBG(ppp, "");
+
+	if (ppp->phase != PPP_PHASE_NETWORK)
+		return;
+
+	ppp->disconnect_reason = G_AT_PPP_REASON_IPV6CP_FAIL;
+	pppcp_signal_close(ppp->ipv6cp);
 	pppcp_signal_close(ppp->ipcp);
 	pppcp_signal_close(ppp->lcp);
 }
@@ -732,6 +772,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
 
 	lcp_free(ppp->lcp);
 	ipcp_free(ppp->ipcp);
+	if (ppp->ipv6cp)
+		ipv6cp_free(ppp->ipv6cp);
 
 	if (ppp->ppp_dead_source) {
 		g_source_remove(ppp->ppp_dead_source);
@@ -772,6 +814,23 @@ void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled)
 	lcp_set_pfc_enabled(ppp->lcp, enabled);
 }
 
+gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
+					const char *local, const char *peer)
+{
+	GError *error = NULL;
+
+	ppp->ipv6cp = ipv6cp_new(ppp, is_server, local, peer, &error);
+	if (ppp->ipv6cp == NULL) {
+		if (error != NULL) {
+			DBG(ppp, "%s", error->message);
+			g_error_free(error);
+		}
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index b5a2234..a41bf08 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -37,6 +37,7 @@ typedef enum _GAtPPPDisconnectReason {
 	G_AT_PPP_REASON_UNKNOWN,
 	G_AT_PPP_REASON_AUTH_FAIL,	/* Failed to authenticate */
 	G_AT_PPP_REASON_IPCP_FAIL,	/* Failed to negotiate IPCP */
+	G_AT_PPP_REASON_IPV6CP_FAIL,	/* Failed to negotiate IPV6CP */
 	G_AT_PPP_REASON_NET_FAIL,	/* Failed to create tun */
 	G_AT_PPP_REASON_PEER_CLOSED,	/* Peer initiated a close */
 	G_AT_PPP_REASON_LINK_DEAD,	/* Link to the peer died */
@@ -81,6 +82,8 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
 
 void g_at_ppp_set_acfc_enabled(GAtPPP *ppp, gboolean enabled);
 void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled);
+gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
+					const char *local, const char *peer);
 
 #ifdef __cplusplus
 }
diff --git a/gatchat/ppp.h b/gatchat/ppp.h
index 718575b..a21ebbc 100644
--- a/gatchat/ppp.h
+++ b/gatchat/ppp.h
@@ -128,6 +128,9 @@ 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);
+void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer);
+void ppp_ipv6cp_down_notify(GAtPPP *ppp);
+void ppp_ipv6cp_finished_notify(GAtPPP *ppp);
 void ppp_lcp_up_notify(GAtPPP *ppp);
 void ppp_lcp_down_notify(GAtPPP *ppp);
 void ppp_lcp_finished_notify(GAtPPP *ppp);
diff --git a/gatchat/ppp_ipv6cp.c b/gatchat/ppp_ipv6cp.c
index ec89fb7..75ce8f2 100644
--- a/gatchat/ppp_ipv6cp.c
+++ b/gatchat/ppp_ipv6cp.c
@@ -95,7 +95,19 @@ static void ipv6cp_reset_config_options(struct ipv6cp_data *ipv6cp)
 
 static void ipv6cp_up(struct pppcp_data *pppcp)
 {
+	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
+	struct in6_addr local_addr, peer_addr;
+	char local[INET6_ADDRSTRLEN], peer[INET6_ADDRSTRLEN];
 
+	memset(&local_addr, 0, sizeof(local_addr));
+	memcpy(&local_addr.s6_addr[8], &ipv6cp->local_addr,
+					sizeof(ipv6cp->local_addr));
+	memset(&peer_addr, 0, sizeof(peer_addr));
+	memcpy(&peer_addr.s6_addr[8], &ipv6cp->peer_addr,
+					sizeof(ipv6cp->peer_addr));
+	ppp_ipv6cp_up_notify(pppcp_get_ppp(pppcp),
+		inet_ntop(AF_INET6, &local_addr, local, INET6_ADDRSTRLEN),
+		inet_ntop(AF_INET6, &peer_addr, peer, INET6_ADDRSTRLEN));
 }
 
 static void ipv6cp_down(struct pppcp_data *pppcp)
@@ -105,11 +117,12 @@ static void ipv6cp_down(struct pppcp_data *pppcp)
 	ipv6cp_reset_config_options(ipv6cp);
 
 	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
+	ppp_ipv6cp_down_notify(pppcp_get_ppp(pppcp));
 }
 
 static void ipv6cp_finished(struct pppcp_data *pppcp)
 {
-
+	ppp_ipv6cp_finished_notify(pppcp_get_ppp(pppcp));
 }
 
 static enum rcr_result ipv6cp_server_rcr(struct ipv6cp_data *ipv6cp,
-- 
1.7.5.4

[PATCHv2 3/6] gsmdial: Add IPv6 CP option

[Oleg Zhurakivskyy]

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

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

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index be159eb..b4b2015 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -59,6 +59,7 @@ static gchar *option_pppdump = NULL;
 static gboolean option_bluetooth = FALSE;
 static gboolean option_acfc = FALSE;
 static gboolean option_pfc = FALSE;
+static gboolean option_ipv6cp = FALSE;
 
 static GAtPPP *ppp;
 static GAtChat *control;
@@ -372,6 +373,9 @@ static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	if (option_pppdump)
 		g_at_ppp_set_recording(ppp, option_pppdump);
 
+	if (option_ipv6cp)
+		g_at_ppp_set_ipv6cp_info(ppp, FALSE, NULL, NULL);
+
 	g_at_ppp_set_credentials(ppp, option_username, option_password);
 
 	g_at_ppp_set_acfc_enabled(ppp, option_acfc);
@@ -687,6 +691,8 @@ static GOptionEntry options[] = {
 				"Use Protocol Field Compression" },
 	{ "acfc", 0, 0, G_OPTION_ARG_NONE, &option_acfc,
 				"Use Address & Control Field Compression" },
+	{ "ipv6cp", '6', 0, G_OPTION_ARG_NONE, &option_ipv6cp,
+				"Enable IPv6 CP" },
 	{ NULL },
 };
 
-- 
1.7.5.4

[PATCHv2 4/6] test-server: Add IPv6 CP options

[Oleg Zhurakivskyy]

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

---
 gatchat/test-server.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gatchat/test-server.c b/gatchat/test-server.c
index 6f1d06d..cd14da7 100644
--- a/gatchat/test-server.c
+++ b/gatchat/test-server.c
@@ -58,6 +58,9 @@ static int modem_creg = 0;
 static int modem_cgreg = 0;
 static int network_status = 4;
 static int network_attach = 0;
+static gboolean ipv6cp = FALSE;
+static const char *ipv6_local_addr = NULL;
+static const char *ipv6_peer_addr = NULL;
 
 struct sock_server{
 	int server_sock;
@@ -176,6 +179,10 @@ static gboolean setup_ppp(GAtServer *server)
 	g_at_ppp_set_server_info(ppp, "192.168.1.2",
 					"10.10.10.10", "10.10.10.11");
 
+	if (ipv6cp)
+		g_at_ppp_set_ipv6cp_info(ppp, TRUE, ipv6_local_addr,
+							ipv6_peer_addr);
+
 	return TRUE;
 }
 
@@ -1118,11 +1125,14 @@ static void usage(void)
 {
 	g_print("test-server - AT Server testing\n"
 		"Usage:\n");
-	g_print("\ttest-server [-t type]\n");
+	g_print("\ttest-server [-t type] [-6] [-l addr] [-p addr]\n");
 	g_print("Types:\n"
 		"\t0: Pseudo TTY port (default)\n"
 		"\t1: TCP sock at port 12346)\n"
 		"\t2: Unix sock at ./server_sock\n");
+	g_print("-6\tEnable IPv6 CP\n");
+	g_print("-l\tSpecify local IPv6 address\n");
+	g_print("-p\tSpecify peer IPv6 address\n");
 }
 
 int main(int argc, char **argv)
@@ -1130,11 +1140,20 @@ int main(int argc, char **argv)
 	int opt, signal_source;
 	int type = 0;
 
-	while ((opt = getopt(argc, argv, "ht:")) != EOF) {
+	while ((opt = getopt(argc, argv, "h6l:p:t:")) != EOF) {
 		switch (opt) {
 		case 't':
 			type = atoi(optarg);
 			break;
+		case '6':
+			ipv6cp = TRUE;
+			break;
+		case 'l':
+			ipv6_local_addr = optarg;
+			break;
+		case 'p':
+			ipv6_peer_addr = optarg;
+			break;
 		case 'h':
 			usage();
 			exit(1);
-- 
1.7.5.4

[PATCHv2 5/6] gatppp: Add IPv6 CP connect, disconnect callbacks

[Oleg Zhurakivskyy]

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

---
 gatchat/gatppp.c |   38 ++++++++++++++++++++++++++++++++++++++
 gatchat/gatppp.h |    9 ++++++++-
 2 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index c425eae..5a34535 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -64,6 +64,7 @@ struct _GAtPPP {
 	struct pppcp_data *ipcp;
 	struct pppcp_data *ipv6cp;
 	struct ppp_net *net;
+	struct ppp_net *net6;
 	struct ppp_chap *chap;
 	GAtHDLC *hdlc;
 	gint mru;
@@ -75,6 +76,10 @@ struct _GAtPPP {
 	GAtPPPDisconnectFunc disconnect_cb;
 	gpointer disconnect_data;
 	GAtPPPDisconnectReason disconnect_reason;
+	GAtPPPConnect6Func connect6_cb;
+	gpointer connect6_data;
+	GAtPPPDisconnectFunc disconnect6_cb;
+	gpointer disconnect6_data;
 	GAtDebugFunc debugf;
 	gpointer debug_data;
 	gboolean sta_pending;
@@ -454,7 +459,19 @@ void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer)
 {
 	DBG(ppp, "local: %s, peer: %s", local, peer);
 
+	ppp->net6 = ppp_net_new(ppp, -1);
+
+	if (ppp->net6 == NULL) {
+		ppp->disconnect_reason = G_AT_PPP_REASON_NET_FAIL;
+		pppcp_signal_close(ppp->lcp);
+		return;
+	}
+
 	ppp_enter_phase(ppp, PPP_PHASE_LINK_UP);
+
+	if (ppp->connect6_cb)
+		ppp->connect6_cb(ppp_net_get_interface(ppp->net6),
+					local, peer, ppp->connect6_data);
 }
 
 void ppp_ipv6cp_down_notify(GAtPPP *ppp)
@@ -831,6 +848,27 @@ gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
 	return TRUE;
 }
 
+void g_at_ppp_set_ipv6_connect_function(GAtPPP *ppp, GAtPPPConnect6Func func,
+							gpointer user_data)
+{
+	if (func == NULL)
+		return;
+
+	ppp->connect6_cb = func;
+	ppp->connect6_data = user_data;
+}
+
+void g_at_ppp_set_ipv6_disconnect_function(GAtPPP *ppp,
+						GAtPPPDisconnectFunc func,
+						gpointer user_data)
+{
+	if (func == NULL)
+		return;
+
+	ppp->disconnect6_cb = func;
+	ppp->disconnect6_data = user_data;
+}
+
 static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
 {
 	GAtPPP *ppp;
diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
index a41bf08..8fa1cd4 100644
--- a/gatchat/gatppp.h
+++ b/gatchat/gatppp.h
@@ -50,7 +50,8 @@ typedef void (*GAtPPPConnectFunc)(const char *iface, const char *local,
 					gpointer user_data);
 typedef void (*GAtPPPDisconnectFunc)(GAtPPPDisconnectReason reason,
 					gpointer user_data);
-
+typedef void (*GAtPPPConnect6Func)(const char *iface, const char *local,
+					const char *peer, gpointer user_data);
 GAtPPP *g_at_ppp_new(void);
 GAtPPP *g_at_ppp_server_new(const char *local);
 GAtPPP *g_at_ppp_server_new_full(const char *local, int fd);
@@ -84,6 +85,12 @@ void g_at_ppp_set_acfc_enabled(GAtPPP *ppp, gboolean enabled);
 void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled);
 gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
 					const char *local, const char *peer);
+void g_at_ppp_set_ipv6_connect_function(GAtPPP *ppp,
+					GAtPPPConnect6Func callback,
+					gpointer user_data);
+void g_at_ppp_set_ipv6_disconnect_function(GAtPPP *ppp,
+						GAtPPPDisconnectFunc func,
+						gpointer user_data);
 
 #ifdef __cplusplus
 }
-- 
1.7.5.4

[PATCHv2 6/6] gsmdial: Add IPv6 CP connect hook

[Oleg Zhurakivskyy]

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

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

diff --git a/gatchat/gsmdial.c b/gatchat/gsmdial.c
index b4b2015..8031245 100644
--- a/gatchat/gsmdial.c
+++ b/gatchat/gsmdial.c
@@ -344,6 +344,44 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
 	g_at_chat_resume(modem);
 }
 
+static void ppp_ipv6_connect(const char *iface, const char *local,
+				const char *peer, gpointer user_data)
+{
+	struct in6_addr in6_local, in6_peer;
+	char local_addr[INET6_ADDRSTRLEN], peer_addr[INET6_ADDRSTRLEN];
+	char buf[512];
+
+	/* Add IPv6 link-local prefix */
+
+	inet_pton(AF_INET6, local, &in6_local);
+	in6_local.s6_addr16[0] = htons(0xfe80);
+	inet_ntop(AF_INET6, &in6_local, local_addr, INET6_ADDRSTRLEN);
+
+	inet_pton(AF_INET6, peer, &in6_peer);
+	in6_peer.s6_addr16[0] = htons(0xfe80);
+	inet_ntop(AF_INET6, &in6_peer, peer_addr, INET6_ADDRSTRLEN);
+
+	g_print("Network Device: %s\n", iface);
+	g_print("IPv6 Local Address: %s\n", local_addr);
+	g_print("IPv6 Peer Address: %s\n", peer_addr);
+
+	if (getuid() != 0) {
+		g_print("Need root privilege to config PPP interface\n");
+		return;
+	}
+
+	snprintf(buf, sizeof(buf), "%s %s up", IFCONFIG_PATH, iface);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "%s %s inet6 add %s", IFCONFIG_PATH,
+			iface, local_addr);
+	execute(buf);
+
+	snprintf(buf, sizeof(buf), "ip -6 route add %s dev %s", peer_addr,
+								iface);
+	execute(buf);
+}
+
 static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	GAtIO *io;
@@ -373,8 +411,12 @@ static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	if (option_pppdump)
 		g_at_ppp_set_recording(ppp, option_pppdump);
 
-	if (option_ipv6cp)
+	if (option_ipv6cp) {
 		g_at_ppp_set_ipv6cp_info(ppp, FALSE, NULL, NULL);
+		g_at_ppp_set_ipv6_connect_function(ppp, ppp_ipv6_connect, NULL);
+		g_at_ppp_set_ipv6_disconnect_function(ppp, ppp_disconnect,
+							NULL);
+	}
 
 	g_at_ppp_set_credentials(ppp, option_username, option_password);
 
-- 
1.7.5.4

Re: [PATCHv2 1/6] gatchat: Add IPv6 Control Protocol

[Denis Kenzior]

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

Hi Oleg,

On 11/14/2011 03:03 AM, Oleg Zhurakivskyy wrote:
> Hello Denis,
> 
>> The break statement seems to be in error and I fixed it in a follow on
>> commit.  Let me know if I was wrong.
> 
> Yes, that is error, thanks for correction. And sorry for the mistake.
> 
> What is still missing from the integration point of view? Thanks in
> advance!

I've reviewed the other patches, however I'm not completely happy with
the API changes and a few scenarios do not seem to work.  I spent most
of Friday trying to figure out better alternatives, but have not come up
with anything yet.  Let it stew in my brain for a bit longer.

Regards,
-Denis

Re: [PATCHv2 1/6] gatchat: Add IPv6 Control Protocol

[Oleg Zhurakivskyy]

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

Hello Denis,

> The break statement seems to be in error and I fixed it in a follow on
> commit.  Let me know if I was wrong.

Yes, that is error, thanks for correction. And sorry for the mistake.

What is still missing from the integration point of view? Thanks in advance!

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCHv2 2/6] gatppp: Add IPv6 CP hooks

[Denis Kenzior]

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

Hi Oleg,

On 11/08/2011 06:46 AM, Oleg Zhurakivskyy wrote:
> ---
>  gatchat/gatppp.c     |   61 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  gatchat/gatppp.h     |    3 ++
>  gatchat/ppp.h        |    3 ++
>  gatchat/ppp_ipv6cp.c |   17 ++++++++++++-
>  4 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
> index f767f4a..c425eae 100644
> --- a/gatchat/gatppp.c
> +++ b/gatchat/gatppp.c
> @@ -62,6 +62,7 @@ struct _GAtPPP {
>  	enum ppp_phase phase;
>  	struct pppcp_data *lcp;
>  	struct pppcp_data *ipcp;
> +	struct pppcp_data *ipv6cp;
>  	struct ppp_net *net;
>  	struct ppp_chap *chap;
>  	GAtHDLC *hdlc;
> @@ -157,7 +158,8 @@ static inline gboolean ppp_drop_packet(GAtPPP *ppp, guint16 protocol)
>  		return TRUE;
>  	case PPP_PHASE_NETWORK:
>  		if (protocol != LCP_PROTOCOL && protocol != CHAP_PROTOCOL &&
> -					protocol != IPCP_PROTO)
> +						protocol != IPCP_PROTO &&
> +						protocol != IPV6CP_PROTO)

This looks fine

>  			return TRUE;
>  		break;
>  	case PPP_PHASE_LINK_UP:
> @@ -222,6 +224,10 @@ static void ppp_receive(const unsigned char *buf, gsize len, void *data)
>  	case IPCP_PROTO:
>  		pppcp_process_packet(ppp->ipcp, packet, len - offset);
>  		break;
> +	case IPV6CP_PROTO:
> +		if (ppp->ipv6cp)
> +			pppcp_process_packet(ppp->ipv6cp, packet, len - offset);
> +		break;

Are you sure you don't want to send a protocol reject here?

>  	case CHAP_PROTOCOL:
>  		if (ppp->chap) {
>  			ppp_chap_process_packet(ppp->chap, packet,
> @@ -381,6 +387,12 @@ void ppp_auth_notify(GAtPPP *ppp, gboolean success)
>  
>  	ppp_enter_phase(ppp, PPP_PHASE_NETWORK);
>  
> +	/* Send UP & OPEN events to the IPv6 CP */
> +	if (ppp->ipv6cp) {
> +		pppcp_signal_open(ppp->ipv6cp);
> +		pppcp_signal_up(ppp->ipv6cp);
> +	}
> +

This looks fine

>  	/* Send UP & OPEN events to the IPCP layer */
>  	pppcp_signal_open(ppp->ipcp);
>  	pppcp_signal_up(ppp->ipcp);
> @@ -431,6 +443,34 @@ void ppp_ipcp_finished_notify(GAtPPP *ppp)
>  
>  	/* Our IPCP parameter negotiation failed */
>  	ppp->disconnect_reason = G_AT_PPP_REASON_IPCP_FAIL;
> +
> +	if (ppp->ipv6cp)
> +		pppcp_signal_close(ppp->ipv6cp);

This also looks fine, but please add a newline here

> +	pppcp_signal_close(ppp->ipcp);
> +	pppcp_signal_close(ppp->lcp);
> +}
> +
> +void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer)
> +{
> +	DBG(ppp, "local: %s, peer: %s", local, peer);
> +
> +	ppp_enter_phase(ppp, PPP_PHASE_LINK_UP);
> +}
> +
> +void ppp_ipv6cp_down_notify(GAtPPP *ppp)
> +{
> +	DBG(ppp, "");
> +}
> +
> +void ppp_ipv6cp_finished_notify(GAtPPP *ppp)
> +{
> +	DBG(ppp, "");
> +
> +	if (ppp->phase != PPP_PHASE_NETWORK)
> +		return;
> +
> +	ppp->disconnect_reason = G_AT_PPP_REASON_IPV6CP_FAIL;
> +	pppcp_signal_close(ppp->ipv6cp);
>  	pppcp_signal_close(ppp->ipcp);

We probably only want to close ipcp if it exists, see my later comments

>  	pppcp_signal_close(ppp->lcp);
>  }
> @@ -732,6 +772,8 @@ void g_at_ppp_unref(GAtPPP *ppp)
>  
>  	lcp_free(ppp->lcp);
>  	ipcp_free(ppp->ipcp);
> +	if (ppp->ipv6cp)
> +		ipv6cp_free(ppp->ipv6cp);
>  
>  	if (ppp->ppp_dead_source) {
>  		g_source_remove(ppp->ppp_dead_source);
> @@ -772,6 +814,23 @@ void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled)
>  	lcp_set_pfc_enabled(ppp->lcp, enabled);
>  }
>  
> +gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
> +					const char *local, const char *peer)
> +{
> +	GError *error = NULL;
> +
> +	ppp->ipv6cp = ipv6cp_new(ppp, is_server, local, peer, &error);
> +	if (ppp->ipv6cp == NULL) {
> +		if (error != NULL) {
> +			DBG(ppp, "%s", error->message);
> +			g_error_free(error);
> +		}
> +		return FALSE;
> +	}
> +
> +	return TRUE;
> +}
> +
>  static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
>  {
>  	GAtPPP *ppp;
> diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
> index b5a2234..a41bf08 100644
> --- a/gatchat/gatppp.h
> +++ b/gatchat/gatppp.h
> @@ -37,6 +37,7 @@ typedef enum _GAtPPPDisconnectReason {
>  	G_AT_PPP_REASON_UNKNOWN,
>  	G_AT_PPP_REASON_AUTH_FAIL,	/* Failed to authenticate */
>  	G_AT_PPP_REASON_IPCP_FAIL,	/* Failed to negotiate IPCP */
> +	G_AT_PPP_REASON_IPV6CP_FAIL,	/* Failed to negotiate IPV6CP */
>  	G_AT_PPP_REASON_NET_FAIL,	/* Failed to create tun */
>  	G_AT_PPP_REASON_PEER_CLOSED,	/* Peer initiated a close */
>  	G_AT_PPP_REASON_LINK_DEAD,	/* Link to the peer died */
> @@ -81,6 +82,8 @@ void g_at_ppp_set_server_info(GAtPPP *ppp, const char *remote_ip,
>  
>  void g_at_ppp_set_acfc_enabled(GAtPPP *ppp, gboolean enabled);
>  void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled);
> +gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
> +					const char *local, const char *peer);

So for consistency sake I think we should introduce
g_at_ppp_set_ipv6_server_info(GAtPPP *ppp, const char *remote_prefix);

For the local prefix, I'm leaning towards creating a new set of v6 and
dual-stack specific constructors:

g_at_ppp_ipv6_new(const char *local_prefix);
g_at_ppp_ipv6_server_new(const char *local_prefix);
g_at_ppp_ipv4v6_new(const char *local_prefix);
g_at_ppp_ipv4v6_server_new(const char *local_ip, const char *local_prefix);

The key difference between constructors being whether to create ipcp,
ipv6cp or both.  We might need a _full version of the server
constructors as well.  I'm not entirely happy with this, so if you have
better ideas, I'd love to hear them.

>  
>  #ifdef __cplusplus
>  }
> diff --git a/gatchat/ppp.h b/gatchat/ppp.h
> index 718575b..a21ebbc 100644
> --- a/gatchat/ppp.h
> +++ b/gatchat/ppp.h
> @@ -128,6 +128,9 @@ 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);
> +void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer);
> +void ppp_ipv6cp_down_notify(GAtPPP *ppp);
> +void ppp_ipv6cp_finished_notify(GAtPPP *ppp);
>  void ppp_lcp_up_notify(GAtPPP *ppp);
>  void ppp_lcp_down_notify(GAtPPP *ppp);
>  void ppp_lcp_finished_notify(GAtPPP *ppp);
> diff --git a/gatchat/ppp_ipv6cp.c b/gatchat/ppp_ipv6cp.c
> index ec89fb7..75ce8f2 100644
> --- a/gatchat/ppp_ipv6cp.c
> +++ b/gatchat/ppp_ipv6cp.c
> @@ -95,7 +95,19 @@ static void ipv6cp_reset_config_options(struct ipv6cp_data *ipv6cp)
>  
>  static void ipv6cp_up(struct pppcp_data *pppcp)
>  {
> +	struct ipv6cp_data *ipv6cp = pppcp_get_data(pppcp);
> +	struct in6_addr local_addr, peer_addr;
> +	char local[INET6_ADDRSTRLEN], peer[INET6_ADDRSTRLEN];
>  
> +	memset(&local_addr, 0, sizeof(local_addr));
> +	memcpy(&local_addr.s6_addr[8], &ipv6cp->local_addr,
> +					sizeof(ipv6cp->local_addr));
> +	memset(&peer_addr, 0, sizeof(peer_addr));
> +	memcpy(&peer_addr.s6_addr[8], &ipv6cp->peer_addr,
> +					sizeof(ipv6cp->peer_addr));
> +	ppp_ipv6cp_up_notify(pppcp_get_ppp(pppcp),
> +		inet_ntop(AF_INET6, &local_addr, local, INET6_ADDRSTRLEN),
> +		inet_ntop(AF_INET6, &peer_addr, peer, INET6_ADDRSTRLEN));
>  }
>  
>  static void ipv6cp_down(struct pppcp_data *pppcp)
> @@ -105,11 +117,12 @@ static void ipv6cp_down(struct pppcp_data *pppcp)
>  	ipv6cp_reset_config_options(ipv6cp);
>  
>  	pppcp_set_local_options(pppcp, ipv6cp->options, ipv6cp->options_len);
> +	ppp_ipv6cp_down_notify(pppcp_get_ppp(pppcp));
>  }
>  
>  static void ipv6cp_finished(struct pppcp_data *pppcp)
>  {
> -
> +	ppp_ipv6cp_finished_notify(pppcp_get_ppp(pppcp));
>  }
>  
>  static enum rcr_result ipv6cp_server_rcr(struct ipv6cp_data *ipv6cp,

Regards,
-Denis

Re: [PATCHv2 5/6] gatppp: Add IPv6 CP connect, disconnect callbacks

[Denis Kenzior]

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

Hi Oleg,

On 11/08/2011 06:46 AM, Oleg Zhurakivskyy wrote:
> ---
>  gatchat/gatppp.c |   38 ++++++++++++++++++++++++++++++++++++++
>  gatchat/gatppp.h |    9 ++++++++-
>  2 files changed, 46 insertions(+), 1 deletions(-)
> 
> diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
> index c425eae..5a34535 100644
> --- a/gatchat/gatppp.c
> +++ b/gatchat/gatppp.c
> @@ -64,6 +64,7 @@ struct _GAtPPP {
>  	struct pppcp_data *ipcp;
>  	struct pppcp_data *ipv6cp;
>  	struct ppp_net *net;
> +	struct ppp_net *net6;

Actually you can't do it this way.  There's only a single IP layer, so
it is considered 'live' as soon as one of the NCPs (e.g. IPCP or IPv6CP)
has been negotiated (e.g. went to 'up' state).

>  	struct ppp_chap *chap;
>  	GAtHDLC *hdlc;
>  	gint mru;
> @@ -75,6 +76,10 @@ struct _GAtPPP {
>  	GAtPPPDisconnectFunc disconnect_cb;
>  	gpointer disconnect_data;
>  	GAtPPPDisconnectReason disconnect_reason;
> +	GAtPPPConnect6Func connect6_cb;
> +	gpointer connect6_data;

I can see this one, but...

> +	GAtPPPDisconnectFunc disconnect6_cb;
> +	gpointer disconnect6_data;

Why do you need this one?

>  	GAtDebugFunc debugf;
>  	gpointer debug_data;
>  	gboolean sta_pending;
> @@ -454,7 +459,19 @@ void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer)
>  {
>  	DBG(ppp, "local: %s, peer: %s", local, peer);
>  
> +	ppp->net6 = ppp_net_new(ppp, -1);
> +
> +	if (ppp->net6 == NULL) {
> +		ppp->disconnect_reason = G_AT_PPP_REASON_NET_FAIL;
> +		pppcp_signal_close(ppp->lcp);
> +		return;
> +	}
> +
>  	ppp_enter_phase(ppp, PPP_PHASE_LINK_UP);
> +
> +	if (ppp->connect6_cb)
> +		ppp->connect6_cb(ppp_net_get_interface(ppp->net6),
> +					local, peer, ppp->connect6_data);

See my previous comments, entering of the LINK_UP phase can happen as
soon as one of IPCP or IPV6CP has been negotiated, so this needs to be
reworked a bit.  If it makes things easier, for now we can assume that
if IPCP or IPV6CP goes down, then the other one should probably be shut
down as well, if it exists.

>  }
>  
>  void ppp_ipv6cp_down_notify(GAtPPP *ppp)
> @@ -831,6 +848,27 @@ gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
>  	return TRUE;
>  }
>  
> +void g_at_ppp_set_ipv6_connect_function(GAtPPP *ppp, GAtPPPConnect6Func func,
> +							gpointer user_data)
> +{
> +	if (func == NULL)
> +		return;
> +
> +	ppp->connect6_cb = func;
> +	ppp->connect6_data = user_data;
> +}
> +
> +void g_at_ppp_set_ipv6_disconnect_function(GAtPPP *ppp,
> +						GAtPPPDisconnectFunc func,
> +						gpointer user_data)
> +{
> +	if (func == NULL)
> +		return;
> +
> +	ppp->disconnect6_cb = func;
> +	ppp->disconnect6_data = user_data;
> +}
> +
>  static GAtPPP *ppp_init_common(gboolean is_server, guint32 ip)
>  {
>  	GAtPPP *ppp;
> diff --git a/gatchat/gatppp.h b/gatchat/gatppp.h
> index a41bf08..8fa1cd4 100644
> --- a/gatchat/gatppp.h
> +++ b/gatchat/gatppp.h
> @@ -50,7 +50,8 @@ typedef void (*GAtPPPConnectFunc)(const char *iface, const char *local,
>  					gpointer user_data);
>  typedef void (*GAtPPPDisconnectFunc)(GAtPPPDisconnectReason reason,
>  					gpointer user_data);
> -
> +typedef void (*GAtPPPConnect6Func)(const char *iface, const char *local,
> +					const char *peer, gpointer user_data);
>  GAtPPP *g_at_ppp_new(void);
>  GAtPPP *g_at_ppp_server_new(const char *local);
>  GAtPPP *g_at_ppp_server_new_full(const char *local, int fd);
> @@ -84,6 +85,12 @@ void g_at_ppp_set_acfc_enabled(GAtPPP *ppp, gboolean enabled);
>  void g_at_ppp_set_pfc_enabled(GAtPPP *ppp, gboolean enabled);
>  gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
>  					const char *local, const char *peer);
> +void g_at_ppp_set_ipv6_connect_function(GAtPPP *ppp,
> +					GAtPPPConnect6Func callback,
> +					gpointer user_data);
> +void g_at_ppp_set_ipv6_disconnect_function(GAtPPP *ppp,
> +						GAtPPPDisconnectFunc func,
> +						gpointer user_data);
>  
>  #ifdef __cplusplus
>  }

Regards,
-Denis

Re: [PATCHv2 5/6] gatppp: Add IPv6 CP connect, disconnect callbacks

[Denis Kenzior]

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

Hi Oleg,

On 11/16/2011 08:05 AM, Oleg Zhurakivskyy wrote:
> Hello Denis,
> 
> On 11/15/2011 04:59 AM, Denis Kenzior wrote:
>>> @@ -64,6 +64,7 @@ struct _GAtPPP {
>>>       struct pppcp_data *ipcp;
>>>       struct pppcp_data *ipv6cp;
>>>       struct ppp_net *net;
>>> +    struct ppp_net *net6;
>>
>> Actually you can't do it this way.  There's only a single IP layer, so
>> it is considered 'live' as soon as one of the NCPs (e.g. IPCP or IPv6CP)
>> has been negotiated (e.g. went to 'up' state).
> 
> Thanks for the explanation of the semantics here.
> 
> Regarding the ppp_net, it might be possible to do just with one
> instance, enhancing struct ppp_net with guint8 proto field. The protocol
> that goes up first, creates the interface, the second one just adds
> relevant protocol configuration. Maybe, the reference counter, would be
> useful here then.
> 
> What would be the preferred approach here?
> 

That sounds fine, basically we need a single TUN interface to handle
both IPv6 and IPv4 packets.  We can probably get by with simply checking
the state of each NCP inside ppp_drop_packet on the rx side.

On the tx side we most likely need to peek inside the IP packet to
figure out and set the protocol accordingly.

Regards,
-Denis

Re: [PATCHv2 2/6] gatppp: Add IPv6 CP hooks

[Denis Kenzior]

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

Hi Oleg,

>>> +gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
>>> +                    const char *local, const char *peer);
>>
>> So for consistency sake I think we should introduce
>> g_at_ppp_set_ipv6_server_info(GAtPPP *ppp, const char *remote_prefix);
>>
>> For the local prefix, I'm leaning towards creating a new set of v6 and
>> dual-stack specific constructors:
>>
>> g_at_ppp_ipv6_new(const char *local_prefix);
>> g_at_ppp_ipv6_server_new(const char *local_prefix);
>> g_at_ppp_ipv4v6_new(const char *local_prefix);
>> g_at_ppp_ipv4v6_server_new(const char *local_ip, const char
>> *local_prefix);
>>
>> The key difference between constructors being whether to create ipcp,
>> ipv6cp or both.  We might need a _full version of the server
>> constructors as well.  I'm not entirely happy with this, so if you have
>> better ideas, I'd love to hear them.
> 
> That looks OK to me. Regarding alternatives, one would be:
> 
> enum ofono_ppp_proto {
>     OFONO_PPP_PROTO_IP     = 0x1,
>     OFONO_PPP_PROTO_IPV6   = 0x2,
>     OFONO_PPP_PROTO_IPV4V6 = 0x4,
> };
> 
> struct _GAtPPP {
>     [...]
>     guint8 proto;
> };
> 
> g_at_ppp_new(guint8 proto);
> 
> or/and
> 
> g_at_ppp_set_proto(GAtPPP *ppp, guint8 proto, ...);
> 
> The usage would be:
> 
> - IPv4 only:
> 
>     g_at_ppp_new(OFONO_GPRS_PROTO_IP);
> 
> - IPv6 only:
> 
>     g_at_ppp_new(OFONO_GPRS_PROTO_IPV6);
> 
> - IPv4v6, both optional, but at least one is mandatory:
> 
>     g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6);
> 
> - IPv4v6, both mandatory:
> 
>     g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6 |
>          OFONO_GPRS_PROTO_IP | OFONO_GPRS_PROTO_IPV6);
> 
> - IPv4v6, IPv4 mandatory:
> 
>     g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6 | OFONO_GPRS_PROTO_IP);
> 
> and so on.
> 
> For g_at_ppp_set_proto(GAtPPP *ppp, guint8 proto, ...) there might be
> optional parameters (one IPv4/IPv6 local address or both IPv4 and IPv6
> local addresses), depending on the need. And similarly for
> g_at_ppp_set_server_info().
> 

There are some interesting ideas here, but I want to keep the API
changes and the impact on existing code minimal for now.  This means not
touching the original API if possible.

Besides, we simply don't know if we need all this flexibility, so lets
just consider the ipv4 only, ipv6 only and both ipv4 and ipv6 being
required as use cases we want to address.

Regards,
-Denis

Re: [PATCHv2 2/6] gatppp: Add IPv6 CP hooks

[Oleg Zhurakivskyy]

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

Hello Denis,

On 11/15/2011 04:53 AM, Denis Kenzior wrote:
>> +	case IPV6CP_PROTO:
>> +		if (ppp->ipv6cp)
>> +			pppcp_process_packet(ppp->ipv6cp, packet, len - offset);
>> +		break;
>
> Are you sure you don't want to send a protocol reject here?

Sure, will make it so, thanks for the idea.

>> +
>> +	if (ppp->ipv6cp)
>> +		pppcp_signal_close(ppp->ipv6cp);
>
> This also looks fine, but please add a newline here

OK.

>> +gboolean g_at_ppp_set_ipv6cp_info(GAtPPP *ppp, gboolean is_server,
>> +					const char *local, const char *peer);
>
> So for consistency sake I think we should introduce
> g_at_ppp_set_ipv6_server_info(GAtPPP *ppp, const char *remote_prefix);
>
> For the local prefix, I'm leaning towards creating a new set of v6 and
> dual-stack specific constructors:
>
> g_at_ppp_ipv6_new(const char *local_prefix);
> g_at_ppp_ipv6_server_new(const char *local_prefix);
> g_at_ppp_ipv4v6_new(const char *local_prefix);
> g_at_ppp_ipv4v6_server_new(const char *local_ip, const char *local_prefix);
>
> The key difference between constructors being whether to create ipcp,
> ipv6cp or both.  We might need a _full version of the server
> constructors as well.  I'm not entirely happy with this, so if you have
> better ideas, I'd love to hear them.

That looks OK to me. Regarding alternatives, one would be:

enum ofono_ppp_proto {
	OFONO_PPP_PROTO_IP     = 0x1,
	OFONO_PPP_PROTO_IPV6   = 0x2,
	OFONO_PPP_PROTO_IPV4V6 = 0x4,
};

struct _GAtPPP {
	[...]
	guint8 proto;
};

g_at_ppp_new(guint8 proto);

or/and

g_at_ppp_set_proto(GAtPPP *ppp, guint8 proto, ...);

The usage would be:

- IPv4 only:

	g_at_ppp_new(OFONO_GPRS_PROTO_IP);

- IPv6 only:

	g_at_ppp_new(OFONO_GPRS_PROTO_IPV6);

- IPv4v6, both optional, but at least one is mandatory:

	g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6);

- IPv4v6, both mandatory:

	g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6 |
	 	OFONO_GPRS_PROTO_IP | OFONO_GPRS_PROTO_IPV6);

- IPv4v6, IPv4 mandatory:

	g_at_ppp_new(OFONO_GPRS_PROTO_IPV4V6 | OFONO_GPRS_PROTO_IP);

and so on.

For g_at_ppp_set_proto(GAtPPP *ppp, guint8 proto, ...) there might be optional 
parameters (one IPv4/IPv6 local address or both IPv4 and IPv6 local addresses), 
depending on the need. And similarly for g_at_ppp_set_server_info().

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCHv2 5/6] gatppp: Add IPv6 CP connect, disconnect callbacks

[Oleg Zhurakivskyy]

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

Hello Denis,

On 11/15/2011 04:59 AM, Denis Kenzior wrote:
>> @@ -64,6 +64,7 @@ struct _GAtPPP {
>>   	struct pppcp_data *ipcp;
>>   	struct pppcp_data *ipv6cp;
>>   	struct ppp_net *net;
>> +	struct ppp_net *net6;
>
> Actually you can't do it this way.  There's only a single IP layer, so
> it is considered 'live' as soon as one of the NCPs (e.g. IPCP or IPv6CP)
> has been negotiated (e.g. went to 'up' state).

Thanks for the explanation of the semantics here.

Regarding the ppp_net, it might be possible to do just with one instance, 
enhancing struct ppp_net with guint8 proto field. The protocol that goes up 
first, creates the interface, the second one just adds relevant protocol 
configuration. Maybe, the reference counter, would be useful here then.

What would be the preferred approach here?

>> +	GAtPPPDisconnectFunc disconnect6_cb;
>> +	gpointer disconnect6_data;
>
> Why do you need this one?

I added it for consistency. But, true, it isn't needed, I will drop it.

>> @@ -454,7 +459,19 @@ void ppp_ipv6cp_up_notify(GAtPPP *ppp, const char *local, const char *peer)
>>   {
>>   	DBG(ppp, "local: %s, peer: %s", local, peer);
>>
>> +	ppp->net6 = ppp_net_new(ppp, -1);
>> +
>> +	if (ppp->net6 == NULL) {
>> +		ppp->disconnect_reason = G_AT_PPP_REASON_NET_FAIL;
>> +		pppcp_signal_close(ppp->lcp);
>> +		return;
>> +	}
>> +
>>   	ppp_enter_phase(ppp, PPP_PHASE_LINK_UP);
>
> See my previous comments, entering of the LINK_UP phase can happen as
> soon as one of IPCP or IPV6CP has been negotiated, so this needs to be
> reworked a bit.  If it makes things easier, for now we can assume that
> if IPCP or IPV6CP goes down, then the other one should probably be shut
> down as well, if it exists.

OK, understood. Let me check this once again, I will try to address this.

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCHv2 2/6] gatppp: Add IPv6 CP hooks

[Oleg Zhurakivskyy]

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

Hello Denis,

On 11/15/2011 03:59 PM, Denis Kenzior wrote:
> There are some interesting ideas here, but I want to keep the API
> changes and the impact on existing code minimal for now.  This means not
> touching the original API if possible.
>
> Besides, we simply don't know if we need all this flexibility, so lets
> just consider the ipv4 only, ipv6 only and both ipv4 and ipv6 being
> required as use cases we want to address.

Sure, let's keep it simple then. I will prepare another patch.

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCHv2 5/6] gatppp: Add IPv6 CP connect, disconnect callbacks

[Oleg Zhurakivskyy]

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

Hello Denis,

On 11/15/2011 03:43 PM, Denis Kenzior wrote:
> That sounds fine, basically we need a single TUN interface to handle
> both IPv6 and IPv4 packets.  We can probably get by with simply checking
> the state of each NCP inside ppp_drop_packet on the rx side.
>
> On the tx side we most likely need to peek inside the IP packet to
> figure out and set the protocol accordingly.

OK, will make it so. Thanks for the idea!

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki