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. Re: [PATCH v3] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call. (Naveen Singh)
2. Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect (Naveen Singh)
3. Re: [PATCH] Supporting wifi load balancing and band steering
(Naveen Singh)
4. Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect (Naveen Singh)
----------------------------------------------------------------------
Message: 1
Date: Tue, 22 Mar 2016 12:25:46 -0700
From: Naveen Singh <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH v3] gsupplicant: Optimize AddNetwork Handler by
avoiding DBUS call.
Message-ID:
<cafk1zrbs7xf4nsm37rj4o_tcsfc_wyvih89kk39v_jissyh...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
On Tue, Mar 22, 2016 at 3:53 AM, Patrik Flykt <[email protected]>
wrote:
> On Thu, 2016-03-17 at 17:33 -0700, [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.
>
> This was also applied, thanks!
>
> Patrik
Hi Patrik
I think this patch standalone would break things. This patch needs to be
applied with the other patch "Do Not disable network on Disconnect".
What would happen now with just this patch is that on disconnect
notification, we will disable the network. This will cause wpa_supplicant
to stop trying to connect. But at this point network_path is still valid.
Any further connect attempt with the same network would not go through
because we do this additional check if it is same SSID. And since in this
case SSID is same, AddNetwork will be dropped. Now this is a complete
deadlock until we have a different SSID to attempt to.
It was my bad that I split this patch in two but when I did it, it was
supposed to answer two different issues and later on i realized that there
is dependency. Can you revert back this patch. I am really very sorry for
this.
I can create another patch with other changes and you can review it.
Let me know what do you think
Regards
Naveen
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20160322/b98d5193/attachment-0001.html>
------------------------------
Message: 2
Date: Tue, 22 Mar 2016 14:26:05 -0700
From: Naveen Singh <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect
Message-ID:
<CAFK1zRBFo56802g-+H9YkKx3XJkFyT=duwv3n-+hsfya2tc...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
On Tue, Mar 22, 2016 at 4:57 AM, Patrik Flykt <[email protected]>
wrote:
> On Mon, 2016-03-21 at 13:23 -0700, Naveen Singh wrote:
> > So unless the proper set of reason codes cannot be obtained
> > from
> > wpa_supplicant indicating that the disconnection is due to a
> > lower level
> > issue that can be ignored, wpa_supplicant should not be left
> > connecting
> > by its own.
> > Tying this up with a specific reason code may not be correct as reason
> > code are always not available. Consider an example where device
> > intermittently faced beacon loss (because of interference) and we know
> > AP exists. wpa_s will get device connected back immediately. In the
> > other case if we would have disabled the network (as reason code
> > coming with this disconnect notification is not 6, 7 or 4),
> > re-connection would have taken longer.
>
> This is the main source of issues with wpa_supplicant. It should not
> tell its users that the network disconnected until the network really
> was cut and the few immediate retries didn't succeed either.
>
wpa_s does not work this way. wpa_s once armed with profile never gives up
till profile is removed or disabled. It's only intention is to get you
connected There are applications (for example Android phones) which rely
completely on wpa_s for getting device connected to wifi. So there is no
notion of wpa_s ever giving up. The disconnect notification is mainly for
apps to assume that under lying L2 connectivity is down hence do not try to
send data packets.
>
> Are the reason codes sent by certain wpa_supplicant D-Bus signals
> actually documented anywhere? Last time I looked no exhaustive list was
> available, do you have any empirical evidence of the reason codes sent
> in different states due to non-terminal and terminal network disconnect
> events?
>
There is no documentation. If disconnect is because of an explicit
deauth/disassoc frame, wireless LAN driver should just pass the same reason
code (from wireless frame) to wpa_s and hence to connman. For internally
generated disconnect wpa_s decided to use reason code 3. But then there are
disconnect that happens because of beacon loss, and in that case reason
code that is sent to connman is dependent on what underlying driver is
doing. This is the reason why tying our fate with reason code is probably
not a good idea.
>
> Cheers,
>
> Patrik
>
Regards
Naveen
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20160322/12b00269/attachment-0001.html>
------------------------------
Message: 3
Date: Tue, 22 Mar 2016 17:18:39 -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:
<cafk1zrckbxdst6mnvdxuqzob2aqoaemnrgsxxypsd9zbj3w...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
On Mon, Mar 21, 2016 at 7:49 AM, Patrik Flykt <[email protected]>
wrote:
> On Mon, 2016-03-14 at 18:49 -0700, Naveen Singh wrote:
> > > + 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?
>
> In other parts of the code functions like
> callback_interface_added(GSupplicantInterface *interface) are used that
> then check for the interface pointer being non-NULL as well as for the
> respective callbacks_pointer member.
>
I can make this change to be consistent with other part of code.
>
> The division into gsupplicant and plugins/wifi.c is a bit artificial
> sometimes. Yes, gsupplicant is a "communcations" library, while the
> (re)action takes place in plugins/wifi.c. It is actually a matter of
> debate which one is the correct place, the idea should be that
> gsupplicant plasters over the biggest wrinkles wpa_supplicant will
> produce making the intention and (re)action clear in plugins/wifi.c.
>
> Whether this particular case deserves handling and where is interesting;
> the criteria is basically that the developer goes through a total memory
> erase and sees the code anew after that; armed with only the code and
> commit messages the implementation split which is then clearer to the
> unaided mind is the one we'd want in the code...
>
If we want to contain the complete change in gsupplicant directory, is
there a need for the callback functions. I think we need to store following
in gsupplicant layer:
a) Reason code
b) Status code
c) variable to track retry
Gsupplicant state handling code will have to look into state and act
differently. Like if current state is ASSOCIATING and going to be state is
DISCONNECTED, it needs to look what was previous assoc status code? Also
once the state becomes CONNECTED, it needs to clear the reason and assoc
status code and also reset the counter variable (which was tracking retry).
This is going to make that code cluttered.
This is place in code where we would handle above mentioned
} else if (g_strcmp0(key, "State") == 0) {
2062 const char *str = NULL;
2063
2064 dbus_message_iter_get_basic(iter, &str);
2065 if (str)
2066 if (string2state(str) != interface->state) {
2067 interface->state = string2state(str);
2068 callback_interface_state(interface);
2069 }
2070
If you think this is correct I can send you a patch with all these changes.
Let me know
Regards
Naveen
>
> Cheers,
>
> Patrik
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20160322/7915612b/attachment-0001.html>
------------------------------
Message: 4
Date: Tue, 22 Mar 2016 17:42:10 -0700
From: Naveen Singh <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected], Naveen Singh <[email protected]>
Subject: Re: [PATCH] plugins/wifi: Do not disable network on
Disconnect
Message-ID:
<CAFK1zRAi_Qj8frowdEMijJwsfTv0m2ho=TxN=gs84stijpy...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
On Tue, Mar 22, 2016 at 4:57 AM, Patrik Flykt <[email protected]>
wrote:
> On Mon, 2016-03-21 at 13:23 -0700, Naveen Singh wrote:
> > So unless the proper set of reason codes cannot be obtained
> > from
> > wpa_supplicant indicating that the disconnection is due to a
> > lower level
> > issue that can be ignored, wpa_supplicant should not be left
> > connecting
> > by its own.
> > Tying this up with a specific reason code may not be correct as reason
> > code are always not available. Consider an example where device
> > intermittently faced beacon loss (because of interference) and we know
> > AP exists. wpa_s will get device connected back immediately. In the
> > other case if we would have disabled the network (as reason code
> > coming with this disconnect notification is not 6, 7 or 4),
> > re-connection would have taken longer.
>
> This is the main source of issues with wpa_supplicant. It should not
> tell its users that the network disconnected until the network really
> was cut and the few immediate retries didn't succeed either.
>
> Are the reason codes sent by certain wpa_supplicant D-Bus signals
> actually documented anywhere? Last time I looked no exhaustive list was
> available, do you have any empirical evidence of the reason codes sent
> in different states due to non-terminal and terminal network disconnect
> events?
>
> Cheers,
>
> Patrik
>
Hi Patrik
I know this change makes you very nervous and you want to be very cautious
about making this. But as I mentioned we are not letting wpa_s try for
ever. The only case where it could fail if there is any bug in wpa_s, where
wpa_s stops trying to connect. The only time when wpa_s will stop trying to
connect is when the profile gets disabled. And disabling the profile
happens only via a DBUS setter API which only connman can call. We have
been running test with these changes for last few weeks and till now we
have not found anything where it went wrong.
In fact this change combined with "DBUS optimization change" is behaving
good on our test beds. The logs look really clean. Without this change if
device gets a disconnect notification, it would see flurry of disconnect
happening with reason code 3. The reason is that disabling the network
leads to a locally generate disconnect from supplicant and based on whether
it is a cfg80211 or mac80211 based driver it could lead to even more
disconnect from device. This all looks very confusing as well as increases
the re-connection time.
Now with this change, device gets disconnected, connman gets notified and
connman releases IP address and makes the service state disconnected. From
now on connman is not doing anything. If wpa_s gets to find the same
network (does not matter same or different BSSID), it would generate a
connect event and that would do the appropriate service transition. And in
the meantime if user decided to connect to some other network or connman
has multiple favorite profiles, it removes the previous network and then do
a AddNetwork for this new network.
And if you agree to apply this change we do not have to revert "DBUS
optimization change".
Let me know if you have any concern
Regards
Naveen
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20160322/9f889f1e/attachment.html>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 5, Issue 28
**************************************
