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. [PATCH v2] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call. ([email protected])
2. Re: [PATCH] Supporting wifi load balancing and band steering
(Naveen Singh)
----------------------------------------------------------------------
Message: 1
Date: Mon, 14 Mar 2016 18:36:14 -0700
From: [email protected]
To: [email protected]
Cc: Naveen Singh <[email protected]>
Subject: [PATCH v2] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call.
Message-ID: <[email protected]>
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 | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index c8fbef6..c4f7e0c 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;
+ }
+
intf_data = dbus_malloc0(sizeof(*intf_data));
if (!intf_data)
return -ENOMEM;
@@ -4756,6 +4771,9 @@ static void network_remove_result(const char *error,
g_free(data->interface->network_path);
data->interface->network_path = NULL;
+ g_free(data->interface->added_ssid);
+ data->interface->added_ssid = NULL;
+
if (data->network_remove_in_progress == TRUE) {
data->network_remove_in_progress = FALSE;
connect_data = dbus_malloc0(sizeof(*connect_data));
--
2.7.0.rc3.207.g0ac5344
------------------------------
Message: 2
Date: Mon, 14 Mar 2016 18:49:30 -0700
From: Naveen Singh <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] Supporting wifi load balancing and band steering
Message-ID:
<cafk1zrbmqdrbzgb+bcy2sm26e5kty_qhuyweks5_c-2io0m...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
On Mon, Mar 14, 2016 at 2:41 AM, Patrik Flykt <[email protected]>
wrote:
>
> 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.
Do you mean instead of update_assoc_status_code and
update_disconnect_reasoncode
make it callback_update_assoc_status_code and
callback_update_disconnect_reasoncode
respectively. I looked into the code and I do not see any of this. Am I
missing something here?
>
> > + } 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20160314/4636c558/attachment.html>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 5, Issue 21
**************************************
