Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. R: R: [RFC] Storage based service retrieval + removal
(MANIEZZO Marco (MM))
2. Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect (Patrik Flykt)
3. Re: [PATCH] Supporting wifi load balancing and band steering
(Patrik Flykt)
4. Re: [PATCH] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call. (Patrik Flykt)
----------------------------------------------------------------------
Message: 1
Date: Mon, 14 Mar 2016 07:28:16 +0000
From: "MANIEZZO Marco (MM)" <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: R: R: [RFC] Storage based service retrieval + removal
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
Hello,
Is there any further feedback on this subject?
Thanks,
Marco
-----Messaggio originale-----
Da: MANIEZZO Marco (MM)
Inviato: luned? 22 febbraio 2016 14:44
A: 'Patrik Flykt'
Cc: Marcel Holtmann; [email protected]
Oggetto: R: R: [RFC] Storage based service retrieval + removal
Hello Patrick,
Sorry I was not clear, I'll try to explain better with the help of connmanctl:
Suppose to have connected the ethernet and mutable service WiFi AP named
Digicom; current information provided by "services" is like:
connmanctl> services
*AR Wired ethernet_c8cbb8564ae1_cable
*AR Digicom wifi_6067208eac78_44696769636f6d_managed_psk
FG-WiFi wifi_6067208eac78_46472d57694669_managed_ieee8021x
This corresponds to the following data in /var/lib/connman:
drwx------ 2 root root 4096 Feb 22 13:53 ethernet_c8cbb8564ae1_cable/
-rw------- 1 root root 142 Feb 22 13:53 settings
drwx------ 2 root root 4096 Feb 22 13:55
wifi_6067208eac78_44696769636f6d_managed_psk/
the folder wifi_6067208eac78_44696769636f6d_managed_psk is created for Digicom
AP and contains files settings and data. The "Remove" method of Service is
currently deleting some data (passphrase included) in ram and setting file, but
it is not deleting folder and files.
With the proposed RFC using know-services in connmanctl that calls
Manager/GetKnownServices method will give:
connmanctl> known-services
*AR Digicom wifi_6067208eac78_44696769636f6d_managed_psk_stored
*AR Wired ethernet_c8cbb8564ae1_cable_stored
At this point on the d-bus the following are available:
1. wifi_6067208eac78_44696769636f6d_managed_psk_stored
2. ethernet_c8cbb8564ae1_cable_stored
3. ethernet_c8cbb8564ae1_cable
4. wifi_6067208eac78_44696769636f6d_managed_psk
5. wifi_6067208eac78_46472d57694669_managed_ieee8021x
(1) and (2) will be available on the d-bus (currently only with Remove method)
regardless of (3) and (4) that would disappear if ethernet is unplugged or
Digicom disappear. Ethernet Service type does not support remove, but Digicom
can be removed so the user can do:
connmanctl> config wifi_6067208eac78_44696769636f6d_managed_psk_stored --remove
or if still nearby
connmanctl> config wifi_6067208eac78_44696769636f6d_managed_psk --remove
(but I would deprecate this)
and
- the service is disconnected (if it was connected)
- its data in ram is cleared (if available, maybe it is not visible), like
before (passphrase, identity ..)
- d-bus object wifi_6067208eac78_44696769636f6d_managed_psk_stored disappears
- /var/lib/connman/wifi_6067208eac78_44696769636f6d_managed_psk folder and its
content is deleted
and the new situation is:
connmanctl> services
*AR Wired ethernet_c8cbb8564ae1_cable
Digicom wifi_6067208eac78_44696769636f6d_managed_psk
FG-WiFi wifi_6067208eac78_46472d57694669_managed_ieee8021x
connmanctl> known-services
*AR Wired ethernet_c8cbb8564ae1_cable_stored
The Manager/GetKnownServices method provides path objects + properties of each,
like GetServices. We'll provide only the path to save memory in case of
thousands services stored.
Regards,
Marco
-----Messaggio originale-----
Da: Patrik Flykt [mailto:[email protected]]
Inviato: luned? 22 febbraio 2016 12:21
A: MANIEZZO Marco (MM)
Cc: Marcel Holtmann; [email protected]
Oggetto: Re: R: [RFC] Storage based service retrieval + removal
Hi,
On Mon, 2016-02-22 at 10:52 +0000, MANIEZZO Marco (MM) wrote:
> Hello Patrick,
>
> Thank you for your feedback. Sorry for the email problem, I just
> copied/pasted the output of git diff in the mail, for the patch.
> Please find the patch in the attached txt.
>
> I agree with you that with 2000 services the d-bus message with this
> implementation will be big, our was an implementation more related to
> pc / mobile devices where the stored services would be two orders of
> magnitude smaller. Moreover the single services GetProperties is
> currently deprecated, I wasn't sure to go that direction for
> knownServices. We will think about the change accordingly to your
> request, maybe in separate phases: first would be to provide only the
> path object and second to implement the GetProperties method.
>
> Related to d-bus path names consider that they are used by the Remove
> method so I thought that the easiest way was to use the actual file
> system path (I mean service_path in var/lib/connman/<service_path>)
> plus a postfix; after having stripped the postfix it is easy to remove
> the file system data relying on
> __connman_storage_remove_service(service->identifier): this is also
> granting
> 1) uniqueness
> 2) relevant information already included in the path itself, like
> technology, security methods, SSID
Point 2) works only if the service ID is interpreted, by default this should
not be the case (except for giving sane clues for developers, but that's
another story).
> 3) for application using connman easy matching with nearby services to
> understand if a stored service is also available for connection
I think there needs to be a service object path property that explicitly tells
what the corresponding service object path is, if any. The other question from
a long time ago was the life cycle of these stored services, were they going to
disappear when they are detected and created as services or how was the
situation supposed to be resolved.
> I can change it to have "stored" as a prefix instead of postfix but I
> would keep the <service_path> especially for point 1, but if you have
> a different suggestion it is welcome; please let me know your
> feedback.
I'd appreciate that the intended changes were to be documented in ./doc first
so that the API can be discussed and agreed on. Then the implementation won't
be dragged through all discussions and changes.
> We'll provide the remove patch separated from knowServices one.
Thanks.
Patrik
________________________________
VISITA IL NOSTRO NUOVO SITO WEB! - VISIT OUR NEW WEB SITE!
www.magnetimarelli.com
Confidential Notice: This message - including its attachments - may contain
proprietary, confidential and/or legally protected information and is intended
solely for the use of the designated addressee(s) above. If you are not the
intended recipient be aware that any downloading, copying, disclosure,
distribution or use of the contents of the above information is strictly
prohibited.
If you have received this communication by mistake, please forward the message
back to the sender at the email address above, delete the message from all
mailboxes and any other electronic storage medium and destroy all copies.
Disclaimer Notice: Internet communications cannot be guaranteed to be safe or
error-free. Therefore we do not assure that this message is complete or
accurate and we do not accept liability for any errors or omissions in the
contents of this message.
------------------------------
Message: 2
Date: Mon, 14 Mar 2016 11:23:31 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Mon, 2016-03-07 at 18:44 -0800, [email protected] wrote:
> From: Naveen Singh <[email protected]>
>
> Connman currently disables network on a disconnect event. This severely
> impacts wpa supplicant ability to roam to same or different BSSID. The
> act of disabling network disables the complete network profile in wpa
> supplicant and it does not try to connect on its own and then waits for
> a new connection attempt from connman. This delays the re-connection
> timings. Once a network is disabled, wpa supplicant will generate a
> locally generated deauth while a connection is going through.
> wpa_supplicant is currently has the best knowledge of getting back L2 level
> connectivity.With this change this is how it would appear in log lines
> when a deauth with reason code 7/6/4 is received:
> 1. wpa_supplicant gets the reason code 7 deauth
> 2. wpa_supplicant starts a fast connect attempt
> 3. wpa_supplicant generates a disconnect notification to connman
> 4. connman calls set_disconnected which would make device lose its
> current IP address.
> 5. service is disconnected as well.
> 6. wpa_supplicant gets device connected back to same AP in this case
> 7. wpa_supplicant generates connected event.
> 8. connman starts with DHCP request (having same IP as the previous
> one).
This change makes me feel a bit uncomfortable. The code below has been
moved out from the 4-way handshake failure to apply in all disconnect
situations. Currently the code only takes
G_SUPPLICANT_STATE_DISCONNECTED state into account, should an additional
check be made for reason codes as well not reintroduce the issue the
original patch was fixing?
Cheers,
Patrik
> ---
> plugins/wifi.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index bb1cabc..382ed81 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -2407,12 +2407,6 @@ static void interface_state(GSupplicantInterface
> *interface)
> network, wifi))
> break;
>
> - /* We disable the selected network, if not then
> - * wpa_supplicant will loop retrying */
> - if (g_supplicant_interface_enable_selected_network(interface,
> - FALSE) != 0)
> - DBG("Could not disable selected network");
> -
> connman_network_set_connected(network, false);
> connman_network_set_associating(network, false);
> wifi->disconnecting = false;
------------------------------
Message: 3
Date: Mon, 14 Mar 2016 11:41:20 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] Supporting wifi load balancing and band steering
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Mon, 2016-03-07 at 16:47 -0800, [email protected] wrote:
> From: Naveen Singh <[email protected]>
>
> Enterprise APS sometimes refuse association with assoc response
> status code 17. AP basically wants to steer client to a right
> band or a better AP for load balancing purposes. This does not
> use to work as connman has no way to know that association was
> denied and it would treat this disconnect notification as a normal
> disconnect. wpa supplicant was sending DisconnectReason on DBUS as
> a part of PropertyChanged signal but there was no AssocStatusCode.
> In this commit id of hostapd (c7fb678f3109e62af1ef39be9b12bf8370c35bde)
In which version of wpa_supplicant is this change?
> wpa supplicant is also sending assoc status code as a part of
> PropertyChanged DBUS signal. Idea is that on a disconnect notification
> from wpa supplicant, if the prior interface state is associating
> (means a conection is underway) and association was denied with status
> code 17 then do not proceed. This would let wpa_supplicant chose another
> BSSID where association would stick.
>
> This is been tested by running a stress test which tries connection with
> an enterprise AP which frequently denies association with status code 17.
Ok.
> ---
> gsupplicant/gsupplicant.h | 2 ++
> gsupplicant/supplicant.c | 19 ++++++++++++++++++-
> plugins/wifi.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/gsupplicant/gsupplicant.h b/gsupplicant/gsupplicant.h
> index a2a7605..2f9806e 100644
> --- a/gsupplicant/gsupplicant.h
> +++ b/gsupplicant/gsupplicant.h
> @@ -353,6 +353,8 @@ struct _GSupplicantCallbacks {
> GSupplicantPeerState state);
> void (*peer_request) (GSupplicantPeer *peer);
> void (*debug) (const char *str);
> + void (*update_disconnect_reasoncode)(GSupplicantInterface *interface,
> int reasoncode);
> + void (*update_assoc_status_code)(GSupplicantInterface *interface, int
> reasoncode);
> };
>
> typedef struct _GSupplicantCallbacks GSupplicantCallbacks;
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index c8fbef6..a5195b1 100644
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -2135,9 +2135,26 @@ static void interface_property(const char *key,
> DBusMessageIter *iter,
> } else if (g_strcmp0(key, "Networks") == 0) {
> supplicant_dbus_array_foreach(iter, interface_network_added,
> interface);
> - } else
> + } else if (g_strcmp0(key, "DisconnectReason") == 0) {
> + int reason;
> + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INVALID) {
> + dbus_message_iter_get_basic(iter, &reason);
> + if (callbacks_pointer &&
> callbacks_pointer->update_disconnect_reasoncode && reason != 0) {
> +
> callbacks_pointer->update_disconnect_reasoncode(interface, reason);
> + }
> + }
> + } else if (g_strcmp0(key, "AssocStatusCode") == 0) {
> + int status_code;
> + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INVALID) {
> + dbus_message_iter_get_basic(iter, &status_code);
> + if (callbacks_pointer &&
> callbacks_pointer->update_assoc_status_code) {
> +
> callbacks_pointer->update_assoc_status_code(interface, status_code);
> + }
> + }
Currently the order of wpa_supplicant properties must be such that all
necessary information is available when the callback call is made here.
If (or when) something changes with wpa_supplicant, it is safer if the
attributes are collected first with the calling code reacting after
that. Otherwise some information might not have been collected from the
messages.
Here one should follow the callback_*() convention so that the code is
more uniform overall.
> + } else {
> SUPPLICANT_DBG("key %s type %c",
> key, dbus_message_iter_get_arg_type(iter));
> + }
> }
>
> static void scan_network_update(DBusMessageIter *iter, void *user_data)
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index bb1cabc..6219b81 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -71,6 +71,8 @@
> #define P2P_LISTEN_PERIOD 500
> #define P2P_LISTEN_INTERVAL 2000
>
> +#define GSUP_80211_ASSOC_STATUS_NO_ADDITIONAL_CLIENT 17
> +#define WPA_SUP_LOAD_SHAPING_MAX_RETRIES 3
> static struct connman_technology *wifi_technology = NULL;
> static struct connman_technology *p2p_technology = NULL;
>
> @@ -128,6 +130,7 @@ struct wifi_data {
> unsigned flags;
> unsigned int watch;
> int retries;
> + int wpa_sup_load_shaping_retries;
> struct hidden_params *hidden;
> bool postpone_hidden;
> struct wifi_tethering_info *tethering_param;
> @@ -144,6 +147,8 @@ struct wifi_data {
> bool p2p_connecting;
> bool p2p_device;
> int servicing;
> + int disconnect_reasoncode;
> + int assoc_statuscode;
> };
>
> static GList *iface_list = NULL;
> @@ -2291,6 +2296,19 @@ static bool handle_wps_completion(GSupplicantInterface
> *interface,
> return true;
> }
>
> +static bool handle_assoc_status_code(GSupplicantInterface *interface,
> + struct wifi_data *wifi)
> +{
> + if (wifi->state == G_SUPPLICANT_STATE_ASSOCIATING &&
> + wifi->assoc_statuscode ==
> GSUP_80211_ASSOC_STATUS_NO_ADDITIONAL_CLIENT &&
> + wifi->wpa_sup_load_shaping_retries <
> WPA_SUP_LOAD_SHAPING_MAX_RETRIES) {
> + wifi->wpa_sup_load_shaping_retries ++;
> + return TRUE;
> + }
> + wifi->wpa_sup_load_shaping_retries = 0;
> + return FALSE;
> +}
Hmm. Is it less changes and cross-file dependencies if the assoc status
code is handled in gsupplicant/supplicant.c instead of the wifi plugin?
With that wpa_supplicant specifics would be handled earlier and the wifi
pluging could do with less logic.
If there won't be 3 retries, the connect attempt will be cancelled with
the service timer, right?
> +
> static bool handle_4way_handshake_failure(GSupplicantInterface *interface,
> struct connman_network *network,
> struct wifi_data *wifi)
> @@ -2382,6 +2400,10 @@ static void interface_state(GSupplicantInterface
> *interface)
> break;
>
> connman_network_set_connected(network, true);
> +
> + wifi->disconnect_reasoncode = 0;
> + wifi->assoc_statuscode = 0;
> + wifi->wpa_sup_load_shaping_retries = 0;
> break;
>
> case G_SUPPLICANT_STATE_DISCONNECTED:
> @@ -2399,6 +2421,9 @@ static void interface_state(GSupplicantInterface
> *interface)
> if (is_idle(wifi))
> break;
>
> + if (handle_assoc_status_code(interface, wifi))
> + break;
> +
> /* If previous state was 4way-handshake, then
> * it's either: psk was incorrect and thus we retry
> * or if we reach the maximum retries we declare the
> @@ -2935,6 +2960,24 @@ static void debug(const char *str)
> connman_debug("%s", str);
> }
>
> +static void wifi_disconnect_reasoncode(GSupplicantInterface *interface, int
> reasoncode)
> +{
> + struct wifi_data *wifi = g_supplicant_interface_get_data(interface);
> +
> + if (wifi != NULL) {
> + wifi->disconnect_reasoncode = reasoncode;
> + }
> +}
> +
> +static void wifi_assoc_status_code(GSupplicantInterface *interface, int
> status_code)
> +{
> + struct wifi_data *wifi = g_supplicant_interface_get_data(interface);
> +
> + if (wifi != NULL) {
> + wifi->assoc_statuscode = status_code;
> + }
> +}
> +
> static const GSupplicantCallbacks callbacks = {
> .system_ready = system_ready,
> .system_killed = system_killed,
> @@ -2953,6 +2996,8 @@ static const GSupplicantCallbacks callbacks = {
> .peer_changed = peer_changed,
> .peer_request = peer_request,
> .debug = debug,
> + .update_disconnect_reasoncode = wifi_disconnect_reasoncode,
> + .update_assoc_status_code = wifi_assoc_status_code,
> };
>
>
Patrik
------------------------------
Message: 4
Date: Mon, 14 Mar 2016 11:46:11 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Mon, 2016-03-07 at 17:55 -0800, [email protected] wrote:
> From: Naveen Singh <[email protected]>
>
> In case network path is not NULL, network should only be removed
> if the new network (to be added) is different from what is sitting
> in wpa_supplicant. This would avoid two DBUS calls: AddNetwork and
> SelectNetwork.
> ---
> gsupplicant/supplicant.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index c8fbef6..b4920fc 100644
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -181,6 +181,7 @@ struct _GSupplicantInterface {
> GHashTable *bss_mapping;
> void *data;
> const char *pending_peer_path;
> + char * added_ssid;
> };
>
> struct g_supplicant_bss {
> @@ -4111,6 +4112,9 @@ static void interface_add_network_result(const char
> *error,
>
> interface->network_path = g_strdup(path);
>
> + g_free(interface->added_ssid);
> + interface->added_ssid = g_strdup(data->ssid->ssid);
> +
> supplicant_dbus_method_call(data->interface->path,
> SUPPLICANT_INTERFACE ".Interface", "SelectNetwork",
> interface_select_network_params,
> @@ -4708,6 +4712,17 @@ int
> g_supplicant_interface_connect(GSupplicantInterface *interface,
> g_free(data->path);
> dbus_free(data);
>
> + /* If this add network is for the same SSID for which
> + * wpa_supplicant already has a profile then do not need
> + * to add another profile. Only if the profile that
> needs
> + * to get added is different from what is there in wpa_s
> + * delete the current one. interface->added_ssid is
> populated
> + * once add network request is successful.*/
> +
> + if (g_strcmp0(interface->added_ssid, ssid->ssid) == 0) {
> + return -EALREADY;
> + }
Shouldn't interface->added_ssid also be cleared on disconnect? It seems
it will be remembered over a previous disconnect (or then there is a
memory leak somewhere) if only touched in
interface_add_network_result()?
> +
> intf_data = dbus_malloc0(sizeof(*intf_data));
> if (!intf_data)
> return -ENOMEM;
Patrik
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 5, Issue 15
**************************************
