Discussion:
[RFC 0/3] Implementation of simultaneous internet contexts activation
(too old to reply)
Mylene JOSSERAND
2015-10-09 06:37:49 UTC
Permalink
This patchset updates and implements in ofono's plugin a simultaneous internet contexts activation per modem.
Thanks to these patches, we can create two (or more) contexts for one modem and activates them simultaneous (if the modem handles it).
It has been tested with a cinterion pxs8 for two contexts : one for QMI and another for PPP interface.

The patch splitting is not a definitive one, it is a proposal.

Thank you in advance for patch reviews.

Mylene JOSSERAND (3):
ofono: create a list of contexts per modem
ofono: Implementation of simultaneous APNS
ofono: add array parsing when contexts are added

plugins/ofono.c | 532 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 338 insertions(+), 194 deletions(-)
--
2.1.4
Mylene JOSSERAND
2015-10-09 06:37:50 UTC
Permalink
This is a first patch of a serie to implement a simultaneous APNs context activation.

The current commit removes the previous implementation of one context per modem and
adds a list of contexts instead.

Some modifications are necessary : some properties in the modem structure are moved
to the context structure (such as connman_network, valid_apn, etc) as they are properties
specifics to context.
Some functions are implemented to search for a specific context in the list by his path
or by his network. A function to remove all the context of one modem
is also implemented.
All others modifications are related to the transfer of the above properties.
---
plugins/ofono.c | 513 ++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 329 insertions(+), 184 deletions(-)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 4337d45..485608a 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -133,6 +133,7 @@ static GHashTable *context_hash;
struct network_context {
char *path;
int index;
+ struct connman_network *network;

enum connman_ipconfig_method ipv4_method;
struct connman_ipaddress *ipv4_address;
@@ -141,15 +142,17 @@ struct network_context {
enum connman_ipconfig_method ipv6_method;
struct connman_ipaddress *ipv6_address;
char *ipv6_nameservers;
+
+ bool active;
+ bool valid_apn; /* APN is 'valid' if length > 0 */
};

struct modem_data {
char *path;

struct connman_device *device;
- struct connman_network *network;

- struct network_context *context;
+ GSList *context_list;

/* Modem Interface */
char *serial;
@@ -167,10 +170,6 @@ struct modem_data {
bool attached;
bool cm_powered;

- /* ConnectionContext Interface */
- bool active;
- bool valid_apn; /* APN is 'valid' if length > 0 */
-
/* SimManager Interface */
char *imsi;

@@ -219,6 +218,41 @@ static char *get_ident(const char *path)
return pos + 1;
}

+
+static struct network_context *get_context_with_path(GSList *context_list,
+ const gchar *path)
+{
+ GSList *list;
+
+ DBG("path %s", path);
+
+ for (list = context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (g_strcmp0(context->path, path) == 0)
+ return context;
+ }
+
+ return NULL;
+}
+
+static struct network_context *get_context_with_network(GSList *context_list,
+ const struct connman_network *network)
+{
+ GSList *list;
+
+ DBG("network %p", network);
+
+ for (list = context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (context->network == network)
+ return context;
+ }
+
+ return NULL;
+}
+
static struct network_context *network_context_alloc(const char *path)
{
struct network_context *context;
@@ -254,31 +288,38 @@ static void network_context_free(struct network_context *context)
free(context);
}

-static void set_connected(struct modem_data *modem)
+static void set_connected(struct modem_data *modem, int context_id)
{
struct connman_service *service;
bool setip = false;
enum connman_ipconfig_method method;
char *nameservers;
int index;
+ struct network_context *context = NULL;

DBG("%s", modem->path);

- index = modem->context->index;
+ context = g_slist_nth_data(modem->context_list, context_id);
+ if (!context) {
+ connman_error("Context does not exist !");
+ return;
+ }

- method = modem->context->ipv4_method;
- if (index < 0 || (!modem->context->ipv4_address &&
+ index = context->index;
+
+ method = context->ipv4_method;
+ if (index < 0 || (!context->ipv4_address &&
method == CONNMAN_IPCONFIG_METHOD_FIXED)) {
connman_error("Invalid index and/or address");
return;
}

- service = connman_service_lookup_from_network(modem->network);
+ service = connman_service_lookup_from_network(context->network);
if (!service)
return;

connman_service_create_ip4config(service, index);
- connman_network_set_ipv4_method(modem->network, method);
+ connman_network_set_ipv4_method(context->network, method);

if (method == CONNMAN_IPCONFIG_METHOD_FIXED ||
method == CONNMAN_IPCONFIG_METHOD_DHCP) {
@@ -286,69 +327,70 @@ static void set_connected(struct modem_data *modem)
}

if (method == CONNMAN_IPCONFIG_METHOD_FIXED) {
- connman_network_set_ipaddress(modem->network,
- modem->context->ipv4_address);
+ connman_network_set_ipaddress(context->network,
+ context->ipv4_address);
}

- method = modem->context->ipv6_method;
+ method = context->ipv6_method;
connman_service_create_ip6config(service, index);
- connman_network_set_ipv6_method(modem->network, method);
+ connman_network_set_ipv6_method(context->network, method);

if (method == CONNMAN_IPCONFIG_METHOD_AUTO) {
setip = true;
}

/* Set the nameservers */
- if (modem->context->ipv4_nameservers &&
- modem->context->ipv6_nameservers) {
+ if (context->ipv4_nameservers &&
+ context->ipv6_nameservers) {
nameservers = g_strdup_printf("%s %s",
- modem->context->ipv4_nameservers,
- modem->context->ipv6_nameservers);
- connman_network_set_nameservers(modem->network, nameservers);
+ context->ipv4_nameservers,
+ context->ipv6_nameservers);
+ connman_network_set_nameservers(context->network, nameservers);
g_free(nameservers);
- } else if (modem->context->ipv4_nameservers) {
- connman_network_set_nameservers(modem->network,
- modem->context->ipv4_nameservers);
- } else if (modem->context->ipv6_nameservers) {
- connman_network_set_nameservers(modem->network,
- modem->context->ipv6_nameservers);
+ } else if (context->ipv4_nameservers) {
+ connman_network_set_nameservers(context->network,
+ context->ipv4_nameservers);
+ } else if (context->ipv6_nameservers) {
+ connman_network_set_nameservers(context->network,
+ context->ipv6_nameservers);
}

if (setip) {
- connman_network_set_index(modem->network, index);
- connman_network_set_connected(modem->network, true);
+ connman_network_set_index(context->network, index);
+ connman_network_set_connected(context->network, true);
}
}

-static void set_disconnected(struct modem_data *modem)
+static void set_disconnected(struct network_context *context)
{
- DBG("%s", modem->path);
+ DBG("%s", context->path);

- if (modem->network)
- connman_network_set_connected(modem->network, false);
+ if (context->network)
+ connman_network_set_connected(context->network, false);

- if (modem->context) {
- g_free(modem->context->ipv4_nameservers);
- modem->context->ipv4_nameservers = NULL;
- if (modem->context->ipv4_method != CONNMAN_IPCONFIG_METHOD_OFF)
- modem->context->ipv4_method =
+ if (context) {
+ g_free(context->ipv4_nameservers);
+ context->ipv4_nameservers = NULL;
+ if (context->ipv4_method != CONNMAN_IPCONFIG_METHOD_OFF)
+ context->ipv4_method =
CONNMAN_IPCONFIG_METHOD_UNKNOWN;

- g_free(modem->context->ipv6_nameservers);
- modem->context->ipv6_nameservers = NULL;
- if (modem->context->ipv6_method != CONNMAN_IPCONFIG_METHOD_OFF)
- modem->context->ipv6_method =
+ g_free(context->ipv6_nameservers);
+ context->ipv6_nameservers = NULL;
+ if (context->ipv6_method != CONNMAN_IPCONFIG_METHOD_OFF)
+ context->ipv6_method =
CONNMAN_IPCONFIG_METHOD_UNKNOWN;
}
}

typedef void (*set_property_cb)(struct modem_data *data,
- bool success);
+ int context_id, bool success);
typedef void (*get_properties_cb)(struct modem_data *data,
DBusMessageIter *dict);

struct property_info {
struct modem_data *modem;
+ int context_id;
const char *path;
const char *interface;
const char *property;
@@ -381,14 +423,15 @@ static void set_property_reply(DBusPendingCall *call, void *user_data)
}

if (info->set_property_cb)
- (*info->set_property_cb)(info->modem, success);
+ (*info->set_property_cb)(info->modem, info->context_id,
+ success);

dbus_message_unref(reply);

dbus_pending_call_unref(call);
}

-static int set_property(struct modem_data *modem,
+static int set_property(struct modem_data *modem, int context_id,
const char *path, const char *interface,
const char *property, int type, void *value,
set_property_cb notify)
@@ -435,6 +478,7 @@ static int set_property(struct modem_data *modem,
}

info->modem = modem;
+ info->context_id = context_id;
info->path = path;
info->interface = interface;
info->property = property;
@@ -542,9 +586,12 @@ static int get_properties(const char *path, const char *interface,
}

static void context_set_active_reply(struct modem_data *modem,
- bool success)
+ int context_id, bool success)
{
- DBG("%s", modem->path);
+ struct network_context *context;
+
+ context = g_slist_nth_data(modem->context_list, context_id);
+ DBG("%s", context->path);

if (success) {
/*
@@ -561,7 +608,7 @@ static void context_set_active_reply(struct modem_data *modem,
* cycle the modem in such cases?
*/

- if (!modem->network) {
+ if (!context->network) {
/*
* In the case where we power down the device
* we don't wait for the reply, therefore the network
@@ -570,18 +617,21 @@ static void context_set_active_reply(struct modem_data *modem,
return;
}

- connman_network_set_error(modem->network,
+ connman_network_set_error(context->network,
CONNMAN_NETWORK_ERROR_ASSOCIATE_FAIL);
}

-static int context_set_active(struct modem_data *modem,
- dbus_bool_t active)
+static int context_set_active(struct modem_data *modem, int context_id,
+ dbus_bool_t active)
{
int err;
+ struct network_context *context;
+
+ context = g_slist_nth_data(modem->context_list, context_id);

DBG("%s active %d", modem->path, active);

- err = set_property(modem, modem->context->path,
+ err = set_property(modem, context_id, context->path,
OFONO_CONTEXT_INTERFACE,
"Active", DBUS_TYPE_BOOLEAN,
&active,
@@ -594,9 +644,12 @@ static int context_set_active(struct modem_data *modem,
}

static void cdma_cm_set_powered_reply(struct modem_data *modem,
- bool success)
+ int context_id, bool success)
{
- DBG("%s", modem->path);
+ struct network_context *context;
+
+ context = g_slist_nth_data(modem->context_list, context_id);
+ DBG("%s", context->path);

if (success) {
/*
@@ -613,7 +666,7 @@ static void cdma_cm_set_powered_reply(struct modem_data *modem,
* cycle the modem in such cases?
*/

- if (!modem->network) {
+ if (!context->network) {
/*
* In the case where we power down the device
* we don't wait for the reply, therefore the network
@@ -622,7 +675,7 @@ static void cdma_cm_set_powered_reply(struct modem_data *modem,
return;
}

- connman_network_set_error(modem->network,
+ connman_network_set_error(context->network,
CONNMAN_NETWORK_ERROR_ASSOCIATE_FAIL);
}

@@ -632,7 +685,7 @@ static int cdma_cm_set_powered(struct modem_data *modem, dbus_bool_t powered)

DBG("%s powered %d", modem->path, powered);

- err = set_property(modem, modem->path, OFONO_CDMA_CM_INTERFACE,
+ err = set_property(modem, 0, modem->path, OFONO_CDMA_CM_INTERFACE,
"Powered", DBUS_TYPE_BOOLEAN,
&powered,
cdma_cm_set_powered_reply);
@@ -647,7 +700,7 @@ static int modem_set_online(struct modem_data *modem, dbus_bool_t online)
{
DBG("%s online %d", modem->path, online);

- return set_property(modem, modem->path,
+ return set_property(modem, -1, modem->path,
OFONO_MODEM_INTERFACE,
"Online", DBUS_TYPE_BOOLEAN,
&online,
@@ -660,7 +713,7 @@ static int cm_set_powered(struct modem_data *modem, dbus_bool_t powered)

DBG("%s powered %d", modem->path, powered);

- err = set_property(modem, modem->path,
+ err = set_property(modem, -1, modem->path,
OFONO_CM_INTERFACE,
"Powered", DBUS_TYPE_BOOLEAN,
&powered,
@@ -680,7 +733,7 @@ static int modem_set_powered(struct modem_data *modem, dbus_bool_t powered)

modem->set_powered = powered;

- err = set_property(modem, modem->path,
+ err = set_property(modem, -1, modem->path,
OFONO_MODEM_INTERFACE,
"Powered", DBUS_TYPE_BOOLEAN,
&powered,
@@ -1017,71 +1070,72 @@ static void destroy_device(struct modem_data *modem)

connman_device_set_powered(modem->device, false);

- if (modem->network) {
- connman_device_remove_network(modem->device, modem->network);
- connman_network_unref(modem->network);
- modem->network = NULL;
- }
-
connman_device_unregister(modem->device);
connman_device_unref(modem->device);

modem->device = NULL;
}

-static void add_network(struct modem_data *modem)
+static void add_network(struct modem_data *modem, int context_id)
{
const char *group;
+ struct network_context *context;

DBG("%s", modem->path);

- if (modem->network)
+ if (!modem->context_list)
return;

- modem->network = connman_network_create(modem->context->path,
- CONNMAN_NETWORK_TYPE_CELLULAR);
- if (!modem->network)
+ context = g_slist_nth_data(modem->context_list, context_id);
+ context->network = connman_network_create(context->path,
+ CONNMAN_NETWORK_TYPE_CELLULAR);
+ if (!context->network)
return;

- DBG("network %p", modem->network);
+ DBG("network %p", context->network);

- connman_network_set_data(modem->network, modem);
+ connman_network_set_data(context->network, modem);

- connman_network_set_string(modem->network, "Path",
- modem->context->path);
+ connman_network_set_string(context->network, "Path",
+ context->path);

if (modem->name)
- connman_network_set_name(modem->network, modem->name);
+ connman_network_set_name(context->network, modem->name);
else
- connman_network_set_name(modem->network, "");
+ connman_network_set_name(context->network, "");

- connman_network_set_strength(modem->network, modem->strength);
+ connman_network_set_strength(context->network, modem->strength);

- group = get_ident(modem->context->path);
- connman_network_set_group(modem->network, group);
+ group = get_ident(context->path);
+ connman_network_set_group(context->network, group);

- connman_network_set_bool(modem->network, "Roaming",
- modem->roaming);
+ connman_network_set_bool(context->network, "Roaming",
+ modem->roaming);

- if (connman_device_add_network(modem->device, modem->network) < 0) {
- connman_network_unref(modem->network);
- modem->network = NULL;
+ if (connman_device_add_network(modem->device, context->network) < 0) {
+ connman_network_unref(context->network);
+ context->network = NULL;
return;
}
}

-static void remove_network(struct modem_data *modem)
+static void remove_network(struct modem_data *modem, int context_id)
{
- DBG("%s", modem->path);
+ struct network_context *context;

- if (!modem->network)
+ DBG("%s", modem->path);
+ if (!modem->context_list)
+ return;
+ context = g_slist_nth_data(modem->context_list, context_id);
+ if (!context)
return;

- DBG("network %p", modem->network);
+ DBG("network %p", context->network);

- connman_device_remove_network(modem->device, modem->network);
- connman_network_unref(modem->network);
- modem->network = NULL;
+ if (modem->device)
+ connman_device_remove_network(modem->device, context->network);
+ connman_network_unref(context->network);
+ context->network = NULL;
}

static int set_context_ipconfig(struct network_context *context,
@@ -1178,9 +1232,9 @@ static int add_cm_context(struct modem_data *modem, const char *context_path,

dbus_message_iter_get_basic(&value, &apn);
if (apn && strlen(apn) > 0)
- modem->valid_apn = true;
+ context->valid_apn = true;
else
- modem->valid_apn = false;
+ context->valid_apn = false;

DBG("%s AccessPointName '%s'", modem->path, apn);
} else if (g_str_equal(key, "Protocol") &&
@@ -1202,44 +1256,69 @@ static int add_cm_context(struct modem_data *modem, const char *context_path,
if (ip_protocol)
set_context_ipconfig(context, ip_protocol);

- modem->context = context;
- modem->active = active;
-
+ context->active = active;
+ modem->context_list = g_slist_append(modem->context_list, context);
g_hash_table_replace(context_hash, g_strdup(context_path), modem);

- if (modem->valid_apn && modem->attached &&
- has_interface(modem->interfaces,
- OFONO_API_NETREG)) {
- add_network(modem);
- }
-
+ if (context->valid_apn && modem->attached &&
+ has_interface(modem->interfaces, OFONO_API_NETREG))
+ add_network(modem, g_slist_index(modem->context_list, context));
return 0;
}

static void remove_cm_context(struct modem_data *modem,
- const char *context_path)
+ int context_id)
+{
+ struct network_context *context;
+
+ if (!modem->context_list)
+ return;
+ context = g_slist_nth_data(modem->context_list, context_id);
+ if (!context)
+ return;
+ g_hash_table_remove(context_hash, context->path);
+
+ if (context->network)
+ remove_network(modem, context_id);
+ modem->context_list = g_slist_remove(modem->context_list, context);
+}
+
+static void remove_all_contexts(struct modem_data *modem)
{
- if (!modem->context)
+ GSList *list = NULL;
+
+ DBG("");
+
+ if (modem->context_list == NULL)
return;

- if (modem->network)
- remove_network(modem);
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;

- g_hash_table_remove(context_hash, context_path);
+ remove_cm_context(modem, g_slist_index(modem->context_list,
+ context));
+ }
+ g_slist_free(modem->context_list);
+ modem->context_list = NULL;
+}

- network_context_free(modem->context);
- modem->context = NULL;
+static void remove_all_networks(struct modem_data *modem)
+{
+ GSList *list;

- modem->valid_apn = false;
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;

- if (modem->network)
- remove_network(modem);
+ remove_network(modem, g_slist_index(modem->context_list,
+ context));
+ }
}

static gboolean context_changed(DBusConnection *conn,
DBusMessage *message,
void *user_data)
{
+ struct network_context *context = NULL;
const char *context_path = dbus_message_get_path(message);
struct modem_data *modem = NULL;
DBusMessageIter iter, value;
@@ -1251,6 +1330,10 @@ static gboolean context_changed(DBusConnection *conn,
if (!modem)
return TRUE;

+ context = get_context_with_path(modem->context_list, context_path);
+ if (!context)
+ return TRUE;
+
if (!dbus_message_iter_init(message, &iter))
return TRUE;

@@ -1267,23 +1350,24 @@ static gboolean context_changed(DBusConnection *conn,
if (g_str_equal(key, "Settings")) {
DBG("%s Settings", modem->path);

- extract_ipv4_settings(&value, modem->context);
+ extract_ipv4_settings(&value, context);
} else if (g_str_equal(key, "IPv6.Settings")) {
DBG("%s IPv6.Settings", modem->path);

- extract_ipv6_settings(&value, modem->context);
+ extract_ipv6_settings(&value, context);
} else if (g_str_equal(key, "Active")) {
dbus_bool_t active;

dbus_message_iter_get_basic(&value, &active);
- modem->active = active;
+ context->active = active;

- DBG("%s Active %d", modem->path, modem->active);
+ DBG("%s Active %d", modem->path, context->active);

- if (modem->active)
- set_connected(modem);
+ if (context->active)
+ set_connected(modem, g_slist_index(modem->context_list,
+ context));
else
- set_disconnected(modem);
+ set_disconnected(context);
} else if (g_str_equal(key, "AccessPointName")) {
const char *apn;

@@ -1292,9 +1376,9 @@ static gboolean context_changed(DBusConnection *conn,
DBG("%s AccessPointName %s", modem->path, apn);

if (apn && strlen(apn) > 0) {
- modem->valid_apn = true;
+ context->valid_apn = true;

- if (modem->network)
+ if (context->network)
return TRUE;

if (!modem->attached)
@@ -1304,26 +1388,29 @@ static gboolean context_changed(DBusConnection *conn,
OFONO_API_NETREG))
return TRUE;

- add_network(modem);
+ add_network(modem, g_slist_index(modem->context_list,
+ context));

- if (modem->active)
- set_connected(modem);
+ if (context->active)
+ set_connected(modem, g_slist_index(modem->context_list,
+ context));
} else {
- modem->valid_apn = false;
+ context->valid_apn = false;

- if (!modem->network)
+ if (!context->network)
return TRUE;

- remove_network(modem);
+ remove_network(modem, g_slist_index(modem->context_list,
+ context));
}

} else if (g_str_equal(key, "Protocol") &&
- dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING ) {
+ dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
const char *ip_protocol;

dbus_message_iter_get_basic(&value, &ip_protocol);

- set_context_ipconfig(modem->context, ip_protocol);
+ set_context_ipconfig(context, ip_protocol);
}

return TRUE;
@@ -1451,6 +1538,7 @@ static gboolean cm_context_removed(DBusConnection *conn,
const char *path = dbus_message_get_path(message);
const char *context_path;
struct modem_data *modem;
+ struct network_context *context;
DBusMessageIter iter;

DBG("context path %s", path);
@@ -1464,15 +1552,17 @@ static gboolean cm_context_removed(DBusConnection *conn,
if (!modem)
return TRUE;

- remove_cm_context(modem, context_path);
+ context = get_context_with_path(modem->context_list, context_path);
+ remove_cm_context(modem, g_slist_index(modem->context_list, context));

return TRUE;
}

static void netreg_update_name(struct modem_data *modem,
- DBusMessageIter* value)
+ DBusMessageIter *value)
{
char *name;
+ GSList *list;

dbus_message_iter_get_basic(value, &name);

@@ -1481,21 +1571,30 @@ static void netreg_update_name(struct modem_data *modem,
g_free(modem->name);
modem->name = g_strdup(name);

- if (!modem->network)
+ if (!modem->context_list)
return;

- connman_network_set_name(modem->network, modem->name);
- connman_network_update(modem->network);
+ /* For all the context */
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (context->network) {
+ connman_network_set_name(context->network, modem->name);
+ connman_network_update(context->network);
+ }
+ }
}

static void netreg_update_strength(struct modem_data *modem,
DBusMessageIter *value)
{
+ GSList *list;
+
dbus_message_iter_get_basic(value, &modem->strength);

DBG("%s Strength %d", modem->path, modem->strength);

- if (!modem->network)
+ if (!modem->context_list)
return;

/*
@@ -1514,19 +1613,29 @@ static void netreg_update_strength(struct modem_data *modem,
if (modem->data_strength != 0)
return;

- connman_network_set_strength(modem->network, modem->strength);
- connman_network_update(modem->network);
+ /* For all the context */
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (context->network) {
+ connman_network_set_strength(context->network,
+ modem->strength);
+ connman_network_update(context->network);
+ }
+ }
}

/* Retrieve 1xEVDO Data Strength signal */
static void netreg_update_datastrength(struct modem_data *modem,
DBusMessageIter *value)
{
+ GSList *list;
+
dbus_message_iter_get_basic(value, &modem->data_strength);

DBG("%s Data Strength %d", modem->path, modem->data_strength);

- if (!modem->network)
+ if (!modem->context_list)
return;

/*
@@ -1537,8 +1646,16 @@ static void netreg_update_datastrength(struct modem_data *modem,
if (modem->data_strength == 0)
return;

- connman_network_set_strength(modem->network, modem->data_strength);
- connman_network_update(modem->network);
+ /* For all the context */
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (context->network) {
+ connman_network_set_strength(context->network,
+ modem->data_strength);
+ connman_network_update(context->network);
+ }
+ }
}

static void netreg_update_status(struct modem_data *modem,
@@ -1546,6 +1663,7 @@ static void netreg_update_status(struct modem_data *modem,
{
char *status;
bool roaming;
+ GSList *list;

dbus_message_iter_get_basic(value, &status);

@@ -1557,12 +1675,19 @@ static void netreg_update_status(struct modem_data *modem,

modem->roaming = roaming;

- if (!modem->network)
+ if (!modem->context_list)
return;

- connman_network_set_bool(modem->network,
+ /* For all the context */
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ if (context->network) {
+ connman_network_set_bool(context->network,
"Roaming", modem->roaming);
- connman_network_update(modem->network);
+ connman_network_update(context->network);
+ }
+ }
}

static void netreg_update_regdom(struct modem_data *modem,
@@ -1624,6 +1749,8 @@ static gboolean netreg_changed(DBusConnection *conn, DBusMessage *message,
static void netreg_properties_reply(struct modem_data *modem,
DBusMessageIter *dict)
{
+ GSList *list = NULL;
+
DBG("%s", modem->path);

while (dbus_message_iter_get_arg_type(dict) == DBUS_TYPE_DICT_ENTRY) {
@@ -1648,7 +1775,7 @@ static void netreg_properties_reply(struct modem_data *modem,
dbus_message_iter_next(dict);
}

- if (!modem->context) {
+ if (!modem->context_list) {
/*
* netgreg_get_properties() was issued after we got
* cm_get_contexts_reply() where we create the
@@ -1659,12 +1786,16 @@ static void netreg_properties_reply(struct modem_data *modem,
*/
return;
}
+ /* Check for all contexts if they are valids and/or actives */
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+ int context_id = g_slist_index(modem->context_list, context);

- if (modem->valid_apn)
- add_network(modem);
-
- if (modem->active)
- set_connected(modem);
+ if (context->valid_apn)
+ add_network(modem, context_id);
+ if (context->active)
+ set_connected(modem, context_id);
+ }
}

static int netreg_get_properties(struct modem_data *modem)
@@ -1675,6 +1806,7 @@ static int netreg_get_properties(struct modem_data *modem)

static void add_cdma_network(struct modem_data *modem)
{
+ struct network_context *context;
/* Be sure that device is created before adding CDMA network */
if (!modem->device)
return;
@@ -1683,16 +1815,19 @@ static void add_cdma_network(struct modem_data *modem)
* CDMA modems don't need contexts for data call, however the current
* add_network() logic needs one, so we create one to proceed.
*/
- if (!modem->context)
- modem->context = network_context_alloc(modem->path);
+ if (!modem->context_list) {
+ context = network_context_alloc(modem->path);
+ modem->context_list = g_slist_append(modem->context_list,
+ context);
+ }

if (!modem->name)
modem->name = g_strdup("CDMA Network");

- add_network(modem);
+ add_network(modem, 0);

if (modem->cdma_cm_powered)
- set_connected(modem);
+ set_connected(modem, 0);
}

static gboolean cdma_netreg_changed(DBusConnection *conn,
@@ -1733,7 +1868,7 @@ static gboolean cdma_netreg_changed(DBusConnection *conn,
if (modem->registered)
add_cdma_network(modem);
else
- remove_network(modem);
+ remove_network(modem, 0);

return TRUE;
}
@@ -1768,7 +1903,7 @@ static void cdma_netreg_properties_reply(struct modem_data *modem,
if (modem->registered)
add_cdma_network(modem);
else
- remove_network(modem);
+ remove_network(modem, 0);
}

static int cdma_netreg_get_properties(struct modem_data *modem)
@@ -1788,7 +1923,14 @@ static void cm_update_attached(struct modem_data *modem,
DBG("%s Attached %d", modem->path, modem->attached);

if (!modem->attached) {
- remove_network(modem);
+ GSList *list;
+
+ for (list = modem->context_list; list; list = list->next) {
+ struct network_context *context = list->data;
+
+ remove_network(modem, g_slist_index(modem->context_list,
+ context));
+ }
return;
}

@@ -1856,13 +1998,13 @@ static void cdma_cm_update_powered(struct modem_data *modem,

DBG("%s CDMA cm Powered %d", modem->path, modem->cdma_cm_powered);

- if (!modem->network)
+ if (!modem->context_list)
return;

if (modem->cdma_cm_powered)
- set_connected(modem);
+ set_connected(modem, 0);
else
- set_disconnected(modem);
+ set_disconnected(g_slist_nth_data(modem->context_list, 0));
}

static void cdma_cm_update_settings(struct modem_data *modem,
@@ -1870,7 +2012,7 @@ static void cdma_cm_update_settings(struct modem_data *modem,
{
DBG("%s Settings", modem->path);

- extract_ipv4_settings(value, modem->context);
+ extract_ipv4_settings(value, g_slist_nth_data(modem->context_list, 0));
}

static gboolean cdma_cm_changed(DBusConnection *conn,
@@ -1885,7 +2027,7 @@ static gboolean cdma_cm_changed(DBusConnection *conn,
if (!modem)
return TRUE;

- if (modem->online && !modem->network)
+ if (modem->online && !modem->context_list)
cdma_netreg_get_properties(modem);

if (!dbus_message_iter_init(message, &iter))
@@ -2142,27 +2284,17 @@ static void modem_update_interfaces(struct modem_data *modem,
if (api_added(old_ifaces, new_ifaces, OFONO_API_CDMA_NETREG))
cdma_netreg_get_properties(modem);

- if (api_removed(old_ifaces, new_ifaces, OFONO_API_CM)) {
- if (modem->call_get_contexts) {
- DBG("cancelling pending GetContexts call");
- dbus_pending_call_cancel(modem->call_get_contexts);
- dbus_pending_call_unref(modem->call_get_contexts);
- modem->call_get_contexts = NULL;
- }
- if (modem->context) {
- DBG("removing context %s", modem->context->path);
- remove_cm_context(modem, modem->context->path);
- }
- }
+ if (api_removed(old_ifaces, new_ifaces, OFONO_API_CM))
+ remove_all_contexts(modem);

if (api_removed(old_ifaces, new_ifaces, OFONO_API_CDMA_CM))
- remove_cm_context(modem, modem->context->path);
+ remove_all_contexts(modem);

if (api_removed(old_ifaces, new_ifaces, OFONO_API_NETREG))
- remove_network(modem);
+ remove_all_networks(modem);

if (api_removed(old_ifaces, new_ifaces, OFONO_API_CDMA_NETREG))
- remove_network(modem);
+ remove_all_networks(modem);
}

static gboolean modem_changed(DBusConnection *conn, DBusMessage *message,
@@ -2196,8 +2328,8 @@ static gboolean modem_changed(DBusConnection *conn, DBusMessage *message,

DBG("%s Powered %d", modem->path, modem->powered);

- if (!modem->powered)
- modem_set_powered(modem, TRUE);
+ /* Set the powered according to the value */
+ modem_set_powered(modem, powered);
} else if (g_str_equal(key, "Online")) {
dbus_bool_t online;

@@ -2365,12 +2497,13 @@ static void remove_modem(gpointer data)
dbus_pending_call_unref(modem->call_get_contexts);
}

+ /* Must remove the contexts before the device */
+ if (modem->context_list)
+ remove_all_contexts(modem);
+
if (modem->device)
destroy_device(modem);

- if (modem->context)
- remove_cm_context(modem, modem->context->path);
-
g_free(modem->serial);
g_free(modem->name);
g_free(modem->imsi);
@@ -2546,12 +2679,18 @@ static void network_remove(struct connman_network *network)

static int network_connect(struct connman_network *network)
{
+ struct network_context *context;
struct modem_data *modem = connman_network_get_data(network);

DBG("%s network %p", modem->path, network);

+ context = get_context_with_network(modem->context_list, network);
+ if (!context)
+ return -ENODEV;
+
if (has_interface(modem->interfaces, OFONO_API_CM))
- return context_set_active(modem, TRUE);
+ return context_set_active(modem, g_slist_index(modem->context_list,
+ context), TRUE);
else if (has_interface(modem->interfaces, OFONO_API_CDMA_CM))
return cdma_cm_set_powered(modem, TRUE);

@@ -2562,12 +2701,18 @@ static int network_connect(struct connman_network *network)

static int network_disconnect(struct connman_network *network)
{
+ struct network_context *context;
struct modem_data *modem = connman_network_get_data(network);

DBG("%s network %p", modem->path, network);

+ context = get_context_with_network(modem->context_list, network);
+ if (!context)
+ return -ENODEV;
+
if (has_interface(modem->interfaces, OFONO_API_CM))
- return context_set_active(modem, FALSE);
+ return context_set_active(modem, g_slist_index(modem->context_list,
+ context), FALSE);
else if (has_interface(modem->interfaces, OFONO_API_CDMA_CM))
return cdma_cm_set_powered(modem, FALSE);
--
2.1.4
Patrik Flykt
2015-10-19 12:16:58 UTC
Permalink
Hi,
Post by Mylene JOSSERAND
This is a first patch of a serie to implement a simultaneous APNs context activation.
The current commit removes the previous implementation of one context per modem and
adds a list of contexts instead.
As this patch is really quite long, it looks it would be possible to add
GSList *context_list without removing struct network_context *context
immediately. So the code would function as before, but the contexts are
added/removed to/from the list. The next patch could then remove struct
network_context *context and change the functionality to always use the
list.

Instead of looking up the context with g_slist_nth_data and a
context_id, can't the code simply pass around a struct network_context *
all the time? For example in network_connect() the corresponding call
would then look like 'context_set_active(modem, context, TRUE)'. It's
much simpler that way and avoids iterating over the list twice. Right
now it is not immediately clear what a context_id with values 0 or -1
means.

Also, if you have a singly-linked list, please always prepend items as
the appending function has to always reach the end of the list before an
item is added. Since the order makes no difference here, prepeding is a
much more elegant (and faster) solution.
Post by Mylene JOSSERAND
Some modifications are necessary : some properties in the modem structure are moved
to the context structure (such as connman_network, valid_apn, etc) as they are properties
specifics to context.
Ok.
Post by Mylene JOSSERAND
Some functions are implemented to search for a specific context in the list by his path
or by his network. A function to remove all the context of one modem
is also implemented.
Yes, those need to be implemented. The other two patches are really nice
and small.

Cheers,

Patrik
Mylène JOSSERAND
2015-10-19 19:31:47 UTC
Permalink
Hi Patrik,


Thank you for the time you spent on my patches.
Post by Mylene JOSSERAND
Hi,
Post by Mylene JOSSERAND
This is a first patch of a serie to implement a simultaneous APNs
context activation.
Post by Mylene JOSSERAND
The current commit removes the previous implementation of one context
per modem and
Post by Mylene JOSSERAND
adds a list of contexts instead.
As this patch is really quite long, it looks it would be possible to add
GSList *context_list without removing struct network_context *context
immediately. So the code would function as before, but the contexts are
added/removed to/from the list. The next patch could then remove struct
network_context *context and change the functionality to always use the
list.
okay, I understand, I did not see this "kind" of patch splitting. I will do
so.
Post by Mylene JOSSERAND
Instead of looking up the context with g_slist_nth_data and a
context_id, can't the code simply pass around a struct network_context *
all the time? For example in network_connect() the corresponding call
would then look like 'context_set_active(modem, context, TRUE)'. It's
much simpler that way and avoids iterating over the list twice. Right
now it is not immediately clear what a context_id with values 0 or -1
means.
You are right, thank you for pointing it.
Post by Mylene JOSSERAND
Also, if you have a singly-linked list, please always prepend items as
the appending function has to always reach the end of the list before an
item is added. Since the order makes no difference here, prepeding is a
much more elegant (and faster) solution.
Indeed, I will use the prepending function, thank you for the tips.
Post by Mylene JOSSERAND
Post by Mylene JOSSERAND
Some modifications are necessary : some properties in the modem
structure are moved
Post by Mylene JOSSERAND
to the context structure (such as connman_network, valid_apn, etc) as
they are properties
Post by Mylene JOSSERAND
specifics to context.
Ok.
Post by Mylene JOSSERAND
Some functions are implemented to search for a specific context in the
list by his path
Post by Mylene JOSSERAND
or by his network. A function to remove all the context of one modem
is also implemented.
Yes, those need to be implemented. The other two patches are really nice
and small.
Cool !

I will send a second version (maybe, directly with PATCH flag and not RFC
?) with these corrections.


Best regards,

Mylene JOSSERAND

Mylene JOSSERAND
2015-10-09 06:37:51 UTC
Permalink
This second patch of this serie implements the use of simultaneous APNS
(and so internet contexts).
It removes the restriction about one Internet context per modem and
inverts the logic of the GetContexts function reply (which exited on first context).
---
plugins/ofono.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 485608a..8f0f146 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -1188,14 +1188,6 @@ static int add_cm_context(struct modem_data *modem, const char *context_path,

DBG("%s context path %s", modem->path, context_path);

- if (modem->context) {
- /*
- * We have already assigned a context to this modem
- * and we do only support one Internet context.
- */
- return -EALREADY;
- }
-
context = network_context_alloc(context_path);
if (!context)
return -ENOMEM;
@@ -1454,7 +1446,7 @@ static void cm_get_contexts_reply(DBusPendingCall *call, void *user_data)
dbus_message_iter_next(&entry);
dbus_message_iter_recurse(&entry, &value);

- if (add_cm_context(modem, context_path, &value) == 0)
+ if (add_cm_context(modem, context_path, &value) != 0)
break;

dbus_message_iter_next(&dict);
--
2.1.4
Mylene JOSSERAND
2015-10-09 06:37:52 UTC
Permalink
In the case there are more than one context saved by ofono, the signal
sent is an array and not a dictionary.
This commit implements this case by recursing it of one step.
---
plugins/ofono.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 8f0f146..9975f64 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -1501,7 +1501,7 @@ static gboolean cm_context_added(DBusConnection *conn,
const char *path = dbus_message_get_path(message);
char *context_path;
struct modem_data *modem;
- DBusMessageIter iter, properties;
+ DBusMessageIter iter, properties, dict;

DBG("%s", path);

@@ -1517,6 +1517,13 @@ static gboolean cm_context_added(DBusConnection *conn,
dbus_message_iter_next(&iter);
dbus_message_iter_recurse(&iter, &properties);

+ /* Sometimes, we get an array instead of dict */
+ if (dbus_message_iter_get_arg_type(&properties) == DBUS_TYPE_ARRAY) {
+ /* Must recurse again */
+ dbus_message_iter_recurse(&properties, &dict);
+ if (add_cm_context(modem, context_path, &dict) != 0)
+ return TRUE;
+ }
if (add_cm_context(modem, context_path, &properties) != 0)
return TRUE;
--
2.1.4
Loading...