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 3/3][v2] wifi: Abort tethering enabling if bridge
creation/configuration fails (Jose Blanquicet)
2. [PATCH] TODO: Add task for tethering (Jose Blanquicet)
3. Re: [PATCH 3/3] wifi: Abort tethering enabling if bridge
creation/configuration fails (Jose Blanquicet)
----------------------------------------------------------------------
Message: 1
Date: Tue, 25 Oct 2016 14:50:50 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected],
[email protected]
Subject: [PATCH 3/3][v2] wifi: Abort tethering enabling if bridge
creation/configuration fails
Message-ID: <[email protected]>
In case there is an error while setting up the bridge for WiFi tethering, the
selected interface for tethering will remain removed and could not be use
anymore. This patch aims to avoid such a situation by using
connman_technology_tethering_notify()'s return value to abort tethering
enabling thus do not unnecessary remove the interface in case of something was
wrong with bridge.
---
plugins/wifi.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 8bde9c0..68b231d 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -3138,13 +3138,14 @@ static void sta_remove_callback(int result,
DBG("ifname %s result %d ", info->ifname, result);
if ((result < 0) || (info->wifi->ap_supported != WIFI_AP_SUPPORTED)) {
- info->wifi->tethering = true;
+ info->wifi->tethering = false;
+ connman_technology_tethering_notify(info->technology, false);
g_free(info->ifname);
g_free(info->ssid);
g_free(info);
- if (info->wifi->ap_supported == WIFI_AP_SUPPORTED){
+ if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
g_free(info->wifi->tethering_param->ssid);
g_free(info->wifi->tethering_param);
info->wifi->tethering_param = NULL;
@@ -3154,8 +3155,6 @@ static void sta_remove_callback(int result,
info->wifi->interface = NULL;
- connman_technology_tethering_notify(info->technology, true);
-
g_supplicant_interface_create(info->ifname, driver, info->wifi->bridge,
ap_create_callback,
info);
@@ -3171,7 +3170,7 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
struct wifi_tethering_info *info;
const char *ifname;
unsigned int mode;
- int err;
+ int err, berr = 0;
for (list = iface_list; list; list = list->next) {
wifi = list->data;
@@ -3230,6 +3229,10 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
info->wifi->tethering = true;
info->wifi->ap_supported = WIFI_AP_SUPPORTED;
+ berr = connman_technology_tethering_notify(technology, true);
+ if (berr < 0)
+ goto failed;
+
err = g_supplicant_interface_remove(interface,
sta_remove_callback,
info);
@@ -3244,6 +3247,17 @@ static int enable_wifi_tethering(struct
connman_technology *technology,
g_free(info);
g_free(wifi->tethering_param);
wifi->tethering_param = NULL;
+
+ /*
+ * Remove bridge if it was correctly created but remove
+ * operation failed. Instead, if bridge creation failed then
+ * break out and do not try again on another interface,
+ * bridge set-up does not depend on it.
+ */
+ if (berr == 0)
+ connman_technology_tethering_notify(technology, false);
+ else
+ break;
}
return -EOPNOTSUPP;
--
1.9.1
------------------------------
Message: 2
Date: Tue, 25 Oct 2016 14:51:14 +0000
From: Jose Blanquicet <[email protected]>
To: [email protected], [email protected],
[email protected]
Subject: [PATCH] TODO: Add task for tethering
Message-ID: <[email protected]>
---
TODO | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/TODO b/TODO
index 00d9676..c1694e3 100644
--- a/TODO
+++ b/TODO
@@ -99,6 +99,20 @@ Core
ObjectManager common to Linux desktops can be implemented.
+Tethering
+=========
+
+- Verify if bridge has been correctly created and configured
+
+ Priority: Low
+ Complexity: C1
+
+ When enabling tethering check if there was any error while creating and
+ configuring the bridge before continue. It has been done only for WiFi
+ technology, for other tethering technologies it should be evaluated
+ and implemented in case it is advantageous.
+
+
WiFi
====
--
1.9.1
------------------------------
Message: 3
Date: Tue, 25 Oct 2016 14:52:01 +0000
From: Jose Blanquicet <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected], "MANIEZZO Marco (MM)"
<[email protected]>
Subject: Re: [PATCH 3/3] wifi: Abort tethering enabling if bridge
creation/configuration fails
Message-ID:
<CAFC8iJL4OtW=+m3NMjmqqR5dkvZ71x4nHjYNRZTv=oF=kqo...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Hi,
>>> After the first successful call with enabled=true it is a no op
>>
>> (return -EALREDAY). Hmm, even worse, since you will break out with
>>>
>>>
>>> if (bridge_result < 0)
>>> break
>>>
>>> you will only enable the first interface. I suggest you call
>>> connman_technology_tethering_notify() before the while loop
>>> or after. Depending what is simpler to do (error handling).
>>
>>
>> Now I got your point. However, I do not know why you continue iterating
>> in the loop after the first successful call, we only want to enable
>> tethering on one interface (The first one that supports tethering).
>> That's the reason loop return when everything went well:
>>
>> err = g_supplicant_interface_remove(interface,
>> sta_remove_callback,
>> info);
>> if (err >= 0) {
>> DBG("tethering wifi %p ifname %s", wifi, ifname);
>> return 0;
>> }
>>
>> Therefore, connman_technology_tethering_notify() will get called only
>> once. However, what you made me notice is that I have to remove the
>> bridge in case g_supplicant_interface_remove() fails.
>>
>> Is there something I am missing?
>
>
> Nah, I didn't read it correctly when giving a quick glance. I suggest to
move the whole functionality into a function and just call it from the
loop. If it returns success leave the loop otherwise continue looping. This
is just a bit too easy to get it wrong as it stands now.
You are right. It has become a complex code even before this patch. To do
what you say will make it more clear/readable but unfortunately I am
currently quite busy to modify and retest all again.
On the other hand, I found that it is required to initialize the variable
berr (previously bridge_error) because it could be evaluated without being
assigned in case of one of these two actions fail:
info->ssid = ssid_ap_init(identifier, passphrase);
if (!info->ssid)
goto failed;
or ...
wifi->tethering_param->ssid = ssid_ap_init(identifier, passphrase);
if (!wifi->tethering_param->ssid)
goto failed;
Please check the PATCH v2. Do you agree?
Regards,
Jose Blanquicet
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20161025/ee25177e/attachment-0001.html>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 12, Issue 27
***************************************
