Send connman mailing list submissions to
        connman@lists.01.org

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
        connman-requ...@lists.01.org

You can reach the person managing the list at
        connman-ow...@lists.01.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. [PATCH] supplicant: fixed return value of ssid getter
      (Vasyl Vavrychuk)
   2. [PATCH] fixed noisy warning on every connect (Vasyl Vavrychuk)
   3. if we try to connect to network during autoscan we may get
      OperationAborted (Vasyl Vavrychuk)
   4. Re: [PATCH] supplicant: fixed return value of ssid getter
      (Daniel Wagner)
   5. Re: [PATCH] fixed noisy warning on every connect (Daniel Wagner)
   6. Re: if we try to connect to network during autoscan we may
      get OperationAborted (Daniel Wagner)
   7. Re: [PATCH] fixed noisy warning on every connect (Vasyl Vavrychuk)


----------------------------------------------------------------------

Message: 1
Date: Fri, 23 Feb 2018 22:26:44 +0200
From: Vasyl Vavrychuk <vvavryc...@gmail.com>
To: connman@lists.01.org
Subject: [PATCH] supplicant: fixed return value of ssid getter
Message-ID: <20180223202644.28956-1-vvavryc...@gmail.com>

Previously for network without SSID g_supplicant_network_get_ssid
returned pointer to empty string. Then some naive users of it treated
such value as a valid SSID.

This fixes "Network SSID is not set" for hidden networks.
---
 gsupplicant/supplicant.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 5246c80b..ec2f4307 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -1189,7 +1189,7 @@ const char 
*g_supplicant_network_get_security(GSupplicantNetwork *network)
 const void *g_supplicant_network_get_ssid(GSupplicantNetwork *network,
                                                unsigned int *ssid_len)
 {
-       if (!network) {
+       if (!network || !network->ssid_len) {
                *ssid_len = 0;
                return NULL;
        }
-- 
2.11.0



------------------------------

Message: 2
Date: Fri, 23 Feb 2018 22:27:04 +0200
From: Vasyl Vavrychuk <vvavryc...@gmail.com>
To: connman@lists.01.org
Subject: [PATCH] fixed noisy warning on every connect
Message-ID: <20180223202704.29237-1-vvavryc...@gmail.com>

On every connect I get 'Skipping disconnect of ..., network is connecting'.
This is happening because __connman_network_connect sets connecting
flag before __connman_device_disconnect. The last function then prints
warning due to this connecting flag.

Changed order of assigning connecting and __connman_device_disconnect
fixes this. Connman logic is not effected due to the way how flags
connected and associating are handled in __connman_network_connect
and __connman_network_disconnect.
---
 src/network.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/network.c b/src/network.c
index 8c5979f7..4757d0d7 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1468,10 +1468,10 @@ int __connman_network_connect(struct connman_network 
*network)
        if (!network->device)
                return -ENODEV;
 
-       network->connecting = true;
-
        __connman_device_disconnect(network->device);
 
+       network->connecting = true;
+
        err = network->driver->connect(network);
        if (err < 0) {
                if (err == -EINPROGRESS)
-- 
2.11.0



------------------------------

Message: 3
Date: Fri, 23 Feb 2018 22:30:21 +0200
From: Vasyl Vavrychuk <vvavryc...@gmail.com>
To: connman@lists.01.org
Subject: if we try to connect to network during autoscan we may get
        OperationAborted
Message-ID:
        <cagj4m+5majxmptkh5-shxqbqcopak-gkbjg0qxhdv8vmbfr...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"

Suppose there is Wi-Fi network X that is saved in connman. Consider
the following scenario:

1. Autoscan is started by connman.
2. Network X will be marked as unavailable as result of
connman_device_set_scanning(true).
3. Application issue connect to network X.
4. Connect to X starts and
interface_state(G_SUPPLICANT_STATE_AUTHENTICATING) is received.
5. As result of this stop_autoscan >
connman_device_set_scanning(false) > __connman_device_cleanup_networks
is issued that free_network X because it was marked as unavailable.
6. It leads to network_remove > __connman_service_remove_from_network
> connman_service_unref_debug > __connman_service_disconnect > ...
reply_pending(ECONNABORTED)

Stack trace is at the end. Does this means that it is better to not
connect to a network during scan?

__connman_error_operation_aborted() at error.c:167 0x5555555a732c
__connman_error_failed() at error.c:57 0x5555555a700d
connman_dbus_reply_pending() at dbus.c:663 0x5555555e48a9
reply_pending() at service.c:3,761 0x5555555b98b0
__connman_service_disconnect() at service.c:6,446 0x5555555bed1d
connman_service_unref_debug() at service.c:4,972 0x5555555bc2e6
__connman_service_remove_from_network() at service.c:7,266 0x5555555c05f7
network_remove() at network.c:809 0x5555555ac2ee
__connman_network_set_device() at network.c:2,015 0x5555555ae65a
free_network() at device.c:398 0x5555555a9190
0x7ffff7afd961
__connman_device_cleanup_networks() at device.c:721 0x5555555a9a33
connman_device_set_scanning() at device.c:778 0x5555555a9ba1
stop_autoscan() at wifi.c:861 0x5555555842d4
interface_state() at wifi.c:2,503 0x555555587a1c


------------------------------

Message: 4
Date: Sat, 24 Feb 2018 15:22:17 +0100
From: Daniel Wagner <w...@monom.org>
To: Vasyl Vavrychuk <vvavryc...@gmail.com>
Cc: connman@lists.01.org
Subject: Re: [PATCH] supplicant: fixed return value of ssid getter
Message-ID: <a86fc122-7a79-bdec-89eb-8055bc780...@monom.org>
Content-Type: text/plain; charset=utf-8

Hi Vasyl,

On 02/23/2018 09:26 PM, Vasyl Vavrychuk wrote:
> Previously for network without SSID g_supplicant_network_get_ssid
> returned pointer to empty string. Then some naive users of it treated
> such value as a valid SSID.
> 
> This fixes "Network SSID is not set" for hidden networks.

Can you elaborate a bit what is wrong? I tried to understand what you
mean with 'previously'. Does a newer version of wpa_s behave
differently compared to an older version?

Also I found following commit:

commit 91913c40054d9c55c6d38f0c626a78bdbcf4279a
Author: Tomasz Bursztyka <tomasz.burszt...@linux.intel.com>
Date:   Thu Dec 12 15:57:05 2013 +0200

    gsupplicant: A network ssid of length 0 is valid, it's an hidden one
    
    In case of an hidden AP around, its ssid has a length of 0, but it is
    obviously a valid network anyway.
    
    Fixes such regression from commit 363393cfb1a5f95f8892f40662486c87b80d0091
    
    Reported by Sameer Naik

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 23ea41a03642..559dfbc6fd2f 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -853,7 +853,7 @@ const char 
*g_supplicant_network_get_security(GSupplicantNetwork *network)
 const void *g_supplicant_network_get_ssid(GSupplicantNetwork *network,
                                                unsigned int *ssid_len)
 {
-       if (!network || network->ssid_len == 0) {
+       if (!network) {
                *ssid_len = 0;
                return NULL;
        }


The report from Sameer was 

"""
I am looking into the connman plugin/wifi.c to figure out the issue.
so far i have found that in  scan_callback_hidden  function,
g_supplicant_interface_get_max_scan_ssids(wifi->interface), returns 0 and
therefore no attempt to connect to provisioned hidden wifi networks is
performed.

Will trace the calls further to figure out why
g_supplicant_interface_get_max_scan_ssids returns 0.
"""

https://lists.01.org/pipermail/connman/2013-March/014020.html

> ---
>   gsupplicant/supplicant.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index 5246c80b..ec2f4307 100644
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -1189,7 +1189,7 @@ const char 
> *g_supplicant_network_get_security(GSupplicantNetwork *network)
>   const void *g_supplicant_network_get_ssid(GSupplicantNetwork *network,
>                                               unsigned int *ssid_len)
>   {
> -     if (!network) {
> +     if (!network || !network->ssid_len) {
>               *ssid_len = 0;
>               return NULL;
>       }

Your patch is reverting commit 91913c40054d ("gsupplicant: A network
ssid of length 0 is valid, it's an hidden one").

Thanks,
Daniel


------------------------------

Message: 5
Date: Sat, 24 Feb 2018 15:34:20 +0100
From: Daniel Wagner <w...@monom.org>
To: Vasyl Vavrychuk <vvavryc...@gmail.com>
Cc: connman@lists.01.org
Subject: Re: [PATCH] fixed noisy warning on every connect
Message-ID: <94e996d4-507a-6a76-3fd8-eed2c62b2...@monom.org>
Content-Type: text/plain; charset=utf-8; format=flowed

Hi Vasyl,

On 02/23/2018 09:27 PM, Vasyl Vavrychuk wrote:
> On every connect I get 'Skipping disconnect of ..., network is connecting'.
> This is happening because __connman_network_connect sets connecting
> flag before __connman_device_disconnect. The last function then prints
> warning due to this connecting flag.
> 
> Changed order of assigning connecting and __connman_device_disconnect
> fixes this. Connman logic is not effected due to the way how flags
> connected and associating are handled in __connman_network_connect
> and __connman_network_disconnect.
> ---
>   src/network.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/network.c b/src/network.c
> index 8c5979f7..4757d0d7 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1468,10 +1468,10 @@ int __connman_network_connect(struct connman_network 
> *network)
>       if (!network->device)
>               return -ENODEV;
>   
> -     network->connecting = true;
> -
>       __connman_device_disconnect(network->device);
>   
> +     network->connecting = true;
> +
>       err = network->driver->connect(network);
>       if (err < 0) {
>               if (err == -EINPROGRESS)
> 

I found this part of always confusing but never took time to really try 
to figure out why it is there. After some digging in the history I think 
we can safely remove the __connman_device_disconnect() call instead of 
moving the network->connecting part. With setting network->connecting = 
true we have a no-op in __connman_device_disconnect(). So why bother to 
call it anyway from here?

Thanks,
Daniel


------------------------------

Message: 6
Date: Sat, 24 Feb 2018 16:06:39 +0100
From: Daniel Wagner <w...@monom.org>
To: Vasyl Vavrychuk <vvavryc...@gmail.com>
Cc: connman@lists.01.org
Subject: Re: if we try to connect to network during autoscan we may
        get OperationAborted
Message-ID: <4dcc5a8c-756c-d326-b873-e0971bce1...@monom.org>
Content-Type: text/plain; charset=utf-8; format=flowed

Hi Vasyl,

On 02/23/2018 09:30 PM, Vasyl Vavrychuk wrote:
> Suppose there is Wi-Fi network X that is saved in connman. Consider
> the following scenario:
> 
> 1. Autoscan is started by connman.
> 2. Network X will be marked as unavailable as result of
> connman_device_set_scanning(true).
> 3. Application issue connect to network X.
> 4. Connect to X starts and
> interface_state(G_SUPPLICANT_STATE_AUTHENTICATING) is received.
> 5. As result of this stop_autoscan >
> connman_device_set_scanning(false) > __connman_device_cleanup_networks
> is issued that free_network X because it was marked as unavailable.
> 6. It leads to network_remove > __connman_service_remove_from_network
>> connman_service_unref_debug > __connman_service_disconnect > ...
> reply_pending(ECONNABORTED)
> 
> Stack trace is at the end. Does this means that it is better to not
> connect to a network during scan?

If I understood it correctly we have a race between scan and connect. My 
naive attempt would be to extend remove_unavailable_network() by 
checking if we are in the connecting state. There is already the 
connecting state check in mark_network_unavailable() but the connect 
attempt happens after the mark_network_unavailable() call.

Hmm, this is very fragile. I wonder if we could get rid of the mark 
thing completely.

Thanks,
Daniel


------------------------------

Message: 7
Date: Sat, 24 Feb 2018 18:10:57 +0200
From: Vasyl Vavrychuk <vvavryc...@gmail.com>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: Re: [PATCH] fixed noisy warning on every connect
Message-ID:
        <CAGj4m+62xFSOuYH4=altckf_cfoa+dzqoejgphhkgf4jdox...@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"

>> On every connect I get 'Skipping disconnect of ..., network is
>> connecting'.
>> This is happening because __connman_network_connect sets connecting
>> flag before __connman_device_disconnect. The last function then prints
>> warning due to this connecting flag.
>>
>> Changed order of assigning connecting and __connman_device_disconnect
>> fixes this. Connman logic is not effected due to the way how flags
>> connected and associating are handled in __connman_network_connect
>> and __connman_network_disconnect.
>> ---
>>   src/network.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/network.c b/src/network.c
>> index 8c5979f7..4757d0d7 100644
>> --- a/src/network.c
>> +++ b/src/network.c
>> @@ -1468,10 +1468,10 @@ int __connman_network_connect(struct
>> connman_network *network)
>>         if (!network->device)
>>                 return -ENODEV;
>>   -     network->connecting = true;
>> -
>>         __connman_device_disconnect(network->device);
>>   +     network->connecting = true;
>> +
>>         err = network->driver->connect(network);
>>         if (err < 0) {
>>                 if (err == -EINPROGRESS)
>>
>
> I found this part of always confusing but never took time to really try to
> figure out why it is there. After some digging in the history I think we can
> safely remove the __connman_device_disconnect() call instead of moving the
> network->connecting part. With setting network->connecting = true we have a
> no-op in __connman_device_disconnect(). So why bother to call it anyway from
> here?

Suppose we are connecting to network A belonging to device B. But
besides network A
there can be other networks belonging to device B. And
__connman_device_disconnect
will perform disconnect from those networks. How about this? Can we
still safely remove
__connman_device_disconnect?


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
connman@lists.01.org
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 28, Issue 18
***************************************

Reply via email to