Re: [RFC 1/3] ofono: create a list of contexts per modem
Hi Patrik, Thank you for the time you spent on my patches. 2015-10-19 14:16 GMT+02:00 Patrik Flykt : > > Hi, > > On Fri, 2015-10-09 at 08:37 +0200, Mylene JOSSERAND wrote: > > 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. > okay, I understand, I did not see this "kind" of patch splitting. I will do so. > 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. > > 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. > > > 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. > > > 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. > > Cool ! I will send a second version (maybe, directly with PATCH flag and not RFC ?) with these corrections. Best regards, Mylene JOSSERAND ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [RFC 1/3] ofono: create a list of contexts per modem
Hi, On Fri, 2015-10-09 at 08:37 +0200, Mylene JOSSERAND wrote: > 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. > 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. > 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 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[RFC 1/3] ofono: create a list of contexts per modem
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
