Re: [PATCH] service: Do not reply via D-Bus when clearing service error
On Thu, 2014-09-25 at 15:20 +0300, Patrik Flykt wrote: When clearing a service in error state, set both ipconfigs to idle causing the service states in both ipconfig and service to be reset. Save the D-Bus id before setting the service to idle and restore it afterwards so that a possible D-Bus client won't be informed of a connection error which actually never happened. Clean up unnecessary checks at the same time. Applied. Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] vpn: Indicating error no longer needs state to be set to failure
On Fri, 2014-09-26 at 12:10 +0300, Patrik Flykt wrote: Indicating an error will set the states correctly and nowadays properly decides what the state will be. Therefore the extra call setting state to failed is removed. Applied. Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] vpn-provider: Properly set VPN connection state on failure
On Fri, 2014-09-26 at 12:09 +0300, Patrik Flykt wrote: Since the net.connman.vpn.Agent RequestInput D-Bus method call is used to fetch fresh credentials from the user, set the VPN connection state to idle after VPN authetication or login error. Other VPN errors will keep the VPN connection in failure state. Fix the code to allow the state to be set to failure if needed instead of automatically transitioning to idle after failure. Applied. Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 0/3] Some memory issue fixes
On Mon, 2014-09-29 at 09:19 +0300, Hannu Mallat wrote: Some fixes to memory issues I've encountered in testing, please review. These have all come up when testing connecting to and disconnecting from different networks while restarting wpa_supplicant and/or reloading wlan kernel module in the background, so that things can fail during pending operations for various reasons. Patches 1/3 and 2/3 applied, thanks! The fix I made to wifi_tethering_info pointer to freed memory feels a bit clumsy to me, so feedback is appreciated. This is the quickes fix for the issue. All other options require more code shuffling and writing. It's a bit difficult to see immediately what the best solution is, so I'll sit on patch 3/3 for a while and see if something comes to mind. Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] ofono: Bring device attached to modem down and up if IMSI has really changed
Hi, On Mon, 2014-09-29 at 11:20 +0300, pasi.sjoh...@jolla.com wrote: From: Pasi Sjöholm pasi.sjoh...@jollamobile.com Bringing the device down when the IMSI is really changed will force the service have correct identifier and not to use the old one (eg. when SIM- card is changed). --- plugins/ofono.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/ofono.c b/plugins/ofono.c index 7af551b..0d296d7 100644 --- a/plugins/ofono.c +++ b/plugins/ofono.c @@ -1908,6 +1908,7 @@ static gboolean sim_changed(DBusConnection *conn, DBusMessage *message, struct modem_data *modem; DBusMessageIter iter, value; const char *key; + char *new_imsi; modem = g_hash_table_lookup(modem_hash, path); if (!modem) @@ -1925,7 +1926,14 @@ static gboolean sim_changed(DBusConnection *conn, DBusMessage *message, dbus_message_iter_recurse(iter, value); if (g_str_equal(key, SubscriberIdentity)) { - sim_update_imsi(modem, value); + dbus_message_iter_get_basic(value, new_imsi); + + if (g_strcmp0(modem-imsi,new_imsi) != 0) { What is the benefit of checking the existing imsi against the new one? Isn't it known already that the sim is tied to this particular modem, as the modem is looked using the sending D-Bus path? + sim_update_imsi(modem, value); + + if (modem-device) + destroy_device(modem); + } Shouldn't this then be run unconditionally? if (!ready_to_create_device(modem)) return TRUE; BTW, the sim_* functions are in need of better checking that the messages contain the desired properties. Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 04/14] client: Support basic WiFi Display IEs (un)registration
Hi, On Mon, 2014-09-29 at 15:19 +0300, Tomasz Bursztyka wrote: @@ -2280,13 +2304,27 @@ static int cmd_peer_service(char *args[], int num, { unsigned char bjr_query[1024] = {}; unsigned char bjr_response[1024] = {}; + unsigned char wfd_ies[1024] = {}; char *upnp_service = NULL; int bjr_query_len = 0, bjr_response_len = 0; - int version = 0, master = 0; + int version = 0, master = 0, wfd_ies_len = 0;; + int limit; Two ;; on the preceeding line gives client/commands.c: In function ‘cmd_peer_service’: client/commands.c:2311:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] int limit; Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 04/14] client: Support basic WiFi Display IEs (un)registration
Hi Patrik, int bjr_query_len = 0, bjr_response_len = 0; - int version = 0, master = 0; + int version = 0, master = 0, wfd_ies_len = 0;; + int limit; Two ;; on the preceeding line gives client/commands.c: In function ‘cmd_peer_service’: client/commands.c:2311:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] int limit; And of course my compilers (4.7 and 4.8) did not spot that. Tomasz ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH v2] client: Support basic WiFi Display IEs (un)registration
This is a useful addition only for testing purposes as connmanctl will never serve any WiFi Display service. The wfd_ies is the tlv formated array of WiFi Display IE expressed in hexadecimal. --- client/commands.c | 60 +-- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/client/commands.c b/client/commands.c index d7b76a6..9208016 100644 --- a/client/commands.c +++ b/client/commands.c @@ -2117,6 +2117,8 @@ struct _peer_service { int bjr_query_len; unsigned char *bjr_response; int bjr_response_len; + unsigned char *wfd_ies; + int wfd_ies_len; char *upnp_service; int version; int master; @@ -2157,6 +2159,9 @@ static void append_peer_service_dict(DBusMessageIter *iter, void *user_data) DBUS_TYPE_INT32, service-version); __connmanctl_dbus_append_dict_entry(iter, UpnpService, DBUS_TYPE_STRING, service-upnp_service); + } else if (service-wfd_ies) { + append_dict_entry_fixed_array(iter, WiFiDisplayIEs, + service-wfd_ies, service-wfd_ies_len); } } @@ -2177,7 +2182,8 @@ static void peer_service_append(DBusMessageIter *iter, void *user_data) static struct _peer_service *fill_in_peer_service(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, char *upnp_service, - int version) + int version, unsigned char *wfd_ies, + int wfd_ies_len) { struct _peer_service *service; @@ -2194,6 +2200,13 @@ static struct _peer_service *fill_in_peer_service(unsigned char *bjr_query, } else if (upnp_service version) { service-upnp_service = strdup(upnp_service); service-version = version; + } else if (wfd_ies wfd_ies_len) { + service-wfd_ies = dbus_malloc0(wfd_ies_len); + memcpy(service-wfd_ies, wfd_ies, wfd_ies_len); + service-wfd_ies_len = wfd_ies_len; + } else { + dbus_free(service); + service = NULL; } return service; @@ -2203,20 +2216,26 @@ static void free_peer_service(struct _peer_service *service) { dbus_free(service-bjr_query); dbus_free(service-bjr_response); + dbus_free(service-wfd_ies); free(service-upnp_service); dbus_free(service); } static int peer_service_register(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, - char *upnp_service, int version, int master) + char *upnp_service, int version, + unsigned char *wfd_ies, int wfd_ies_len, int master) { struct _peer_service *service; bool registration = true; int ret; service = fill_in_peer_service(bjr_query, bjr_query_len, bjr_response, - bjr_response_len, upnp_service, version); + bjr_response_len, upnp_service, version, + wfd_ies, wfd_ies_len); + if (!service) + return -EINVAL; + service-master = master; ret = __connmanctl_dbus_method_call(connection, net.connman, /, @@ -2231,14 +2250,19 @@ static int peer_service_register(unsigned char *bjr_query, int bjr_query_len, static int peer_service_unregister(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, - char *upnp_service, int version) + char *upnp_service, int version, + unsigned char *wfd_ies, int wfd_ies_len) { struct _peer_service *service; bool registration = false; int ret; service = fill_in_peer_service(bjr_query, bjr_query_len, bjr_response, - bjr_response_len, upnp_service, version); + bjr_response_len, upnp_service, version, + wfd_ies, wfd_ies_len); + if (!service) + return -EINVAL; + service-master = -1; ret = __connmanctl_dbus_method_call(connection, net.connman, /, @@ -2280,13 +2304,27 @@ static int cmd_peer_service(char *args[], int num, { unsigned char bjr_query[1024] = {}; unsigned char bjr_response[1024] = {}; + unsigned char wfd_ies[1024] = {}; char *upnp_service = NULL; int bjr_query_len = 0, bjr_response_len = 0; - int version = 0, master = 0; + int version = 0, master = 0, wfd_ies_len = 0; + int limit; + + if (num 4) + return -EINVAL; + + if (!strcmp(args[2],
[PATCH v2] client: Support basic WiFi Display IEs (un)registration
This is a useful addition only for testing purposes as connmanctl will never serve any WiFi Display service. The wfd_ies is the tlv formated array of WiFi Display IE expressed in hexadecimal. --- client/commands.c | 60 +-- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/client/commands.c b/client/commands.c index d7b76a6..9208016 100644 --- a/client/commands.c +++ b/client/commands.c @@ -2117,6 +2117,8 @@ struct _peer_service { int bjr_query_len; unsigned char *bjr_response; int bjr_response_len; + unsigned char *wfd_ies; + int wfd_ies_len; char *upnp_service; int version; int master; @@ -2157,6 +2159,9 @@ static void append_peer_service_dict(DBusMessageIter *iter, void *user_data) DBUS_TYPE_INT32, service-version); __connmanctl_dbus_append_dict_entry(iter, UpnpService, DBUS_TYPE_STRING, service-upnp_service); + } else if (service-wfd_ies) { + append_dict_entry_fixed_array(iter, WiFiDisplayIEs, + service-wfd_ies, service-wfd_ies_len); } } @@ -2177,7 +2182,8 @@ static void peer_service_append(DBusMessageIter *iter, void *user_data) static struct _peer_service *fill_in_peer_service(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, char *upnp_service, - int version) + int version, unsigned char *wfd_ies, + int wfd_ies_len) { struct _peer_service *service; @@ -2194,6 +2200,13 @@ static struct _peer_service *fill_in_peer_service(unsigned char *bjr_query, } else if (upnp_service version) { service-upnp_service = strdup(upnp_service); service-version = version; + } else if (wfd_ies wfd_ies_len) { + service-wfd_ies = dbus_malloc0(wfd_ies_len); + memcpy(service-wfd_ies, wfd_ies, wfd_ies_len); + service-wfd_ies_len = wfd_ies_len; + } else { + dbus_free(service); + service = NULL; } return service; @@ -2203,20 +2216,26 @@ static void free_peer_service(struct _peer_service *service) { dbus_free(service-bjr_query); dbus_free(service-bjr_response); + dbus_free(service-wfd_ies); free(service-upnp_service); dbus_free(service); } static int peer_service_register(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, - char *upnp_service, int version, int master) + char *upnp_service, int version, + unsigned char *wfd_ies, int wfd_ies_len, int master) { struct _peer_service *service; bool registration = true; int ret; service = fill_in_peer_service(bjr_query, bjr_query_len, bjr_response, - bjr_response_len, upnp_service, version); + bjr_response_len, upnp_service, version, + wfd_ies, wfd_ies_len); + if (!service) + return -EINVAL; + service-master = master; ret = __connmanctl_dbus_method_call(connection, net.connman, /, @@ -2231,14 +2250,19 @@ static int peer_service_register(unsigned char *bjr_query, int bjr_query_len, static int peer_service_unregister(unsigned char *bjr_query, int bjr_query_len, unsigned char *bjr_response, int bjr_response_len, - char *upnp_service, int version) + char *upnp_service, int version, + unsigned char *wfd_ies, int wfd_ies_len) { struct _peer_service *service; bool registration = false; int ret; service = fill_in_peer_service(bjr_query, bjr_query_len, bjr_response, - bjr_response_len, upnp_service, version); + bjr_response_len, upnp_service, version, + wfd_ies, wfd_ies_len); + if (!service) + return -EINVAL; + service-master = -1; ret = __connmanctl_dbus_method_call(connection, net.connman, /, @@ -2280,13 +2304,27 @@ static int cmd_peer_service(char *args[], int num, { unsigned char bjr_query[1024] = {}; unsigned char bjr_response[1024] = {}; + unsigned char wfd_ies[1024] = {}; char *upnp_service = NULL; int bjr_query_len = 0, bjr_response_len = 0; - int version = 0, master = 0; + int version = 0, master = 0, wfd_ies_len = 0; + int limit; + + if (num 4) + return -EINVAL; + + if (!strcmp(args[2],
RE: [PATCH] ofono: Bring device attached to modem down and up if IMSI has really changed
if (g_str_equal(key, SubscriberIdentity)) { - sim_update_imsi(modem, value); + dbus_message_iter_get_basic(value, new_imsi); + + if (g_strcmp0(modem-imsi,new_imsi) != 0) { What is the benefit of checking the existing imsi against the new one? Isn't it known already that the sim is tied to this particular modem, as the modem is looked using the sending D-Bus path? + sim_update_imsi(modem, value); + + if (modem-device) + destroy_device(modem); + } Shouldn't this then be run unconditionally? For some reason (didn't dig it further) the sim_changed is called multiple times with the SubscriberIdentity as key when hot-swapping the sim card and the connman already has the new IMSI so we should not destroy the device in that case to get service identier corrected as it is already correct. It can lead to connman missing the cellular context if it's done (yes, I tried.. and it seems to occur when the sim has a pin set but not when it does not). BTW, the sim_* functions are in need of better checking that the messages contain the desired properties. Noticed that also when discussing with Hannu Mallat. :) ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 0/4] Handle PeerServiceRegister master parameter
When set, the go intent will be set to a high value ensuring that it advertizes it's will to be the GO when negocating about it. Tomasz Bursztyka (4): gsupplicant: Set the go intent to maximum when relevant peer_service: Handle registrations which specify to be the master peer_service: Add a function to tell if local peer is master or not wifi: Add master parameter when connecting to a peer gsupplicant/gsupplicant.h | 1 + gsupplicant/supplicant.c | 3 +++ include/peer.h| 2 ++ plugins/wifi.c| 2 ++ src/connman.h | 3 ++- src/manager.c | 2 +- src/peer_service.c| 28 ++-- 7 files changed, 37 insertions(+), 4 deletions(-) -- 1.8.5.5 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/4] gsupplicant: Set the go intent to maximum when relevant
If being the master is requested, it will set the go intent to maximum value. --- gsupplicant/gsupplicant.h | 1 + gsupplicant/supplicant.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/gsupplicant/gsupplicant.h b/gsupplicant/gsupplicant.h index 29e54b6..7ca30ce 100644 --- a/gsupplicant/gsupplicant.h +++ b/gsupplicant/gsupplicant.h @@ -171,6 +171,7 @@ struct _GSupplicantScanParams { typedef struct _GSupplicantScanParams GSupplicantScanParams; struct _GSupplicantPeerParams { + bool master; char *wps_pin; char *path; }; diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 82d4d63..3e87612 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -4831,6 +4831,9 @@ static void interface_p2p_connect_params(DBusMessageIter *iter, void *user_data) supplicant_dbus_dict_open(iter, dict); + if (data-peer-master) + go_intent = 15; + if (data-peer-wps_pin) wps = pin; -- 1.8.5.5 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/4] peer_service: Handle registrations which specify to be the master
We internally keep a count of registered services which are set as master. This will be then useful afterwards to set the GO intent from the wifi plugin and gsupplicant, transparently. --- src/connman.h | 3 ++- src/manager.c | 2 +- src/peer_service.c | 20 ++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/connman.h b/src/connman.h index a871e82..daad623 100644 --- a/src/connman.h +++ b/src/connman.h @@ -806,7 +806,8 @@ int __connman_peer_service_register(const char *owner, DBusMessage *msg, const unsigned char *specification, int specification_length, const unsigned char *query, - int query_length, int version); + int query_length, int version, + bool master); int __connman_peer_service_unregister(const char *owner, const unsigned char *specification, int specification_length, diff --git a/src/manager.c b/src/manager.c index 3c5636c..03ea523 100644 --- a/src/manager.c +++ b/src/manager.c @@ -460,7 +460,7 @@ static DBusMessage *register_peer_service(DBusConnection *conn, dbus_message_iter_get_basic(iter, master); ret = __connman_peer_service_register(owner, msg, spec, spec_len, - query, query_len, version); + query, query_len, version,master); if (!ret) return g_dbus_create_reply(msg, DBUS_TYPE_INVALID); if (ret == -EINPROGRESS) diff --git a/src/peer_service.c b/src/peer_service.c index f1985fd..8fc0247 100644 --- a/src/peer_service.c +++ b/src/peer_service.c @@ -39,6 +39,8 @@ struct _peer_service { GBytes *specification; GBytes *query; int version; + + bool master; }; struct _peer_service_owner { @@ -51,6 +53,7 @@ static struct connman_peer_driver *peer_driver; static GHashTable *owners_map; static GHashTable *services_map; +static int peer_master; static void reply_pending(struct _peer_service *service, int error) { @@ -149,6 +152,9 @@ static void remove_peer_service(gpointer user_data) if (service-query) g_bytes_unref(service-query); + if (service-master) + peer_master--; + g_free(service); } @@ -197,6 +203,8 @@ static void service_registration_result(int result, void *user_data) if (result == 0) { service-registered = true; + if (service-master) + peer_master++; return; } @@ -247,7 +255,8 @@ int __connman_peer_service_register(const char *owner, DBusMessage *msg, const unsigned char *specification, int specification_length, const unsigned char *query, - int query_length, int version) + int query_length, int version, + bool master) { struct _peer_service_owner *ps_owner; GBytes *spec, *query_spec = NULL; @@ -306,6 +315,7 @@ int __connman_peer_service_register(const char *owner, DBusMessage *msg, service-specification = spec; service-query = query_spec; service-version = version; + service-master = master; g_hash_table_insert(services_map, spec, ps_owner); spec = query_spec = NULL; @@ -315,8 +325,11 @@ int __connman_peer_service_register(const char *owner, DBusMessage *msg, goto error; else if (ret == -EINPROGRESS) service-pending = dbus_message_ref(msg); - else + else { service-registered = true; + if (master) + peer_master++; + } ps_owner-services = g_list_prepend(ps_owner-services, service); @@ -387,6 +400,8 @@ int __connman_peer_service_init(void) remove_peer_service_owner); services_map = g_hash_table_new_full(g_bytes_hash, g_bytes_equal, NULL, NULL); + peer_master = 0; + return 0; } @@ -399,6 +414,7 @@ void __connman_peer_service_cleanup(void) g_hash_table_destroy(owners_map); g_hash_table_destroy(services_map); + peer_master = 0; dbus_connection_unref(connection); connection = NULL; -- 1.8.5.5 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 3/4] peer_service: Add a function to tell if local peer is master or not
This will be used in the wifi plugin, and then gsupplicant, to set the go intent relevantly. The function is declared in peer.h but the actual implementation is in peer_service.h, I thought there were no need to create yet another header file moreover there will not be much from peer_service.c available for plugins. This function might be the only one. --- include/peer.h | 2 ++ src/peer_service.c | 8 2 files changed, 10 insertions(+) diff --git a/include/peer.h b/include/peer.h index e98e442..8a690f5 100644 --- a/include/peer.h +++ b/include/peer.h @@ -107,6 +107,8 @@ struct connman_peer_driver { int connman_peer_driver_register(struct connman_peer_driver *driver); void connman_peer_driver_unregister(struct connman_peer_driver *driver); +bool connman_peer_service_is_master(void); + #ifdef __cplusplus } #endif diff --git a/src/peer_service.c b/src/peer_service.c index 8fc0247..053672a 100644 --- a/src/peer_service.c +++ b/src/peer_service.c @@ -387,6 +387,14 @@ int __connman_peer_service_unregister(const char *owner, return 0; } +bool connman_peer_service_is_master(void) +{ + if (!peer_master || !peer_driver) + return false; + + return true; +} + int __connman_peer_service_init(void) { DBG(); -- 1.8.5.5 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH v2 00/14] Support for WiFi Display as a Peer Service
On Mon, 2014-09-29 at 15:19 +0300, Tomasz Bursztyka wrote: Now that wpa_supplicant supports setting the WFD through DBus API, I am resending this patch-set, rebased against latest ConnMan's master branch. Tomasz Bursztyka (14): gsupplicant: Add a callback enabled setter for the WFD IEs manager: Support WiFiDisplayIEs as a new Peer Service to handle wifi: Support registering WiFi Display IEs when relevant client: Support basic WiFi Display IEs (un)registration gsupplicant: Refactor how Peer's groups is detected to have changed gsupplicant: Get Peer's WiFi Display information elements if present gsupplicant: Add a getter for the WiFi Display information elements gsupplicant: Rename the peer state enum for future changes gsupplicant: Add a Peer service changed dedicated state gsupplicant: Notify about peer's services on WiFi Display IEs change peer: Add the core logic to handle a peer service list peer: Implement peer's Services DBus property wifi: Expose WiFi Display IEs as a peer service doc: Update peer Services property about WiFi Display IEs All patches applied, v2 for 4/14 applied last. Thanks! Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 0/4] Handle PeerServiceRegister master parameter
On Tue, 2014-09-30 at 13:41 +0300, Tomasz Bursztyka wrote: When set, the go intent will be set to a high value ensuring that it advertizes it's will to be the GO when negocating about it. All patches applied, thanks! Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH v2] gdhcp: Workaround for buggy AP that handle broadcast flag incorrectly
On Wed, 2014-09-24 at 11:18 +0300, Jukka Rissanen wrote: Some routers/AP handle the DHCP broadcast flag incorrectly. This means that some AP discard the DHCP packet if broadcast flag is present and some discard it if broadcast flag is missing. The workaround is to first send DISCOVER packet in INIT state without broadcast flag. If there is a timeout we send the second packet with broadcast flag set. In a case of second timeout the next DISCOVER will not set broadcast flag etc. In REBOOTING state the REQUEST packet will not set the broadcast flag. If we do not get a reply, we switch to INIT state anyway which will set the broadcast flag. --- Hi, this is v2 of the DHCP broadcast flag handling. This version will by default start with unicast flag and only if there is no reply we try with broadcast flag. Let's try with this one. Applied, thanks! Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman