[PATCH 0/12] IPv6 support

[PATCH 0/12] IPv6 support

[Mika Liljeberg]

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

Hi All,

Here a set of patches with IPv6 support for oFono. Based on
feedback to the RFC patches, I've taken out the emulated ipv4v6
context support for modems conforming to 3GPP rel7 or older.
This simplifies the patches quite a bit and hopefully makes
them more accetable at this stage. As a result, only a single
network interface and a single set of interface configuration
parameters is exposed via each oFono gprs context.

It should be noted that the operator requirements I have seen so
far state that IPv6 capable UEs should fall back to parallel
IPv4 and IPv6 contexts if either the network or the UE does
not support the rel8 IPv4v6 combined context. 

This has implications to any IPv6 capable product using a rel7
(or older) modem. Either connman will have to support parallel
IPv4 and IPv6 connections, or the specific oFono modem driver
will have to fake it by providing an emulated ipv4v6 context
underneath oFono.

Whether oFono will be used in any IPv6 capable products with
a rel7 modem is a topic for other forums. People should be aware
of the limitation, though.

Br,

	MikaL

[PATCH 01/12] gprs: Update documentation for IPv6
[PATCH 02/12] gprs: driver interface changes for IPv6
[PATCH 03/12] gprs: core support for IPv6
[PATCH 04/12] test: modify test scripts for IPv6
[PATCH 05/12] isimodem: IPv6 support
[PATCH 06/12] atmodem: update to new gprs context interface
[PATCH 07/12] huaweimodem: update to new gprs context interface
[PATCH 08/12] mbmmodem: update to new gprs context interface
[PATCH 09/12] hsomodem: update to new gprs context interface
[PATCH 10/12] ifxmodem: update to new gprs context interface
[PATCH 11/12] stemodem: update to new gprs context interface
[PATCH 12/12] phonesim: add IPv6 support

 Makefile.am                        |    3 +-
 doc/connman-api.txt                |    9 +-
 drivers/atmodem/gprs-context.c     |   41 +++---
 drivers/hsomodem/gprs-context.c    |   48 ++++---
 drivers/huaweimodem/gprs-context.c |   43 +++---
 drivers/ifxmodem/gprs-context.c    |   36 +++--
 drivers/isimodem/gprs-context.c    |  119 +++++++++------
 drivers/mbmmodem/gprs-context.c    |   65 +++++---
 drivers/stemodem/gprs-context.c    |   31 +++--
 include/gprs-context.h             |   23 +++-
 plugins/phonesim.c                 |   32 +++-
 src/gprs.c                         |  296 +++++++++++++++++++++++-------------
 test/list-contexts                 |    6 +-
 test/set-context-property          |   38 +++++
 14 files changed, 514 insertions(+), 276 deletions(-)

[PATCH 01/12] gprs: Update documentation for IPv6

[Mika Liljeberg]

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

---
 doc/connman-api.txt |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/connman-api.txt b/doc/connman-api.txt
index 22c59dc..de1b938 100644
--- a/doc/connman-api.txt
+++ b/doc/connman-api.txt
@@ -182,7 +182,7 @@ Properties	boolean Active [readwrite]
 		string Protocol [readwrite]
 
 			Holds the protocol for this context.  Valid values
-			are: "ip" and "ipv6".
+			are: "ip", "ipv6" and "ipv4v6".
 
 		string Name [readwrite]
 
@@ -196,7 +196,7 @@ Properties	boolean Active [readwrite]
 
 			string Interface [readonly, optional]
 
-				Holds the interface of the network interface
+				Holds the name of the network interface
 				used by this context (e.g. "ppp0" "usb0")
 
 			string Method [readonly, optional]
@@ -242,6 +242,11 @@ Properties	boolean Active [readwrite]
 				via this proxy.  All other values are left
 				out in this case.
 
+			string IPv6Address [readonly, optional]
+
+				Holds the IPv6 link local address for this
+				context.
+
 		string MessageProxy [readwrite, MMS only]
 
 			Holds the MMS Proxy setting.
-- 
1.7.1

[PATCH 02/12] gprs: driver interface changes for IPv6

[Mika Liljeberg]

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

---
 include/gprs-context.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index c29c0dc..88530f6 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -37,6 +37,7 @@ struct ofono_gprs_context;
 enum ofono_gprs_proto {
 	OFONO_GPRS_PROTO_IP = 0,
 	OFONO_GPRS_PROTO_IPV6,
+	OFONO_GPRS_PROTO_IPV4V6,
 };
 
 enum ofono_gprs_context_type {
@@ -47,6 +48,12 @@ enum ofono_gprs_context_type {
 	OFONO_GPRS_CONTEXT_TYPE_IMS,
 };
 
+enum ofono_gprs_addrconf {
+	OFONO_GPRS_ADDRCONF_NONE,
+	OFONO_GPRS_ADDRCONF_STATIC,
+	OFONO_GPRS_ADDRCONF_DHCP,
+};
+
 struct ofono_gprs_primary_context {
 	unsigned int cid;
 	int direction;
@@ -94,6 +101,20 @@ struct ofono_modem *ofono_gprs_context_get_modem(struct ofono_gprs_context *gc);
 
 void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
 					enum ofono_gprs_context_type type);
+void ofono_gprs_context_set_interface(struct ofono_gprs_context *gc,
+					const char *interface);
+void ofono_gprs_context_set_ip_addrconf(struct ofono_gprs_context *gc,
+					enum ofono_gprs_addrconf method);
+void ofono_gprs_context_set_ip_address(struct ofono_gprs_context *gc,
+					const char *address);
+void ofono_gprs_context_set_ip_netmask(struct ofono_gprs_context *gc,
+					const char *netmask);
+void ofono_gprs_context_set_ip_gateway(struct ofono_gprs_context *gc,
+					const char *netmask);
+void ofono_gprs_context_set_ipv6_address(struct ofono_gprs_context *gc,
+					const char *address);
+void ofono_gprs_context_set_dns_servers(struct ofono_gprs_context *gc,
+					const char **dns);
 
 #ifdef __cplusplus
 }
-- 
1.7.1

[PATCH 03/12] gprs: core support for IPv6

[Mika Liljeberg]

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

---
 include/gprs-context.h |    2 +-
 src/gprs.c             |  296 +++++++++++++++++++++++++++++++-----------------
 2 files changed, 192 insertions(+), 106 deletions(-)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index 88530f6..d03dd47 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -77,7 +77,7 @@ struct ofono_gprs_context_driver {
 	void (*remove)(struct ofono_gprs_context *gc);
 	void (*activate_primary)(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data);
+				ofono_gprs_context_cb_t cb, void *data);
 	void (*deactivate_primary)(struct ofono_gprs_context *gc,
 					unsigned int id,
 					ofono_gprs_context_cb_t cb, void *data);
diff --git a/src/gprs.c b/src/gprs.c
index 7d36633..b5dd5f2 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -99,10 +99,12 @@ struct ofono_gprs {
 	struct ofono_atom *atom;
 };
 
+struct pri_context;
+
 struct ofono_gprs_context {
 	struct ofono_gprs *gprs;
+	struct pri_context *pri;
 	enum ofono_gprs_context_type type;
-	ofono_bool_t inuse;
 	const struct ofono_gprs_context_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
@@ -111,12 +113,13 @@ struct ofono_gprs_context {
 struct context_settings {
 	enum ofono_gprs_context_type type;
 	char *interface;
-	gboolean static_ip;
+	enum ofono_gprs_addrconf method;
 	char *ip;
 	char *netmask;
 	char *gateway;
 	char **dns;
 	char *proxy;
+	char *ipv6addr;
 };
 
 struct pri_context {
@@ -226,6 +229,8 @@ static const char *gprs_proto_to_string(enum ofono_gprs_proto proto)
 		return "ip";
 	case OFONO_GPRS_PROTO_IPV6:
 		return "ipv6";
+	case OFONO_GPRS_PROTO_IPV4V6:
+		return "ipv4v6";
 	};
 
 	return NULL;
@@ -240,11 +245,28 @@ static gboolean gprs_proto_from_string(const char *str,
 	} else if (g_str_equal(str, "ipv6")) {
 		*proto = OFONO_GPRS_PROTO_IPV6;
 		return TRUE;
+	} else if (g_str_equal(str, "ipv4v6")) {
+		*proto = OFONO_GPRS_PROTO_IPV4V6;
+		return TRUE;
 	}
 
 	return FALSE;
 }
 
+static const char *gprs_addrconf_to_string(enum ofono_gprs_addrconf method)
+{
+	switch (method) {
+	case OFONO_GPRS_ADDRCONF_NONE:
+		return NULL;
+	case OFONO_GPRS_ADDRCONF_STATIC:
+		return "static";
+	case OFONO_GPRS_ADDRCONF_DHCP:
+		return "dhcp";
+	}
+
+	return NULL;
+}
+
 static unsigned int gprs_cid_alloc(struct ofono_gprs *gprs)
 {
 	return idmap_alloc(gprs->cid_map);
@@ -255,6 +277,49 @@ static void gprs_cid_release(struct ofono_gprs *gprs, unsigned int id)
 	idmap_put(gprs->cid_map, id);
 }
 
+static gboolean assign_context(struct pri_context *ctx)
+{
+	struct idmap *cidmap = ctx->gprs->cid_map;
+	unsigned int cid_min;
+	GSList *l;
+
+	if (cidmap == NULL)
+		return FALSE;
+
+	cid_min = idmap_get_min(cidmap);
+
+	ctx->context.cid = gprs_cid_alloc(ctx->gprs);
+	if (ctx->context.cid == 0)
+		return FALSE;
+
+	for (l = ctx->gprs->context_drivers; l; l = l->next) {
+		struct ofono_gprs_context *gc = l->data;
+
+		if (gc->pri != NULL)
+			continue;
+
+		if (gc->type == OFONO_GPRS_CONTEXT_TYPE_ANY ||
+						gc->type == ctx->type) {
+			gc->pri = ctx;
+			ctx->context_driver = gc;
+			return TRUE;
+		}
+	}
+
+	return FALSE;
+}
+
+static void release_context(struct pri_context *ctx)
+{
+	if (ctx == NULL || ctx->gprs == NULL)
+		return;
+
+	gprs_cid_release(ctx->gprs, ctx->context.cid);
+	ctx->context.cid = 0;
+	ctx->context_driver->pri = NULL;
+	ctx->context_driver = NULL;
+}
+
 static struct pri_context *gprs_context_by_path(struct ofono_gprs *gprs,
 						const char *ctx_path)
 {
@@ -278,6 +343,7 @@ static void context_settings_free(struct context_settings *settings)
 	g_free(settings->gateway);
 	g_strfreev(settings->dns);
 	g_free(settings->proxy);
+	g_free(settings->ipv6addr);
 
 	g_free(settings);
 }
@@ -316,12 +382,10 @@ static void context_settings_append_variant(struct context_settings *settings,
 		goto done;
 	}
 
-	if (settings->static_ip == TRUE)
-		method = "static";
-	else
-		method = "dhcp";
-
-	ofono_dbus_dict_append(&array, "Method", DBUS_TYPE_STRING, &method);
+	method = gprs_addrconf_to_string(settings->method);
+	if (method != NULL)
+		ofono_dbus_dict_append(&array, "Method", DBUS_TYPE_STRING,
+					&method);
 
 	if (settings->ip)
 		ofono_dbus_dict_append(&array, "Address", DBUS_TYPE_STRING,
@@ -335,6 +399,19 @@ static void context_settings_append_variant(struct context_settings *settings,
 		ofono_dbus_dict_append(&array, "Gateway", DBUS_TYPE_STRING,
 					&settings->gateway);
 
+	if (settings->ipv6addr != NULL) {
+		uint8_t addr[16];
+		char *strll = alloca(INET6_ADDRSTRLEN);
+
+		inet_pton(AF_INET6, settings->ipv6addr, addr);
+		memset(addr, 0, 8);
+		addr[0] = 0xfe;
+		addr[1] = 0x80;
+		inet_ntop(AF_INET6, addr, strll, INET6_ADDRSTRLEN);
+		ofono_dbus_dict_append(&array, "IPv6Address", DBUS_TYPE_STRING,
+					&strll);
+	}
+
 	if (settings->dns)
 		ofono_dbus_dict_append_array(&array, "DomainNameServers",
 						DBUS_TYPE_STRING,
@@ -578,42 +655,27 @@ static void pri_reset_context_settings(struct pri_context *ctx)
 	g_free(interface);
 }
 
-static void pri_update_context_settings(struct pri_context *ctx,
-					const char *interface,
-					ofono_bool_t static_ip,
-					const char *ip, const char *netmask,
-					const char *gateway, const char **dns)
+static void pri_update_context_settings(struct pri_context *ctx)
 {
-	if (ctx->settings)
-		context_settings_free(ctx->settings);
-
-	ctx->settings = g_try_new0(struct context_settings, 1);
-	if (ctx->settings == NULL)
+	if (ctx->settings == NULL || ctx->settings->interface == NULL)
 		return;
 
 	ctx->settings->type = ctx->type;
 
-	ctx->settings->interface = g_strdup(interface);
-	ctx->settings->static_ip = static_ip;
-	ctx->settings->ip = g_strdup(ip);
-	ctx->settings->netmask = g_strdup(netmask);
-	ctx->settings->gateway = g_strdup(gateway);
-	ctx->settings->dns = g_strdupv((char **)dns);
-
 	if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS && ctx->message_proxy)
 		ctx->settings->proxy = g_strdup(ctx->message_proxy);
 
-	pri_ifupdown(interface, TRUE);
+	pri_ifupdown(ctx->settings->interface, TRUE);
 
 	if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
 		pri_parse_proxy(ctx, ctx->message_proxy);
 
 		DBG("proxy %s port %u", ctx->proxy_host, ctx->proxy_port);
 
-		pri_setaddr(interface, ip);
+		pri_setaddr(ctx->settings->interface, ctx->settings->ip);
 
 		if (ctx->proxy_host)
-			pri_setproxy(interface, ctx->proxy_host);
+			pri_setproxy(ctx->settings->interface, ctx->proxy_host);
 	}
 
 	pri_context_signal_settings(ctx);
@@ -685,30 +747,18 @@ static DBusMessage *pri_get_properties(DBusConnection *conn,
 	return reply;
 }
 
-static void pri_activate_callback(const struct ofono_error *error,
-					const char *interface,
-					ofono_bool_t static_ip,
-					const char *ip, const char *netmask,
-					const char *gateway, const char **dns,
-					void *data)
+static void pri_activate_callback(const struct ofono_error *error, void *data)
 {
 	struct pri_context *ctx = data;
 	DBusConnection *conn = ofono_dbus_get_connection();
 	dbus_bool_t value;
 
-	DBG("%p %s", ctx, interface);
-
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		DBG("Activating context failed with error: %s",
 				telephony_error_to_str(error));
 		__ofono_dbus_pending_reply(&ctx->pending,
 					__ofono_error_failed(ctx->pending));
-
-		gprs_cid_release(ctx->gprs, ctx->context.cid);
-		ctx->context.cid = 0;
-		ctx->context_driver->inuse = FALSE;
-		ctx->context_driver = NULL;
-
+		release_context(ctx);
 		return;
 	}
 
@@ -716,14 +766,7 @@ static void pri_activate_callback(const struct ofono_error *error,
 	__ofono_dbus_pending_reply(&ctx->pending,
 				dbus_message_new_method_return(ctx->pending));
 
-	/*
-	 * If we don't have the interface, don't bother emitting any settings,
-	 * as nobody can make use of them
-	 */
-	if (interface != NULL)
-		pri_update_context_settings(ctx, interface, static_ip,
-						ip, netmask, gateway, dns);
-
+	pri_update_context_settings(ctx);
 	value = ctx->active;
 	ofono_dbus_signal_property_changed(conn, ctx->path,
 					OFONO_CONNECTION_CONTEXT_INTERFACE,
@@ -744,11 +787,8 @@ static void pri_deactivate_callback(const struct ofono_error *error, void *data)
 		return;
 	}
 
-	gprs_cid_release(ctx->gprs, ctx->context.cid);
-	ctx->context.cid = 0;
+	release_context(ctx);
 	ctx->active = FALSE;
-	ctx->context_driver->inuse = FALSE;
-	ctx->context_driver = NULL;
 
 	__ofono_dbus_pending_reply(&ctx->pending,
 				dbus_message_new_method_return(ctx->pending));
@@ -995,38 +1035,6 @@ static DBusMessage *pri_set_message_center(struct pri_context *ctx,
 	return NULL;
 }
 
-static gboolean assign_context(struct pri_context *ctx)
-{
-	struct idmap *cidmap = ctx->gprs->cid_map;
-	unsigned int cid_min;
-	GSList *l;
-
-	if (cidmap == NULL)
-		return FALSE;
-
-	cid_min = idmap_get_min(cidmap);
-
-	ctx->context.cid = gprs_cid_alloc(ctx->gprs);
-	if (ctx->context.cid == 0)
-		return FALSE;
-
-	for (l = ctx->gprs->context_drivers; l; l = l->next) {
-		struct ofono_gprs_context *gc = l->data;
-
-		if (gc->inuse == TRUE)
-			continue;
-
-		if (gc->type == OFONO_GPRS_CONTEXT_TYPE_ANY ||
-						gc->type == ctx->type) {
-			ctx->context_driver = gc;
-			ctx->context_driver->inuse = TRUE;
-			return TRUE;
-		}
-	}
-
-	return FALSE;
-}
-
 static DBusMessage *pri_set_property(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
@@ -1179,6 +1187,8 @@ static GDBusSignalTable context_signals[] = {
 	{ }
 };
 
+
+
 static struct pri_context *pri_context_create(struct ofono_gprs *gprs,
 					const char *name,
 					enum ofono_gprs_context_type type)
@@ -1344,14 +1354,9 @@ static void gprs_attached_update(struct ofono_gprs *gprs)
 			if (ctx->active == FALSE)
 				continue;
 
-			gprs_cid_release(gprs, ctx->context.cid);
-			ctx->context.cid = 0;
+			release_context(ctx);
 			ctx->active = FALSE;
-			ctx->context_driver->inuse = FALSE;
-			ctx->context_driver = NULL;
-
 			pri_reset_context_settings(ctx);
-
 			value = FALSE;
 			ofono_dbus_signal_property_changed(conn, ctx->path,
 					OFONO_CONNECTION_CONTEXT_INTERFACE,
@@ -1736,10 +1741,7 @@ static void gprs_deactivate_for_remove(const struct ofono_error *error,
 		return;
 	}
 
-	gprs_cid_release(gprs, ctx->context.cid);
-	ctx->context.cid = 0;
-	ctx->context_driver->inuse = FALSE;
-	ctx->context_driver = NULL;
+	release_context(ctx);
 
 	if (gprs->settings) {
 		g_key_file_remove_group(gprs->settings, ctx->key, NULL);
@@ -1831,12 +1833,9 @@ static void gprs_deactivate_for_all(const struct ofono_error *error,
 		return;
 	}
 
-	gprs_cid_release(gprs, ctx->context.cid);
-	ctx->active = FALSE;
-	ctx->context.cid = 0;
-	ctx->context_driver->inuse = FALSE;
-	ctx->context_driver = NULL;
+	release_context(ctx);
 
+	ctx->active = FALSE;
 	pri_reset_context_settings(ctx);
 
 	value = ctx->active;
@@ -2087,11 +2086,8 @@ void ofono_gprs_context_deactivated(struct ofono_gprs_context *gc,
 		if (ctx->active == FALSE)
 			break;
 
-		gprs_cid_release(ctx->gprs, ctx->context.cid);
-		ctx->context.cid = 0;
+		release_context(ctx);
 		ctx->active = FALSE;
-		ctx->context_driver->inuse = FALSE;
-		ctx->context_driver = NULL;
 
 		pri_reset_context_settings(ctx);
 
@@ -2130,6 +2126,8 @@ static void gprs_context_remove(struct ofono_atom *atom)
 	if (gc == NULL)
 		return;
 
+	release_context(gc->pri);
+
 	if (gc->driver && gc->driver->remove)
 		gc->driver->remove(gc);
 
@@ -2202,6 +2200,94 @@ void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
 	gc->type = type;
 }
 
+static struct context_settings *get_settings(struct ofono_gprs_context *gc)
+{
+	if (gc == NULL || gc->pri == NULL)
+		return NULL;
+
+	if (gc->pri->settings == NULL)
+		gc->pri->settings = g_try_new0(struct context_settings, 1);
+
+	return gc->pri->settings;
+}
+
+void ofono_gprs_context_set_interface(struct ofono_gprs_context *gc,
+					const char *interface)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->interface != NULL)
+		return;
+
+	settings->interface = g_strdup(interface);
+}
+
+void ofono_gprs_context_set_ip_addrconf(struct ofono_gprs_context *gc,
+					enum ofono_gprs_addrconf method)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL)
+		return;
+
+	settings->method = method;
+}
+
+void ofono_gprs_context_set_ip_address(struct ofono_gprs_context *gc,
+					const char *address)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->ip != NULL)
+		return;
+
+	settings->ip = g_strdup(address);
+}
+
+void ofono_gprs_context_set_ip_netmask(struct ofono_gprs_context *gc,
+					const char *netmask)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->netmask != NULL)
+		return;
+
+	settings->netmask = g_strdup(netmask);
+}
+
+void ofono_gprs_context_set_ip_gateway(struct ofono_gprs_context *gc,
+					const char *gateway)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->gateway != NULL)
+		return;
+
+	settings->gateway = g_strdup(gateway);
+}
+
+void ofono_gprs_context_set_ipv6_address(struct ofono_gprs_context *gc,
+						const char *address)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->ipv6addr != NULL)
+		return;
+
+	settings->ipv6addr = g_strdup(address);
+}
+
+void ofono_gprs_context_set_dns_servers(struct ofono_gprs_context *gc,
+						const char **dns)
+{
+	struct context_settings *settings = get_settings(gc);
+
+	if (settings == NULL || settings->dns != NULL)
+		return;
+
+	settings->dns = g_strdupv((char **)dns);
+}
+
 int ofono_gprs_driver_register(const struct ofono_gprs_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
-- 
1.7.1

[PATCH 04/12] test: modify test scripts for IPv6

[Mika Liljeberg]

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

---
 Makefile.am               |    3 ++-
 test/list-contexts        |    6 +++---
 test/set-context-property |   38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)
 create mode 100755 test/set-context-property

diff --git a/Makefile.am b/Makefile.am
index a38fcb9..3002f56 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -478,7 +478,8 @@ test_scripts = test/backtrace \
 		test/cdma-dial-number \
 		test/cdma-hangup \
 		test/disable-call-forwarding \
-		test/list-messages
+		test/list-messages \
+		test/set-context-property
 
 if TEST
 testdir = $(pkglibdir)/test
diff --git a/test/list-contexts b/test/list-contexts
index 68dae6a..9519ade 100755
--- a/test/list-contexts
+++ b/test/list-contexts
@@ -24,15 +24,15 @@ for path, properties in modems:
 		print "    [ %s ]" % (path)
 
 		for key in properties.keys():
-			if key in ["Settings"]:
+			if key in ["Settings"] or key in ["IPv6Settings"]:
 				val = "{"
 				for i in properties[key].keys():
 					val += " " + i + "="
 					if i in ["DomainNameServers"]:
 						for n in properties[key][i]:
-							val += n + ","
+							val += str(n) + ","
 					else:
-						val += properties[key][i]
+						val += str(properties[key][i])
 				val += " }"
 			else:
 				val = str(properties[key])
diff --git a/test/set-context-property b/test/set-context-property
new file mode 100755
index 0000000..8ea0e3a
--- /dev/null
+++ b/test/set-context-property
@@ -0,0 +1,38 @@
+#!/usr/bin/python
+
+import sys
+import dbus
+
+if len(sys.argv) < 4:
+	print "Usage: set-context-property <context> <name> <value>"
+	sys.exit(1)
+
+bus = dbus.SystemBus()
+
+manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+						'org.ofono.Manager')
+
+modems = manager.GetModems()
+
+for path, properties in modems:
+	if "org.ofono.ConnectionManager" not in properties["Interfaces"]:
+		continue
+
+	connman = dbus.Interface(bus.get_object('org.ofono', path),
+					'org.ofono.ConnectionManager')
+
+	contexts = connman.GetContexts()
+
+	if (len(contexts) == 0):
+		print "No context available"
+		sys.exit(1)
+
+	path = contexts[int(sys.argv[1])][0]
+	context = dbus.Interface(bus.get_object('org.ofono', path),
+					'org.ofono.ConnectionContext')
+
+	try:
+		context.SetProperty(sys.argv[2], sys.argv[3])
+	except dbus.DBusException, e:
+		print "Error setting context %s property %s: %s" % (path, sys.argv[2], str(e))
+		exit(2)
-- 
1.7.1

[PATCH 05/12] isimodem: IPv6 support

[Mika Liljeberg]

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

---
 drivers/isimodem/gprs-context.c |  119 ++++++++++++++++++++++++---------------
 1 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/isimodem/gprs-context.c b/drivers/isimodem/gprs-context.c
index 6d579d8..942cb13 100644
--- a/drivers/isimodem/gprs-context.c
+++ b/drivers/isimodem/gprs-context.c
@@ -62,10 +62,7 @@ struct context_data {
 	uint16_t gpds;	/* GPDS object handle */
 	unsigned cid;	/* oFono core context ID */
 	struct ofono_gprs_context *context;
-	union {
-		ofono_gprs_context_up_cb_t up_cb;
-		ofono_gprs_context_cb_t down_cb;
-	};
+	ofono_gprs_context_cb_t cb;
 	void *data;
 
 	GIsiPEP *pep;
@@ -112,15 +109,14 @@ typedef void (*ContextFailFunc)(struct context_data *cd);
 
 static void gprs_up_fail(struct context_data *cd)
 {
-	CALLBACK_WITH_FAILURE(cd->up_cb, NULL, 0, NULL, NULL, NULL, NULL,
-				cd->data);
 	reset_context(cd);
+	CALLBACK_WITH_FAILURE(cd->cb, cd->data);
 }
 
 static void gprs_down_fail(struct context_data *cd)
 {
-	CALLBACK_WITH_FAILURE(cd->down_cb, cd->data);
 	reset_context(cd);
+	CALLBACK_WITH_FAILURE(cd->cb, cd->data);
 }
 
 static gboolean check_resp(const GIsiMessage *msg, uint8_t id, size_t minlen,
@@ -206,12 +202,12 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 {
 	struct context_data *cd = opaque;
 	GIsiSubBlockIter iter;
+	const char *dns[5];
+	int dns_count = 0;
 
 	char ifname[IF_NAMESIZE];
-	char *ip = NULL;
-	char *pdns = NULL;
-	char *sdns = NULL;
-	const char *dns[3];
+	char *ip_addr = NULL;
+	char *ipv6_addr = NULL;
 
 	if (!check_ind(msg, 2, cd))
 		return;
@@ -225,8 +221,6 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 
 		switch (g_isi_sb_iter_get_id(&iter)) {
 
-		/* TODO: IPv6 address support */
-
 		case GPDS_PDP_ADDRESS_INFO:
 
 			if (!g_isi_sb_iter_get_byte(&iter, &addr_len, 3))
@@ -236,9 +230,15 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 							4))
 				goto error;
 
-			ip = alloca(INET_ADDRSTRLEN);
-			inet_ntop(AF_INET, (const void *)addr_value, ip,
-					INET_ADDRSTRLEN);
+			if (addr_len == 4) {
+				ip_addr = alloca(INET_ADDRSTRLEN);
+				inet_ntop(AF_INET, (const void *)addr_value,
+						ip_addr, INET_ADDRSTRLEN);
+			} else if (addr_len == 16) {
+				ipv6_addr = alloca(INET6_ADDRSTRLEN);
+				inet_ntop(AF_INET6, (const void *)addr_value,
+						ipv6_addr, INET6_ADDRSTRLEN);
+			}
 			break;
 
 		case GPDS_PDNS_ADDRESS_INFO:
@@ -250,9 +250,17 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 							4))
 				break;
 
-			pdns = alloca(INET_ADDRSTRLEN);
-			inet_ntop(AF_INET, (const void *)addr_value, pdns,
-					INET_ADDRSTRLEN);
+			if (addr_len == 4) {
+				char *addr = alloca(INET_ADDRSTRLEN);
+				inet_ntop(AF_INET, (const void *)addr_value,
+						addr, INET_ADDRSTRLEN);
+				dns[dns_count++] = addr;
+			} else if (addr_len == 16) {
+				char *addr = alloca(INET6_ADDRSTRLEN);
+				inet_ntop(AF_INET6, (const void *)addr_value,
+						addr, INET6_ADDRSTRLEN);
+				dns[dns_count++] = addr;
+			}
 			break;
 
 		case GPDS_SDNS_ADDRESS_INFO:
@@ -264,9 +272,17 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 							4))
 				break;
 
-			sdns = alloca(INET_ADDRSTRLEN);
-			inet_ntop(AF_INET, (const void *)addr_value, sdns,
-					INET_ADDRSTRLEN);
+			if (addr_len == 4) {
+				char *addr = alloca(INET_ADDRSTRLEN);
+				inet_ntop(AF_INET, (const void *)addr_value,
+						addr, INET_ADDRSTRLEN);
+				dns[dns_count++] = addr;
+			} else if (addr_len == 16) {
+				char *addr = alloca(INET6_ADDRSTRLEN);
+				inet_ntop(AF_INET6, (const void *)addr_value,
+						addr, INET6_ADDRSTRLEN);
+				dns[dns_count++] = addr;
+			}
 			break;
 
 		default:
@@ -279,26 +295,26 @@ static void activate_ind_cb(const GIsiMessage *msg, void *opaque)
 	if (!g_isi_pep_get_ifname(cd->pep, ifname))
 		goto error;
 
-	dns[0] = pdns;
-	dns[1] = sdns;
-	dns[2] = 0;
+	ofono_gprs_context_set_interface(cd->context, ifname);
 
-	CALLBACK_WITH_SUCCESS(cd->up_cb, ifname, TRUE, (const char *)ip,
-					STATIC_IP_NETMASK, NULL,
-					dns, cd->data);
-	return;
+	if (ip_addr != NULL) {
+		ofono_gprs_context_set_ip_addrconf(cd->context,
+						OFONO_GPRS_ADDRCONF_STATIC);
+		ofono_gprs_context_set_ip_address(cd->context, ip_addr);
+		ofono_gprs_context_set_ip_netmask(cd->context,
+							STATIC_IP_NETMASK);
+	}
 
-error:
-	gprs_up_fail(cd);
-}
+	if (ipv6_addr != NULL)
+		ofono_gprs_context_set_ipv6_address(cd->context, ipv6_addr);
 
-static void activate_fail_ind_cb(const GIsiMessage *msg, void *opaque)
-{
-	struct context_data *cd = opaque;
+	dns[dns_count] = 0;
+	ofono_gprs_context_set_dns_servers(cd->context, dns);
 
-	if (!check_ind(msg, 2, cd))
-		return;
+	CALLBACK_WITH_SUCCESS(cd->cb, cd->data);
+	return;
 
+error:
 	gprs_up_fail(cd);
 }
 
@@ -319,8 +335,6 @@ static void send_context_activate(GIsiClient *client, void *opaque)
 
 	g_isi_client_ind_subscribe(client, GPDS_CONTEXT_ACTIVATE_IND,
 				activate_ind_cb, cd);
-	g_isi_client_ind_subscribe(client, GPDS_CONTEXT_ACTIVATE_FAIL_IND,
-				activate_fail_ind_cb, cd);
 	g_isi_client_ind_subscribe(client, GPDS_CONTEXT_DEACTIVATE_IND,
 				deactivate_ind_cb, cd);
 
@@ -460,7 +474,7 @@ static void create_pipe_cb(GIsiPipe *pipe)
 
 static void isi_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct context_data *cd = ofono_gprs_context_get_data(gc);
 
@@ -468,8 +482,7 @@ static void isi_gprs_activate_primary(struct ofono_gprs_context *gc,
 
 	if (cd == NULL || !cd->gpds) {
 		/* GPDS is not reachable */
-		CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL,
-					NULL, data);
+		CALLBACK_WITH_FAILURE(cb, data);
 		return;
 	}
 
@@ -480,12 +493,26 @@ static void isi_gprs_activate_primary(struct ofono_gprs_context *gc,
 	}
 
 	cd->cid = ctx->cid;
-	cd->up_cb = cb;
+	cd->cb = cb;
 	cd->data = data;
 	cd->pep = NULL;
 	cd->pipe = NULL;
 	cd->handle = INVALID_ID;
-	cd->type = GPDS_PDP_TYPE_IPV4;
+
+	switch (ctx->proto) {
+	case OFONO_GPRS_PROTO_IP:
+		cd->type = GPDS_PDP_TYPE_IPV4;
+		break;
+
+	case OFONO_GPRS_PROTO_IPV6:
+		cd->type = GPDS_PDP_TYPE_IPV6;
+		break;
+
+	case OFONO_GPRS_PROTO_IPV4V6:
+		/* Not supported by modem */
+		CALLBACK_WITH_FAILURE(cb, data);
+		return;
+	}
 
 	if (strlen(ctx->apn) >= GPDS_MAX_APN_STRING_LENGTH
 			|| strlen(ctx->username) >= GPDS_MAX_USERNAME_LENGTH
@@ -527,7 +554,7 @@ static void context_deactivate_cb(const GIsiMessage *msg, void *opaque)
 			gprs_down_fail))
 		return;
 
-	CALLBACK_WITH_SUCCESS(cd->down_cb, cd->data);
+	CALLBACK_WITH_SUCCESS(cd->cb, cd->data);
 	reset_context(cd);
 }
 
@@ -545,7 +572,7 @@ static void isi_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 	if (cd == NULL)
 		return;
 
-	cd->down_cb = cb;
+	cd->cb = cb;
 	cd->data = data;
 
 	msg[1] = cd->handle;
-- 
1.7.1

[PATCH 06/12] atmodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/atmodem/gprs-context.c |   41 ++++++++++++++++++++-------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 56d65d4..2c4797b 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -62,10 +62,7 @@ struct gprs_context_data {
 	char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
 	GAtPPP *ppp;
 	enum state state;
-	union {
-		ofono_gprs_context_cb_t down_cb;        /* Down callback */
-		ofono_gprs_context_up_cb_t up_cb;       /* Up callback */
-	};
+	ofono_gprs_context_cb_t cb;
 	void *cb_data;                                  /* Callback data */
 };
 
@@ -93,9 +90,12 @@ static void ppp_connect(const char *interface, const char *local,
 	ofono_info("DNS: %s, %s", dns1, dns2);
 
 	gcd->state = STATE_ACTIVE;
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, local,
-					STATIC_IP_NETMASK, NULL,
-					dns, gcd->cb_data);
+	ofono_gprs_context_set_interface(gc, interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_STATIC);
+	ofono_gprs_context_set_ip_address(gc, local);
+	ofono_gprs_context_set_ip_netmask(gc, STATIC_IP_NETMASK);
+	ofono_gprs_context_set_dns_servers(gc, dns);
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 }
 
 static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
@@ -110,11 +110,10 @@ static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data)
 
 	switch (gcd->state) {
 	case STATE_ENABLING:
-		CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, FALSE, NULL,
-					NULL, NULL, NULL, gcd->cb_data);
+		CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 		break;
 	case STATE_DISABLING:
-		CALLBACK_WITH_SUCCESS(gcd->down_cb, gcd->cb_data);
+		CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 		break;
 	default:
 		ofono_gprs_context_deactivated(gc, gcd->active_context);
@@ -181,8 +180,7 @@ static void at_cgdata_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		gcd->state = STATE_IDLE;
 
 		decode_at_error(&error, g_at_result_final_response(result));
-		gcd->up_cb(&error, NULL, 0, NULL, NULL, NULL, NULL,
-				gcd->cb_data);
+		gcd->cb(&error, gcd->cb_data);
 		return;
 	}
 
@@ -204,8 +202,7 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		gcd->state = STATE_IDLE;
 
 		decode_at_error(&error, g_at_result_final_response(result));
-		gcd->up_cb(&error, NULL, 0, NULL, NULL, NULL, NULL,
-				gcd->cb_data);
+		gcd->cb(&error, gcd->cb_data);
 		return;
 	}
 
@@ -217,22 +214,25 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	gcd->active_context = 0;
 	gcd->state = STATE_IDLE;
 
-	CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, 0, NULL, NULL, NULL, NULL,
-				gcd->cb_data);
+	CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 }
 
 static void at_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	char buf[OFONO_GPRS_MAX_APN_LENGTH + 128];
 	int len;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error;
+
 	DBG("cid %u", ctx->cid);
 
 	gcd->active_context = ctx->cid;
-	gcd->up_cb = cb;
+	gcd->cb = cb;
 	gcd->cb_data = data;
 	memcpy(gcd->username, ctx->username, sizeof(ctx->username));
 	memcpy(gcd->password, ctx->password, sizeof(ctx->password));
@@ -249,7 +249,8 @@ static void at_gprs_activate_primary(struct ofono_gprs_context *gc,
 				at_cgdcont_cb, gc, NULL) > 0)
 		return;
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void at_gprs_deactivate_primary(struct ofono_gprs_context *gc,
@@ -261,7 +262,7 @@ static void at_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 	DBG("cid %u", cid);
 
 	gcd->state = STATE_DISABLING;
-	gcd->down_cb = cb;
+	gcd->cb = cb;
 	gcd->cb_data = data;
 
 	g_at_ppp_shutdown(gcd->ppp);
-- 
1.7.1

[PATCH 07/12] huaweimodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/huaweimodem/gprs-context.c |   43 +++++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/huaweimodem/gprs-context.c b/drivers/huaweimodem/gprs-context.c
index bbc9c96..67d933e 100644
--- a/drivers/huaweimodem/gprs-context.c
+++ b/drivers/huaweimodem/gprs-context.c
@@ -53,10 +53,7 @@ struct gprs_context_data {
 	unsigned int dhcp_source;
 	unsigned int dhcp_count;
 	guint ndis_watch;
-	union {
-		ofono_gprs_context_cb_t down_cb;	/* Down callback */
-		ofono_gprs_context_up_cb_t up_cb;	/* Up callback */
-	};
+	ofono_gprs_context_cb_t cb;
 	void *cb_data;					/* Callback data */
 };
 
@@ -68,8 +65,7 @@ static gboolean dhcp_poll(gpointer user_data)
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 
 	if (gcd->dhcp_count > 20)
-		CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, 0, NULL, NULL,
-						NULL, NULL, gcd->cb_data);
+		CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 	else
 		check_dhcp(gc);
 
@@ -192,9 +188,15 @@ static void dhcp_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	interface = "invalid";
 
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, ip,
-					netmask, gateway, dns, gcd->cb_data);
-	gcd->up_cb = NULL;
+	ofono_gprs_context_set_interface(gc, interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_STATIC);
+	ofono_gprs_context_set_ip_address(gc, ip);
+	ofono_gprs_context_set_ip_netmask(gc, netmask);
+	ofono_gprs_context_set_ip_gateway(gc, gateway);
+	ofono_gprs_context_set_dns_servers(gc, dns);
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+	gcd->cb = NULL;
 	gcd->cb_data = NULL;
 
 	g_free(ip);
@@ -224,7 +226,7 @@ static void at_ndisdup_down_cb(gboolean ok, GAtResult *result,
 	DBG("ok %d", ok);
 
 	if (ok) {
-		gcd->down_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 
 		if (gcd->ndis_watch > 0) {
@@ -241,7 +243,7 @@ static void at_ndisdup_up_cb(gboolean ok, GAtResult *result,
 						gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct ofono_error error;
@@ -249,7 +251,7 @@ static void at_ndisdup_up_cb(gboolean ok, GAtResult *result,
 	DBG("ok %d", ok);
 
 	if (ok) {
-		gcd->up_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 
 		gcd->dhcp_count = 0;
@@ -261,13 +263,13 @@ static void at_ndisdup_up_cb(gboolean ok, GAtResult *result,
 	gcd->active_context = 0;
 
 	decode_at_error(&error, g_at_result_final_response(result));
-	cb(&error, NULL, FALSE, NULL, NULL, NULL, NULL, cbd->data);
+	cb(&error, cbd->data);
 }
 
 static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *ncbd;
@@ -281,7 +283,7 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		gcd->active_context = 0;
 
 		decode_at_error(&error, g_at_result_final_response(result));
-		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -297,18 +299,22 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	gcd->active_context = 0;
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
 static void huawei_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[64];
 	int len;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error;
+
 	DBG("cid %u", ctx->cid);
 
 	gcd->active_context = ctx->cid;
@@ -325,9 +331,10 @@ static void huawei_gprs_activate_primary(struct ofono_gprs_context *gc,
 				at_cgdcont_cb, cbd, g_free) > 0)
 		return;
 
+error:
 	g_free(cbd);
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void huawei_gprs_deactivate_primary(struct ofono_gprs_context *gc,
-- 
1.7.1

[PATCH 08/12] mbmmodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/mbmmodem/gprs-context.c |   65 ++++++++++++++++++++++++---------------
 1 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/drivers/mbmmodem/gprs-context.c b/drivers/mbmmodem/gprs-context.c
index 322f96d..972b35e 100644
--- a/drivers/mbmmodem/gprs-context.c
+++ b/drivers/mbmmodem/gprs-context.c
@@ -70,10 +70,7 @@ struct gprs_context_data {
 	gboolean have_e2ipcfg;
 	unsigned int enap_source;
 	enum mbm_state mbm_state;
-	union {
-		ofono_gprs_context_cb_t down_cb;        /* Down callback */
-		ofono_gprs_context_up_cb_t up_cb;       /* Up callback */
-	};
+	ofono_gprs_context_cb_t cb;
 	void *cb_data;                                  /* Callback data */
 	int enap;                                   /* State of the call */
 };
@@ -141,11 +138,23 @@ out:
 	ofono_info("IP: %s  Gateway: %s", ip, gateway);
 	ofono_info("DNS: %s, %s", dns[0], dns[1]);
 
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, success, ip,
-					STATIC_IP_NETMASK, gateway,
-					success ? dns : NULL, gcd->cb_data);
+	ofono_gprs_context_set_interface(gc, interface);
+
+	if (success) {
+		ofono_gprs_context_set_ip_addrconf(gc,
+						OFONO_GPRS_ADDRCONF_STATIC);
+		ofono_gprs_context_set_ip_address(gc, ip);
+		ofono_gprs_context_set_ip_netmask(gc, STATIC_IP_NETMASK);
+		ofono_gprs_context_set_dns_servers(gc, dns);
+	} else {
+		ofono_gprs_context_set_ip_addrconf(gc,
+						OFONO_GPRS_ADDRCONF_DHCP);
+	}
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+
 	gcd->mbm_state = MBM_NONE;
-	gcd->up_cb = NULL;
+	gcd->cb = NULL;
 	gcd->cb_data = NULL;
 }
 
@@ -169,11 +178,14 @@ static void mbm_get_ip_details(struct ofono_gprs_context *gc)
 
 	modem = ofono_gprs_context_get_modem(gc);
 	interface = ofono_modem_get_string(modem, "NetworkInterface");
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, FALSE, NULL, NULL,
-			NULL, NULL, gcd->cb_data);
+
+	ofono_gprs_context_set_interface(gc, interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_DHCP);
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 
 	gcd->mbm_state = MBM_NONE;
-	gcd->up_cb = NULL;
+	gcd->cb = NULL;
 	gcd->cb_data = NULL;
 }
 
@@ -191,12 +203,11 @@ static void mbm_state_changed(struct ofono_gprs_context *gc, int state)
 		DBG("disconnected");
 
 		if (gcd->mbm_state == MBM_DISABLING) {
-			CALLBACK_WITH_SUCCESS(gcd->down_cb, gcd->cb_data);
-			gcd->down_cb = NULL;
+			CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+			gcd->cb = NULL;
 		} else if (gcd->mbm_state == MBM_ENABLING) {
-			CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, 0, NULL, NULL,
-						NULL, NULL, gcd->cb_data);
-			gcd->up_cb = NULL;
+			CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
+			gcd->cb = NULL;
 		} else {
 			ofono_gprs_context_deactivated(gc, gcd->active_context);
 		}
@@ -275,7 +286,7 @@ static void at_enap_down_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	/* Now we have to wait for the unsolicited notification to arrive */
 	if (ok && gcd->enap != 0) {
 		gcd->mbm_state = MBM_DISABLING;
-		gcd->down_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 
 		if (gcd->have_e2nap == FALSE)
@@ -292,7 +303,7 @@ static void at_enap_down_cb(gboolean ok, GAtResult *result, gpointer user_data)
 static void mbm_enap_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct ofono_error error;
@@ -301,7 +312,7 @@ static void mbm_enap_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	if (ok) {
 		gcd->mbm_state = MBM_ENABLING;
-		gcd->up_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 
 		if (gcd->have_e2nap == FALSE)
@@ -314,13 +325,13 @@ static void mbm_enap_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	gcd->active_context = 0;
 
 	decode_at_error(&error, g_at_result_final_response(result));
-	cb(&error, NULL, FALSE, NULL, NULL, NULL, NULL, cbd->data);
+	cb(&error, cbd->data);
 }
 
 static void mbm_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *ncbd;
@@ -334,7 +345,7 @@ static void mbm_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		gcd->active_context = 0;
 
 		decode_at_error(&error, g_at_result_final_response(result));
-		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -350,18 +361,22 @@ static void mbm_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	gcd->active_context = 0;
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
 static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[AUTH_BUF_LENGTH];
 	int len;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error;
+
 	DBG("cid %u", ctx->cid);
 
 	gcd->active_context = ctx->cid;
@@ -393,7 +408,7 @@ static void mbm_gprs_activate_primary(struct ofono_gprs_context *gc,
 error:
 	g_free(cbd);
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void mbm_gprs_deactivate_primary(struct ofono_gprs_context *gc,
-- 
1.7.1

[PATCH 09/12] hsomodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/hsomodem/gprs-context.c |   48 ++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/hsomodem/gprs-context.c b/drivers/hsomodem/gprs-context.c
index c132846..f9f1696 100644
--- a/drivers/hsomodem/gprs-context.c
+++ b/drivers/hsomodem/gprs-context.c
@@ -62,10 +62,7 @@ struct gprs_context_data {
 	GAtChat *chat;
 	unsigned int active_context;			/* Currently active */
 	enum hso_state hso_state;			/* Are we in req ? */
-	union {
-		ofono_gprs_context_cb_t down_cb;	/* Down callback */
-		ofono_gprs_context_up_cb_t up_cb;	/* Up callback */
-	};
+	ofono_gprs_context_cb_t cb;
 	void *cb_data;					/* Callback data */
 	int owancall;					/* State of the call */
 };
@@ -82,7 +79,7 @@ static void at_owancall_down_cb(gboolean ok, GAtResult *result,
 	/* Now we have to wait for the unsolicited notification to arrive */
 	if (ok && gcd->owancall != 0) {
 		gcd->hso_state = HSO_DISABLING;
-		gcd->down_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 		return;
 	}
@@ -95,14 +92,14 @@ static void at_owancall_up_cb(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct ofono_error error;
 
 	if (ok) {
 		gcd->hso_state = HSO_ENABLING;
-		gcd->up_cb = cb;
+		gcd->cb = cb;
 		gcd->cb_data = cbd->data;
 		return;
 	}
@@ -110,13 +107,13 @@ static void at_owancall_up_cb(gboolean ok, GAtResult *result,
 	gcd->active_context = 0;
 
 	decode_at_error(&error, g_at_result_final_response(result));
-	cb(&error, NULL, FALSE, NULL, NULL, NULL, NULL, cbd->data);
+	cb(&error, cbd->data);
 }
 
 static void hso_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *ncbd;
@@ -128,7 +125,7 @@ static void hso_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		gcd->active_context = 0;
 
 		decode_at_error(&error, g_at_result_final_response(result));
-		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -144,18 +141,22 @@ static void hso_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	gcd->active_context = 0;
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
 static void hso_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[AUTH_BUF_LENGTH];
 	int len;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error;
+
 	gcd->active_context = ctx->cid;
 
 	cbd->user = gc;
@@ -186,7 +187,7 @@ static void hso_gprs_activate_primary(struct ofono_gprs_context *gc,
 error:
 	g_free(cbd);
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void hso_gprs_deactivate_primary(struct ofono_gprs_context *gc,
@@ -265,11 +266,17 @@ static void owandata_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_info("IP: %s, Gateway: %s", ip, gateway);
 	ofono_info("DNS: %s, %s", dns1, dns2);
 
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, ip,
-				STATIC_IP_NETMASK, gateway, dns, gcd->cb_data);
+	ofono_gprs_context_set_interface(gc, interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_STATIC);
+	ofono_gprs_context_set_ip_address(gc, ip);
+	ofono_gprs_context_set_ip_netmask(gc, STATIC_IP_NETMASK);
+	ofono_gprs_context_set_ip_gateway(gc, gateway);
+	ofono_gprs_context_set_dns_servers(gc, dns);
+
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 
 	gcd->hso_state = HSO_NONE;
-	gcd->up_cb = NULL;
+	gcd->cb = NULL;
 	gcd->cb_data = NULL;
 }
 
@@ -300,9 +307,9 @@ static void owancall_notifier(GAtResult *result, gpointer user_data)
 		DBG("HSO Context: disconnected");
 
 		if (gcd->hso_state == HSO_DISABLING) {
-			CALLBACK_WITH_SUCCESS(gcd->down_cb, gcd->cb_data);
+			CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 			gcd->hso_state = HSO_NONE;
-			gcd->down_cb = NULL;
+			gcd->cb = NULL;
 			gcd->cb_data = NULL;
 		} else {
 			ofono_gprs_context_deactivated(gc, gcd->active_context);
@@ -330,10 +337,9 @@ static void owancall_notifier(GAtResult *result, gpointer user_data)
 		DBG("HSO Context: failed");
 
 		if (gcd->hso_state == HSO_ENABLING) {
-			CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, 0, NULL,
-						NULL, NULL, NULL, gcd->cb_data);
+			CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 			gcd->hso_state = HSO_NONE;
-			gcd->up_cb = NULL;
+			gcd->cb = NULL;
 			gcd->cb_data = NULL;
 		}
 
-- 
1.7.1

[PATCH 10/12] ifxmodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/ifxmodem/gprs-context.c |   36 +++++++++++++++++++++---------------
 1 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 2c68b44..91dee8c 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -67,10 +67,7 @@ struct gprs_context_data {
 	char address[32];
 	char dns1[32];
 	char dns2[32];
-	union {
-		ofono_gprs_context_cb_t down_cb;        /* Down callback */
-		ofono_gprs_context_up_cb_t up_cb;       /* Up callback */
-	};
+	ofono_gprs_context_cb_t cb;
 	void *cb_data;                                  /* Callback data */
 };
 
@@ -123,13 +120,12 @@ static void failed_setup(struct ofono_gprs_context *gc,
 	gcd->state = STATE_IDLE;
 
 	if (result == NULL) {
-		CALLBACK_WITH_FAILURE(gcd->up_cb, NULL, 0, NULL, NULL,
-						NULL, NULL, gcd->cb_data);
+		CALLBACK_WITH_FAILURE(gcd->cb, gcd->cb_data);
 		return;
 	}
 
 	decode_at_error(&error, g_at_result_final_response(result));
-	gcd->up_cb(&error, NULL, 0, NULL, NULL, NULL, NULL, gcd->cb_data);
+	gcd->cb(&error, gcd->cb_data);
 }
 
 static void session_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -157,10 +153,15 @@ static void session_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	if (interface == NULL)
 		interface = "invalid";
 
-	CALLBACK_WITH_SUCCESS(gcd->up_cb, interface, TRUE, gcd->address,
-				STATIC_IP_NETMASK, NULL, dns, gcd->cb_data);
+	ofono_gprs_context_set_interface(gc, interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_STATIC);
+	ofono_gprs_context_set_ip_address(gc, gcd->address);
+	ofono_gprs_context_set_ip_netmask(gc, STATIC_IP_NETMASK);
+	ofono_gprs_context_set_dns_servers(gc, dns);
 
-	gcd->up_cb = NULL;
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
+
+	gcd->cb = NULL;
 	gcd->cb_data = NULL;
 }
 
@@ -316,16 +317,20 @@ error:
 
 static void ifx_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	char buf[OFONO_GPRS_MAX_APN_LENGTH + 128];
 	int len;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error;
+
 	DBG("cid %u", ctx->cid);
 
 	gcd->active_context = ctx->cid;
-	gcd->up_cb = cb;
+	gcd->cb = cb;
 	gcd->cb_data = data;
 	memcpy(gcd->username, ctx->username, sizeof(ctx->username));
 	memcpy(gcd->password, ctx->password, sizeof(ctx->password));
@@ -342,7 +347,8 @@ static void ifx_gprs_activate_primary(struct ofono_gprs_context *gc,
 				setup_cb, gc, NULL) > 0)
 		return;
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void deactivate_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -360,7 +366,7 @@ static void deactivate_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	g_at_chat_resume(gcd->chat);
 
-	CALLBACK_WITH_SUCCESS(gcd->down_cb, gcd->cb_data);
+	CALLBACK_WITH_SUCCESS(gcd->cb, gcd->cb_data);
 }
 
 static void ifx_gprs_deactivate_primary(struct ofono_gprs_context *gc,
@@ -374,7 +380,7 @@ static void ifx_gprs_deactivate_primary(struct ofono_gprs_context *gc,
 	DBG("cid %u", cid);
 
 	gcd->state = STATE_DISABLING;
-	gcd->down_cb = cb;
+	gcd->cb = cb;
 	gcd->cb_data = data;
 
 	g_at_rawip_shutdown(gcd->rawip);
-- 
1.7.1

[PATCH 11/12] stemodem: update to new gprs context interface

[Mika Liljeberg]

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

---
 drivers/stemodem/gprs-context.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/stemodem/gprs-context.c b/drivers/stemodem/gprs-context.c
index e247f35..7266810 100644
--- a/drivers/stemodem/gprs-context.c
+++ b/drivers/stemodem/gprs-context.c
@@ -246,7 +246,7 @@ static void ste_eppsd_down_cb(gboolean ok, GAtResult *result,
 static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct conn_info *conn;
@@ -276,7 +276,7 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		conn->cid = 0;
 		gcd->active_context = 0;
 		decode_at_error(&error, g_at_result_final_response(result));
-		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -305,9 +305,13 @@ static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dns[1] = rsp.dns_server2;
 	dns[2] = NULL;
 
-	CALLBACK_WITH_SUCCESS(cb, conn->interface, TRUE, rsp.ip_address,
-				rsp.subnet_mask, NULL,
-				dns, cbd->data);
+	ofono_gprs_context_set_interface(gc, conn->interface);
+	ofono_gprs_context_set_ip_addrconf(gc, OFONO_GPRS_ADDRCONF_STATIC);
+	ofono_gprs_context_set_ip_address(gc, rsp.ip_address);
+	ofono_gprs_context_set_ip_netmask(gc, rsp.subnet_mask);
+	ofono_gprs_context_set_dns_servers(gc, dns);
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
 	return;
 
 error:
@@ -320,13 +324,13 @@ error:
 error_no_device:
 	DBG("ste_eppsd_up_cb error");
 	gcd->active_context = 0;
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
 static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *ncbd;
@@ -351,7 +355,7 @@ static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		conn->cid = 0;
 		gcd->active_context = 0;
 		decode_at_error(&error, g_at_result_final_response(result));
-		cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data);
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -368,13 +372,12 @@ static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 error_no_device:
 	gcd->active_context = 0;
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL,
-				NULL, NULL, cbd->data);
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
 }
 
 static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *cbd = cb_data_new(cb, data);
@@ -383,6 +386,10 @@ static void ste_gprs_activate_primary(struct ofono_gprs_context *gc,
 	GSList *l;
 	struct conn_info *conn;
 
+	/* IPv6 support not implemented */
+	if (ctx->proto != OFONO_GPRS_PROTO_IP)
+		goto error_no_device;
+
 	gcd->active_context = ctx->cid;
 	cbd->user = gc;
 
@@ -432,7 +439,7 @@ error_no_device:
 	gcd->active_context = 0;
 	g_free(cbd);
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc,
-- 
1.7.1

[PATCH 12/12] phonesim: add IPv6 support

[Mika Liljeberg]

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

---
 plugins/phonesim.c |   32 +++++++++++++++++++++++++-------
 1 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 2b36fe0..2634d23 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -83,14 +83,17 @@ struct gprs_context_data {
 static void at_cgact_up_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_gprs_context_up_cb_t cb = cbd->cb;
+	ofono_gprs_context_cb_t cb = cbd->cb;
 	struct ofono_gprs_context *gc = cbd->user;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct ofono_error error;
 
 	decode_at_error(&error, g_at_result_final_response(result));
-	cb(&error, ok ? gcd->interface : NULL, FALSE,
-			NULL, NULL, NULL, NULL, cbd->data);
+
+	if (ok)
+		ofono_gprs_context_set_interface(gc, gcd->interface);
+
+	cb(&error, cbd->data);
 }
 
 static void at_cgact_down_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -105,16 +108,31 @@ static void at_cgact_down_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 static void phonesim_activate_primary(struct ofono_gprs_context *gc,
 				const struct ofono_gprs_primary_context *ctx,
-				ofono_gprs_context_up_cb_t cb, void *data)
+				ofono_gprs_context_cb_t cb, void *data)
 {
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	struct cb_data *cbd = cb_data_new(cb, data);
 	char buf[OFONO_GPRS_MAX_APN_LENGTH + 128];
-	int len;
+	int len = 0;
 
 	cbd->user = gc;
 
-	len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"", ctx->cid);
+	switch (ctx->proto) {
+	case OFONO_GPRS_PROTO_IP:
+		len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IP\"",
+				ctx->cid);
+		break;
+
+	case OFONO_GPRS_PROTO_IPV6:
+		len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV6\"",
+				ctx->cid);
+		break;
+
+	case OFONO_GPRS_PROTO_IPV4V6:
+		len = snprintf(buf, sizeof(buf), "AT+CGDCONT=%u,\"IPV4V6\"",
+				ctx->cid);
+		break;
+	}
 
 	if (ctx->apn)
 		snprintf(buf + len, sizeof(buf) - len - 3, ",\"%s\"",
@@ -132,7 +150,7 @@ static void phonesim_activate_primary(struct ofono_gprs_context *gc,
 error:
 	g_free(cbd);
 
-	CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data);
+	CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void phonesim_deactivate_primary(struct ofono_gprs_context *gc,
-- 
1.7.1

Re: [PATCH 02/12] gprs: driver interface changes for IPv6

[Denis Kenzior]

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

Hi Mika,

> +enum ofono_gprs_addrconf {
> +	OFONO_GPRS_ADDRCONF_NONE,
> +	OFONO_GPRS_ADDRCONF_STATIC,
> +	OFONO_GPRS_ADDRCONF_DHCP,
> +};
> +

What exactly is the _NONE addrconf for?  And why don't we use a gboolean
here like before?

>  struct ofono_gprs_primary_context {
>  	unsigned int cid;
>  	int direction;
> @@ -94,6 +101,20 @@ struct ofono_modem *ofono_gprs_context_get_modem(struct ofono_gprs_context *gc);
>  
>  void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
>  					enum ofono_gprs_context_type type);
> +void ofono_gprs_context_set_interface(struct ofono_gprs_context *gc,
> +					const char *interface);
> +void ofono_gprs_context_set_ip_addrconf(struct ofono_gprs_context *gc,
> +					enum ofono_gprs_addrconf method);
> +void ofono_gprs_context_set_ip_address(struct ofono_gprs_context *gc,
> +					const char *address);
> +void ofono_gprs_context_set_ip_netmask(struct ofono_gprs_context *gc,
> +					const char *netmask);
> +void ofono_gprs_context_set_ip_gateway(struct ofono_gprs_context *gc,
> +					const char *netmask);
> +void ofono_gprs_context_set_ipv6_address(struct ofono_gprs_context *gc,
> +					const char *address);
> +void ofono_gprs_context_set_dns_servers(struct ofono_gprs_context *gc,
> +					const char **dns);
>  

So I'm actually fine with this approach.

>  #ifdef __cplusplus
>  }

Regards,
-Denis

Re: [PATCH 03/12] gprs: core support for IPv6

[Denis Kenzior]

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

Hi Mika,

On 02/03/2011 03:03 AM, Mika Liljeberg wrote:
> ---
>  include/gprs-context.h |    2 +-
>  src/gprs.c             |  296 +++++++++++++++++++++++++++++++-----------------
>  2 files changed, 192 insertions(+), 106 deletions(-)
> 
> diff --git a/include/gprs-context.h b/include/gprs-context.h
> index 88530f6..d03dd47 100644
> --- a/include/gprs-context.h
> +++ b/include/gprs-context.h
> @@ -77,7 +77,7 @@ struct ofono_gprs_context_driver {
>  	void (*remove)(struct ofono_gprs_context *gc);
>  	void (*activate_primary)(struct ofono_gprs_context *gc,
>  				const struct ofono_gprs_primary_context *ctx,
> -				ofono_gprs_context_up_cb_t cb, void *data);
> +				ofono_gprs_context_cb_t cb, void *data);
>  	void (*deactivate_primary)(struct ofono_gprs_context *gc,
>  					unsigned int id,
>  					ofono_gprs_context_cb_t cb, void *data);
> diff --git a/src/gprs.c b/src/gprs.c
> index 7d36633..b5dd5f2 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -99,10 +99,12 @@ struct ofono_gprs {
>  	struct ofono_atom *atom;
>  };
>  
> +struct pri_context;
> +
>  struct ofono_gprs_context {
>  	struct ofono_gprs *gprs;
> +	struct pri_context *pri;
>  	enum ofono_gprs_context_type type;
> -	ofono_bool_t inuse;

I don't see how getting rid of this is not a good idea.  As soon as the
context driver is assigned to a context being activated we set it as
'inuse'.  If you don't set this variable then it is possible to pick the
same driver, even if that driver is busy but has not started to set its
settings yet.

>  	const struct ofono_gprs_context_driver *driver;
>  	void *driver_data;
>  	struct ofono_atom *atom;
> @@ -111,12 +113,13 @@ struct ofono_gprs_context {
>  struct context_settings {
>  	enum ofono_gprs_context_type type;
>  	char *interface;
> -	gboolean static_ip;
> +	enum ofono_gprs_addrconf method;
>  	char *ip;
>  	char *netmask;
>  	char *gateway;
>  	char **dns;
>  	char *proxy;
> +	char *ipv6addr;

Nitpick but please name this ipv6 and put it right after ip

>  };
>  
>  struct pri_context {
> @@ -226,6 +229,8 @@ static const char *gprs_proto_to_string(enum ofono_gprs_proto proto)
>  		return "ip";
>  	case OFONO_GPRS_PROTO_IPV6:
>  		return "ipv6";
> +	case OFONO_GPRS_PROTO_IPV4V6:
> +		return "ipv4v6";
>  	};
>  
>  	return NULL;
> @@ -240,11 +245,28 @@ static gboolean gprs_proto_from_string(const char *str,
>  	} else if (g_str_equal(str, "ipv6")) {
>  		*proto = OFONO_GPRS_PROTO_IPV6;
>  		return TRUE;
> +	} else if (g_str_equal(str, "ipv4v6")) {
> +		*proto = OFONO_GPRS_PROTO_IPV4V6;
> +		return TRUE;
>  	}
>  
>  	return FALSE;
>  }
>  
> +static const char *gprs_addrconf_to_string(enum ofono_gprs_addrconf method)
> +{
> +	switch (method) {
> +	case OFONO_GPRS_ADDRCONF_NONE:
> +		return NULL;
> +	case OFONO_GPRS_ADDRCONF_STATIC:
> +		return "static";
> +	case OFONO_GPRS_ADDRCONF_DHCP:
> +		return "dhcp";
> +	}
> +
> +	return NULL;
> +}
> +
>  static unsigned int gprs_cid_alloc(struct ofono_gprs *gprs)
>  {
>  	return idmap_alloc(gprs->cid_map);
> @@ -255,6 +277,49 @@ static void gprs_cid_release(struct ofono_gprs *gprs, unsigned int id)
>  	idmap_put(gprs->cid_map, id);
>  }
>  
> +static gboolean assign_context(struct pri_context *ctx)
> +{
> +	struct idmap *cidmap = ctx->gprs->cid_map;
> +	unsigned int cid_min;
> +	GSList *l;
> +
> +	if (cidmap == NULL)
> +		return FALSE;
> +
> +	cid_min = idmap_get_min(cidmap);
> +
> +	ctx->context.cid = gprs_cid_alloc(ctx->gprs);
> +	if (ctx->context.cid == 0)
> +		return FALSE;
> +
> +	for (l = ctx->gprs->context_drivers; l; l = l->next) {
> +		struct ofono_gprs_context *gc = l->data;
> +
> +		if (gc->pri != NULL)
> +			continue;
> +
> +		if (gc->type == OFONO_GPRS_CONTEXT_TYPE_ANY ||
> +						gc->type == ctx->type) {
> +			gc->pri = ctx;
> +			ctx->context_driver = gc;
> +			return TRUE;
> +		}
> +	}
> +
> +	return FALSE;
> +}
> +
> +static void release_context(struct pri_context *ctx)
> +{
> +	if (ctx == NULL || ctx->gprs == NULL)
> +		return;
> +
> +	gprs_cid_release(ctx->gprs, ctx->context.cid);
> +	ctx->context.cid = 0;
> +	ctx->context_driver->pri = NULL;
> +	ctx->context_driver = NULL;
> +}
> +

For something like this I'd really like to see it in a separate patch.
You're moving the function and also modifying the implementation.  It is
much easier to review a patch set if you move a function in one patch
and then modify its behavior in another.  Same with release_context.  It
is probably a good idea to put the common code in here, but modify the
behavior as a part of a different patch.

Ideally this patch should be at least 2:
- For moving assign_context and factoring out common code into
release_context
- The rest of this patch + modifications to the two above.

>  static struct pri_context *gprs_context_by_path(struct ofono_gprs *gprs,
>  						const char *ctx_path)
>  {
> @@ -278,6 +343,7 @@ static void context_settings_free(struct context_settings *settings)
>  	g_free(settings->gateway);
>  	g_strfreev(settings->dns);
>  	g_free(settings->proxy);
> +	g_free(settings->ipv6addr);
>  
>  	g_free(settings);
>  }
> @@ -316,12 +382,10 @@ static void context_settings_append_variant(struct context_settings *settings,
>  		goto done;
>  	}
>  
> -	if (settings->static_ip == TRUE)
> -		method = "static";
> -	else
> -		method = "dhcp";
> -
> -	ofono_dbus_dict_append(&array, "Method", DBUS_TYPE_STRING, &method);
> +	method = gprs_addrconf_to_string(settings->method);
> +	if (method != NULL)
> +		ofono_dbus_dict_append(&array, "Method", DBUS_TYPE_STRING,
> +					&method);
>  
>  	if (settings->ip)
>  		ofono_dbus_dict_append(&array, "Address", DBUS_TYPE_STRING,
> @@ -335,6 +399,19 @@ static void context_settings_append_variant(struct context_settings *settings,
>  		ofono_dbus_dict_append(&array, "Gateway", DBUS_TYPE_STRING,
>  					&settings->gateway);
>  
> +	if (settings->ipv6addr != NULL) {
> +		uint8_t addr[16];
> +		char *strll = alloca(INET6_ADDRSTRLEN);
> +
> +		inet_pton(AF_INET6, settings->ipv6addr, addr);
> +		memset(addr, 0, 8);
> +		addr[0] = 0xfe;
> +		addr[1] = 0x80;
> +		inet_ntop(AF_INET6, addr, strll, INET6_ADDRSTRLEN);
> +		ofono_dbus_dict_append(&array, "IPv6Address", DBUS_TYPE_STRING,
> +					&strll);
> +	}
> +
>  	if (settings->dns)
>  		ofono_dbus_dict_append_array(&array, "DomainNameServers",
>  						DBUS_TYPE_STRING,
> @@ -578,42 +655,27 @@ static void pri_reset_context_settings(struct pri_context *ctx)
>  	g_free(interface);
>  }
>  
> -static void pri_update_context_settings(struct pri_context *ctx,
> -					const char *interface,
> -					ofono_bool_t static_ip,
> -					const char *ip, const char *netmask,
> -					const char *gateway, const char **dns)
> +static void pri_update_context_settings(struct pri_context *ctx)
>  {
> -	if (ctx->settings)
> -		context_settings_free(ctx->settings);
> -
> -	ctx->settings = g_try_new0(struct context_settings, 1);
> -	if (ctx->settings == NULL)
> +	if (ctx->settings == NULL || ctx->settings->interface == NULL)
>  		return;
>  
>  	ctx->settings->type = ctx->type;
>  
> -	ctx->settings->interface = g_strdup(interface);
> -	ctx->settings->static_ip = static_ip;
> -	ctx->settings->ip = g_strdup(ip);
> -	ctx->settings->netmask = g_strdup(netmask);
> -	ctx->settings->gateway = g_strdup(gateway);
> -	ctx->settings->dns = g_strdupv((char **)dns);
> -
>  	if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS && ctx->message_proxy)
>  		ctx->settings->proxy = g_strdup(ctx->message_proxy);
>  
> -	pri_ifupdown(interface, TRUE);
> +	pri_ifupdown(ctx->settings->interface, TRUE);
>  
>  	if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
>  		pri_parse_proxy(ctx, ctx->message_proxy);
>  
>  		DBG("proxy %s port %u", ctx->proxy_host, ctx->proxy_port);
>  
> -		pri_setaddr(interface, ip);
> +		pri_setaddr(ctx->settings->interface, ctx->settings->ip);
>  
>  		if (ctx->proxy_host)
> -			pri_setproxy(interface, ctx->proxy_host);
> +			pri_setproxy(ctx->settings->interface, ctx->proxy_host);
>  	}
>  
>  	pri_context_signal_settings(ctx);
> @@ -685,30 +747,18 @@ static DBusMessage *pri_get_properties(DBusConnection *conn,
>  	return reply;
>  }
>  
> -static void pri_activate_callback(const struct ofono_error *error,
> -					const char *interface,
> -					ofono_bool_t static_ip,
> -					const char *ip, const char *netmask,
> -					const char *gateway, const char **dns,
> -					void *data)
> +static void pri_activate_callback(const struct ofono_error *error, void *data)
>  {
>  	struct pri_context *ctx = data;
>  	DBusConnection *conn = ofono_dbus_get_connection();
>  	dbus_bool_t value;
>  
> -	DBG("%p %s", ctx, interface);
> -
>  	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>  		DBG("Activating context failed with error: %s",
>  				telephony_error_to_str(error));
>  		__ofono_dbus_pending_reply(&ctx->pending,
>  					__ofono_error_failed(ctx->pending));
> -
> -		gprs_cid_release(ctx->gprs, ctx->context.cid);
> -		ctx->context.cid = 0;
> -		ctx->context_driver->inuse = FALSE;
> -		ctx->context_driver = NULL;
> -
> +		release_context(ctx);
>  		return;
>  	}
>  
> @@ -716,14 +766,7 @@ static void pri_activate_callback(const struct ofono_error *error,
>  	__ofono_dbus_pending_reply(&ctx->pending,
>  				dbus_message_new_method_return(ctx->pending));
>  
> -	/*
> -	 * If we don't have the interface, don't bother emitting any settings,
> -	 * as nobody can make use of them
> -	 */
> -	if (interface != NULL)
> -		pri_update_context_settings(ctx, interface, static_ip,
> -						ip, netmask, gateway, dns);
> -
> +	pri_update_context_settings(ctx);
>  	value = ctx->active;
>  	ofono_dbus_signal_property_changed(conn, ctx->path,
>  					OFONO_CONNECTION_CONTEXT_INTERFACE,
> @@ -744,11 +787,8 @@ static void pri_deactivate_callback(const struct ofono_error *error, void *data)
>  		return;
>  	}
>  
> -	gprs_cid_release(ctx->gprs, ctx->context.cid);
> -	ctx->context.cid = 0;
> +	release_context(ctx);
>  	ctx->active = FALSE;
> -	ctx->context_driver->inuse = FALSE;
> -	ctx->context_driver = NULL;
>  
>  	__ofono_dbus_pending_reply(&ctx->pending,
>  				dbus_message_new_method_return(ctx->pending));
> @@ -995,38 +1035,6 @@ static DBusMessage *pri_set_message_center(struct pri_context *ctx,
>  	return NULL;
>  }
>  
> -static gboolean assign_context(struct pri_context *ctx)
> -{
> -	struct idmap *cidmap = ctx->gprs->cid_map;
> -	unsigned int cid_min;
> -	GSList *l;
> -
> -	if (cidmap == NULL)
> -		return FALSE;
> -
> -	cid_min = idmap_get_min(cidmap);
> -
> -	ctx->context.cid = gprs_cid_alloc(ctx->gprs);
> -	if (ctx->context.cid == 0)
> -		return FALSE;
> -
> -	for (l = ctx->gprs->context_drivers; l; l = l->next) {
> -		struct ofono_gprs_context *gc = l->data;
> -
> -		if (gc->inuse == TRUE)
> -			continue;
> -
> -		if (gc->type == OFONO_GPRS_CONTEXT_TYPE_ANY ||
> -						gc->type == ctx->type) {
> -			ctx->context_driver = gc;
> -			ctx->context_driver->inuse = TRUE;
> -			return TRUE;
> -		}
> -	}
> -
> -	return FALSE;
> -}
> -
>  static DBusMessage *pri_set_property(DBusConnection *conn,
>  					DBusMessage *msg, void *data)
>  {
> @@ -1179,6 +1187,8 @@ static GDBusSignalTable context_signals[] = {
>  	{ }
>  };
>  
> +
> +
>  static struct pri_context *pri_context_create(struct ofono_gprs *gprs,
>  					const char *name,
>  					enum ofono_gprs_context_type type)
> @@ -1344,14 +1354,9 @@ static void gprs_attached_update(struct ofono_gprs *gprs)
>  			if (ctx->active == FALSE)
>  				continue;
>  
> -			gprs_cid_release(gprs, ctx->context.cid);
> -			ctx->context.cid = 0;
> +			release_context(ctx);
>  			ctx->active = FALSE;
> -			ctx->context_driver->inuse = FALSE;
> -			ctx->context_driver = NULL;
> -
>  			pri_reset_context_settings(ctx);
> -
>  			value = FALSE;
>  			ofono_dbus_signal_property_changed(conn, ctx->path,
>  					OFONO_CONNECTION_CONTEXT_INTERFACE,
> @@ -1736,10 +1741,7 @@ static void gprs_deactivate_for_remove(const struct ofono_error *error,
>  		return;
>  	}
>  
> -	gprs_cid_release(gprs, ctx->context.cid);
> -	ctx->context.cid = 0;
> -	ctx->context_driver->inuse = FALSE;
> -	ctx->context_driver = NULL;
> +	release_context(ctx);
>  
>  	if (gprs->settings) {
>  		g_key_file_remove_group(gprs->settings, ctx->key, NULL);
> @@ -1831,12 +1833,9 @@ static void gprs_deactivate_for_all(const struct ofono_error *error,
>  		return;
>  	}
>  
> -	gprs_cid_release(gprs, ctx->context.cid);
> -	ctx->active = FALSE;
> -	ctx->context.cid = 0;
> -	ctx->context_driver->inuse = FALSE;
> -	ctx->context_driver = NULL;
> +	release_context(ctx);
>  
> +	ctx->active = FALSE;
>  	pri_reset_context_settings(ctx);
>  
>  	value = ctx->active;
> @@ -2087,11 +2086,8 @@ void ofono_gprs_context_deactivated(struct ofono_gprs_context *gc,
>  		if (ctx->active == FALSE)
>  			break;
>  
> -		gprs_cid_release(ctx->gprs, ctx->context.cid);
> -		ctx->context.cid = 0;
> +		release_context(ctx);
>  		ctx->active = FALSE;
> -		ctx->context_driver->inuse = FALSE;
> -		ctx->context_driver = NULL;
>  
>  		pri_reset_context_settings(ctx);
>  
> @@ -2130,6 +2126,8 @@ static void gprs_context_remove(struct ofono_atom *atom)
>  	if (gc == NULL)
>  		return;
>  
> +	release_context(gc->pri);
> +
>  	if (gc->driver && gc->driver->remove)
>  		gc->driver->remove(gc);
>  
> @@ -2202,6 +2200,94 @@ void ofono_gprs_context_set_type(struct ofono_gprs_context *gc,
>  	gc->type = type;
>  }
>  
> +static struct context_settings *get_settings(struct ofono_gprs_context *gc)
> +{
> +	if (gc == NULL || gc->pri == NULL)
> +		return NULL;
> +
> +	if (gc->pri->settings == NULL)
> +		gc->pri->settings = g_try_new0(struct context_settings, 1);
> +

I appreciate the lazy allocation, however you get into trouble if the
driver calls one of the _set functions but still returns a failure in
the callback.  Overall this seems a more complicated approach than
returning values in the callback of activate_primary.

> +	return gc->pri->settings;
> +}
> +
> +void ofono_gprs_context_set_interface(struct ofono_gprs_context *gc,
> +					const char *interface)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->interface != NULL)
> +		return;

I'd let the driver call these functions as often as they want and just
take the last value.

> +
> +	settings->interface = g_strdup(interface);
> +}
> +
> +void ofono_gprs_context_set_ip_addrconf(struct ofono_gprs_context *gc,
> +					enum ofono_gprs_addrconf method)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL)
> +		return;
> +
> +	settings->method = method;
> +}
> +
> +void ofono_gprs_context_set_ip_address(struct ofono_gprs_context *gc,
> +					const char *address)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->ip != NULL)
> +		return;
> +
> +	settings->ip = g_strdup(address);
> +}
> +
> +void ofono_gprs_context_set_ip_netmask(struct ofono_gprs_context *gc,
> +					const char *netmask)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->netmask != NULL)
> +		return;
> +
> +	settings->netmask = g_strdup(netmask);
> +}
> +
> +void ofono_gprs_context_set_ip_gateway(struct ofono_gprs_context *gc,
> +					const char *gateway)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->gateway != NULL)
> +		return;
> +
> +	settings->gateway = g_strdup(gateway);
> +}
> +
> +void ofono_gprs_context_set_ipv6_address(struct ofono_gprs_context *gc,
> +						const char *address)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->ipv6addr != NULL)
> +		return;
> +
> +	settings->ipv6addr = g_strdup(address);
> +}
> +
> +void ofono_gprs_context_set_dns_servers(struct ofono_gprs_context *gc,
> +						const char **dns)
> +{
> +	struct context_settings *settings = get_settings(gc);
> +
> +	if (settings == NULL || settings->dns != NULL)
> +		return;
> +
> +	settings->dns = g_strdupv((char **)dns);
> +}
> +
>  int ofono_gprs_driver_register(const struct ofono_gprs_driver *d)
>  {
>  	DBG("driver: %p, name: %s", d, d->name);

Regards,
-Denis

RE: [PATCH 02/12] gprs: driver interface changes for IPv6

[Mika.Liljeberg]

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

Hi Denis,

Thanks for the comments. 

> > +enum ofono_gprs_addrconf {
> > +	OFONO_GPRS_ADDRCONF_NONE,
> > +	OFONO_GPRS_ADDRCONF_STATIC,
> > +	OFONO_GPRS_ADDRCONF_DHCP,
> > +};
> > +
> 
> What exactly is the _NONE addrconf for?  And why don't we use 
> a gboolean
> here like before?

Neither "static" nor "DHCP" makes sense for IPv6. If we have an IPv6 only context, the "Method" property will not show up at all.

I also wanted to leave the door open for additional address configuration protocols in case e.g. IPv6 capable modems with virtual ethernet intefaces require some weird quirks. I also felt this was a bit cleaner, although that is a matter of opinion, of course.

Normally, IPv6 addresses and routes are autoconfigured using IPv6 stateless address autoconfiguration followed optionally by DHCPv6 to get additional settings. The kernel actually starts stateless address autoconfiguration immediately when the interface is configured up (which oFono does), assuming the interface already has a link-local IPv6 address (which all Ethernet interface do have, as it is calculated from the MAC address and added automatically).

Point-to-point interfaces, on the other, require the link-local address to be added manually to the interface before the autoconfiguration can happen. oFono should probably add the address to be consistent with Ethernet interfaces.

The upshot would be that IPv6 could be autoconfigured immediately without connman's intervention. IPv6 does not have private address spaces, so in theory connman does not really have to care about IPv6 addresses and routes. However, the optional DHCPv6 step would probably be controlled by connman. Configuration of IPv6 tethering would be another optional followup step controlled by connman.

On the other hand, if we want connman to be fully in charge, we could change oFono to always leave the network interface down and let connman do everything. Marcels input on this would be welcome as well.

Regards,

	MikaL

RE: [PATCH 03/12] gprs: core support for IPv6

[Mika.Liljeberg]

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

Hi Denis,

> > +struct pri_context;
> > +
> >  struct ofono_gprs_context {
> >  	struct ofono_gprs *gprs;
> > +	struct pri_context *pri;
> >  	enum ofono_gprs_context_type type;
> > -	ofono_bool_t inuse;
> 
> I don't see how getting rid of this is not a good idea.  As 
> soon as the
> context driver is assigned to a context being activated we set it as
> 'inuse'.  If you don't set this variable then it is possible 
> to pick the
> same driver, even if that driver is busy but has not started 
> to set its
> settings yet.

The pri pointer points to the owner of the context driver. It is assigned when the context driver is allocated and cleared when the context driver is released. Therefore, the inuse flag becomes redundant. It is enough to check if pri != NULL to know if the context is in use.

> > +static gboolean assign_context(struct pri_context *ctx)
..
> > +static void release_context(struct pri_context *ctx)
..
> For something like this I'd really like to see it in a separate patch.
> You're moving the function and also modifying the 
> implementation.  It is
> much easier to review a patch set if you move a function in one patch
> and then modify its behavior in another.  Same with 
> release_context.  It
> is probably a good idea to put the common code in here, but modify the
> behavior as a part of a different patch.

Ok, I'll split it out. I decided to do this when in the middle of making my changes I noticed that the release code is actually copy pasted many times over.

> Ideally this patch should be at least 2:
> - For moving assign_context and factoring out common code into
> release_context
> - The rest of this patch + modifications to the two above.

Will do.

> > +static struct context_settings *get_settings(struct 
> ofono_gprs_context *gc)
> > +{
> > +	if (gc == NULL || gc->pri == NULL)
> > +		return NULL;
> > +
> > +	if (gc->pri->settings == NULL)
> > +		gc->pri->settings = g_try_new0(struct 
> context_settings, 1);
> > +
> 
> I appreciate the lazy allocation, however you get into trouble if the
> driver calls one of the _set functions but still returns a failure in
> the callback.  Overall this seems a more complicated approach than
> returning values in the callback of activate_primary.

Oops. I thought I had the failure path covered but this bit changed from the RFC patches. The lazy allocation is a remnant from having separate dicts for IPv4 and IPv6 settings. I'll get rid of it.

> > +void ofono_gprs_context_set_interface(struct 
> ofono_gprs_context *gc,
> > +					const char *interface)
> > +{
> > +	struct context_settings *settings = get_settings(gc);
> > +
> > +	if (settings == NULL || settings->interface != NULL)
> > +		return;
> 
> I'd let the driver call these functions as often as they want and just
> take the last value.

Ok. Makes sense.

Regards,

	MikaL

Re: [PATCH 02/12] gprs: driver interface changes for IPv6

[Denis Kenzior]

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

Hi Mika,

On 02/07/2011 03:55 AM, Mika.Liljeberg(a)nokia.com wrote:
> Hi Denis,
> 
> Thanks for the comments. 
> 
>>> +enum ofono_gprs_addrconf {
>>> +	OFONO_GPRS_ADDRCONF_NONE,
>>> +	OFONO_GPRS_ADDRCONF_STATIC,
>>> +	OFONO_GPRS_ADDRCONF_DHCP,
>>> +};
>>> +
>>
>> What exactly is the _NONE addrconf for?  And why don't we use 
>> a gboolean
>> here like before?
> 
> Neither "static" nor "DHCP" makes sense for IPv6. If we have an IPv6 only context, the "Method" property will not show up at all.

What about combined V4 V6 contexts? What method do you use there, since
NONE doesn't exactly fit?  I'd rather see this be ignored for IPv6
contexts or that we separate the IPv4 and IPv6 configuration into two
separate batches.

Regards,
-Denis

RE: [PATCH 02/12] gprs: driver interface changes for IPv6

[Marcel Holtmann]

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

Hi Mika,

> > > +enum ofono_gprs_addrconf {
> > > +	OFONO_GPRS_ADDRCONF_NONE,
> > > +	OFONO_GPRS_ADDRCONF_STATIC,
> > > +	OFONO_GPRS_ADDRCONF_DHCP,
> > > +};
> > > +
> > 
> > What exactly is the _NONE addrconf for?  And why don't we use 
> > a gboolean
> > here like before?
> 
> Neither "static" nor "DHCP" makes sense for IPv6. If we have an IPv6 only context, the "Method" property will not show up at all.
> 
> I also wanted to leave the door open for additional address configuration protocols in case e.g. IPv6 capable modems with virtual ethernet intefaces require some weird quirks. I also felt this was a bit cleaner, although that is a matter of opinion, of course.
> 
> Normally, IPv6 addresses and routes are autoconfigured using IPv6 stateless address autoconfiguration followed optionally by DHCPv6 to get additional settings. The kernel actually starts stateless address autoconfiguration immediately when the interface is configured up (which oFono does), assuming the interface already has a link-local IPv6 address (which all Ethernet interface do have, as it is calculated from the MAC address and added automatically).
> 
> Point-to-point interfaces, on the other, require the link-local address to be added manually to the interface before the autoconfiguration can happen. oFono should probably add the address to be consistent with Ethernet interfaces.

I am not sure we really wanna be consistent with how Ethernet works
since we are not actually Ethernet.

So I can see benefits for having ConnMan control the link-local in case
of point-to-point, but then I also do see the benefit oFono doing it.

We might have to spin of ideas and see what is the most logical one and
what works best. Especially also in the context of LTE support in the
future.

> The upshot would be that IPv6 could be autoconfigured immediately without connman's intervention. IPv6 does not have private address spaces, so in theory connman does not really have to care about IPv6 addresses and routes. However, the optional DHCPv6 step would probably be controlled by connman. Configuration of IPv6 tethering would be another optional followup step controlled by connman.

Yes, DHCPv6 step needs to be done by ConnMan. We do not have DHCPv6
support right now. We only plan to use it for extra settings like
proxies, timeservers etc.

> On the other hand, if we want connman to be fully in charge, we could change oFono to always leave the network interface down and let connman do everything. Marcels input on this would be welcome as well.

This is something we need to figure out. And of course this all needs to
be aligned with IPv4 and IPv6. Right now we are treating this similar to
how wpa_supplicant for WiFi does this.

So going back to potential LTE and IMS support; if that runs IPv6 then
it might make sense to just autoconfigure it without ConnMan
interaction.

Regards

Marcel

RE: [PATCH 02/12] gprs: driver interface changes for IPv6

[Mika.Liljeberg]

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

Hi Marcel, 

Thanks for the comments.

> > Normally, IPv6 addresses and routes are autoconfigured 
> using IPv6 stateless address autoconfiguration followed 
> optionally by DHCPv6 to get additional settings. The kernel 
> actually starts stateless address autoconfiguration 
> immediately when the interface is configured up (which oFono 
> does), assuming the interface already has a link-local IPv6 
> address (which all Ethernet interface do have, as it is 
> calculated from the MAC address and added automatically).
> > 
> > Point-to-point interfaces, on the other, require the 
> link-local address to be added manually to the interface 
> before the autoconfiguration can happen. oFono should 
> probably add the address to be consistent with Ethernet interfaces.
> 
> I am not sure we really wanna be consistent with how Ethernet works
> since we are not actually Ethernet.
> 
> So I can see benefits for having ConnMan control the 
> link-local in case
> of point-to-point, but then I also do see the benefit oFono doing it.
> 
> We might have to spin of ideas and see what is the most 
> logical one and
> what works best. Especially also in the context of LTE support in the
> future.

I agree. The pragmatic way would be to just choose one option now and see how that works for us. We can revisit the issue later when we understand the requirements better.

> > The upshot would be that IPv6 could be autoconfigured 
> immediately without connman's intervention. IPv6 does not 
> have private address spaces, so in theory connman does not 
> really have to care about IPv6 addresses and routes. However, 
> the optional DHCPv6 step would probably be controlled by 
> connman. Configuration of IPv6 tethering would be another 
> optional followup step controlled by connman.
> 
> Yes, DHCPv6 step needs to be done by ConnMan. We do not have DHCPv6
> support right now. We only plan to use it for extra settings like
> proxies, timeservers etc.
> 
> > On the other hand, if we want connman to be fully in 
> charge, we could change oFono to always leave the network 
> interface down and let connman do everything. Marcels input 
> on this would be welcome as well.
> 
> This is something we need to figure out. And of course this 
> all needs to
> be aligned with IPv4 and IPv6. Right now we are treating this 
> similar to
> how wpa_supplicant for WiFi does this.
> 
> So going back to potential LTE and IMS support; if that runs IPv6 then
> it might make sense to just autoconfigure it without ConnMan
> interaction.

Ok. If you agree, I'll add link-local address configuration to my patches for all IPv6 contexts for the time being. Autonomous autoconfiguration is in accordance with IPv6 philosophy anyway. Might as well see how far it takes us. We can always move to a connman controlled approach later if that turns out to be necessary.

Regards,

	MikaL

RE: [PATCH 02/12] gprs: driver interface changes for IPv6

[Mika.Liljeberg]

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

Hi Denis, 

> >>> +enum ofono_gprs_addrconf {
> >>> +	OFONO_GPRS_ADDRCONF_NONE,
> >>> +	OFONO_GPRS_ADDRCONF_STATIC,
> >>> +	OFONO_GPRS_ADDRCONF_DHCP,
> >>> +};
> >>> +
> >>
> >> What exactly is the _NONE addrconf for?  And why don't we use 
> >> a gboolean
> >> here like before?
> > 
> > Neither "static" nor "DHCP" makes sense for IPv6. If we 
> have an IPv6 only context, the "Method" property will not 
> show up at all.
> 
> What about combined V4 V6 contexts? What method do you use 
> there, since
> NONE doesn't exactly fit?

The driver will call the ofono_gprs_context_set_ip_addrconf() method to set either STATIC or DHCP configuration method for IPv4. NONE is just a deault intializer, so the "Method" property doesn't show up in the "Settings" dict, unless actually set by the driver.

> I'd rather see this be ignored for IPv6
> contexts or that we separate the IPv4 and IPv6 configuration into two 
> separate batches.

That's the idea. If we find that IPv6 needs some additional support, the plan is to add a new ofono_gprs_context_set_ipv6_addrconf() method, which sets the the address configuration method for IPv6. That would be associated with a new "IPv6Method" property in the settings dict. At the moment I don't know if we need this, until the driver writers start implemeting IPv6 support for their modems.

Regards,

	MikaL