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] dhcpv6: use correct dhcp renew time when valid
      life-time is infinity. (wangfe-nestlabs)
   2. [PATCH] config: Fix indentation (Jaakko Hannikainen)
   3. [PATCH 0/2] DHCPv6 infinite expiry times (Patrik Flykt)
   4. [PATCH 2/2] gdhcp: Set T1 and T2 to infinite if expiry time
      is infinite (Patrik Flykt)
   5. [PATCH 1/2] dhcpv6: Return -EISCONN when the expiry time is
      inifinite (Patrik Flykt)
   6. DBus properties race condition (David Lechner)
   7. Re: [PATCH 1/2] dhcpv6: Return -EISCONN when the expiry time
      is inifinite (Feng Wang)


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

Message: 1
Date: Wed, 13 Jul 2016 12:01:30 -0700
From: wangfe-nestlabs <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] dhcpv6: use correct dhcp renew time when valid
        life-time is infinity.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii

Hi Patrik,

You think the patch is good?

Thanks,

Feng


> On Jul 13, 2016, at 4:25 AM, Patrik Flykt <[email protected]> 
> wrote:
> 
> 
>       Hi,
> 
> On Tue, 2016-06-28 at 14:34 -0700, Feng Wang wrote:
>> Based on RFC 3315, 22.6, the valid life-time is infinite when its
>> value is 0xffffffff. In the g_dhcpv6_client_get_timeouts, the expire
>> data type is time_t. If time_t is uint32, the last_request time plus
>> 0xffffffff will wrapover so that expire time is smaller than current
>> time. Thus the dhcpv6 will restart immediately(dhcpv6_restart called).
>> ---
>>  gdhcp/client.c | 9 +++++++--
>>  src/dhcpv6.c   | 6 +++++-
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/gdhcp/client.c b/gdhcp/client.c
>> index 9012b38..2be3982 100644
>> --- a/gdhcp/client.c
>> +++ b/gdhcp/client.c
>> @@ -835,8 +835,13 @@ int g_dhcpv6_client_get_timeouts(GDHCPClient 
>> *dhcp_client,
>>      if (started)
>>              *started = dhcp_client->last_request;
>>  
>> -    if (expire)
>> -            *expire = dhcp_client->last_request + dhcp_client->expire;
>> +    if (expire) {
>> +            if (dhcp_client->expire == 0xffffffff)
>> +                    /* RFC3315 22.6 infinite valid-lifetime */
>> +                    *expire = 0xffffffff;
>> +            else
>> +                    *expire = dhcp_client->last_request + 
>> dhcp_client->expire;
>> +    }
> 
> Coming back to this, RFC 3315, Section 22.5, indicates that "...If the
> "shortest" preferred lifetime is 0xffffffff ("infinity"), the
> recommended T1 and T2 values are also 0xffffffff..."
> 
> So this all could be made a bit easier by always setting T1, T2 and
> expire to 0xffffffff when dhcp->expire is 0xffffffff.
> 
> And this would be patch #2. 
> 
>>      return 0;
>>  }
>> diff --git a/src/dhcpv6.c b/src/dhcpv6.c
>> index 9e21040..cd5733a 100644
>> --- a/src/dhcpv6.c
>> +++ b/src/dhcpv6.c
>> @@ -1195,7 +1195,7 @@ static int check_restart(struct connman_dhcpv6 *dhcp)
>>                              NULL, &expired);
>>      current = time(NULL);
>>  
>> -    if (current >= expired) {
>> +    if (current >= expired && expired != 0xffffffff) {
>>              DBG("expired by %d secs", (int)(current - expired));
>>  
>>              g_timeout_add(0, dhcpv6_restart, dhcp);
>> @@ -1442,6 +1442,10 @@ int __connman_dhcpv6_start_renew(struct 
>> connman_network *network,
>>              /* RFC 3315, 22.4
>>               * Client can choose the timeout.
>>               */
>> +            if (expired == 0xffffffff) {
>> +                /* RFC 3315, 22.6 infinite valid-lifetime */
>> +                return 0;
>> +            }
>>              T1 = (expired - started) / 2;
>>              T2 = (expired - started) / 10 * 8;
>>      }
> 
> All instances where T1 and T2 are handled should be checked for
> "infinity" as RFC 3315, Section 22.4 also says that "...a client will
> never attempt to extend the lifetimes of any addresses in an IA with T1
> set to 0xffffffff.  A client will never attempt to use a Rebind message
> to locate a different server to extend the lifetimes of any addresses
> in an IA with T2 set to 0xffffffff...". To get to this behavior, all
> usage of T1 and T2 should be checked.
> 
> This would be then patch #1.
> 
> 
> Cheers,
> 
>       Patrik
> 



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

Message: 2
Date: Thu, 14 Jul 2016 13:13:13 +0300
From: Jaakko Hannikainen <[email protected]>
To: [email protected]
Subject: [PATCH] config: Fix indentation
Message-ID: <[email protected]>

gcc 6.1.1 complains about misleading indentation and halts because of
-Werror. Fix the indentation.
---
 src/config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/config.c b/src/config.c
index dcbee06..ba10fbb 100644
--- a/src/config.c
+++ b/src/config.c
@@ -716,9 +716,9 @@ static bool load_service(GKeyFile *keyfile, const char 
*group,
                                        "setting %s = %s",
                                        SERVICE_KEY_SECURITY, str);
 
-                       service->security = CONNMAN_SERVICE_SECURITY_NONE;
+               service->security = CONNMAN_SERVICE_SECURITY_NONE;
        } else
-                       service->security = CONNMAN_SERVICE_SECURITY_NONE;
+               service->security = CONNMAN_SERVICE_SECURITY_NONE;
 
        g_free(str);
 
-- 
2.9.0



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

Message: 3
Date: Thu, 14 Jul 2016 14:52:12 +0300
From: Patrik Flykt <[email protected]>
To: [email protected],       [email protected]
Subject: [PATCH 0/2] DHCPv6 infinite expiry times
Message-ID:
        <[email protected]>


        Hi,

This patch set is heavily influenced by the patch set from
[email protected]. The difference is that T1, T2 and expiry
timeouts are all set to 0xffffffff (infinite) when the expiry
time is infinite. With this it is believed any further changes
will be much smaller, as the code already checks for T1 and
T2 being unequal to 0xffffffff.

Wang Feng, does this work with your setup?

Cheers,

       Patrik



Patrik Flykt (2):
  dhcpv6: Return -EISCONN when the expiry time is inifinite
  gdhcp: Set T1 and T2 to infinite if expiry time is infinite

 gdhcp/client.c | 9 ++++++---
 src/dhcpv6.c   | 5 +++++
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.8.1



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

Message: 4
Date: Thu, 14 Jul 2016 14:52:14 +0300
From: Patrik Flykt <[email protected]>
To: [email protected],       [email protected]
Subject: [PATCH 2/2] gdhcp: Set T1 and T2 to infinite if expiry time
        is infinite
Message-ID:
        <[email protected]>

RFC 3315, Section 22.5, indicates that

..If the "shortest" preferred lifetime is 0xffffffff ("infinity"),
the recommended T1 and T2 values are also 0xffffffff...

Use value 0xffffffff to indicate inifinite lease time and infinite
T1 and T2 to simplify the decision making in the caller of this
function.

Based on a patch by [email protected]
---
 gdhcp/client.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 9012b38..fbb40ab 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -827,16 +827,19 @@ int g_dhcpv6_client_get_timeouts(GDHCPClient *dhcp_client,
                return -EINVAL;
 
        if (T1)
-               *T1 = dhcp_client->T1;
+               *T1 = (dhcp_client->expire == 0xffffffff) ? 0xffffffff:
+                       dhcp_client->T1;
 
        if (T2)
-               *T2 = dhcp_client->T2;
+               *T2 = (dhcp_client->expire == 0xffffffff) ? 0xffffffff:
+                       dhcp_client->T2;
 
        if (started)
                *started = dhcp_client->last_request;
 
        if (expire)
-               *expire = dhcp_client->last_request + dhcp_client->expire;
+               *expire = (dhcp_client->expire == 0xffffffff) ? 0xffffffff:
+                       dhcp_client->last_request + dhcp_client->expire;
 
        return 0;
 }
-- 
2.8.1



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

Message: 5
Date: Thu, 14 Jul 2016 14:52:13 +0300
From: Patrik Flykt <[email protected]>
To: [email protected],       [email protected]
Subject: [PATCH 1/2] dhcpv6: Return -EISCONN when the expiry time is
        inifinite
Message-ID:
        <[email protected]>

When the expiry time for an DHCPv6 address is infinite, no renewals
are needed. Return -EISCONN to notify the callers of this function
that no timeouts need to be set up.

Based on a patch by [email protected]
---
 src/dhcpv6.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/dhcpv6.c b/src/dhcpv6.c
index 9e21040..1bd0000 100644
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -1193,6 +1193,11 @@ static int check_restart(struct connman_dhcpv6 *dhcp)
 
        g_dhcpv6_client_get_timeouts(dhcp->dhcp_client, NULL, NULL,
                                NULL, &expired);
+
+       /* infinite lifetime for an DHCPv6 address */
+       if (expired == 0xffffffff)
+               return -EISCONN;
+
        current = time(NULL);
 
        if (current >= expired) {
-- 
2.8.1



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

Message: 6
Date: Thu, 14 Jul 2016 11:10:47 -0500
From: David Lechner <[email protected]>
To: [email protected]
Subject: DBus properties race condition
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"; Format="flowed"

Here is what is going on:

When implementing a ConnMan client, the docs say to use, for example, 
Manager.GetTechnologies() instead of the deprecated 
Technology.GetProperties() method. However, if I follow this advice, 
there is a race condition where property changes can be missed. I have 
observed this first hand and I also found a bug against connman-qt that 
is explaining the same situation[1].

Pseudo code goes like this:

    manager_proxy = get_proxy("net.connman", "/", "net.connman.Manager")
    technologies = manager_proxy.GetTechnologies()
    foreach (technology in technologies) {
         technology_proxy = get_proxy("net.connman", technology.Path,
    "net.connman.Technology")
         technology_proxy.connect_signals() *<-- Properties can change
    in the time between the call to GetTechnologies() and when we
    connect signals!*
         technology_proxy.set_properties(technology.properties)
    }


So, to avoid the race condition, I am forced to use the deprecated 
GetProperties() method and ignore the properties from 
Manager.GetTechnology().

    manager_proxy = get_proxy("net.connman", "/", "net.connman.Manager")
    technologies = manager_proxy.GetTechnologies()
    foreach (technology in technologies) {
         technology_proxy = get_proxy("net.connman", technology.Path,
    "net.connman.Technology")
         technology_proxy.connect_signals()
         technology_proxy.set_properties(technology.GetProperties())
    *<-- getting the properties again to avoid race condition*
    }

It is there some other way I should be doing this or is this really an 
oversight in the ConnMan DBus API?


[1]: https://bugs.merproject.org/show_bug.cgi?id=1039
-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://lists.01.org/pipermail/connman/attachments/20160714/64282181/attachment-0001.html>

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

Message: 7
Date: Thu, 14 Jul 2016 11:07:43 -0700
From: Feng Wang <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 1/2] dhcpv6: Return -EISCONN when the expiry time
        is inifinite
Message-ID:
        <CAG09-dJkHUa=zz7c0twzqncht0yhp56tvuxtujkcrjb49w6...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"

Hi Patrik,

I tried the 2 patches.  They are working.

Thanks,

Feng


On Thu, Jul 14, 2016 at 4:52 AM, Patrik Flykt <[email protected]>
wrote:

> When the expiry time for an DHCPv6 address is infinite, no renewals
> are needed. Return -EISCONN to notify the callers of this function
> that no timeouts need to be set up.
>
> Based on a patch by [email protected]
> ---
>  src/dhcpv6.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/dhcpv6.c b/src/dhcpv6.c
> index 9e21040..1bd0000 100644
> --- a/src/dhcpv6.c
> +++ b/src/dhcpv6.c
> @@ -1193,6 +1193,11 @@ static int check_restart(struct connman_dhcpv6
> *dhcp)
>
>         g_dhcpv6_client_get_timeouts(dhcp->dhcp_client, NULL, NULL,
>                                 NULL, &expired);
> +
> +       /* infinite lifetime for an DHCPv6 address */
> +       if (expired == 0xffffffff)
> +               return -EISCONN;
> +
>         current = time(NULL);
>
>         if (current >= expired) {
> --
> 2.8.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://lists.01.org/pipermail/connman/attachments/20160714/79270adb/attachment-0001.html>

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

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


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

End of connman Digest, Vol 9, Issue 7
*************************************

Reply via email to