Re: [PATCH] gdbus: move typedefs for interwork with strict compilers

[Grant Erickson]

On Oct 21, 2015, at 8:40 AM, connman-requ...@connman.net wrote:
> From: Marcel Holtmann 
> Subject: Re: [PATCH] gdbus: move typedefs for interwork with strict compilers
> Date: October 21, 2015 at 3:35:48 AM PDT
> To: connman@connman.net
> Reply-To: connman@connman.net
> 
> Hi Grant,
> 
>> Move enumeration type defintions AFTER the enumerations themselves are 
>> declared 
>> and defined such that the header works with strict compilers.
> 
> which compiler is this? This code has been in there for a long time and in 
> multiple projects. I have not seen any problems with it.

Marcel,

This occurs when building a plugin, compiled with C++, and occurs on all of:

arm-none-linux-gnueabi-g++ (Sourcery G++ Lite 2010q1-202) 4.4.1
arm-poky-linux-gnueabi-g++ (GCC) 4.8.2
g++ (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4

Best,

Grant

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] gdbus: move typedefs for interwork with strict compilers

[Grant Erickson]

Move enumeration type defintions AFTER the enumerations themselves are declared 
and defined such that the header works with strict compilers.
---
 gdbus/gdbus.h | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index d99c254..7a6a18f 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -31,10 +31,6 @@ extern "C" {
 #include 
 #include 
 
-typedef enum GDBusMethodFlags GDBusMethodFlags;
-typedef enum GDBusSignalFlags GDBusSignalFlags;
-typedef enum GDBusPropertyFlags GDBusPropertyFlags;
-typedef enum GDBusSecurityFlags GDBusSecurityFlags;
 
 typedef struct GDBusArgInfo GDBusArgInfo;
 typedef struct GDBusMethodTable GDBusMethodTable;
@@ -115,6 +111,11 @@ enum GDBusSecurityFlags {
G_DBUS_SECURITY_FLAG_ALLOW_INTERACTION = (1 << 2),
 };
 
+typedef enum GDBusMethodFlags GDBusMethodFlags;
+typedef enum GDBusSignalFlags GDBusSignalFlags;
+typedef enum GDBusPropertyFlags GDBusPropertyFlags;
+typedef enum GDBusSecurityFlags GDBusSecurityFlags;
+
 struct GDBusArgInfo {
const char *name;
const char *signature;
-- 
2.3.5

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: service state transition to failure

[Grant Erickson]

On Oct 13, 2015, at 11:01 PM, Patrik Flykt  wrote:
> On Tue, 2015-10-13 at 14:42 -0700, Naveen Singh wrote:
> 
>> There is nothing that application knows that connman does not know.
>> In fact application gets to know through connman that connection did
>> not go through. The way application gets connected back is to initiate
>> a scan and hoping that one of these scan would find the AP (or
>> services) and then run autoconnect would trigger and get device
>> connected. But in this case run autoconnect would not attempt
>> connection because service state was left to failure.
> 
> This approach won't work at all. When ConnMan receives a Scan() over
> D-Bus, it will scan immediately and reset its own autoscan timer. Which
> means that if the application issues a scan after less than ~6 minutes,
> it causes ConnMan to scan so often that wpa_s never times out its WiFi
> networks. So if the service has failed, it has failed for all eternity.
> 
> The solution here is very simple. Get rid of the application assisted
> scan and let ConnMan handle it instead.

Patrik,

The solution, it turns out, is not that simple, at least for the application at 
hand.

This particular application is embedded, largely user-unattended, and sleepy 
(from a CPU perspective). It must work day-in, day-out, week-in, week-out, 
month-in, and month-out without user intervention after the user has 
interactively established the first initial connection.

Due to these constraints, the application knows best when to scan and does so 
at its discretion. Neither connman nor wpa_s autoscan infrastructure scan at 
the application-desired times. When they do scan automatically, it typically 
results in one or both of an undesirable expenditure of power or a momentary 
and inopportune move off-channel.

Absent the proposed change, per doc/overview-api.txt, with a WiFi network 
continually refreshed, there is no exit path from the Failure service state 
and, consequently, no unattended means by which the application can recover 
WiFi connectivity under these circumstances in a deterministic and 
application-controlled way.

The proposed change offers, for unattended applications that wish to avail 
themselves of it, an exit gate to effectively acknowledge a service error by 
clearing it. There is no downside or consequence to interactive applications 
that wish to continue to operate as they do today.

Best,

Grant


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [RFC] Exposing network credentials to an application

[Grant Erickson]

On Jul 8, 2014, at 5:39 AM, Tomasz Bursztyka  
wrote:
>> Thanks Grant for the reply. The config option sounds right to me as a
>> general solution. Connman folks, thoughts?
> 
> I don't think we will ever put this PassPhrase property back again.
> And we don't do #ifdef here and there for small configurable parts.
> 
> Too few real use cases, which could be themselves be solved out of ConnMan.

Tomasz,

Thanks for the reply. As a clarification, this wasn't proposed as an 
preprocessor conditional. The proposal at hand is for a connman.conf/main.conf 
runtime configuration option, defaulting to disabled. There are ample examples 
of using the configuration file for such things today.

As for use cases, I believe Drew and I have just presented a very real world 
and credible use case. This use case, when measured in shipping unit volumes 
that likely continues to make it one of the largest single (and possibly 
aggregate) connman implementations, is all the more real world.

What alternative proposals would you suggest that provide similar functionality 
and utility in a manner that do not rely on side-channel approaches leveraging 
unofficial, unsupported implementation internals of connman?

Best,

Grant
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [RFC] Exposing network credentials to an application

[Grant Erickson]

On Jun 27, 2014, at 12:00 PM, Jukka Rissanen  
wrote:
> On to, 2014-06-26 at 16:52 -0700, Drew Stebbins wrote:
>> Hi folks,
>> 
>> I'm working on a project where devices on a WiFi network must occasionally
>> provide an application with the WiFi network's security credentials. These
>> credentials are used to provision new devices via an alternate
>> communication mechanism. For consistency's sake, I'd like for the
>> application to get these credentials from ConnMan via D-Bus. For now, I've
>> added a "Passphrase" property back to the D-Bus service dictionary for WiFi
>> networks. This property was previously removed back in commit
>> eff06f8e1ce3c3dcf5a3d8158d01403ecf6d62a8.
> 
> Although the Passphrase is not very secret information it was too easy
> to get it via dbus without any apparent real use case in mind. That is
> probably why it was removed from dubs.

Certainly the one use case that was prevalent around the time of this refactor 
was agent support. In the particular use case where an old PSK fails and the 
agent interface prompts the client for a new one, the old PSK is provided in an 
adjunct dictionary for the purposes of rendering it directly or substituted by 
bullets (•) in a GUI.

>> The above commit would seem to imply that sensitive information such as
>> passphrases should be kept out of the publicly-visible service dictionary.
>> If so, how would the community suggest ConnMan expose network credentials
>> to an application? I'd like to upstream this functionality if at all
>> possible, as I suspect I'm not the only one with this sort of use case.
> 
> What kind of use case you have in mind? Also note that most people would
> probably not want to expose this information via dubs.
> 
> If you are really eager to get passphrase information, you can always
> dig the settings files in /var/lib/connman directory.

Imagine a deeply-embedded system with WiFi in which there is no physical user 
interface on the device whatsoever. Now, imagine an out-of-band channel in 
which a mobile device, with appropriate authorization over that out-of-band 
channel, would like to extract the currently-provisioned WiFi services for the 
purposes of provisioning another new system.

Imagine another system, different from a mobile device, perhaps like the first 
that can communicate and authenticate with it out-of-band, based on some 
provided user authority. Over that channel, the first device would like to tell 
the second, "When you encounter this WiFi network, you may connect to it with 
these credentials.".

While you're correct that one could certainly root about int he 
/var/lib/connman directory and pull this, such a mechanism will not survive 
connman version changes due to a high-reliance on internal details that are 
subject to change by virtue of being not being part of connman's public API.

Perhaps there could be a config option, deserted by default, that allows this 
property to be exposed when that configuration is asserted?

Best,

Grant
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [RFC 0/3] Publish dhcp statistics to dbus

[Grant Erickson]

On Nov 4, 2013, at 12:20 AM, Patrik Flykt wrote:
> On Fri, 2013-11-01 at 13:54 -0700, Andrew LeCain wrote:
>> 
>> We do indeed have a sleep manager that will put our system to deep
>> sleep/suspend. This sleep manager also uses the RTC to schedule a wake
>> for
>> future tasks. However, the wifi chip we use performs suspend-idle
>> association and is configured with wake-on-WLAN filters to wake the
>> host
>> for certain network traffic addressed to the device. This allows us to
>> keep
>> a consistent association to the network despite the primary processor
>> being
>> asleep.
> 
> Indeed I suspected this to be the case. Thanks for the explanation!
> 
>> Unless the DHCP lease statistics are known. it's not possible for the
>> sleep
>> manager to schedule wakeups to renew the DHCP lease. If there are no
>> scheduled wakeup events that occur during the renewal period of the
>> lease
>> then we will lose our lease and no longer are woken up by packets
>> addressed
>> to us. Additionally, we don't want to suspend in the middle of a DHCP
>> renewal, and therefore need to be able to query the DHCP state from
>> our
>> client. It doesn't appear that the session api provides this
>> information.
> 
> The API does not provide this information as it really is not a useful
> metric.
> 
> What you actually must do is a separate plugin in plugins/ that properly
> takes care of negotiating sleep states with this external daemon. This
> way also other events considered important enough can be taken care of
> later on. With a specific plugin it will be clear what is going on;
> extra properties that have no direct connection with or used by anything
> else have a tendency to be plainly broken or just removed. Adding DHCP
> stats that are very important but implicit in their nature for a
> specific use case is not the way forward.

Plug-in or not, DHCP will have to expose some state. In the proposal on the 
table, that exposure is via the IPv4 dictionary. In the plug-in 
counterproposal, some API provided exposure will need to be provided such that 
the plug-in may use to negotiate with the external daemon.

> The next issue that comes to mind is how a system "sleep API" like this
> can be generalized to be useful for other daemons.

This sounds like a nice opportunity for future architectural enhancement; 
however, it seems a bit out of scope based on the proposal on the table.

In a "CanSuspend" / "WillSuspend" type registration entity within connman to 
support something like this, do you envision anything beyond DHCPv4 would 
register with it?

Best,

Grant
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Setting Network Enabled to False Disables via DBus Disables the SSID Forever

[Grant Erickson]

Patrik:

I am debugging an issue in which there are reports of not being able to connect 
to or see a particular SSID scanned (let me know if you want specific details) 
for an extended period (several hours).

I've been able to correlate them back to this check-in:

commit b0cb897fe9fc4d29b07eb1356d61f641cbe594a1
Author: Patrik Flykt 
Date:   Fri Dec 2 13:55:36 2011 +0200

wifi: Disable network in disconnected state

Disable WiFi network when it ends up in disconnected state
in order to prevent wpa_supplicant looping forever retrying.

Fixes BMC#23973

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 745c2cb..538230a 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -912,12 +912,6 @@ static connman_bool_t 
handle_4way_handshake_failure(GSupplicantInterface *interf
if (wifi->retries < MAXIMUM_RETRIES)
return TRUE;
 
-   /* We disable the selected network, if not then
-* wpa_supplicant will loop retrying */
-   if (g_supplicant_interface_enable_selected_network(interface,
-   FALSE) != 0)
-   DBG("Could not disables selected network");
-
connman_network_set_error(network, 
CONNMAN_NETWORK_ERROR_INVALID_KEY);
 
return FALSE;
@@ -987,6 +981,12 @@ static void interface_state(GSupplicantInterface 
*interface)
network, wifi) == TRUE)
break;
 
+   /* We disable the selected network, if not then
+* wpa_supplicant will loop retrying */
+   if (g_supplicant_interface_enable_selected_network(interface,
+   FALSE) != 0)
+   DBG("Could not disables selected network");
+
connman_network_set_associating(network, FALSE);
connman_network_set_connected(network, FALSE);
break;

Under what circumstances would such a network get re-enabled? As far as I can 
tell, a complete system reboot or network stack restart is it; otherwise, that 
SSID / network appears to be disabled  indefinitely.

Thoughts or insights?

Best,

Grant
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

RE: wifi: scan all stored hidden networks relevantly

[Grant Erickson]

On Thu May 16 05:32:29 PDT 2013, Tomasz Bursztyka 
 wrote:
> Currently a scan will only scan the first found stored hidden network.
> This patch fixes it: it will scan all, taking into account the limit of
> scan parameters the driver can take.
> ---
> @@ -548,32 +640,21 @@ static void scan_callback_hidden(int result,
>   struct connman_device *device = user_data;
>   struct wifi_data *wifi = connman_device_get_data(device);
>   GSupplicantScanParams *scan_params;
> - int driver_max_ssids, ret;
> + int ret;
>  
>   DBG("result %d wifi %p", result, wifi);
>  
>   if (wifi == NULL)
>   goto out;
>  
> - /*
> -  * Scan hidden networks so that we can autoconnect to them.
> -  * We will assume 1 as a default number of ssid to scan.
> -  */
> - driver_max_ssids = g_supplicant_interface_get_max_scan_ssids(
> - wifi->interface);
> - if (driver_max_ssids == 0)
> - driver_max_ssids = 1;
> -
> - DBG("max ssids %d", driver_max_ssids);
> -
>   scan_params = g_try_malloc0(sizeof(GSupplicantScanParams));
>   if (scan_params == NULL)
>   goto out;
>  
> - if (get_hidden_connections(driver_max_ssids, scan_params) > 0) {
> + if (get_hidden_connections_params(wifi, scan_params) > 0) {
>   ret = g_supplicant_interface_scan(wifi->interface,
>   scan_params,
> - scan_callback,
> + scan_callback_hidden,

Tomasz,

I found this surprising. Why did the callback change from scan_callback to 
scan_callback_hidden? Isn't the appropriate closure for the scan, scan_callback?

Best,

Grant
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] wifi: improve fix to avoid device double release on disable

[Grant Erickson]

Per MEEGO-25999 , there
exists a race between scan_callback and wifi_disable such that a care-
fully-timed deassertion of the technology Powered property between
throw_wifi_scan and scan_callback will lead to a device reference
count underflow. A second Powered property false -> true -> false
transition will then deallocate the device and Wi-Fi will no longer
function until connman or the system is restarted.

This patch improves upon and revises the prior patch to address this
issue by recognizing that hidden networks involve a slightly different
code path and timing and that both this and the prior case can be
addressed by tracking the device scanning state and only releasing the
device when the state is asserted.
---
 plugins/wifi.c | 46 +++---
 1 file changed, 11 insertions(+), 35 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index f862703..07180d6 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -93,7 +93,6 @@ struct wifi_data {
GSList *networks;
GSupplicantInterface *interface;
GSupplicantState state;
-   connman_bool_t disabling;
connman_bool_t connected;
connman_bool_t disconnecting;
connman_bool_t tethering;
@@ -172,7 +171,6 @@ static int wifi_probe(struct connman_device *device)
if (wifi == NULL)
return -ENOMEM;
 
-   wifi->disabling = FALSE;
wifi->connected = FALSE;
wifi->disconnecting = FALSE;
wifi->tethering = FALSE;
@@ -512,6 +510,7 @@ static void scan_callback(int result, GSupplicantInterface 
*interface,
 {
struct connman_device *device = user_data;
struct wifi_data *wifi = connman_device_get_data(device);
+   connman_bool_t scanning;
 
DBG("result %d wifi %p", result, wifi);
 
@@ -524,18 +523,22 @@ static void scan_callback(int result, 
GSupplicantInterface *interface,
if (result < 0)
connman_device_reset_scanning(device);
 
-   connman_device_set_scanning(device, FALSE);
+   scanning = connman_device_get_scanning(device);
+
+   if (scanning == TRUE)
+   connman_device_set_scanning(device, FALSE);
 
if (result != -ENOLINK)
start_autoscan(device);
 
/*
-* If we are here then we were scanning; however, if we are also
-* mid-flight disabling the interface, then wifi_disable has
-* already unreferenced the device and we needn't do it here.
+* If we are here then we were scanning; however, if we are
+* also mid-flight disabling the interface, then wifi_disable
+* has already cleared the device scanning state and
+* unreferenced the device, obviating the need to do it here.
 */
 
-   if (wifi->disabling != TRUE)
+   if (scanning == TRUE)
connman_device_unref(device);
 }
 
@@ -733,27 +736,6 @@ static void interface_create_callback(int result,
}
 }
 
-/*
- * The sole function of this callback is to avoid a race between scan 
completion
- * and wifi_disable that can otherwise cause a reference count underflow if the
- * disabling state is not tracked and observed.
- */
-static void interface_remove_callback(int result,
-   GSupplicantInterface *interface,
-   void *user_data)
-{
-   struct wifi_data *wifi = user_data;
-
-   DBG("result %d ifname %s, wifi %p", result,
-   g_supplicant_interface_get_ifname(interface),
-   wifi);
-
-   if (result < 0 || wifi == NULL)
-   return;
-
-   wifi->disabling = FALSE;
-}
-
 static int wifi_enable(struct connman_device *device)
 {
struct wifi_data *wifi = connman_device_get_data(device);
@@ -772,8 +754,6 @@ static int wifi_enable(struct connman_device *device)
if (ret < 0)
return ret;
 
-   wifi->disabling = FALSE;
-
return -EINPROGRESS;
 }
 
@@ -803,14 +783,10 @@ static int wifi_disable(struct connman_device *device)
 
remove_networks(device, wifi);
 
-   ret = g_supplicant_interface_remove(wifi->interface,
-   interface_remove_callback,
-   wifi);
+   ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
if (ret < 0)
return ret;
 
-   wifi->disabling = TRUE;
-
return -EINPROGRESS;
 }
 
-- 
1.8.1.1

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] wifi: avoid device double release on disable

[Grant Erickson]

Per MEEGO-25999 , there
exists a race between scan_callback and wifi_disable such that a care-
fully-timed deassertion of the technology Powered property between
throw_wifi_scan and scan_callback will lead to a device reference
count underflow. A second Powered property false -> true -> false
transition will then deallocate the device and Wi-Fi will no longer
function until connman or the system is restarted.

This patch addresses that race by tracking and observing the disabling
state such that scan_callback ad wifi_disable can effectively negotiate
which has performed the device release, thereby avoiding the double
release and resulting underflow.
---
 plugins/wifi.c | 40 ++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index a0cb966..8758353 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -93,6 +93,7 @@ struct wifi_data {
GSList *networks;
GSupplicantInterface *interface;
GSupplicantState state;
+   connman_bool_t disabling;
connman_bool_t connected;
connman_bool_t disconnecting;
connman_bool_t tethering;
@@ -171,6 +172,7 @@ static int wifi_probe(struct connman_device *device)
if (wifi == NULL)
return -ENOMEM;
 
+   wifi->disabling = FALSE;
wifi->connected = FALSE;
wifi->disconnecting = FALSE;
wifi->tethering = FALSE;
@@ -527,7 +529,14 @@ static void scan_callback(int result, GSupplicantInterface 
*interface,
if (result != -ENOLINK)
start_autoscan(device);
 
-   connman_device_unref(device);
+   /*
+* If we are here then we were scanning; however, if we are also
+* mid-flight disabling the interface, then wifi_disable has
+* already unreferenced the device and we needn't do it here.
+*/
+
+   if (wifi->disabling != TRUE)
+   connman_device_unref(device);
 }
 
 static void scan_callback_hidden(int result,
@@ -724,6 +733,27 @@ static void interface_create_callback(int result,
}
 }
 
+/*
+ * The sole function of this callback is to avoid a race between scan 
completion
+ * and wifi_disable that can otherwise cause a reference count underflow if the
+ * disabling state is not tracked and observed.
+ */
+static void interface_remove_callback(int result,
+   GSupplicantInterface *interface,
+   void *user_data)
+{
+   struct wifi_data *wifi = user_data;
+
+   DBG("result %d ifname %s, wifi %p", result,
+   g_supplicant_interface_get_ifname(interface),
+   wifi);
+
+   if (result < 0 || wifi == NULL)
+   return;
+
+   wifi->disabling = FALSE;
+}
+
 static int wifi_enable(struct connman_device *device)
 {
struct wifi_data *wifi = connman_device_get_data(device);
@@ -742,6 +772,8 @@ static int wifi_enable(struct connman_device *device)
if (ret < 0)
return ret;
 
+   wifi->disabling = FALSE;
+
return -EINPROGRESS;
 }
 
@@ -771,10 +803,14 @@ static int wifi_disable(struct connman_device *device)
 
remove_networks(device, wifi);
 
-   ret = g_supplicant_interface_remove(wifi->interface, NULL, NULL);
+   ret = g_supplicant_interface_remove(wifi->interface,
+   interface_remove_callback,
+   wifi);  
if (ret < 0)
return ret;
 
+   wifi->disabling = TRUE;
+
return -EINPROGRESS;
 }
 
-- 
1.8.1.1

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] wifi: terminate autoscan stop if autoscan is not active

[Grant Erickson]

This terminates the body of stop_autoscan if autoscan is
not active. Per MEEGO-25987 ,
running stop_autoscan when changing connected services can prematurely and
incorrectly terminate a new, in-flight service connection and result in a
failure of that connection with "net.connman.Error.OperationAborted:
Operation aborted".

diff --git a/plugins/wifi.c b/plugins/wifi.c
index dd79cd3..a0cb966 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -233,6 +233,11 @@ static void reset_autoscan(struct connman_device *device)
 
 static void stop_autoscan(struct connman_device *device)
 {
+   const struct wifi_data *wifi = connman_device_get_data(device);
+
+   if (wifi == NULL || wifi->autoscan == NULL)
+   return;
+
reset_autoscan(device);
 
connman_device_set_scanning(device, FALSE);
-- 
1.8.1.1

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Missing Gateway Property for Service IPv6 Dictionary?

[Grant Erickson]

On Dec 27, 2012, at 9:54 AM, Marcel Holtmann  wrote:
>> Is there any reason why a "Gateway" property is missing from the
>> service "IPv6" dictionary? Is this assumed to be implicit from the
>> address prefix and therefore unnecessary?
> 
> it is documented in the API. If it is not returned that normally means
> we do not have it (or we have a bug here). Is that IPv6 connection for
> some reason link-local only?

Marcel,

I think it is a bug. If I run 'ip -6 route show', I see non-link-local default 
routes for both the eth0 and wlan0 interfaces with a prefix of 2601:9:3e00:49 
(see https://bugs.meego.com/attachment.cgi?id=10070) from 25929 
(https://bugs.meego.com/show_bug.cgi?id=25929).

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Missing Gateway Property for Service IPv6 Dictionary?

[Grant Erickson]

Is there any reason why a "Gateway" property is missing from the service
"IPv6" dictionary? Is this assumed to be implicit from the address prefix
and therefore unnecessary?

-Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Connman Periodically, Incorrectly Disables IPv6 Support

[Grant Erickson]

I've been chasing down for the past few days an issue in which I thought the
kernel, but it now appears to be connman, was periodically and incorrectly
disabling IPv6 support. For those interested in following the thread, more
at:

https://bugs.meego.com/show_bug.cgi?id=25929

Best,

Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v3] service: signal error property changes

[Grant Erickson]

In order to allow clients to update the property in a manner similar to
other service properties, signal changes in the service "Error" property
key/value pair.

The value "" is used when no error is asserted because D-Bus property
changed convention has no semantics for indicating a key/value pair was
removed.
---

 v1: Initial submission.
 v2: Incorporated stylistic feedback from Patrik Flykt.
 v3: Incorporated more feedback from Patrik Flykt.

 src/service.c |   35 ++-
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/service.c b/src/service.c
index 9275e54..d0843fc 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3206,11 +3206,34 @@ static DBusMessage *set_property(DBusConnection *conn,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static void set_error(struct connman_service *service,
+   enum connman_service_error error)
+{
+   const char *str;
+
+   if (service->error == error)
+   return;
+
+   service->error = error;
+
+   if (service->path == NULL)
+   return;
+
+   str = error2string(service->error);
+
+   if (str == NULL)
+   str = "";
+
+   connman_dbus_property_changed_basic(service->path,
+   CONNMAN_SERVICE_INTERFACE, "Error",
+   DBUS_TYPE_STRING, &str);
+}
+
 static void set_idle(struct connman_service *service)
 {
service->state = service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_IDLE;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
state_changed(service);
 }
 
@@ -4114,6 +4137,8 @@ static void service_initialize(struct connman_service 
*service)
service->refcount = 1;
service->session_usage_count = 0;
 
+   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+
service->type = CONNMAN_SERVICE_TYPE_UNKNOWN;
service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
 
@@ -4693,7 +4718,7 @@ static void request_input_cb (struct connman_service 
*service,
  done:
if (err >= 0) {
/* We forget any previous error. */
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
__connman_service_connect(service);
 
@@ -4943,7 +4968,7 @@ static int service_indicate_state(struct connman_service 
*service)
return 0;
service_complete(service);
} else
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
iter = g_hash_table_lookup(service_hash, service->identifier);
if (iter != NULL && g_sequence_get_length(service_list) > 1) {
@@ -4969,7 +4994,7 @@ int __connman_service_indicate_error(struct 
connman_service *service,
if (service == NULL)
return -EINVAL;
 
-   service->error = error;
+   set_error(service, error);
 
if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
__connman_service_set_passphrase(service, NULL);
@@ -4995,8 +5020,7 @@ int __connman_service_clear_error(struct connman_service 
*service)
 
service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_UNKNOWN;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
if (service->favorite == TRUE)
set_reconnect_state(service, TRUE);
--
1.7.10
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2] service: signal error property changes

[Grant Erickson]

On Aug 8, 2012, at 3:23 AM, connman-requ...@connman.net wrote:
> From: Patrik Flykt 
> Subject: Re: [PATCH v2] service: signal error property changes
> Date: August 8, 2012 3:15:39 AM PDT
> To: connman 
> Reply-To: connman@connman.net
> 
> Noticed one issue, and some nitpicks.
> 
> On Mon, 2012-08-06 at 09:28 -0700, Grant Erickson wrote:
>> 
>> static void set_idle(struct connman_service *service)
>> {
>>  service->state = service->state_ipv4 = service->state_ipv6 =
>>  CONNMAN_SERVICE_STATE_IDLE;
>> -service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>>  state_changed(service);
>> }
>> 
>> @@ -4100,6 +4125,8 @@ static void service_initialize(struct connman_service 
>> *service)
>>  service->refcount = 1;
>>  service->session_usage_count = 0;
>> 
>> +service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +
>>  service->type = CONNMAN_SERVICE_TYPE_UNKNOWN;
>>  service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
>> 
>> @@ -4679,7 +4706,7 @@ static void request_input_cb (struct connman_service 
>> *service,
>>  done:
>>  if (err >= 0) {
>>  /* We forget any previous error. */
>> -service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
>> +set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>> 
>>  __connman_service_connect(service);
> 
> It'd be better to call set_error() in __connman_service_connect() since
> it clears the error in all of the service connect cases, e.g on
> autoconnect, VPNs, Session API use, etc. This one I missed last time,
> sorry for that.

Patrik,

Thanks for the feedback and review. Regarding the last comment, I'd prefer 
addressing that in a second, orthogonal patch as that does more than just 
replace existing assignments with the function call.

What do you think?

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v4] main: add support for specifying a non-default configuration file.

[Grant Erickson]

This patch adds support for specifying a main configuration file
different from the default value of /etc/connman/main.conf. This
is valuable during debug to specify alternate debug-only
configurations.
---
  v1: Initial submission.
  v2: Incorporated feedback from Marcel about not propagating the
  fully-qualified default configuration file path on the compiler
  command line.
  v3: Incorporated stylistic feedback from Patrik Flykt.
  v4: Incorporated --file/-f -> --config/-c change feedback from Marcel 
Holtmann.

diff --git a/src/main.c b/src/main.c
index eeb2e58..1e4809f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -42,6 +42,8 @@
 #define DEFAULT_INPUT_REQUEST_TIMEOUT 120 * 1000
 #define DEFAULT_BROWSER_LAUNCH_TIMEOUT 300 * 1000
 
+#define CONFIGMAINFILE CONFIGDIR "/main.conf"
+
 static char *default_auto_connect[] = {
"wifi",
"ethernet",
@@ -243,6 +245,18 @@ static void parse_config(GKeyFile *config)
g_clear_error(&error);
 }
 
+static int config_init(const char *file)
+{
+   GKeyFile *config;
+
+   config = load_config(file);
+   parse_config(config);
+   if (config != NULL)
+   g_key_file_free(config);
+
+   return 0;
+}
+
 static GMainLoop *main_loop = NULL;
 
 static unsigned int __terminated = 0;
@@ -322,6 +336,7 @@ static void disconnect_callback(DBusConnection *conn, void 
*user_data)
g_main_loop_quit(main_loop);
 }
 
+static gchar *option_config = NULL;
 static gchar *option_debug = NULL;
 static gchar *option_device = NULL;
 static gchar *option_plugin = NULL;
@@ -344,6 +359,9 @@ static gboolean parse_debug(const char *key, const char 
*value,
 }
 
 static GOptionEntry options[] = {
+   { "config", 'c', 0, G_OPTION_ARG_STRING, &option_config,
+   "Load the specified configuration file "
+   "instead of " CONFIGMAINFILE, "FILE" },
{ "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
G_OPTION_ARG_CALLBACK, parse_debug,
"Specify debug options to enable", "DEBUG" },
@@ -427,7 +445,6 @@ int main(int argc, char *argv[])
GError *error = NULL;
DBusConnection *conn;
DBusError err;
-   GKeyFile *config;
guint signal;
 
 #ifdef NEED_THREADS
@@ -504,10 +521,10 @@ int main(int argc, char *argv[])
 
__connman_dbus_init(conn);
 
-   config = load_config(CONFIGDIR "/main.conf");
-   parse_config(config);
-   if (config != NULL)
-   g_key_file_free(config);
+   if (option_config == NULL)
+   config_init(CONFIGMAINFILE);
+   else
+   config_init(option_config);
 
__connman_storage_migrate();
__connman_technology_init();
@@ -547,6 +564,7 @@ int main(int argc, char *argv[])
__connman_wispr_init();
__connman_rfkill_init();
 
+   g_free(option_config);
g_free(option_device);
g_free(option_plugin);
g_free(option_nodevice);
--
1.7.10
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] service: signal error property changes

[Grant Erickson]

In order to allow clients to update the property in a manner similar to
other service properties, signal changes in the service "Error" property
key/value pair.

The value "none" is used when no error is asserted because D-Bus property
changed convention has no semantics for indicating a key/value pair was
removed.
---

 v1: Initial submission.
 v2: Incorporated stylistic feedback from Patrik Flykt.

 src/service.c |   37 -
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/src/service.c b/src/service.c
index dfe1752..219f05c 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3205,11 +3205,36 @@ static DBusMessage *set_property(DBusConnection *conn,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static void set_error(struct connman_service *service,
+   enum connman_service_error error)
+{
+   const char *str, **strp;
+   const char *none = "";
+
+   if (service->error != error) {
+   service->error = error;
+
+   if (service->path == NULL)
+   return;
+
+   str = error2string(service->error);
+
+   if (str == NULL)
+   strp = &none;
+   else
+   strp = &str;
+
+   connman_dbus_property_changed_basic(service->path,
+   CONNMAN_SERVICE_INTERFACE, "Error",
+   DBUS_TYPE_STRING, strp);
+   }
+}
+
 static void set_idle(struct connman_service *service)
 {
service->state = service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_IDLE;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
state_changed(service);
 }
 
@@ -4100,6 +4125,8 @@ static void service_initialize(struct connman_service 
*service)
service->refcount = 1;
service->session_usage_count = 0;
 
+   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+
service->type = CONNMAN_SERVICE_TYPE_UNKNOWN;
service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
 
@@ -4679,7 +4706,7 @@ static void request_input_cb (struct connman_service 
*service,
  done:
if (err >= 0) {
/* We forget any previous error. */
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
__connman_service_connect(service);
 
@@ -4929,7 +4956,7 @@ static int service_indicate_state(struct connman_service 
*service)
return 0;
service_complete(service);
} else
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
iter = g_hash_table_lookup(service_hash, service->identifier);
if (iter != NULL && g_sequence_get_length(service_list) > 1) {
@@ -4955,7 +4982,7 @@ int __connman_service_indicate_error(struct 
connman_service *service,
if (service == NULL)
return -EINVAL;
 
-   service->error = error;
+   set_error(service, error);
 
if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
__connman_service_set_passphrase(service, NULL);
@@ -4981,7 +5008,7 @@ int __connman_service_clear_error(struct connman_service 
*service)
 
service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_UNKNOWN;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
if (service->favorite == TRUE)
set_reconnect_state(service, TRUE);
--
1.7.10
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v3] main: add support for specifying a non-default configuration file.

[Grant Erickson]

On Aug 6, 2012, at 9:07 AM, Marcel Holtmann wrote:
>> @@ -350,6 +365,9 @@ static GOptionEntry options[] = {
>>  "Specify debug options to enable", "DEBUG" },
>>  { "device", 'i', 0, G_OPTION_ARG_STRING, &option_device,
>>  "Specify networking device or interface", "DEV" },
>> +{ "file", 'f', 0, G_OPTION_ARG_STRING, &option_file,
>> +"Load the specified configuration file "
>> +"instead of " CONFIGMAINFILE, "FILE" },
> 
> we could just start using --config / -c here. The --file does not sound
> that good to me.

I chose '--file / -f' because it didn't conflict with the existing '--compat / 
-c' and had precedence from avahi-daemon and, where possible, I like to follow 
the path of well-worn user muscle memory.

I someone want to submit a patch to obliterate '--compat / -c', since it's 
marked as obsolete, I am happy to send along v4 with '--config / -c'.

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v3] main: add support for specifying a non-default configuration file.

[Grant Erickson]

This patch adds support for specifying a main configuration file
different from the default value of /etc/connman/main.conf. This
is valuable during debug to specify alternate debug-only
configurations.
---
  v1: Initial submission.
  v2: Incorporated feedback from Marcel about not propagating the
  fully-qualified default configuration file path on the compiler
  command line.
  v3: Incorporated stylistic feedback from Patrik Flykt.

 src/main.c |   28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/main.c b/src/main.c
index 5c206e7..3baf61b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -42,6 +42,8 @@
 #define DEFAULT_INPUT_REQUEST_TIMEOUT 120 * 1000
 #define DEFAULT_BROWSER_LAUNCH_TIMEOUT 300 * 1000
 
+#define CONFIGMAINFILE CONFIGDIR "/main.conf"
+
 static char *default_auto_connect[] = {
"wifi",
"ethernet",
@@ -243,6 +245,18 @@ static void parse_config(GKeyFile *config)
g_clear_error(&error);
 }
 
+static int config_init(const char *file)
+{
+   GKeyFile *config;
+
+   config = load_config(file);
+   parse_config(config);
+   if (config != NULL)
+   g_key_file_free(config);
+
+   return 0;
+}
+
 static GMainLoop *main_loop = NULL;
 
 static unsigned int __terminated = 0;
@@ -324,6 +338,7 @@ static void disconnect_callback(DBusConnection *conn, void 
*user_data)
 
 static gchar *option_debug = NULL;
 static gchar *option_device = NULL;
+static gchar *option_file = NULL;
 static gchar *option_plugin = NULL;
 static gchar *option_nodevice = NULL;
 static gchar *option_noplugin = NULL;
@@ -350,6 +365,9 @@ static GOptionEntry options[] = {
"Specify debug options to enable", "DEBUG" },
{ "device", 'i', 0, G_OPTION_ARG_STRING, &option_device,
"Specify networking device or interface", "DEV" },
+   { "file", 'f', 0, G_OPTION_ARG_STRING, &option_file,
+   "Load the specified configuration file "
+   "instead of " CONFIGMAINFILE, "FILE" },
{ "nodevice", 'I', 0, G_OPTION_ARG_STRING, &option_nodevice,
"Specify networking interface to ignore", "DEV" },
{ "plugin", 'p', 0, G_OPTION_ARG_STRING, &option_plugin,
@@ -430,7 +448,6 @@ int main(int argc, char *argv[])
GError *error = NULL;
DBusConnection *conn;
DBusError err;
-   GKeyFile *config;
guint signal;
 
 #ifdef NEED_THREADS
@@ -507,10 +524,10 @@ int main(int argc, char *argv[])
 
__connman_dbus_init(conn);
 
-   config = load_config(CONFIGDIR "/main.conf");
-   parse_config(config);
-   if (config != NULL)
-   g_key_file_free(config);
+   if (option_file == NULL)
+   config_init(CONFIGMAINFILE);
+   else
+   config_init(option_file);
 
__connman_storage_migrate();
__connman_technology_init();
@@ -551,6 +568,7 @@ int main(int argc, char *argv[])
__connman_rfkill_init();
 
g_free(option_device);
+   g_free(option_file);
g_free(option_plugin);
g_free(option_nodevice);
g_free(option_noplugin);
--
1.7.10
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

RE: ConnMan 1.3 and Wifi

[Grant Erickson]

On Fri Jul 20 04:50:07 PDT 2012, Daniel Wagner  wrote:
> Hi Grant,
>
> you reported on IRC that you have troubles with the current HEAD version 
> with your Wifi hardware. Could you post a description what the problem 
> is and add a log? We'd like to resolve this issue if possible.
>
> Thanks a lot!
> 
> cheers,
> daniel

Daniel,

I don't know that I'll have a chance to circle back to this until next week at 
the earliest; however, in testing for the NTP synchronization case, I had a 
loop running in which I did:

cm-connect 

cm-disconnect 
sleep 5

where cm-connect and cm-disconnect are just shell script wrappers around 
dbus-send.

With connman-g4716ee9f + the NTP and gresolv patches, I would reconnect to the 
SSID, successfully complete DHCP and synchronize time every time. However, with 
connman-g13f39a7a, it'd get through association just about every time; however, 
DHCP would fail most of the time and I'd be left with iwconfig reporting no 
valid association. The only way to get out of this state was to reboot the 
system or to restart the networking stack (driver + supplicant + connman) at 
which point it'd connect correctly the first time, but then be very 
inconsistent thereafter.

Had I the time, it'd be reasonable to git bisect from g4716ee9f to g13f39a7a to 
iterate to the commit that broke things.

Best,

Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

RE: [PATCH] wifi: reduce MAXIMUM_RETRIES to 2 retries

[Grant Erickson]

On Jul 19, 2012, at 12:00 PM, Julien Massot  
wrote:
> Some access point exclude station which have tried 3 times a wrong passphrase.
> Decreasing this counter to two gives a chance to the user to connect after
> entering a wrong passphrase.
> ---
> plugins/wifi.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index a6da298..1047202 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -57,7 +57,7 @@
> 
> #define CLEANUP_TIMEOUT   8   /* in seconds */
> #define INACTIVE_TIMEOUT  12  /* in seconds */
> -#define MAXIMUM_RETRIES   4
> +#define MAXIMUM_RETRIES   2
> 
> #define BGSCAN_DEFAULT "simple:30:-45:300"
> #define AUTOSCAN_DEFAULT "exponential:3:300"

Julien:

NACK. IIRC, this value has been adjusted up and down through several iterations 
and four was arrived at as being reasonable based on quite a bit of testing. 
Please check the GIT history and follow-up on those changes to ensure this 
doesn't reintroduce regressions that we've already been through on this value.

Thanks,

Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] main: add support for specifying a non-default configuration file.

[Grant Erickson]

This patch adds support for specifying a main configuration file
different from the default value of /etc/connman/main.conf. This
is valuable during debug to specify alternate debug-only
configurations.
---
  v1: Initial submission.
  v2: Incorporated feedback from Marcel about not propagating the
  fully-qualified default configuration file path on the compiler
  command line.

 src/main.c |   25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/main.c b/src/main.c
index 5c206e7..214b736 100644
--- a/src/main.c
+++ b/src/main.c
@@ -42,6 +42,8 @@
 #define DEFAULT_INPUT_REQUEST_TIMEOUT 120 * 1000
 #define DEFAULT_BROWSER_LAUNCH_TIMEOUT 300 * 1000
 
+#define CONFIGMAINFILE CONFIGDIR "/main.conf"
+
 static char *default_auto_connect[] = {
"wifi",
"ethernet",
@@ -324,6 +326,7 @@ static void disconnect_callback(DBusConnection *conn, void 
*user_data)
 
 static gchar *option_debug = NULL;
 static gchar *option_device = NULL;
+static gchar *option_file = NULL;
 static gchar *option_plugin = NULL;
 static gchar *option_nodevice = NULL;
 static gchar *option_noplugin = NULL;
@@ -333,6 +336,18 @@ static gboolean option_dnsproxy = TRUE;
 static gboolean option_compat = FALSE;
 static gboolean option_version = FALSE;
 
+static int conf_init(const char *file)
+{
+   GKeyFile *config;
+
+   config = load_config(file);
+   parse_config(config);
+   if (config != NULL)
+   g_key_file_free(config);
+
+   return 0;
+}
+
 static gboolean parse_debug(const char *key, const char *value,
gpointer user_data, GError **error)
 {
@@ -350,6 +365,9 @@ static GOptionEntry options[] = {
"Specify debug options to enable", "DEBUG" },
{ "device", 'i', 0, G_OPTION_ARG_STRING, &option_device,
"Specify networking device or interface", "DEV" },
+   { "file", 'f', 0, G_OPTION_ARG_STRING, &option_file,
+   "Load the specified configuration file "
+   "instead of " CONFIGMAINFILE, "FILE" },
{ "nodevice", 'I', 0, G_OPTION_ARG_STRING, &option_nodevice,
"Specify networking interface to ignore", "DEV" },
{ "plugin", 'p', 0, G_OPTION_ARG_STRING, &option_plugin,
@@ -430,7 +448,6 @@ int main(int argc, char *argv[])
GError *error = NULL;
DBusConnection *conn;
DBusError err;
-   GKeyFile *config;
guint signal;
 
 #ifdef NEED_THREADS
@@ -507,10 +524,7 @@ int main(int argc, char *argv[])
 
__connman_dbus_init(conn);
 
-   config = load_config(CONFIGDIR "/main.conf");
-   parse_config(config);
-   if (config != NULL)
-   g_key_file_free(config);
+   conf_init(option_file ? option_file : CONFIGMAINFILE);
 
__connman_storage_migrate();
__connman_technology_init();
@@ -551,6 +565,7 @@ int main(int argc, char *argv[])
__connman_rfkill_init();
 
g_free(option_device);
+   g_free(option_file);
g_free(option_plugin);
g_free(option_nodevice);
g_free(option_noplugin);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] main: add support for specifying a non-default configuration file.

[Grant Erickson]

This patch adds support for specifying a main configuration file
different from the default value of /etc/connman/main.conf. This
is valuable during debug to specify alternate debug-only
configurations.
---
 Makefile.am |3 ++-
 src/main.c  |   23 ++-
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index ca0cf0b..ce505b6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -119,7 +119,8 @@ AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @XTABLES_CFLAGS@ \
-DPLUGINDIR=\""$(build_plugindir)"\" \
-DSCRIPTDIR=\""$(build_scriptdir)"\" \
-DSTORAGEDIR=\""$(storagedir)\"" \
-   -DCONFIGDIR=\""$(configdir)\""
+   -DCONFIGDIR=\""$(configdir)\"" \
+   -DCONFIGMAINFILE=\""$(configdir)/main.conf"\"
 
 INCLUDES = -I$(builddir)/include -I$(builddir)/src -I$(srcdir)/gdbus
 
diff --git a/src/main.c b/src/main.c
index 5c206e7..b0045d7 100644
--- a/src/main.c
+++ b/src/main.c
@@ -324,6 +324,7 @@ static void disconnect_callback(DBusConnection *conn, void 
*user_data)
 
 static gchar *option_debug = NULL;
 static gchar *option_device = NULL;
+static gchar *option_file = NULL;
 static gchar *option_plugin = NULL;
 static gchar *option_nodevice = NULL;
 static gchar *option_noplugin = NULL;
@@ -333,6 +334,18 @@ static gboolean option_dnsproxy = TRUE;
 static gboolean option_compat = FALSE;
 static gboolean option_version = FALSE;
 
+static int conf_init(const char *file)
+{
+   GKeyFile *config;
+
+   config = load_config(file);
+   parse_config(config);
+   if (config != NULL)
+   g_key_file_free(config);
+
+   return 0;
+}
+
 static gboolean parse_debug(const char *key, const char *value,
gpointer user_data, GError **error)
 {
@@ -350,6 +363,9 @@ static GOptionEntry options[] = {
"Specify debug options to enable", "DEBUG" },
{ "device", 'i', 0, G_OPTION_ARG_STRING, &option_device,
"Specify networking device or interface", "DEV" },
+   { "file", 'f', 0, G_OPTION_ARG_STRING, &option_file,
+   "Load the specified configuration file "
+   "instead of " CONFIGMAINFILE, "FILE" },
{ "nodevice", 'I', 0, G_OPTION_ARG_STRING, &option_nodevice,
"Specify networking interface to ignore", "DEV" },
{ "plugin", 'p', 0, G_OPTION_ARG_STRING, &option_plugin,
@@ -430,7 +446,6 @@ int main(int argc, char *argv[])
GError *error = NULL;
DBusConnection *conn;
DBusError err;
-   GKeyFile *config;
guint signal;
 
 #ifdef NEED_THREADS
@@ -507,10 +522,7 @@ int main(int argc, char *argv[])
 
__connman_dbus_init(conn);
 
-   config = load_config(CONFIGDIR "/main.conf");
-   parse_config(config);
-   if (config != NULL)
-   g_key_file_free(config);
+   conf_init(option_file ? option_file : CONFIGMAINFILE);
 
__connman_storage_migrate();
__connman_technology_init();
@@ -551,6 +563,7 @@ int main(int argc, char *argv[])
__connman_rfkill_init();
 
g_free(option_device);
+   g_free(option_file);
g_free(option_plugin);
g_free(option_nodevice);
g_free(option_noplugin);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] service: signal error property changes

[Grant Erickson]

In order to allow clients to update the property in a manner similar to
other service properties, signal changes in the service "Error" property
key/value pair.

The value "none" is used when no error is asserted because D-Bus property
changed convention has no semantics for indicating a key/value pair was
removed.
---
 src/service.c |   39 ++-
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/service.c b/src/service.c
index dfe1752..1b855b6 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1420,6 +1420,21 @@ static void autoconnect_changed(struct connman_service 
*service)
DBUS_TYPE_BOOLEAN, &service->autoconnect);
 }
 
+static void error_changed(struct connman_service *service)
+{
+   const char *str;
+   const char *none = "none";
+
+   if (service->path == NULL)
+   return;
+
+   str = error2string(service->error);
+
+   connman_dbus_property_changed_basic(service->path,
+   CONNMAN_SERVICE_INTERFACE, "Error",
+   DBUS_TYPE_STRING, str ? &str : &none);
+}
+
 static void append_security(DBusMessageIter *iter, void *user_data)
 {
struct connman_service *service = user_data;
@@ -3205,11 +3220,23 @@ static DBusMessage *set_property(DBusConnection *conn,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static void set_error(struct connman_service *service,
+   enum connman_service_error error)
+{
+   enum connman_service_error old_error;
+
+   old_error = service->error;
+   service->error = error;
+
+   if (error != old_error)
+   error_changed(service);
+}
+
 static void set_idle(struct connman_service *service)
 {
service->state = service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_IDLE;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
state_changed(service);
 }
 
@@ -4100,6 +4127,8 @@ static void service_initialize(struct connman_service 
*service)
service->refcount = 1;
service->session_usage_count = 0;
 
+   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+
service->type = CONNMAN_SERVICE_TYPE_UNKNOWN;
service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
 
@@ -4679,7 +4708,7 @@ static void request_input_cb (struct connman_service 
*service,
  done:
if (err >= 0) {
/* We forget any previous error. */
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
__connman_service_connect(service);
 
@@ -4929,7 +4958,7 @@ static int service_indicate_state(struct connman_service 
*service)
return 0;
service_complete(service);
} else
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
iter = g_hash_table_lookup(service_hash, service->identifier);
if (iter != NULL && g_sequence_get_length(service_list) > 1) {
@@ -4955,7 +4984,7 @@ int __connman_service_indicate_error(struct 
connman_service *service,
if (service == NULL)
return -EINVAL;
 
-   service->error = error;
+   set_error(service, error);
 
if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
__connman_service_set_passphrase(service, NULL);
@@ -4981,7 +5010,7 @@ int __connman_service_clear_error(struct connman_service 
*service)
 
service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_UNKNOWN;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
if (service->favorite == TRUE)
set_reconnect_state(service, TRUE);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 13/13] gresolv: use predefined mnemonics rather than magic numbers

[Grant Erickson]

Use predefined mnemonics from arpa/nameserv.h for the return value
from ns_msg_getflag rather than magic numbers.
---
 gweb/gresolv.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index d96f345..79abc9b 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -652,22 +652,22 @@ static void parse_response(struct resolv_nameserver 
*nameserver,
ns_msg_id(msg), rcode, count);
 
switch (rcode) {
-   case 0:
+   case ns_r_noerror:
status = G_RESOLV_RESULT_STATUS_SUCCESS;
break;
-   case 1:
+   case ns_r_formerr:
status = G_RESOLV_RESULT_STATUS_FORMAT_ERROR;
break;
-   case 2:
+   case ns_r_servfail:
status = G_RESOLV_RESULT_STATUS_SERVER_FAILURE;
break;
-   case 3:
+   case ns_r_nxdomain:
status = G_RESOLV_RESULT_STATUS_NAME_ERROR;
break;
-   case 4:
+   case ns_r_notimpl:
status = G_RESOLV_RESULT_STATUS_NOT_IMPLEMENTED;
break;
-   case 5:
+   case ns_r_refused:
status = G_RESOLV_RESULT_STATUS_REFUSED;
break;
default:
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 12/13] gresolv: do not allow unsuccessful response status to override successful status

[Grant Erickson]

When performing a resolver lookup from timeserver or wpad, both
perform queries with an unspecified address family. This means that
both A and  record queries are issued. In cases where a valid,
successful A response comes back but where the  query results in a
timeout, do not smash the successful A status with the time out 
status; otherwise, the timeserver or wpad will appear to fail to them
when, in fact, the A query was successful and more than satisfies its
unspecified address family requirement.

Partial fix for BMC#25486 .
---
 gweb/gresolv.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index caa30ee..d96f345 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -486,10 +486,16 @@ static void sort_and_return_results(struct resolv_lookup 
*lookup)
 
results[n++] = NULL;
 
-   status = lookup->ipv4_status;
-
-   if (status == G_RESOLV_RESULT_STATUS_SUCCESS)
+   if (lookup->resolv->result_family == AF_INET)
+   status = lookup->ipv4_status;
+   else if (lookup->resolv->result_family == AF_INET6)
status = lookup->ipv6_status;
+   else {
+   if (lookup->ipv6_status == G_RESOLV_RESULT_STATUS_SUCCESS)
+   status = lookup->ipv6_status;
+   else
+   status = lookup->ipv4_status;
+   }
 
lookup->result_func(status, results, lookup->result_data);
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 11/13] gresolve: fix a typo in conditional check for returning results

[Grant Erickson]

In both parse_response and query_timeout there exists logic that checks to 
ensure that both an A and  lookups
have either been responded to or timed out before processing and returning 
results to the caller. In query_timeout,
there was a typo in the condition check such that it did not match those 
conditions tested in parse_response.
---
 gweb/gresolv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index e93bf4f..caa30ee 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -514,7 +514,7 @@ static gboolean query_timeout(gpointer user_data)
lookup->ipv6_query = NULL;
}
 
-   if (lookup->ipv4_query == NULL && lookup->ipv4_query == NULL)
+   if (lookup->ipv4_query == NULL && lookup->ipv6_query == NULL)
sort_and_return_results(lookup);
 
destroy_query(query);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 10/13] ntp: add debug message when starting NTP against resolved server

[Grant Erickson]

This patch adds an analagous debug message when starting NTP against a
resolved server as when starting against a numbered server.
---
 src/timeserver.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/timeserver.c b/src/timeserver.c
index 59b299d..f934b58 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -96,6 +96,8 @@ static void resolv_result(GResolvResultStatus status, char 
**results, gpointer u
ts_list, results[i]);
}
 
+   DBG("Using timeserver %s", results[0]);
+
__connman_ntp_start(results[0]);
 
return;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 09/13] ntp: fixed a debug message typo

[Grant Erickson]

---
 src/timeserver.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/timeserver.c b/src/timeserver.c
index 439d6f5..59b299d 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -129,7 +129,7 @@ void __connman_timeserver_sync_next()
 
/* if its a IP , directly query it. */
if (connman_inet_check_ipaddress(server) > 0) {
-   DBG("Using timeservers %s", server);
+   DBG("Using timeserver %s", server);
 
__connman_ntp_start(server);
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 08/13] ntp: retry a given server address multiple times before falling back

[Grant Erickson]

This patch causes the internal NTP client to retry a given server address up to 
three times (four total attempts) at a two second interval before
falling back to another server name or server address.
---
 src/ntp.c |   66 +
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/src/ntp.c b/src/ntp.c
index e15bd08..2ef0cf2 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -69,6 +69,9 @@ struct ntp_msg {
 
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
+#define NTP_SEND_TIMEOUT   2
+#define NTP_SEND_RETRIES   3
+
 #define NTP_FLAG_LI_SHIFT  6
 #define NTP_FLAG_LI_MASK   0x3
 #define NTP_FLAG_LI_NOWARNING  0x0
@@ -113,6 +116,31 @@ static int transmit_fd = 0;
 static char *timeserver = NULL;
 static gint poll_id = 0;
 static gint timeout_id = 0;
+static guint retries = 0;
+
+static void send_packet(int fd, const char *server);
+
+static void next_server(void)
+{
+   if (timeserver != NULL) {
+   g_free(timeserver);
+   timeserver = NULL;
+   }
+
+   __connman_timeserver_sync_next();
+}
+
+static gboolean send_timeout(gpointer user_data)
+{
+   DBG("send timeout (retries %d)", retries);
+
+   if (retries++ == NTP_SEND_RETRIES)
+   next_server();
+   else
+   send_packet(transmit_fd, timeserver);
+
+   return FALSE;
+}
 
 static void send_packet(int fd, const char *server)
 {
@@ -158,18 +186,14 @@ static void send_packet(int fd, const char *server)
connman_error("Broken time request for server %s", server);
return;
}
-}
 
-static gboolean next_server(gpointer user_data)
-{
-   if (timeserver != NULL) {
-   g_free(timeserver);
-   timeserver = NULL;
-   }
-
-   __connman_timeserver_sync_next();
+   /*
+* Add a retry timeout of two seconds to retry the existing
+* request. After a set number of retries, we'll fallback to
+* trying another server.
+*/
 
-   return FALSE;
+   timeout_id = g_timeout_add_seconds(NTP_SEND_TIMEOUT, send_timeout, 
NULL);
 }
 
 static gboolean next_poll(gpointer user_data)
@@ -182,6 +206,14 @@ static gboolean next_poll(gpointer user_data)
return FALSE;
 }
 
+static void reset_timeout(void)
+{
+   if (timeout_id > 0)
+   g_source_remove(timeout_id);
+
+   retries = 0;
+}
+
 static void decode_msg(void *base, size_t len, struct timeval *tv)
 {
struct ntp_msg *msg = base;
@@ -244,8 +276,8 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
DBG("offset=%f delay=%f", offset, delay);
 
/* Remove the timeout, as timeserver has responded */
-   if (timeout_id > 0)
-   g_source_remove(timeout_id);
+
+   reset_timeout();
 
/*
 * Now poll the server every transmit_delay seconds
@@ -422,13 +454,6 @@ int __connman_ntp_start(char *server)
 
start_ntp(timeserver);
 
-   /*
-* Add a fallback timeout , preferably short, 5 sec here,
-* to fallback on the next server.
-*/
-
-   timeout_id = g_timeout_add_seconds(5, next_server, NULL);
-
return 0;
 }
 
@@ -439,8 +464,7 @@ void __connman_ntp_stop()
if (poll_id > 0)
g_source_remove(poll_id);
 
-   if (timeout_id > 0)
-   g_source_remove(timeout_id);
+reset_timeout();
 
if (channel_watch > 0) {
g_source_remove(channel_watch);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 07/13] ntp: use mnemonic when specifying the request precision

[Grant Erickson]

This patch uses a mnemonic to specify the request precision and also adds a 
comment how a future implementation might pass along the actual system
wall clock precision.
---
 src/ntp.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index 3e90741..e15bd08 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -120,10 +120,17 @@ static void send_packet(int fd, const char *server)
struct sockaddr_in addr;
ssize_t len;
 
+   /*
+* At some point, we could specify the actual system precision with:
+*
+*   clock_getres(CLOCK_REALTIME, &ts);
+*   msg.precision = (int)log2(ts.tv_sec + (ts.tv_nsec * 1.0e-9));
+*/
memset(&msg, 0, sizeof(msg));
msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOTINSYNC, 4, 
NTP_FLAG_MD_CLIENT);
msg.poll = 4;   // min
msg.poll = 10;  // max
+   msg.precision = NTP_PRECISION_S;
 
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 06/13] ntp: add mnemonics for common clock precisions

[Grant Erickson]

---
 src/ntp.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index e4267ea..3e90741 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -99,6 +99,13 @@ struct ntp_msg {
 #define NTP_FLAGS_VN_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_VN_SHIFT) & NTP_FLAG_VN_MASK))
 #define NTP_FLAGS_MD_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_MD_SHIFT) & NTP_FLAG_MD_MASK))
 
+#define NTP_PRECISION_S0
+#define NTP_PRECISION_DS   -3
+#define NTP_PRECISION_CS   -6
+#define NTP_PRECISION_MS   -9
+#define NTP_PRECISION_US   -19
+#define NTP_PRECISION_NS   -29
+
 static guint channel_watch = 0;
 static struct timeval transmit_timeval;
 static int transmit_fd = 0;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 05/13] ntp: use mnemonics when checking reply flags

[Grant Erickson]

Use mnemonics when checking reply flags and print out information in
debug mode about why the reply flags were rejected.
---
 src/ntp.c |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 7ef072c..e4267ea 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -199,8 +199,20 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
 
transmit_delay = LOGTOD(msg->poll);
 
-   if (msg->flags != 0x24)
+   if (NTP_FLAGS_LI_DECODE(msg->flags) == NTP_FLAG_LI_NOTINSYNC) {
+   DBG("ignoring unsynchronized peer");
return;
+   }
+
+   if (NTP_FLAGS_VN_DECODE(msg->flags) != 4) {
+   DBG("unsupported version %d", NTP_FLAGS_VN_DECODE(msg->flags));
+   return;
+   }
+
+   if (NTP_FLAGS_MD_DECODE(msg->flags) != NTP_FLAG_MD_SERVER) {
+   DBG("unsupported mode %d", NTP_FLAGS_MD_DECODE(msg->flags));
+   return;
+   }
 
org = transmit_timeval.tv_sec +
(1.0e-6 * transmit_timeval.tv_usec) + OFFSET_1900_1970;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 04/13] ntp: specify leap-not-in-sync in flags

[Grant Erickson]

This patch sets the non-in-sync flag of the leap subfield of the
packet flags field. This reflects behavior implemented by ntpdate or
ntpd in one-shot mode.
---
 src/ntp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 1916a72..7ef072c 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -114,7 +114,7 @@ static void send_packet(int fd, const char *server)
ssize_t len;
 
memset(&msg, 0, sizeof(msg));
-   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);
+   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOTINSYNC, 4, 
NTP_FLAG_MD_CLIENT);
msg.poll = 4;   // min
msg.poll = 10;  // max
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 03/13] ntp: send a transmit time value that reflects current system time

[Grant Erickson]

This sets the ntp request packet transmit time to the current system
time to reflect behavior implemented by ntpdate and ntpd in one-shot
mode.
---
 src/ntp.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ntp.c b/src/ntp.c
index c7224a8..1916a72 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -117,8 +117,6 @@ static void send_packet(int fd, const char *server)
msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);
msg.poll = 4;   // min
msg.poll = 10;  // max
-   msg.xmttime.seconds = random();
-   msg.xmttime.fraction = random();
 
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
@@ -127,6 +125,9 @@ static void send_packet(int fd, const char *server)
 
gettimeofday(&transmit_timeval, NULL);
 
+   msg.xmttime.seconds = htonl(transmit_timeval.tv_sec + OFFSET_1900_1970);
+   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
+
len = sendto(fd, &msg, sizeof(msg), MSG_DONTWAIT,
&addr, sizeof(addr));
if (len < 0) {
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 02/13] ntp: use mnemonics rather than magic numbers

[Grant Erickson]

Convert a magic number into equivalent mnemonics for the flags (leap,
version, mode) packet field.
---
 src/ntp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 7555618..c7224a8 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -114,7 +114,7 @@ static void send_packet(int fd, const char *server)
ssize_t len;
 
memset(&msg, 0, sizeof(msg));
-   msg.flags = 0x23;
+   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);
msg.poll = 4;   // min
msg.poll = 10;  // max
msg.xmttime.seconds = random();
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 00/13] NTP: Address Failures to Synchronize

[Grant Erickson]

This patch set is an omnibus roll-up of several patches submitted
Friday and over the weekend to address BMC#25486
<https://bugs.meego.com/show_bug.cgi?id=25486>.

Two key problems are addressed in this set:

  1) The NTP client will send to a given server address up to four times
 at two second intervals before giving up and moving on to the next
 server.

  2) The resolver will not allow unsuccessful A or  response status to
 override successful status when performing a wildcard lookup
 (i.e. the address family is unspecified).

Grant Erickson (13):
  ntp: add mnemonics for the flags (leap, version, mode) packet field
  ntp: use mnemonics rather than magic numbers
  ntp: send a transmit time value that reflects current system time
  ntp: specify leap-not-in-sync in flags
  ntp: use mnemonics when checking reply flags
  ntp: add mnemonics for common clock precisions
  ntp: use mnemonic when specifying the request precision
  ntp: retry a given server address multiple times before falling back
  ntp: fixed a debug message typo
  ntp: add debug message when starting NTP against resolved server
  gresolve: fix a typo in conditional check for returning results
  gresolv: do not allow unsuccessful response status to override successful 
status
  gresolv: use predefined mnemonics rather than magic numbers

 gweb/gresolv.c   |   26 ++-
 src/ntp.c|  131 +++---
 src/timeserver.c |4 +-
 3 files changed, 125 insertions(+), 36 deletions(-)

-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 01/13] ntp: add mnemonics for the flags (leap, version, mode) packet field

[Grant Erickson]

---
 src/ntp.c |   30 ++
 1 file changed, 30 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index 8c31fdb..7555618 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -69,6 +69,36 @@ struct ntp_msg {
 
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
+#define NTP_FLAG_LI_SHIFT  6
+#define NTP_FLAG_LI_MASK   0x3
+#define NTP_FLAG_LI_NOWARNING  0x0
+#define NTP_FLAG_LI_ADDSECOND  0x1
+#define NTP_FLAG_LI_DELSECOND  0x2
+#define NTP_FLAG_LI_NOTINSYNC  0x3
+
+#define NTP_FLAG_VN_SHIFT  3
+#define NTP_FLAG_VN_MASK   0x7
+
+#define NTP_FLAG_MD_SHIFT  0
+#define NTP_FLAG_MD_MASK   0x7
+#define NTP_FLAG_MD_UNSPEC 0
+#define NTP_FLAG_MD_ACTIVE 1
+#define NTP_FLAG_MD_PASSIVE2
+#define NTP_FLAG_MD_CLIENT 3
+#define NTP_FLAG_MD_SERVER 4
+#define NTP_FLAG_MD_BROADCAST  5
+#define NTP_FLAG_MD_CONTROL6
+#define NTP_FLAG_MD_PRIVATE7
+
+#define NTP_FLAGS_ENCODE(li, vn, md)  ((uint8_t)( \
+  (((li) & NTP_FLAG_LI_MASK) << NTP_FLAG_LI_SHIFT) | \
+  (((vn) & NTP_FLAG_VN_MASK) << NTP_FLAG_VN_SHIFT) | \
+  (((md) & NTP_FLAG_MD_MASK) << NTP_FLAG_MD_SHIFT)))
+
+#define NTP_FLAGS_LI_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_LI_SHIFT) & NTP_FLAG_LI_MASK))
+#define NTP_FLAGS_VN_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_VN_SHIFT) & NTP_FLAG_VN_MASK))
+#define NTP_FLAGS_MD_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_MD_SHIFT) & NTP_FLAG_MD_MASK))
+
 static guint channel_watch = 0;
 static struct timeval transmit_timeval;
 static int transmit_fd = 0;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gresolv: use predefined mnemonics rather than magic numbers

[Grant Erickson]

Use predefined mnemonics from arpa/nameserv.h for the return value
from ns_msg_getflag rather than magic numbers.
---
 gweb/gresolv.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index d96f345..79abc9b 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -652,22 +652,22 @@ static void parse_response(struct resolv_nameserver 
*nameserver,
ns_msg_id(msg), rcode, count);
 
switch (rcode) {
-   case 0:
+   case ns_r_noerror:
status = G_RESOLV_RESULT_STATUS_SUCCESS;
break;
-   case 1:
+   case ns_r_formerr:
status = G_RESOLV_RESULT_STATUS_FORMAT_ERROR;
break;
-   case 2:
+   case ns_r_servfail:
status = G_RESOLV_RESULT_STATUS_SERVER_FAILURE;
break;
-   case 3:
+   case ns_r_nxdomain:
status = G_RESOLV_RESULT_STATUS_NAME_ERROR;
break;
-   case 4:
+   case ns_r_notimpl:
status = G_RESOLV_RESULT_STATUS_NOT_IMPLEMENTED;
break;
-   case 5:
+   case ns_r_refused:
status = G_RESOLV_RESULT_STATUS_REFUSED;
break;
default:
-- 
1.7.10.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gresolv: do not allow unsuccessful A or AAAA response status override successful status

[Grant Erickson]

When performing a resolver lookup from timeserver or wpad, both
perform queries with an unspecified address family. This means that
both A and  record queries are issued. In cases where a valid,
successful A response comes back but where the  query results in a
timeout, do not smash the successful A status with the time out 
status; otherwise, the timeserver or wpad will appear to fail to them
when, in fact, the A query was successful and more than satisfies its
unspecified address family requirement.

Partial fix for BMC#25486 .
---
 gweb/gresolv.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index caa30ee..d96f345 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -486,10 +486,16 @@ static void sort_and_return_results(struct resolv_lookup 
*lookup)
 
results[n++] = NULL;
 
-   status = lookup->ipv4_status;
-
-   if (status == G_RESOLV_RESULT_STATUS_SUCCESS)
+   if (lookup->resolv->result_family == AF_INET)
+   status = lookup->ipv4_status;
+   else if (lookup->resolv->result_family == AF_INET6)
status = lookup->ipv6_status;
+   else {
+   if (lookup->ipv6_status == G_RESOLV_RESULT_STATUS_SUCCESS)
+   status = lookup->ipv6_status;
+   else
+   status = lookup->ipv4_status;
+   }
 
lookup->result_func(status, results, lookup->result_data);
 
-- 
1.7.10.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gresolve: fix a typo in conditional check for returning results

[Grant Erickson]

In both parse_response and query_timeout there exists logic that checks to 
ensure that both an A and  lookups
have either been responded to or timed out before processing and returning 
results to the caller. In query_timeout,
there was a typo in the condition check such that it did not match those 
conditions tested in parse_response.
---
 gweb/gresolv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index e93bf4f..caa30ee 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -514,7 +514,7 @@ static gboolean query_timeout(gpointer user_data)
lookup->ipv6_query = NULL;
}
 
-   if (lookup->ipv4_query == NULL && lookup->ipv4_query == NULL)
+   if (lookup->ipv4_query == NULL && lookup->ipv6_query == NULL)
sort_and_return_results(lookup);
 
destroy_query(query);
-- 
1.7.10.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: add debug message when starting NTP against resolved server

[Grant Erickson]

This patch adds an analagous debug message when starting NTP against a
resolved server as when starting against a numbered server.
---
 src/timeserver.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/timeserver.c b/src/timeserver.c
index 3603c5a..df4de3c 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -90,6 +90,8 @@ static void resolv_result(GResolvResultStatus status, char 
**results, gpointer u
for (i = 0; results[i]; i++)
DBG("result: %s", results[i]);
 
+   DBG("Using timeserver %s", results[0]);
+
__connman_ntp_start(results[0]);
 
return;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: fixed a debug message typo

[Grant Erickson]

---
 src/timeserver.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/timeserver.c b/src/timeserver.c
index 467da52..3603c5a 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -123,7 +123,7 @@ void __connman_timeserver_sync_next()
 
/* if its a IP , directly query it. */
if (connman_inet_check_ipaddress(server) > 0) {
-   DBG("Using timeservers %s", server);
+   DBG("Using timeserver %s", server);
 
__connman_ntp_start(server);
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: retry a given server address multiple times before falling back

[Grant Erickson]

This patch causes the internal NTP client to retry a given server address up to 
three times (four total attempts) at a two second interval before
falling back to another server name or server address.
---
 src/ntp.c |   66 +
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..74aeda8 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -69,6 +69,9 @@ struct ntp_msg {
 
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
+#define NTP_SEND_TIMEOUT   2
+#define NTP_SEND_RETRIES   3
+
 #define NTP_FLAG_LI_SHIFT  6
 #define NTP_FLAG_LI_MASK   0x3
 #define NTP_FLAG_LI_NOWARNING  0x0
@@ -106,6 +109,31 @@ static int transmit_fd = 0;
 static char *timeserver = NULL;
 static gint poll_id = 0;
 static gint timeout_id = 0;
+static guint retries = 0;
+
+static void send_packet(int fd, const char *server);
+
+static void next_server(void)
+{
+   if (timeserver != NULL) {
+   g_free(timeserver);
+   timeserver = NULL;
+   }
+
+   __connman_timeserver_sync_next();
+}
+
+static gboolean send_timeout(gpointer user_data)
+{
+   DBG("send timeout (retries %d)", retries);
+
+   if (retries++ == NTP_SEND_RETRIES)
+   next_server();
+   else
+   send_packet(transmit_fd, timeserver);
+
+   return FALSE;
+}
 
 static void send_packet(int fd, const char *server)
 {
@@ -138,18 +166,14 @@ static void send_packet(int fd, const char *server)
connman_error("Broken time request for server %s", server);
return;
}
-}
 
-static gboolean next_server(gpointer user_data)
-{
-   if (timeserver != NULL) {
-   g_free(timeserver);
-   timeserver = NULL;
-   }
-
-   __connman_timeserver_sync_next();
+   /*
+* Add a retry timeout of two seconds to retry the existing
+* request. After a set number of retries, we'll fallback to
+* trying another server.
+*/
 
-   return FALSE;
+   timeout_id = g_timeout_add_seconds(NTP_SEND_TIMEOUT, send_timeout, 
NULL);
 }
 
 static gboolean next_poll(gpointer user_data)
@@ -162,6 +186,14 @@ static gboolean next_poll(gpointer user_data)
return FALSE;
 }
 
+static void reset_timeout(void)
+{
+   if (timeout_id > 0)
+   g_source_remove(timeout_id);
+
+   retries = 0;
+}
+
 static void decode_msg(void *base, size_t len, struct timeval *tv)
 {
struct ntp_msg *msg = base;
@@ -212,8 +244,8 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
DBG("offset=%f delay=%f", offset, delay);
 
/* Remove the timeout, as timeserver has responded */
-   if (timeout_id > 0)
-   g_source_remove(timeout_id);
+
+   reset_timeout();
 
/*
 * Now poll the server every transmit_delay seconds
@@ -390,13 +422,6 @@ int __connman_ntp_start(char *server)
 
start_ntp(timeserver);
 
-   /*
-* Add a fallback timeout , preferably short, 5 sec here,
-* to fallback on the next server.
-*/
-
-   timeout_id = g_timeout_add_seconds(5, next_server, NULL);
-
return 0;
 }
 
@@ -407,8 +432,7 @@ void __connman_ntp_stop()
if (poll_id > 0)
g_source_remove(poll_id);
 
-   if (timeout_id > 0)
-   g_source_remove(timeout_id);
+reset_timeout();
 
if (channel_watch > 0) {
g_source_remove(channel_watch);
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v3] ntp: use mnemonics when checking reply flags

[Grant Erickson]

Use mnemonics when checking reply flags and print out information in
debug mode about why the reply flags were rejected.

---

 v1: Initial submission.
 v2: Fixed typo that prevented compilation.
 v3: More typo corrections.

 src/ntp.c |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..ca4fa02 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -193,8 +193,20 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
 
transmit_delay = LOGTOD(msg->poll);
 
-   if (msg->flags != 0x24)
+   if (NTP_FLAGS_LI_DECODE(msg->flags) == NTP_FLAG_LI_NOTINSYNC) {
+   DBG("ignoring unsynchronized peer");
return;
+   }
+
+   if (NTP_FLAGS_VN_DECODE(msg->flags) != 4) {
+   DBG("unsupported version %d", NTP_FLAGS_VN_DECODE(msg->flags));
+   return;
+   }
+
+   if (NTP_FLAGS_MD_DECODE(msg->flags) != NTP_FLAG_MD_SERVER) {
+   DBG("unsupported mode %d", NTP_FLAGS_MD_DECODE(msg->flags));
+   return;
+   }
 
org = transmit_timeval.tv_sec +
(1.0e-6 * transmit_timeval.tv_usec) + OFFSET_1900_1970;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] ntp: use mnemonics when checking reply flags

[Grant Erickson]

Use mnemonics when checking reply flags and print out information in
debug mode about why the reply flags were rejected.

---

 v1: Initial submission.
 v2: Fixed typo that prevented compilation.

 src/ntp.c |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..d646161 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -193,9 +193,20 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
 
transmit_delay = LOGTOD(msg->poll);
 
-   if (msg->flags != 0x24)
+   if (PKT_FLAGS_LI_DECODE(msg->flag) == NTP_FLAG_LI_NOTINSYNC) {
+   DBG("ignoring unsynchronized peer");
return;
+   }
+
+   if (PKT_FLAGS_VN_DECODE(msg->flag) != 4) {
+   DBG("unsupported version %d", PKT_FLAGS_VN_DECODE(msg->flag));
+   return;
+   }
+
+   if (PKT_FLAGS_MD_DECODE(msg->flag) != NTP_FLAG_MD_SERVER) {
+   DBG("unsupported mode %d", PKT_FLAGS_MD_DECODE(msg->flag));
+   return;
+   }
 
org = transmit_timeval.tv_sec +
(1.0e-6 * transmit_timeval.tv_usec) + OFFSET_1900_1970;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: use mnemonics when checking reply flags

[Grant Erickson]

Use mnemonics when checking reply flags and print out information in
debug mode about why the reply flags were rejected.

---
 src/ntp.c |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..d646161 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -193,8 +193,20 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
 
transmit_delay = LOGTOD(msg->poll);
 
-   if (msg->flags != 0x24)
+   if (PKT_FLAGS_LI_DECODE(msg->flag) == LEAP_NOTINSYNC) {
+   DBG("ignoring unsynchronized peer");
return;
+   }
+
+   if (PKT_FLAGS_VN_DECODE(msg->flag) != 4) {
+   DBG("unsupported version %d", PKT_FLAGS_VN_DECODE(msg->flag));
+   return;
+   }
+
+   if (PKT_FLAGS_MD_DECODE(msg->flag) != NTP_FLAG_MD_SERVER) {
+   DBG("unsupported mode %d", PKT_FLAGS_MD_DECODE(msg->flag));
+   return;
+   }
 
org = transmit_timeval.tv_sec +
(1.0e-6 * transmit_timeval.tv_usec) + OFFSET_1900_1970;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 2/2] ntp: specify leap-not-in-sync in flags

[Grant Erickson]

This patch sets the non-in-sync flag of the leap subfield of the
packet flags field. This reflects behavior implemented by ntpdate or
ntpd in one-shot mode.

---
 src/ntp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 88c8b8e..623c21e 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -114,7 +114,7 @@ static void send_packet(int fd, const char *server)
ssize_t len;
 
memset(&msg, 0, sizeof(msg));
-   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);;
+   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOTINSYNC, 4, 
NTP_FLAG_MD_CLIENT);;
msg.poll = 4;   // min
msg.poll = 10;  // max
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 1/2] ntp: send a transmit time value that reflects current system time

[Grant Erickson]

This sets the ntp request packet transmit time to the current system
time to reflect behavior implemented by ntpdate and ntpd in one-shot
mode.

---
 src/ntp.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..88c8b8e 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -117,8 +117,6 @@ static void send_packet(int fd, const char *server)
msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);;
msg.poll = 4;   // min
msg.poll = 10;  // max
-   msg.xmttime.seconds = random();
-   msg.xmttime.fraction = random();
 
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
@@ -127,6 +125,9 @@ static void send_packet(int fd, const char *server)
 
gettimeofday(&transmit_timeval, NULL);
 
+   msg.xmttime.seconds = htonl(transmit_timeval.tv_sec + OFFSET_1900_1970);
+   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
+
len = sendto(fd, &msg, sizeof(msg), MSG_DONTWAIT,
&addr, sizeof(addr));
if (len < 0) {
-- 
1.7.10
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 0/2] ntp: behave more like ntpdate or ntpd

[Grant Erickson]

This patch adds two changes that make the NTP request packet sent by
connman more consistent with those sent from ntpdate or ntpd in
one-shot mode.

Grant Erickson (2):
  ntp: send a transmit time value that reflects current system time
  ntp: specify leap-not-in-sync in flags

-- 
1.7.10

>From e06aa8a29fc61e00d9675ea6b4fa6e2ab3c743ae Mon Sep 17 00:00:00 2001
From: Grant Erickson 
Date: Fri, 13 Jul 2012 09:38:52 -0700
Subject: [PATCH 1/2] ntp: send a transmit time value that reflects current
 system time

This sets the ntp request packet transmit time to the current system
time to reflect behavior implemented by ntpdate and ntpd in one-shot
mode.

---
 src/ntp.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ntp.c b/src/ntp.c
index f257cda..88c8b8e 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -117,8 +117,6 @@ static void send_packet(int fd, const char *server)
msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);;
msg.poll = 4;   // min
msg.poll = 10;  // max
-   msg.xmttime.seconds = random();
-   msg.xmttime.fraction = random();
 
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
@@ -127,6 +125,9 @@ static void send_packet(int fd, const char *server)
 
gettimeofday(&transmit_timeval, NULL);
 
+   msg.xmttime.seconds = htonl(transmit_timeval.tv_sec + OFFSET_1900_1970);
+   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
+
len = sendto(fd, &msg, sizeof(msg), MSG_DONTWAIT,
&addr, sizeof(addr));
if (len < 0) {
-- 
1.7.10


>From 3c0a58203fd20df3db5572243d35b29e83abc895 Mon Sep 17 00:00:00 2001
From: Grant Erickson 
Date: Fri, 13 Jul 2012 09:41:20 -0700
Subject: [PATCH 2/2] ntp: specify leap-not-in-sync in flags

This patch sets the non-in-sync flag of the leap subfield of the
packet flags field. This reflects behavior implemented by ntpdate or
ntpd in one-shot mode.

---
 src/ntp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 88c8b8e..623c21e 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -114,7 +114,7 @@ static void send_packet(int fd, const char *server)
ssize_t len;
 
memset(&msg, 0, sizeof(msg));
-   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);;
+   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOTINSYNC, 4, 
NTP_FLAG_MD_CLIENT);;
msg.poll = 4;   // min
msg.poll = 10;  // max
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: use mnemonics rather than magic numbers

[Grant Erickson]

Convert a magic number into equivalent mnemonics for the flags (leap,
version, mode) packet field.

---
 src/ntp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ntp.c b/src/ntp.c
index 87017ed..f257cda 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -114,7 +114,7 @@ static void send_packet(int fd, const char *server)
ssize_t len;
 
memset(&msg, 0, sizeof(msg));
-   msg.flags = 0x23;
+   msg.flags = NTP_FLAGS_ENCODE(NTP_FLAG_LI_NOWARNING, 4, 
NTP_FLAG_MD_CLIENT);;
msg.poll = 4;   // min
msg.poll = 10;  // max
msg.xmttime.seconds = random();
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] ntp: add mnemonics for the flags (leap, version, mode) packet field

[Grant Erickson]

---

 v1: Initial submission.
 v2: Removed trailing white space.

 src/ntp.c |   30 ++
 1 file changed, 30 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index 91a8832..87017ed 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -69,6 +69,36 @@ struct ntp_msg {
 
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
+#define NTP_FLAG_LI_SHIFT  6
+#define NTP_FLAG_LI_MASK   0x3
+#define NTP_FLAG_LI_NOWARNING  0x0
+#define NTP_FLAG_LI_ADDSECOND  0x1
+#define NTP_FLAG_LI_DELSECOND  0x2
+#define NTP_FLAG_LI_NOTINSYNC  0x3
+
+#define NTP_FLAG_VN_SHIFT  3
+#define NTP_FLAG_VN_MASK   0x7
+
+#define NTP_FLAG_MD_SHIFT  0
+#define NTP_FLAG_MD_MASK   0x7
+#define NTP_FLAG_MD_UNSPEC 0
+#define NTP_FLAG_MD_ACTIVE 1
+#define NTP_FLAG_MD_PASSIVE2
+#define NTP_FLAG_MD_CLIENT 3
+#define NTP_FLAG_MD_SERVER 4
+#define NTP_FLAG_MD_BROADCAST  5
+#define NTP_FLAG_MD_CONTROL6
+#define NTP_FLAG_MD_PRIVATE7
+
+#define NTP_FLAGS_ENCODE(li, vn, md)  ((uint8_t)( \
+  (((li) & NTP_FLAG_LI_MASK) << NTP_FLAG_LI_SHIFT) | \
+  (((vn) & NTP_FLAG_VN_MASK) << NTP_FLAG_VN_SHIFT) | \
+  (((md) & NTP_FLAG_MD_MASK) << NTP_FLAG_MD_SHIFT)))
+
+#define NTP_FLAGS_LI_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_LI_SHIFT) & NTP_FLAG_LI_MASK))
+#define NTP_FLAGS_VN_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_VN_SHIFT) & NTP_FLAG_VN_MASK))
+#define NTP_FLAGS_MD_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_MD_SHIFT) & NTP_FLAG_MD_MASK))
+
 static guint channel_watch = 0;
 static struct timeval transmit_timeval;
 static int transmit_fd = 0;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: add mnemonics for the flags (leap, version, mode) packet field

[Grant Erickson]

---
 src/ntp.c |   30 ++
 1 file changed, 30 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index 91a8832..87017ed 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -69,6 +69,36 @@ struct ntp_msg {
 
 #define LOGTOD(a)  ((a) < 0 ? 1. / (1L << -(a)) : 1L << (int)(a))
 
+#define NTP_FLAG_LI_SHIFT  6
+#define NTP_FLAG_LI_MASK   0x3
+#define NTP_FLAG_LI_NOWARNING  0x0
+#define NTP_FLAG_LI_ADDSECOND  0x1
+#define NTP_FLAG_LI_DELSECOND  0x2
+#define NTP_FLAG_LI_NOTINSYNC  0x3
+
+#define NTP_FLAG_VN_SHIFT  3
+#define NTP_FLAG_VN_MASK   0x7
+
+#define NTP_FLAG_MD_SHIFT  0
+#define NTP_FLAG_MD_MASK   0x7
+#define NTP_FLAG_MD_UNSPEC 0
+#define NTP_FLAG_MD_ACTIVE 1
+#define NTP_FLAG_MD_PASSIVE2
+#define NTP_FLAG_MD_CLIENT 3
+#define NTP_FLAG_MD_SERVER 4
+#define NTP_FLAG_MD_BROADCAST  5
+#define NTP_FLAG_MD_CONTROL6
+#define NTP_FLAG_MD_PRIVATE7
+
+#define NTP_FLAGS_ENCODE(li, vn, md)  ((uint8_t)( \
+  (((li) & NTP_FLAG_LI_MASK) << NTP_FLAG_LI_SHIFT) | \
+  (((vn) & NTP_FLAG_VN_MASK) << NTP_FLAG_VN_SHIFT) | \
+  (((md) & NTP_FLAG_MD_MASK) << NTP_FLAG_MD_SHIFT)))
+  
+#define NTP_FLAGS_LI_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_LI_SHIFT) & NTP_FLAG_LI_MASK))
+#define NTP_FLAGS_VN_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_VN_SHIFT) & NTP_FLAG_VN_MASK))
+#define NTP_FLAGS_MD_DECODE(flags)((uint8_t)(((flags) >> 
NTP_FLAG_MD_SHIFT) & NTP_FLAG_MD_MASK))
+
 static guint channel_watch = 0;
 static struct timeval transmit_timeval;
 static int transmit_fd = 0;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Connman 1.2 Open Issues

[Grant Erickson]

On Jun 27, 2012, at 11:01 PM, Marcel Holtmann wrote:
>> I ran connman-1.2 through it's paces today. I noticed two significant issues 
>> right away:
>> 
>> 1) An empty or commented out NetworkInterfaceBlacklist setting in main.conf 
>> does not work. Patch submitted (see 
>> http://lists.connman.net/pipermail/connman/2012-June/010241.html).
> 
> this was stupid of us. I need to spin a ConnMan 1.3 this week with this
> fixed. Thanks for the quick test (and patch) on this.
> 
>> 2) Named (e.g. pool.ntp.org) time servers do not work. They error out with 
>> "Time request for server pool.ntp.org failed". This matches behavior back 
>> from the 0.6x and 0.7x days. 
>>   Replacing named time servers with a raw IP address works fine.
> 
> So I remember that this used to work at some point. Especially since we
> have the internal GResolv for DNS lookups. That needs fixing as well.

For what it is worth, this appears to be a regression between 1.1 and 1.2 as 
1.1 works fine with pool.ntp.org as a fallback time server. That should be a 
fairly small bisect set to iterate through.

-Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH] main: static-qualify default config values

[Grant Erickson]

On Jun 27, 2012, at 10:57 PM, Patrik Flykt wrote:
> On Wed, 2012-06-27 at 17:21 -0700, Grant Erickson wrote:
>> -   char *default_auto_connect[] = {
>> +   static char *default_auto_connect[] = { 
> 
> This isn't strictly necessary as default_auto_connect[] is only used
> locally as input to parse_service_types().

Agreed. It looks like you've opted to move these to file rather than function 
scope upstream.

I agree with Marcel that it'd be preferable to const-qualify these arrays; 
however, as you attest that leads to a const-cascade that will require some 
casting elsewhere.

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Connman 1.2 Open Issues

[Grant Erickson]

I ran connman-1.2 through it's paces today. I noticed two significant issues 
right away:

1) An empty or commented out NetworkInterfaceBlacklist setting in main.conf 
does not work. Patch submitted (see 
http://lists.connman.net/pipermail/connman/2012-June/010241.html).

2) Named (e.g. pool.ntp.org) time servers do not work. They error out with 
"Time request for server pool.ntp.org failed". This matches behavior back from 
the 0.6x and 0.7x days. 
   Replacing named time servers with a raw IP address works fine.

-Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] main: static-qualify default config values

[Grant Erickson]

The default configuration arrays default_auto_connect and 
default_blacklist must be static-qualified. Otherwise, their
stack-allocated storage will go out of scope after parse_config
is finished and empty or commented-out DefaultAutoConnectTechnologies
or NetworkInterfaceBlacklist config entries will refer to
random stack data.
---
 src/main.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main.c b/src/main.c
index fc3812f..a39adb3 100644
--- a/src/main.c
+++ b/src/main.c
@@ -141,13 +141,13 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
gsize len;
-   char *default_auto_connect[] = {
+   static char *default_auto_connect[] = {
"wifi",
"ethernet",
"cellular",
NULL
};
-   char *default_blacklist[] = {
+   static char *default_blacklist[] = {
"vmnet",
"vboxnet",
"virbr",
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Crash with 1.2 with Ctrl-C on Exit

[Grant Erickson]

connmand[1771]: ...
connmand[1771]: src/rtnl.c:connman_rtnl_remove_watch() id 2
connmand[1771]: plugins/wifi.c:network_removed() name WiFi Activation
connmand[1771]: plugins/wifi.c:network_removed() name Rock
connmand[1771]: plugins/wifi.c:network_removed() name Scissors
connmand[1771]: plugins/wifi.c:network_removed() name Paper
connmand[1771]: plugins/wifi.c:network_removed() name 
connmand[1771]: plugins/wifi.c:scan_callback_hidden() result -5
connmand[1771]: Aborting (signal 11) [/usr/sbin/connmand]
connmand[1771]:  backtrace 
connmand[1771]: #0  0x4021d630 in /lib/libc.so.6
connmand[1771]: #1  0x20c04 in scan_callback_hidden() at plugins/wifi.c:469
connmand[1771]: #2  0x219d0 in remove_interface() at 
gsupplicant/supplicant.c:449
connmand[1771]: #3  0x40057604 in /lib/libglib-2.0.so.0
connmand[1771]: #4  0x40057b18 in /lib/libglib-2.0.so.0
connmand[1771]: #5  0x40057bb4 in /lib/libglib-2.0.so.0
connmand[1771]: #6  0x212ac in g_supplicant_unregister() at 
gsupplicant/supplicant.c:3702
connmand[1771]: #7  0x1e18c in wifi_exit() at plugins/wifi.c:1920
connmand[1771]: #8  0x2a05c in __connman_plugin_cleanup() at src/plugin.c:212
connmand[1771]: #9  0x29134 in main() at src/main.c:617
connmand[1771]: +++

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

RE: [PATCH] service: signal error property changes

[Grant Erickson]

Patrik Flykt patrik.flykt at linux.intel.com 
On Wed Jun 13 00:36:52 PDT 2012, Patrik Flykt wrote:
> On Wed, 2012-06-06 at 23:15 -0700, Grant Erickson wrote:
>> In order to allow clients to update the property in a manner similar
>> to other service properties, signal changes in the service "Error"
>> property key/value pair.
>>
>> The value "none" is used when no error is asserted because D-Bus
>> property changed convention has no semantics for indicating a
>> key/value pair was removed.
>
> doc/service-api.txt documents that the Error property '...is only valid
> when the service is in the "failure" state...' Doesn't that already fix
> the problem at hand?

Patrik:

The statement in doc/service-api.txt is no more or less valid in the presence 
of the proposed patch. When the service is not in the failure state, the value 
of the Error property is broadcast as and set to "None" maintaining the 
validity of that statement.

The advantage of this patch, when implementing a connman client, is that the 
client implementation is greatly simplified--and my general impression is that 
client simplification is a motivating goal of connman. The simplification is 
that no special-case logic is required to watch both the service state and 
error properties in tandem and manipulate the latter when the former changes 
to/from "failure".

With this patch, the client can simply perform a dictionary merge/update of the 
object properties when property changed notification is received.

Best,

Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] service: signal error property changes

[Grant Erickson]

In order to allow clients to update the property in a manner similar to other 
service properties, signal changes
in the service "Error" property key/value pair.

The value "none" is used when no error is asserted because D-Bus property 
changed convention has no semantics
for indicating a key/value pair was removed.
---
 src/service.c |   37 +
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/service.c b/src/service.c
index d6bd870..a2cd5ff 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1427,6 +1427,21 @@ static void autoconnect_changed(struct connman_service 
*service)
DBUS_TYPE_BOOLEAN, &service->autoconnect);
 }
 
+static void error_changed(struct connman_service *service)
+{
+   const char *str;
+   const char *none = "none";
+
+   if (service->path == NULL)
+   return;
+
+   str = error2string(service->error);
+
+   connman_dbus_property_changed_basic(service->path,
+   CONNMAN_SERVICE_INTERFACE, "Error",
+   DBUS_TYPE_STRING, str ? &str : &none);
+}
+
 static void append_security(DBusMessageIter *iter, void *user_data)
 {
struct connman_service *service = user_data;
@@ -3193,11 +3208,23 @@ static DBusMessage *set_property(DBusConnection *conn,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static void set_error(struct connman_service *service,
+   enum connman_service_error error)
+{
+   enum connman_service_error old_error;
+
+   old_error = service->error;
+   service->error = error;
+
+   if (error != old_error)
+   error_changed(service);
+}
+
 static void set_idle(struct connman_service *service)
 {
service->state = service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_IDLE;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
state_changed(service);
 }
 
@@ -4023,6 +4050,8 @@ static void service_initialize(struct connman_service 
*service)
service->refcount = 1;
service->session_usage_count = 0;
 
+   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+
service->type = CONNMAN_SERVICE_TYPE_UNKNOWN;
service->security = CONNMAN_SERVICE_SECURITY_UNKNOWN;
 
@@ -4800,7 +4829,7 @@ static int service_indicate_state(struct connman_service 
*service)
return 0;
service_complete(service);
} else
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
iter = g_hash_table_lookup(service_hash, service->identifier);
if (iter != NULL && g_sequence_get_length(service_list) > 1) {
@@ -4826,7 +4855,7 @@ int __connman_service_indicate_error(struct 
connman_service *service,
if (service == NULL)
return -EINVAL;
 
-   service->error = error;
+   set_error(service, error);
 
if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
__connman_service_set_passphrase(service, NULL);
@@ -4852,7 +4881,7 @@ int __connman_service_clear_error(struct connman_service 
*service)
 
service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_UNKNOWN;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
+   set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
 
if (service->favorite == TRUE)
set_reconnect_state(service, TRUE);
-- 
1.7.10.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] service: remove extra semicolon

[Grant Erickson]

---
 src/service.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/service.c b/src/service.c
index de28264..d6bd870 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4852,7 +4852,7 @@ int __connman_service_clear_error(struct connman_service 
*service)
 
service->state_ipv4 = service->state_ipv6 =
CONNMAN_SERVICE_STATE_UNKNOWN;
-   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;;
+   service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
 
if (service->favorite == TRUE)
set_reconnect_state(service, TRUE);
-- 
1.7.10.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Traces for "Wrong wifi pointer"

[Grant Erickson]

On 6/6/12 12:30 AM, Daniel Wagner wrote:
> Thanks for the logs. I have uploaded it here for the rest of us to enjoy
> (it a bit too large for the ml):
> 
> http://www.monom.org/logs/wrong-wifi-pointer.log
> 
> The second log is easier to understand, but
> I think the same problem bis in the first log.

Daniel,

Thanks for investigating.

> ConnMan is terminating and following things happen:
> 
>   wifi_exit():
> 
>   g_supplicant_unregister(&callbacks);
> 
>   service.c shutdown -> cleanup all services -> networks
> 
>   wifi_remove():
> 
>   g_supplicant_interface_set_data(wifi->interface, NULL);
> 
>   interface_removed():
>   wifi = g_supplicant_interface_get_data(interface);
> 
>   if (wifi == NULL || wifi->device == NULL) {
>   connman_error("Wrong wifi pointer");
>   return;
>   }
> 
> So I think we should just not get the final callback
> interface_removed() at all. Could it be that we do not really
> cleanup correctly in g_supplicant_unregister()?

Without having looked at the actual code, I'd expect that calling
g_supplicant_interface_set_data and g_supplicant_interface_get_data would
both be invalid after having called g_supplicant_unregister.

> BTW, I see that there is autoscan between network_remove() and "Wrong
> wifi pointer". Did you see also apply Tomasz's patches which have
> fixed some problems related to autoscan (since the log says
> ConnMan is 1.0).

Yes, the logs represent 1.0+ which included the autoscan patches. I also
tested against 1.1 proper and saw the same behavior.

-Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] device: do not filter out EALREADY or EINPROGRESS errrors

[Grant Erickson]

If a device scan request immediately returns -ELAREADY or -EINPROGRESS from 
device_scan, do not filter the error and effect a
successful pending return. Instead, immediately process and return the pending 
error reply.

Failure to do this results in periodic D-Bus daemon error assertions about 
malformed method_reply messages as well as scan request
timeouts in the client that invoked the request.
---
 src/device.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/device.c b/src/device.c
index e433991..62ecc2e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1086,7 +1086,7 @@ int __connman_device_request_scan(enum 
connman_service_type type)
}
 
err = device_scan(device);
-   if (err == 0 || err == -EALREADY || err == -EINPROGRESS) {
+   if (err == 0) {
success = TRUE;
} else {
last_err = err;
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Connman 1.x Regression / Issue List

[Grant Erickson]

Over the weekend I identified a few add'l regressions in functionality in 
connman-1.x relative to connman-0.7x that I am going to be diving into today:

1) Connman does not automatically reconnect to idle services following 
a technology powered or offline mode assertion / deassertion cycle or following 
an access point reset.

   - However, if connman itself is restarted, it'll then reconnect. 

2) Connman fails to scan after an extended period of time.

   - I found this issue by leaving my UI parked at the Wi-Fi service 
chooser view, which kicks off a periodic scan request every 10 seconds.

   - After several minutes, all scan requests return with -ENXIO.

3) Connecting to hidden networks succeeds but still prematurely fails 
with an Error.InputOutput. See Comment 3 of BMC #21285 


And there's still the known issue:

4) BMC #21285 : Connman 
Broadcasts Service Property Changes after it Has Removed or Before It Has Added 
a Service

Updates on 1 and 2 as I have them.

-Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] ntp: log the time slew delta

[Grant Erickson]

---
 src/ntp.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/ntp.c b/src/ntp.c
index 9bc548d..91a8832 100644
--- a/src/ntp.c
+++ b/src/ntp.c
@@ -196,6 +196,8 @@ static void decode_msg(void *base, size_t len, struct 
timeval *tv)
 
poll_id = g_timeout_add_seconds(transmit_delay, next_poll, NULL);
 
+   connman_info("ntp: time slew %+.6f s", offset);
+
if (offset < STEPTIME_MIN_OFFSET && offset > -STEPTIME_MIN_OFFSET) {
struct timeval adj;
 
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2 0/4] Fix a few issues with Agent API

[Grant Erickson]

> On Wed, 2012-05-23 at 15:24 +0300, patrik.fl...@linux.intel.com wrote:
>> After looking over the first patch once more with Jukka, we came to the
>> conclusion that the service should not go to failure state after the
>> -ENOKEY checking part. If the service is set to failure, it cannot be
>> autoconnected anymore. The existing network disconnect takes care of the
>> service and network being in proper state if __connman_service_connect()
>> fails or reconnects after asking for a passphrase.
> 
> Any comments especially on 1/4 or shall I just go ahead and apply it?
> 
> Cheers,
> 
>   Patrik

They seem to be working well here.

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Service Connect Hiccups with Hidden Services in Connman 1.0

[Grant Erickson]

It's not  show-stopper, but I found another wrinkle with agent connect
after applying Patrik's patches that, in some fashion, relates back to a
long-standing signal sequencing problem (BMC 21285 at
https://bugs.meego.com/show_bug.cgi?id=21285).

*Problem 1:* If I connect to a hidden network, the .Service.Connect
immediately returns with -EIO after the .Agent.RequestInput has been sent
and ACKed with the passphrase and name key/value pairs.

*Problem 2:* Then, I receive two back-to-pack .Service.PropertyChanged
notifications for "Ethernet" on the service corresponding to the hidden
service the .Service.Connect was initiated for. This occurs before I've
received a .Manager.ServicesChanged for that service.

Since .Manager.ServicesChanged is how I setup the services hash, I reject
the two "Ethernet" signals because I don't technically know about that
service yet.

*Problem 3:* I then get a "State" changed for the corresponding service. I
reject that too because .Manager.ServicesChanged still hasn't informed me
of the service yet.

Finally, I get a Manager.ServicesChanged with the new service corresponding
to the hidden service I asked to connect to.

Here are application-level logs documenting the sequence of events:


2012-05-24 07:42:08.725765 [2757:57d730] wifi: connect to hidden, secured
network
2012-05-24 07:42:08.726955 [2757:57d730] wifi: attempting to connect to
network service (Hidden Test) with passphrase (***)
2012-05-24 07:42:08.728084 [2757:57d730] connectlayer: setting network
display name to connect to Hidden Test
2012-05-24 07:42:08.751857 [2757:57d730] connectlayer: setting network
connect state to 0
2012-05-24 07:42:08.755062 [2757:57d730] connectlayer: setting network
connect status to 0
2012-05-24 07:42:08.865352 [2757:57d730] connman: AgentRequestInput
2012-05-24 07:42:08.866725 [2757:57d730] connman:   object path:
/net/connman/service/wifi_000c2926defd_hidden_managed_psk
2012-05-24 07:42:08.867885 [2757:57d730] connman:   array start
2012-05-24 07:42:08.869014 [2757:57d730] connman: dict entry (Name)
2012-05-24 07:42:08.870082 [2757:57d730] connman: variant
2012-05-24 07:42:08.871120 [2757:57d730] connman: array start
2012-05-24 07:42:08.872219 [2757:57d730] connman:   dict entry (Type)
2012-05-24 07:42:08.873256 [2757:57d730] connman:   variant
2012-05-24 07:42:08.874446 [2757:57d730] connman:   string: string
2012-05-24 07:42:08.875606 [2757:57d730] connman:   dict entry
(Requirement)
2012-05-24 07:42:08.877254 [2757:57d730] connman:   variant
2012-05-24 07:42:08.878261 [2757:57d730] connman:   string: mandatory
2012-05-24 07:42:08.879421 [2757:57d730] connman:   dict entry
(Alternates)
2012-05-24 07:42:08.880489 [2757:57d730] connman:   variant
2012-05-24 07:42:08.883235 [2757:57d730] connman:   array start
2012-05-24 07:42:08.884883 [2757:57d730] connman: string: SSID
2012-05-24 07:42:08.885951 [2757:57d730] connman: array end
2012-05-24 07:42:08.887050 [2757:57d730] connman:   array end
2012-05-24 07:42:08.888149 [2757:57d730] connman: dict entry (SSID)
2012-05-24 07:42:08.889186 [2757:57d730] connman: variant
2012-05-24 07:42:08.890285 [2757:57d730] connman: array start
2012-05-24 07:42:08.891323 [2757:57d730] connman:   dict entry (Type)
2012-05-24 07:42:08.892391 [2757:57d730] connman:   variant
2012-05-24 07:42:08.893398 [2757:57d730] connman:   string: ssid
2012-05-24 07:42:08.894496 [2757:57d730] connman:   dict entry
(Requirement)
2012-05-24 07:42:08.895534 [2757:57d730] connman:   variant
2012-05-24 07:42:08.897090 [2757:57d730] connman:   string: alternate
2012-05-24 07:42:08.898372 [2757:57d730] connman:   array end
2012-05-24 07:42:08.899471 [2757:57d730] connman: dict entry
(Passphrase)
2012-05-24 07:42:08.900539 [2757:57d730] connman: variant
2012-05-24 07:42:08.901485 [2757:57d730] connman: array start
2012-05-24 07:42:08.902522 [2757:57d730] connman:   dict entry (Type)
2012-05-24 07:42:08.903560 [2757:57d730] connman:   variant
2012-05-24 07:42:08.904567 [2757:57d730] connman:   string: psk
2012-05-24 07:42:08.905818 [2757:57d730] connman:   dict entry
(Requirement)
2012-05-24 07:42:08.906886 [2757:57d730] connman:   variant
2012-05-24 07:42:08.907924 [2757:57d730] connman:   string: mandatory
2012-05-24 07:42:08.909603 [2757:57d730] connman:   array end
2012-05-24 07:42:08.910671 [2757:57d730] connman: array end
2012-05-24 07:42:08.916072 [2757:57d730] network manager: signaling input
request for service
"/net/connman/service/wifi_000c2926defd_hidden_managed_psk"
2012-05-24 07:42:08.965999 [2757:57d730] connman:   array start
2012-05-24 07:42:08.967159 [2757:57d730] connman: dict entry
(Passphrase)
2012-05-24 07:42:08.968410 [2757:57d730] connman: variant
2012-05-24 07:42:08.969417 [2757:57d730] connman: string: 
2012-05-24 07:42:08.970546 [2757:57d730] connman: dict entry (Name)
2012-05-24 07:42:08.

Re: Agent-based Service Termination Mid-connection

[Grant Erickson]

On May 22, 2012, at 2:03 PM, Grant Erickson wrote:
> Picking up a thread from IRC, I've been working on getting agent-based 
> service connections working with connman-1.0. However, what I've found is 
> that rather than performing the following sequence of operations:
> 
>   1) Issue .Service.Connect call.
>   2) Receive properties dictionary from .Agent.RequestInput call.
>   3) Post message to UI with a closure context for the async pending 
> .Agent.RequestInput call.
>   4) Gather requested input.
>   5) Create a response dictionary
>   6) Complete closure context and send D-Bus reply back to initial 
> .Agent.RequestInput call.
> 
> What I see happening instead is:
> 
>   1) Issue .Service.Connect call.
>   2) Receive .Service.PropertiesChanged call with State = failure.
>   3) Receive net.connman.Error.Failed: Input/output error error in 
> response to (1).
>   4) Receive properties dictionary from .Agent.RequestInput call.
> 
> and things generally fall apart from there. Debugging this, in particular the 
> source of the -EIO asynchronous cancellation in the middle of the connect 
> request, what I find is the following:
> 
>   23.000 debug connmand:  --> (3) connect_service: 
> service 0xaa320
>   23.000 debug connmand:  --> (4) 
> __connman_service_connect: service 0xaa320 state idle
>   23.000 info  connmand: User-requested connection to service "Test" 
> identifier "wifi_02283e3d4347_65547473_managed_psk".
>   23.000 debug connmand: service_connect returned -126 (Required key not 
> available)
>   23.000 debug connmand: __connman_error_failed: msg 0xa0698 errnum 5 
> (Input/output error)
>   23.000 err   connmand:  backtrace 
>   24.000 err   connmand: #0  0x31078 in reply_pending() at 
> src/service.c:3315
>   24.000 err   connmand: #1  0x35010 in service_complete() at 
> src/service.c:4383
>   24.000 err   connmand: #2  0x35288 in service_indicate_state() at 
> src/service.c:4749
>   24.000 err   connmand: #3  0x35628 in 
> __connman_service_ipconfig_indicate_state() at src/service.c:5041
>   24.000 err   connmand: #4  0x35f04 in __connman_service_connect() at 
> src/service.c:5291
>   24.000 err   connmand: #5  0x36520 in connect_service() at 
> src/service.c:3449
>   24.000 err   connmand: #6  0x13130 in process_message() at 
> gdbus/object.c:199
>   24.000 err   connmand: #7  0x40163e84 in /lib/libdbus-1.so.3
>   24.000 err   connmand: #8  0x4014f5d4 in /lib/libdbus-1.so.3
>   24.000 err   connmand: #9  0x12090 in message_dispatch() at 
> gdbus/mainloop.c:76
>   24.000 err   connmand: #10 0x4006773c in /lib/libglib-2.0.so.0
>   24.000 err   connmand: #11 0x40066db4 in /lib/libglib-2.0.so.0
>   24.000 err   connmand: #12 0x4006ab98 in /lib/libglib-2.0.so.0
>   24.000 err   connmand: #13 0x4006b148 in /lib/libglib-2.0.so.0
>   24.000 err   connmand: #14 0x27788 in main() at src/main.c:562
>   24.000 err   connmand: +++
>   24.000 debug connmand: service->network 0xa8e48
>   24.000 debug connmand:  --> (5) 
> __connman_network_disconnect: network 0xa8e48
>   24.000 debug connmand: user connect 1, error is -126 (Required key not 
> available)
>   24.000 debug connmand:  --> (6) 
> __connman_agent_request_passphrase_input: service 0xaa320 callback 0x369d4 
> user_data (nil)
>   24.000 debug connmand:  <-- (6) 
> __connman_agent_request_passphrase_input: err -5 (Input/output error)
>   24.000 debug connmand:  <-- (5) 
> __connman_service_connect: err -115 (Operation now in progress)
>   24.000 debug connmand: __connman_service_connect returned -115 
> (Operation now in progress)
>   24.000 debug connmand: return 2
>   24.000 debug connmand:  <-- (4) 
> connect_service: reply (nil)
> 
> So, it would appear that service_indicate_state is killing the 
> .Service.Connect call prematurely for reasons yet to be determined.

I've done a bit more debugging on this and one factor seems to be setting both 
the IPv4 and IPv6 state in __connman_service_connect to _STATE_FAILURE. As an 
experiment, I changed the values there to _STATE_IDLE and things started to 
behave more as expected with the order of operations occurring as in 1-6 above; 
however, the default .Agent.RequestInput timeout (20s) was entirely too short 
as the call timed out before I was finished entering the passphrase.

Why do we set the IPv

Agent-based Service Termination Mid-connection

[Grant Erickson]

Picking up a thread from IRC, I've been working on getting agent-based service 
connections working with connman-1.0. However, what I've found is that rather 
than performing the following sequence of operations:

1) Issue .Service.Connect call.
2) Receive properties dictionary from .Agent.RequestInput call.
3) Post message to UI with a closure context for the async pending 
.Agent.RequestInput call.
4) Gather requested input.
5) Create a response dictionary
6) Complete closure context and send D-Bus reply back to initial 
.Agent.RequestInput call.

What I see happening instead is:

1) Issue .Service.Connect call.
2) Receive .Service.PropertiesChanged call with State = failure.
3) Receive net.connman.Error.Failed: Input/output error error in 
response to (1).
4) Receive properties dictionary from .Agent.RequestInput call.

and things generally fall apart from there. Debugging this, in particular the 
source of the -EIO asynchronous cancellation in the middle of the connect 
request, what I find is the following:

23.000 debug connmand:  --> (3) connect_service: 
service 0xaa320
23.000 debug connmand:  --> (4) 
__connman_service_connect: service 0xaa320 state idle
23.000 info  connmand: User-requested connection to service "Test" 
identifier "wifi_02283e3d4347_65547473_managed_psk".
23.000 debug connmand: service_connect returned -126 (Required key not 
available)
23.000 debug connmand: __connman_error_failed: msg 0xa0698 errnum 5 
(Input/output error)
23.000 err   connmand:  backtrace 
24.000 err   connmand: #0  0x31078 in reply_pending() at 
src/service.c:3315
24.000 err   connmand: #1  0x35010 in service_complete() at 
src/service.c:4383
24.000 err   connmand: #2  0x35288 in service_indicate_state() at 
src/service.c:4749
24.000 err   connmand: #3  0x35628 in 
__connman_service_ipconfig_indicate_state() at src/service.c:5041
24.000 err   connmand: #4  0x35f04 in __connman_service_connect() at 
src/service.c:5291
24.000 err   connmand: #5  0x36520 in connect_service() at 
src/service.c:3449
24.000 err   connmand: #6  0x13130 in process_message() at 
gdbus/object.c:199
24.000 err   connmand: #7  0x40163e84 in /lib/libdbus-1.so.3
24.000 err   connmand: #8  0x4014f5d4 in /lib/libdbus-1.so.3
24.000 err   connmand: #9  0x12090 in message_dispatch() at 
gdbus/mainloop.c:76
24.000 err   connmand: #10 0x4006773c in /lib/libglib-2.0.so.0
24.000 err   connmand: #11 0x40066db4 in /lib/libglib-2.0.so.0
24.000 err   connmand: #12 0x4006ab98 in /lib/libglib-2.0.so.0
24.000 err   connmand: #13 0x4006b148 in /lib/libglib-2.0.so.0
24.000 err   connmand: #14 0x27788 in main() at src/main.c:562
24.000 err   connmand: +++
24.000 debug connmand: service->network 0xa8e48
24.000 debug connmand:  --> (5) 
__connman_network_disconnect: network 0xa8e48
24.000 debug connmand: user connect 1, error is -126 (Required key not 
available)
24.000 debug connmand:  --> (6) 
__connman_agent_request_passphrase_input: service 0xaa320 callback 0x369d4 
user_data (nil)
24.000 debug connmand:  <-- (6) 
__connman_agent_request_passphrase_input: err -5 (Input/output error)
24.000 debug connmand:  <-- (5) 
__connman_service_connect: err -115 (Operation now in progress)
24.000 debug connmand: __connman_service_connect returned -115 
(Operation now in progress)
24.000 debug connmand: return 2
24.000 debug connmand:  <-- (4) 
connect_service: reply (nil)

So, it would appear that service_indicate_state is killing the .Service.Connect 
call prematurely for reasons yet to be determined.

Best,

Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

RE: Memory Usage in 0.76+ vs. 1.0

[Grant Erickson]

On 2012-05-21, Grant Erickson wrote:
> Following up on an IRC discussion, I had noted on upgrading connman from
> 0.76+ to 1.0 that my previously-set resource limits of:
> 
> rlimit-as=5242880
> rlimit-core=0
> rlimit-data=3145728
> rlimit-nofile=768
> rlimit-stack=262144
> 
> were causing connman-1.0 to crash in glib on start-up. So, I thought I'd
> post the following memory usage summary between those two versions:
> 
> -
> Memory Usage / KiB
> -
> Version0.76+   1.0   Change
> -
> IdleConnected   IdleConnected   IdleConnected
> -
> VmPeak  4248   4248 5588   5588 +32%   +32%
> VmSize  4216   4216 5580   5580 +32%   +32%
> VmLck  0  00  0  +0%+0%
> VmHWM   1972   2068 2016   2300  +2%   +11%
> VmRSS   1972   2068 2016   2300  +2%   +11%
> VmData   304304  316316  +4%+4%
> VmStk 84 84   84 84  +0%+0%
> VmExe372372  448448 +20%   +20%
> VmLib   3132   3132 4292   4292 +37%   +37%
> VmPTE  8  8   10 10 +25%   +25%
> Stack usage   16 16   16 20  +0%   +25%
> -
> 
> It's worth noting that moving from 0.76+ to 1.0 required a glib upgrade from
> 2.24.2 to 2.28.8, so some of the increases, particularly in VmLib, stem from
> that change.

Marcel,

With your --disable-wispr configuration change, the new 1.0 numbers are:

VmPeak: 4300 kB
VmSize: 4292 kB
VmLck: 0 kB
VmHWM:  1892 kB
VmRSS:  1892 kB
VmData:  308 kB
VmStk:84 kB
VmExe:   444 kB
VmLib:  3132 kB
VmPTE: 8 kB
Stack usage:16 kB

Exactly what I'd expect: growth in VmExe to reflect the new code and features 
but fairly stable elsewhere. I'd say GNUTLS is the culprit.

Best,

Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] storage: migrate 0.76-era device enabled / powered state

[Grant Erickson]

This patch ensures that [device_] Powered= device enabled
/ powered state migrates from 0.76-era versions of default.profile.
---
 src/storage.c |   34 +++
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/storage.c b/src/storage.c
index 2faab75..e3111b6 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -418,11 +418,19 @@ void __connman_storage_migrate()
g_key_file_set_boolean(keyfile, "global",
"OfflineMode", val);
 
+   /* Migrate Powered/Enable state key/value pairs from legacy
+* settings
+*/
+
val = g_key_file_get_boolean(keyfile_def, "WiFi",
"Enable", &error);
if (error != NULL) {
g_clear_error(&error);
-   val = FALSE;
+   val = g_key_file_get_boolean(keyfile_def, "device_Wireless", 
"Powered", &error);
+   if (error != NULL) {
+   g_clear_error(&error);
+   val = FALSE;
+   }
}
 
g_key_file_set_boolean(keyfile, "WiFi",
@@ -432,7 +440,11 @@ void __connman_storage_migrate()
"Enable", &error);
if (error != NULL) {
g_clear_error(&error);
-   val = FALSE;
+   val = g_key_file_get_boolean(keyfile_def, "device_Bluetooth", 
"Powered", &error);
+   if (error != NULL) {
+   g_clear_error(&error);
+   val = FALSE;
+   }
}
 
g_key_file_set_boolean(keyfile, "Bluetooth",
@@ -442,7 +454,11 @@ void __connman_storage_migrate()
"Enable", &error);
if (error != NULL) {
g_clear_error(&error);
-   val = FALSE;
+   val = g_key_file_get_boolean(keyfile_def, "device_Ethernet", 
"Powered", &error);
+   if (error != NULL) {
+   g_clear_error(&error);
+   val = FALSE;
+   }
}
 
g_key_file_set_boolean(keyfile, "Wired",
@@ -452,7 +468,11 @@ void __connman_storage_migrate()
"Enable", &error);
if (error != NULL) {
g_clear_error(&error);
-   val = FALSE;
+   val = g_key_file_get_boolean(keyfile_def, "device_Cellular", 
"Powered", &error);
+   if (error != NULL) {
+   g_clear_error(&error);
+   val = FALSE;
+   }
}
 
g_key_file_set_boolean(keyfile, "Cellular",
@@ -462,7 +482,11 @@ void __connman_storage_migrate()
"Enable", &error);
if (error != NULL) {
g_clear_error(&error);
-   val = FALSE;
+   val = g_key_file_get_boolean(keyfile_def, "device_WiMAX", 
"Powered", &error);
+   if (error != NULL) {
+   g_clear_error(&error);
+   val = FALSE;
+   }
}
 
g_key_file_set_boolean(keyfile, "WiMAX",
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Memory Usage in 0.76+ vs. 1.0

[Grant Erickson]

Following up on an IRC discussion, I had noted on upgrading connman from
0.76+ to 1.0 that my previously-set resource limits of:

rlimit-as=5242880
rlimit-core=0
rlimit-data=3145728
rlimit-nofile=768
rlimit-stack=262144

were causing connman-1.0 to crash in glib on start-up. So, I thought I'd
post the following memory usage summary between those two versions:

-
Memory Usage / KiB
-
Version0.76+   1.0   Change
-
IdleConnected   IdleConnected   IdleConnected
-
VmPeak  4248   4248 5588   5588 +32%   +32%
VmSize  4216   4216 5580   5580 +32%   +32%
VmLck  0  00  0  +0%+0%
VmHWM   1972   2068 2016   2300  +2%   +11%
VmRSS   1972   2068 2016   2300  +2%   +11%
VmData   304304  316316  +4%+4%
VmStk 84 84   84 84  +0%+0%
VmExe372372  448448 +20%   +20%
VmLib   3132   3132 4292   4292 +37%   +37%
VmPTE  8  8   10 10 +25%   +25%
Stack usage   16 16   16 20  +0%   +25%
-

It's worth noting that moving from 0.76+ to 1.0 required a glib upgrade from
2.24.2 to 2.28.8, so some of the increases, particularly in VmLib, stem from
that change.

Best,

Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gsupplicant: coding style clean-up

[Grant Erickson]

This resolves several post-commit review coding style comments for raw PSK 
support.
---
 gsupplicant/supplicant.c |   26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 829ceb7..0ae5540 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -2964,23 +2964,22 @@ static dbus_bool_t is_psk_raw_key(const char *psk)
 static unsigned char hexchar2bin(char c)
 {
if ((c >= '0') && (c <= '9'))
-   return (c - '0');
+   return c - '0';
else if ((c >= 'A') && (c <= 'F'))
-   return (c - 'A' + 10);
+   return c - 'A' + 10;
else if ((c >= 'a') && (c <= 'f'))
-   return (c - 'a' + 10);
+   return c - 'a' + 10;
else
-   return (c);
+   return c;
 }
 
 static void hexstring2bin(const char *string, unsigned char *data, size_t 
data_len)
 {
size_t i;
 
-   if ((data != NULL) && (string != NULL))
-   for (i = 0; i < data_len; i++)
-   data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
-  hexchar2bin(string[i * 2 + 1]) << 0);
+   for (i = 0; i < data_len; i++)
+   data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
+  hexchar2bin(string[i * 2 + 1]) << 0);
 }
 
 static void add_network_security_psk(DBusMessageIter *dict,
@@ -2990,15 +2989,18 @@ static void add_network_security_psk(DBusMessageIter 
*dict,
const char *key = "psk";
 
if (is_psk_raw_key(ssid->passphrase) == TRUE) {
-   const size_t size = 32;
-   unsigned char data[size];
+   unsigned char data[32];
unsigned char *datap = data;
 
-   hexstring2bin(ssid->passphrase, datap, size);
+   /* The above pointer alias is required by D-Bus because
+* with D-Bus and GCC, non-heap-allocated arrays cannot
+* be passed directly by their base pointer. */
+
+   hexstring2bin(ssid->passphrase, datap, sizeof(data));
 
supplicant_dbus_dict_append_fixed_array(dict,
key, DBUS_TYPE_BYTE,
-   &datap, size);
+   &datap, sizeof(data));
} else
supplicant_dbus_dict_append_basic(dict,
key, DBUS_TYPE_STRING,
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] gsupplicant: fix "raw" PSK support

[Grant Erickson]

This fixes "raw" PSK support by first converting the "raw" PSK from
a 64-character hexadecimal string to 32-byte array before appending
it to the network dictionary.

---
 v1: Initial submission.
 v2: Fixed white space.

 gsupplicant/supplicant.c |   42 --
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 977c0fb..3af1662 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -2961,17 +2961,47 @@ static dbus_bool_t is_psk_raw_key(const char *psk)
return TRUE;
 }
 
+static unsigned char hexchar2bin(char c)
+{
+   if ((c >= '0') && (c <= '9'))
+   return (c - '0');
+   else if ((c >= 'A') && (c <= 'F'))
+   return (c - 'A' + 10);
+   else if ((c >= 'a') && (c <= 'f'))
+   return (c - 'a' + 10);
+   else
+   return (c);
+}
+
+static void hexstring2bin(const char *string, unsigned char *data, size_t 
data_len)
+{
+   size_t i;
+
+   if ((data != NULL) && (string != NULL))
+   for (i = 0; i < data_len; i++)
+   data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
+  hexchar2bin(string[i * 2 + 1]) << 0);
+}
+
 static void add_network_security_psk(DBusMessageIter *dict,
GSupplicantSSID *ssid)
 {
if (ssid->passphrase && strlen(ssid->passphrase) > 0) {
-   if (is_psk_raw_key(ssid->passphrase) == TRUE)
+   const char *key = "psk";
+
+   if (is_psk_raw_key(ssid->passphrase) == TRUE) {
+   const size_t size = 32;
+   unsigned char data[size];
+   unsigned char *datap = data;
+
+   hexstring2bin(ssid->passphrase, datap, size);
+
supplicant_dbus_dict_append_fixed_array(dict,
-   "psk", DBUS_TYPE_BYTE,
-   &ssid->passphrase, 64);
-   else
-   supplicant_dbus_dict_append_basic(dict, "psk",
-   DBUS_TYPE_STRING,
+   key, DBUS_TYPE_BYTE,
+   &datap, size);
+   } else
+   supplicant_dbus_dict_append_basic(dict,
+   key, DBUS_TYPE_STRING,
&ssid->passphrase);
}
 }
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2] gsupplicant: fix "raw" PSK support

[Grant Erickson]

On May 15, 2012, at 8:09 AM, Marcel Holtmann wrote:
>> This fixes "raw" PSK support by first converting the "raw" PSK from
>> a 64-character hexadecimal string to 32-byte array before appending
>> it to the network dictionary.
>> 
>> ---
>> v1: Initial submission.
>> v2: Fixed white space.
>> 
>> gsupplicant/supplicant.c |   42 --
>> 1 file changed, 36 insertions(+), 6 deletions(-)
>> 
>> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
>> index 977c0fb..3af1662 100644
>> --- a/gsupplicant/supplicant.c
>> +++ b/gsupplicant/supplicant.c
>> @@ -2961,17 +2961,47 @@ static dbus_bool_t is_psk_raw_key(const char *psk)
>>  return TRUE;
>> }
>> 
>> +static unsigned char hexchar2bin(char c)
>> +{
>> +if ((c >= '0') && (c <= '9'))
>> +return (c - '0');
>> +else if ((c >= 'A') && (c <= 'F'))
>> +return (c - 'A' + 10);
>> +else if ((c >= 'a') && (c <= 'f'))
>> +return (c - 'a' + 10);
>> +else
>> +return (c);
>> +}
>> +
>> +static void hexstring2bin(const char *string, unsigned char *data, size_t 
>> data_len)
>> +{
>> +size_t i;
>> +
>> +if ((data != NULL) && (string != NULL))
> 
> Actually "if (data != NULL && string != NULL)" is enough. The extra ( )
> are just useless and will clutter the code.
> 
> Also it seems this check is rather useless since it will never be NULL
> in the first place. It is only called from on location.

Sure. Will update and resubmit.

>> +for (i = 0; i < data_len; i++)
>> +data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
>> +   hexchar2bin(string[i * 2 + 1]) << 0);
>> +}
>> +
>> static void add_network_security_psk(DBusMessageIter *dict,
>>  GSupplicantSSID *ssid)
>> {
>>  if (ssid->passphrase && strlen(ssid->passphrase) > 0) {
>> -if (is_psk_raw_key(ssid->passphrase) == TRUE)
>> +const char *key = "psk";
>> +
>> +if (is_psk_raw_key(ssid->passphrase) == TRUE) {
>> +const size_t size = 32;
>> +unsigned char data[size];
>> +unsigned char *datap = data;
> 
> What is this datap conversion for? Just use data in hexstring2bin.

Sure I can do it at that call point without issue. However, not so later.

> And
> please use unsigned char data[32] and then sizeof(data). Sorry, but this
> is way too complicated.

I wish it were that simple and, were it, I'd have been in bed two hours earlier 
last night. However, per:

http://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html#ga591f3aab5dd2c87e56e05423c2a671d9

for some reason, as far as D-Bus is concerned, all pointers are not equal when 
it comes to non-heap-allocated arrays. I'll add a comment clarifying the 
nonsense.

Best,

Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] gsupplicant: fix "raw" PSK support

[Grant Erickson]

This fixes "raw" PSK support by first converting the "raw" PSK from
a 64-character hexadecimal string to 32-byte array before appending
it to the network dictionary.

---
 v1: Initial submission.
 v2: Fixed white space.

 gsupplicant/supplicant.c |   42 --
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 977c0fb..3af1662 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -2961,17 +2961,47 @@ static dbus_bool_t is_psk_raw_key(const char *psk)
return TRUE;
 }
 
+static unsigned char hexchar2bin(char c)
+{
+   if ((c >= '0') && (c <= '9'))
+   return (c - '0');
+   else if ((c >= 'A') && (c <= 'F'))
+   return (c - 'A' + 10);
+   else if ((c >= 'a') && (c <= 'f'))
+   return (c - 'a' + 10);
+   else
+   return (c);
+}
+
+static void hexstring2bin(const char *string, unsigned char *data, size_t 
data_len)
+{
+   size_t i;
+
+   if ((data != NULL) && (string != NULL))
+   for (i = 0; i < data_len; i++)
+   data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
+  hexchar2bin(string[i * 2 + 1]) << 0);
+}
+
 static void add_network_security_psk(DBusMessageIter *dict,
GSupplicantSSID *ssid)
 {
if (ssid->passphrase && strlen(ssid->passphrase) > 0) {
-   if (is_psk_raw_key(ssid->passphrase) == TRUE)
+   const char *key = "psk";
+
+   if (is_psk_raw_key(ssid->passphrase) == TRUE) {
+   const size_t size = 32;
+   unsigned char data[size];
+   unsigned char *datap = data;
+
+   hexstring2bin(ssid->passphrase, datap, size);
+
supplicant_dbus_dict_append_fixed_array(dict,
-   "psk", DBUS_TYPE_BYTE,
-   &ssid->passphrase, 64);
-   else
-   supplicant_dbus_dict_append_basic(dict, "psk",
-   DBUS_TYPE_STRING,
+   key, DBUS_TYPE_BYTE,
+   &datap, size);
+   } else
+   supplicant_dbus_dict_append_basic(dict,
+   key, DBUS_TYPE_STRING,
&ssid->passphrase);
}
 }
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gsupplicant: fix "raw" PSK support

[Grant Erickson]

This fixes "raw" PSK support by first converting the "raw" PSK from
a 64-character hexadecimal string to 32-byte array before appending
it to the network dictionary.

---
 gsupplicant/supplicant.c |   42 --
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 977c0fb..3af1662 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -2961,17 +2961,47 @@ static dbus_bool_t is_psk_raw_key(const char *psk)
return TRUE;
 }
 
+static unsigned char hexchar2bin(char c)
+{
+   if ((c >= '0') && (c <= '9'))
+   return (c - '0');
+   else if ((c >= 'A') && (c <= 'F'))
+   return (c - 'A' + 10);
+   else if ((c >= 'a') && (c <= 'f'))
+   return (c - 'a' + 10);
+   else
+   return (c);
+}
+
+static void hexstring2bin(const char *string, unsigned char *data, size_t 
data_len)
+{
+   size_t i;
+
+   if ((data != NULL) && (string != NULL))
+   for (i = 0; i < data_len; i++)
+   data[i] = (hexchar2bin(string[i * 2 + 0]) << 4 |
+  hexchar2bin(string[i * 2 + 1]) << 0);
+}
+
 static void add_network_security_psk(DBusMessageIter *dict,
GSupplicantSSID *ssid)
 {
if (ssid->passphrase && strlen(ssid->passphrase) > 0) {
-   if (is_psk_raw_key(ssid->passphrase) == TRUE)
+   const char *key = "psk";
+
+   if (is_psk_raw_key(ssid->passphrase) == TRUE) {
+   const size_t size = 32;
+   unsigned char data[size];
+unsigned char *datap = data;
+
+   hexstring2bin(ssid->passphrase, datap, size);
+
supplicant_dbus_dict_append_fixed_array(dict,
-   "psk", DBUS_TYPE_BYTE,
-   &ssid->passphrase, 64);
-   else
-   supplicant_dbus_dict_append_basic(dict, "psk",
-   DBUS_TYPE_STRING,
+   key, DBUS_TYPE_BYTE,
+   &datap, size);
+   } else
+   supplicant_dbus_dict_append_basic(dict,
+   key, DBUS_TYPE_STRING,
&ssid->passphrase);
}
 }
-- 
1.7.10

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Connman and Raw WPA/WPA2 PSK Values

[Grant Erickson]

On May 14, 2012, at 6:27 PM, Grant Erickson wrote:
> Has anyone recently run a regression with connman against full, 64-character 
> "raw" WPA/WPA2 PSKs? I found the following results today:
> 
>   http://pastebin.com/y73zH074
> 
> While the following non-upstreamed wpa_s patch looks relevant
> 
>   
> http://codereview.chromium.org/6783033/diff/1/wpa_supplicant/dbus/dbus_new_handlers.c
> 
> It doesn't appear to make any difference.
> 
> I need to take a look at what's going across the D-Bus wire, but I suspect 
> that connman is handing the "raw" PSK to the supplicant in a format it 
> doesn't understand or isn't parsing correctly.

As far as I can see, this seems to have changed around commit 
0f37827d4eaa59f0c793225c3723adc16941ef40:

connmand[841]: Skipping disconnect of 472d53706f74_managed_psk, network 
is connecting.
connmand[841]: --> (0) add_network_security_psk: dict 0xbeecd84c ssid 
0x93c80 passphrase 
ca473586684c60e2d3fb994b7b8f79a2712c130a10fa055d098af6c424fd2d2e
connmand[841]:  --> (1) is_psk_raw_key: psk 
ca473586684c60e2d3fb994b7b8f79a2712c130a10fa055d098af6c424fd2d2e
connmand[841]:  <-- (1) is_psk_raw_key: raw 1
connmand[841]: <-- (0) add_network_security_psk: 
946689402.007415: dbus: fi.w1.wpa_supplicant1.Interface.AddNetwork 
(/fi/w1/wpa_supplicant1/Interfaces/0)
946689402.007781: dbus: Register network object 
'/fi/w1/wpa_supplicant1/Interfaces/0/Networks/0'
946689402.025482: set_network_properties: entry.key scan_ssid 
entry.type 117 entry.array_type 0 entry.array_len 0
946689402.025817: wpa_config_set: ssid 0x97dc8 var scan_ssid value 1
946689402.026184: scan_ssid=1 (0x1)
946689402.026519: set_network_properties: entry.key mode entry.type 117 
entry.array_type 0 entry.array_len 0
946689402.026672: wpa_config_set: ssid 0x97dc8 var mode value 0
946689402.026794: mode=0 (0x0)
946689402.028320: set_network_properties: entry.key psk entry.type 97 
entry.array_type 121 entry.array_len 64
946689402.028472: entry.bytearray_value - hexdump(len=64): 63 61 34 37 
33 35 38 36 36 38 34 63 36 30 65 32 64 33 66 62 39 39 34 62 37 62 38 66 37 39 
61 32 37 31 32 63 31 33 30 61 31 30 66 61 30 35 35 64 30 39 38 61 66 36 63 34 
32 34 66 64 32 64 32 65
946689402.034973: set_network_properties: ret 128
946689402.035125: wpa_config_set: ssid 0x97dc8 var psk value 
63613437333538363638346336306532643366623939346237623866373961323731326331333061313066613035356430393861663663343234666432643265
946689402.035247: --> wpa_config_parse_psk data 0x5e570 ssid 0x97dc8 
line 0 value 
63613437333538363638346336306532643366623939346237623866373961323731326331333061313066613035356430393861663663343234666432643265
946689402.035369: --> hexstr2bin hex 
63613437333538363638346336306532643366623939346237623866373961323731326331333061313066613035356430393861663663343234666432643265
 buf 0x97dec len 32
946689402.035491: <-- hexstr2bin retval 0
946689402.035552: Line 0: Invalid PSK 
'63613437333538363638346336306532643366623939346237623866373961323731326331333061313066613035356430393861663663343234666432643265'.
946689402.035736: wpas_dbus_handler_add_network[dbus]:control interface 
couldn't set network properties

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Connman and Raw WPA/WPA2 PSK Values

[Grant Erickson]

Has anyone recently run a regression with connman against full, 64-character 
"raw" WPA/WPA2 PSKs? I found the following results today:

http://pastebin.com/y73zH074

While the following non-upstreamed wpa_s patch looks relevant


http://codereview.chromium.org/6783033/diff/1/wpa_supplicant/dbus/dbus_new_handlers.c

It doesn't appear to make any difference.

I need to take a look at what's going across the D-Bus wire, but I suspect that 
connman is handing the "raw" PSK to the supplicant in a format it doesn't 
understand or isn't parsing correctly.

-Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: connman Digest, Vol 40, Issue 25

[Grant Erickson]

On May 11, 2012, at 2:42 AM, connman-requ...@connman.net wrote:
> From: Marcel Holtmann 
> Subject: Re: Wifi won't connect
> Date: May 11, 2012 1:56:04 AM PDT
> 
> Hi Patrik,
> 
>>> Adding a global section seems to change nothing.
>>> [global]
>> 
>> Reading the code in src/config.c, load_config() reveals that the
>> [global] section was not mandatory to have.
>> 
>>> [service_dunasyswifi]
>>> Type=wifi
>>> SSID=MY_WIFI
>>> Passphrase=secret
>> 
>> In the same file, load_service() expects SSID to be in hexadecimal
>> notation only. If there is no SSID field, a 'Name' field in plain text
>> is used instead. I think it should work right away if you add:
>> 
>> [service_dunasyswifi]
>> Type=wifi
>> Name=MY_WIFI
>> Passphrase=secret
>> 
>> The documentation is obviously buggy when compare with the actual code.
>> I'll fix that.
> 
> in which way is it buggy?
> 
>>> On the contrary if I create a /var/lib/connman/default.profile file 
>> 
>> This works because ConnMan migrates the old default.profile to the new
>> setup into separate directories.
> 
> We should have removed the support for default.profile for 1.0. At least
> lets start by deleting default.profile once we converted it.

Deleting it makes sense; however, please don't remove the conversion support. 
When I roll a large number of deployed devices in the field from 0.76+ to 1.0, 
having the auto-migration in place will be very valuable.

Best,

Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Connman 0.78 : security protection change on a fly on wifi AP

[Grant Erickson]

On 4/25/12 8:15 AM, Ogor Jean-Jacques (AQL) wrote:
> I noticed that changing Wifi security configuration (from Wep to WPA or WPA to
> Wep) is not dynamically corrected handled by connman 0.78
> 
> I experienced inconsistencies when changing security protection on a fly on
> wifi AP. 

Jean-Jacques:

Confirmed. See https://bugs.meego.com/show_bug.cgi?id=21332

Best,

Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Manager state "online"

[Grant Erickson]

On Apr 3, 2012, at 9:51 AM, Marcel Holtmann wrote:
 In the current Connman version the "state"  of the manager API is
 either "offline" or "online".
 If only one device is marked as connected the global state is "online".
 
 So the global online state doesn't have the same signification for the
 service state, it's a little bit confusing.
 
 The service state makes the difference between "online" and "ready"
 and detects well the Internet availability.
 
 Why don't we notifies a global state "online" when we have Internet
 and "ready" when we have an IP configuration ?
 
 He could be usefull for the applications who are waiting for Internet
 without checking the whole list of services state.
>>> 
>>> I rather have applications use the Session API, than this global state.
>>> 
>>> And maybe we just better remove it before 1.0 release.
>> 
>> -1. While it certainly has its limitations, the simplicity of the 
>> Manager.State element has value and, at this point, I'd rather avoid having 
>> to rewrite most of our application when transitioning from 0.76 to 1.0.
> 
> fair enough actually.
> 
> So should we keep it as "online" and "offline"? So pretty simple. Or
> should we introduce an extra state for when we have a connection, but
> not yet confirmed that it is actually a valid Internet connection.

To date, that's absolutely been sufficient. I could see an argument for 
mirroring the service state of "ready" in addition to "offline" and "online"; 
however, it hasn't been an application deal-breaker to date.

Reading through session-overview.txt and session-api.txt in the top-of-tree GIT 
repository reconvinces me that the session API is a whole lot of complexity 
that our application simply doesn't need.

Thanks,

Grant

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Manager state "online"

[Grant Erickson]

On Apr 3, 2012, at 5:59 AM, connman-requ...@connman.net wrote:
> Hi Julien,
> 
>> In the current Connman version the "state"  of the manager API is
>> either "offline" or "online".
>> If only one device is marked as connected the global state is "online".
>> 
>> So the global online state doesn't have the same signification for the
>> service state, it's a little bit confusing.
>> 
>> The service state makes the difference between "online" and "ready"
>> and detects well the Internet availability.
>> 
>> Why don't we notifies a global state "online" when we have Internet
>> and "ready" when we have an IP configuration ?
>> 
>> He could be usefull for the applications who are waiting for Internet
>> without checking the whole list of services state.
> 
> I rather have applications use the Session API, than this global state.
> 
> And maybe we just better remove it before 1.0 release.

-1. While it certainly has its limitations, the simplicity of the Manager.State 
element has value and, at this point, I'd rather avoid having to rewrite most 
of our application when transitioning from 0.76 to 1.0.

-Grant
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Network configuration lost after system reboot

[Grant Erickson]

On 3/28/12 12:54 PM, Mario Domenech Goulart wrote:
> On Wed, 28 Mar 2012 12:26:20 -0700 Grant Erickson 
> wrote:
>> On 3/28/12 11:35 AM, Mario Domenech Goulart wrote:
>>> I've been facing a weird behavior on the system I'm working. I'll
>>> try to describe it here.
>>> 
>>> I'm using a regular PC with an ethernet card (eth0), ConnMan 0.79
>>> under Linux (3.2.7). ConnMan is started by systemd.  Here are the steps
>>> to illustrate the problem:
>>> 
>>> 1. Right after installed, the system boots and the network interface
>>>gets IP configuration via DHCP, as expected.
>>> 
>>> 2. I set the network interface configuration as static and it works as
>>>expected.
>>> 
>>> 3. I set the network interface configuration to be obtained via DHCP.
>>>It is configured via DHCP as expected.
>>> 
>>> 4. I reboot the system.
>>> 
>>> 5. The network interface is not configured.
>>> 
>>> After performing those steps, I cannot make the network interface
>>> obtain its configuration via DHCP on system boot anymore.
>> 
>> Mario:
>> 
>> How is your system terminating connman?
>> 
>> On normal termination, connman will write out the contents of
>> /var/lib/connman/default.profile (among other data). This, of course,
>> doesn't always happen instantaneously. If your system is not set up to
>> actually wait for connman to completely finish before shutting down or if it
>> resorts to killing connman when it doesn't shutdown in time, you could end
>> up with a corrupted, empty or incomplete default.profile.
> 
> Thanks.  That's a valuable tip.
> 
> I'm using systemd. According to
> http://www.freedesktop.org/software/systemd/man/systemd.service.html it
> seems that systemd sends a SIGTERM by default and a SIGKILL, after a
> timeout, in case processes survive SIGTERM.

What is the length of that timeout?

> What is the proper way to shut ConnMan down?

The above cited method is very typical and, in general, should be
sufficient. I defer to Marcel, Samuel and Daniel should there be something
more preferable.

Best,

Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Network configuration lost after system reboot

[Grant Erickson]

On 3/28/12 11:35 AM, Mario Domenech Goulart wrote:
> I've been facing a weird behavior on the system I'm working. I'll
> try to describe it here.
> 
> I'm using a regular PC with an ethernet card (eth0), ConnMan 0.79
> under Linux (3.2.7). ConnMan is started by systemd.  Here are the steps
> to illustrate the problem:
> 
> 1. Right after installed, the system boots and the network interface
>gets IP configuration via DHCP, as expected.
> 
> 2. I set the network interface configuration as static and it works as
>expected.
> 
> 3. I set the network interface configuration to be obtained via DHCP.
>It is configured via DHCP as expected.
> 
> 4. I reboot the system.
> 
> 5. The network interface is not configured.
> 
> After performing those steps, I cannot make the network interface
> obtain its configuration via DHCP on system boot anymore.

Mario:

How is your system terminating connman?

On normal termination, connman will write out the contents of
/var/lib/connman/default.profile (among other data). This, of course,
doesn't always happen instantaneously. If your system is not set up to
actually wait for connman to completely finish before shutting down or if it
resorts to killing connman when it doesn't shutdown in time, you could end
up with a corrupted, empty or incomplete default.profile.

Best,

Grant


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 2/2] style: change coding style for passing NULL to time

[Grant Erickson]

Pass NULL rather than 0 when calling time(2) to follow prefered
stylistic convention.

--
diff --git a/src/dhcpv6.c b/src/dhcpv6.c
index aab5868..462b78d 100644
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -606,7 +606,7 @@ static int check_restart(struct connman_dhcpv6 *dhcp)
 
g_dhcpv6_client_get_timeouts(dhcp->dhcp_client, NULL, NULL,
NULL, NULL, &expired);
-   current = time(0);
+   current = time(NULL);
 
if (current > expired) {
DBG("expired by %d secs", (int)(current - expired));
@@ -816,7 +816,7 @@ int __connman_dhcpv6_start_renew(struct connman_network 
*network,
g_dhcpv6_client_get_timeouts(dhcp->dhcp_client, &T1, &T2,
&last_renew, &last_rebind, &expired);
 
-   current = time(0);
+   current = time(NULL);
 
DBG("T1 %u T2 %u expires %lu current %lu", T1, T2,
(unsigned long)expired, current);
@@ -1207,7 +1207,7 @@ int __connman_dhcpv6_init(void)
 {
DBG("");
 
-   srand(time(0));
+   srand(time(NULL));
 
network_table = g_hash_table_new_full(g_direct_hash, g_direct_equal,
NULL, remove_network);
diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index dc44a46..e0b8da2 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -565,7 +565,7 @@ static gboolean cache_check_is_valid(struct cache_data 
*data,
  */
 static void cache_enforce_validity(struct cache_entry *entry)
 {
-   time_t current_time = time(0);
+   time_t current_time = time(NULL);
 
if (cache_check_is_valid(entry->ipv4, current_time) == FALSE
&& entry->ipv4) {
@@ -588,7 +588,7 @@ static void cache_enforce_validity(struct cache_entry 
*entry)
 static uint16_t cache_check_validity(char *question, uint16_t type,
struct cache_entry *entry)
 {
-   time_t current_time = time(0);
+   time_t current_time = time(NULL);
int want_refresh = 0;
 
/*
@@ -1053,7 +1053,7 @@ static void cache_cleanup(void)
struct cache_timeout data;
int count = 0;
 
-   data.current_time = time(0);
+   data.current_time = time(NULL);
data.max_timeout = 0;
data.try_harder = 0;
 
@@ -1230,7 +1230,7 @@ static int cache_update(struct server_data *srv, unsigned 
char *msg,
return 0;
}
 
-   current_time = time(0);
+   current_time = time(NULL);
 
/* don't do a cache refresh more than twice a minute */
if (next_refresh < current_time) {
@@ -1416,7 +1416,7 @@ static int ns_resolv(struct server_data *server, struct 
request_data *req,
data = entry->ipv6;
 
if (data) {
-   ttl_left = data->valid_until - time(0);
+   ttl_left = data->valid_until - time(NULL);
entry->hits++;
}
 
--
1.7.9.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 1/2] dhcpv4-client: change coding style for passing NULL to time

[Grant Erickson]

Pass NULL rather than 0 when calling time(2) to follow prefered
stylistic convention.

--
diff --git a/gdhcp/client.c b/gdhcp/client.c
index a3b4b8a..fe60308 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -570,7 +570,7 @@ int g_dhcpv6_create_duid(GDHCPDuidType duid_type, int 
index, int type,
get_interface_mac_address(index, &(*duid)[2 + 2 + 4]);
(*duid)[2] = 0;
(*duid)[3] = type;
-   duid_time = time(0) - DUID_TIME_EPOCH;
+   duid_time = time(NULL) - DUID_TIME_EPOCH;
(*duid)[4] = duid_time >> 24;
(*duid)[5] = duid_time >> 16;
(*duid)[6] = duid_time >> 8;
@@ -896,7 +896,7 @@ GDHCPClient *g_dhcp_client_new(GDHCPType type,
dhcp_client->require_list = NULL;
dhcp_client->duid = NULL;
dhcp_client->duid_len = 0;
-   dhcp_client->last_renew = dhcp_client->last_rebind = time(0);
+   dhcp_client->last_renew = dhcp_client->last_rebind = time(NULL);
dhcp_client->expire = 0;
 
*error = G_DHCP_CLIENT_ERROR_NONE;
@@ -2554,7 +2554,7 @@ void g_dhcpv6_client_reset_renew(GDHCPClient *dhcp_client)
if (dhcp_client == NULL || dhcp_client->type == G_DHCP_IPV4)
return;
 
-   dhcp_client->last_renew = time(0);
+   dhcp_client->last_renew = time(NULL);
 }
 
 void g_dhcpv6_client_reset_rebind(GDHCPClient *dhcp_client)
@@ -2562,7 +2562,7 @@ void g_dhcpv6_client_reset_rebind(GDHCPClient 
*dhcp_client)
if (dhcp_client == NULL || dhcp_client->type == G_DHCP_IPV4)
return;
 
-   dhcp_client->last_rebind = time(0);
+   dhcp_client->last_rebind = time(NULL);
 }
 
 void g_dhcpv6_client_set_expire(GDHCPClient *dhcp_client, uint32_t timeout)
@@ -2570,7 +2570,7 @@ void g_dhcpv6_client_set_expire(GDHCPClient *dhcp_client, 
uint32_t timeout)
if (dhcp_client == NULL || dhcp_client->type == G_DHCP_IPV4)
return;
 
-   dhcp_client->expire = time(0) + timeout;
+   dhcp_client->expire = time(NULL) + timeout;
 }
 
 uint16_t g_dhcpv6_client_get_status(GDHCPClient *dhcp_client)
--
1.7.9.2 
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 0/2] change coding style for passing NULL to time

[Grant Erickson]

Pass NULL rather than 0 when calling time(2) to follow prefered
stylistic convention.

  v0: Initial submission.

Grant Erickson (2)

  dhcpv4-client: change coding style for passing NULL to time
  style: change coding style for passing NULL to time

 gdhcp/client.c |   10 +-
 src/dhcpv6.c   |6 +++---
 src/dnsproxy.c |   10 +-
 3 files changed, 13 insertions(+), 13 deletions(-)
--
1.7.9.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2] gdhcp: set secs in a RFC 951- and 2131-compliant manner

[Grant Erickson]

This patch sets the BOOTP secs field in a RFC 951- and 2131-compliant
manner for DHCPv4 DISCOVER and REQUEST/SELECT packets.

Certain DHCP servers, such as that implemented in Mac OS X (< 10.7)
for its "Internet Sharing" feature, refuse to issue a DHCP lease to
clients that have not set a non-zero value in their DISCOVER or
REQUEST packets. In fact, based on http://hints.macworld.com/article.php?
story=20071223001432304, it's not non-zero but a value greater than four (4)
seconds to allow another "authoritative" DHCP server on the subnet to reply
first.

Side-by-side packet analysis of Mac OS X, iOS, Android, ISC and
Windows clients show that these clients set the BOOTP 'secs' field and
are successfully issued a DHCP lease by Mac OS X. By contrast, a
connman-based client will issue 10 back-to-back DISCOVER packets and
will not be returned a DHCP OFFER from the server.

---
 v1: Initial submission.
 v2: Change 0 argument passed to time to NULL based on feedback from Daniel
 Wagner. Enhanced dhcp_attempt_seconds comment.

 gdhcp/client.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index a3b4b8a..761e6f9 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -85,6 +85,7 @@ struct _GDHCPClient {
char *interface;
uint8_t mac_address[6];
uint32_t xid;
+   time_t start;
uint32_t server_ip;
uint32_t requested_ip;
char *assigned_ip;
@@ -339,6 +340,15 @@ static void add_send_options(GDHCPClient *dhcp_client,
add_binary_option, packet);
 }
 
+/* Return an RFC 951- and 2131-complaint BOOTP 'secs' value that
+ * represents the number of seconds elapsed from the start of
+ * attempting DHCP to satisfy some DHCP servers that allow for an
+ * "authoritative" reply before responding. */
+static uint16_t dhcp_attempt_secs(GDHCPClient *dhcp_client)
+{
+   return htons(MIN(time(NULL) - dhcp_client->start, UINT16_MAX));
+}
+
 static int send_discover(GDHCPClient *dhcp_client, uint32_t requested)
 {
struct dhcp_packet packet;
@@ -348,6 +357,7 @@ static int send_discover(GDHCPClient *dhcp_client, uint32_t 
requested)
init_packet(dhcp_client, &packet, DHCPDISCOVER);
 
packet.xid = dhcp_client->xid;
+   packet.secs = dhcp_attempt_secs(dhcp_client);
 
if (requested)
dhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested);
@@ -374,6 +384,7 @@ static int send_select(GDHCPClient *dhcp_client)
init_packet(dhcp_client, &packet, DHCPREQUEST);
 
packet.xid = dhcp_client->xid;
+   packet.secs = dhcp_attempt_secs(dhcp_client);
 
dhcp_add_simple_option(&packet, DHCP_REQUESTED_IP,
dhcp_client->requested_ip);
@@ -2240,7 +2251,7 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client, const 
char *last_address)
return re;
 
dhcp_client->xid = rand();
+   dhcp_client->start = time(NULL);
}
 
if (last_address == NULL) {
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] gdhcp: set secs in a RFC 951- and 2131-compliant manner

[Grant Erickson]

This patch sets the BOOTP secs field in a RFC 951- and 2131-compliant
manner for DHCPv4 DISCOVER and REQUEST/SELECT packets.

---
Certain DHCP servers, such as that implemented in Mac OS X (< 10.7)
for its "Internet Sharing" feature, refuse to issue a DHCP lease to
clients that have not set a non-zero value in their DISCOVER or
REQUEST packets.

Side-by-side packet analysis of Mac OS X, iOS, Android, ISC and
Windows clients show that these clients set the BOOTP 'secs' field and
are successfully issued a DHCP lease by Mac OS X. By contrast, a
connman-based client will issue 10 back-to-back DISCOVER packets and
will not be returned a DHCP OFFER from the server.

 gdhcp/client.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index a3b4b8a..761e6f9 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -85,6 +85,7 @@ struct _GDHCPClient {
char *interface;
uint8_t mac_address[6];
uint32_t xid;
+   time_t start;
uint32_t server_ip;
uint32_t requested_ip;
char *assigned_ip;
@@ -339,6 +340,14 @@ static void add_send_options(GDHCPClient *dhcp_client,
add_binary_option, packet);
 }
 
+/* Return an RFC 951- and 2131-complaint BOOTP 'secs' value that
+ * represents the number of seconds elapsed from the start of
+ * attempting DHCP. */
+static uint16_t dhcp_attempt_secs(GDHCPClient *dhcp_client)
+{
+   return htons(MIN(time(0) - dhcp_client->start, UINT16_MAX));
+}
+
 static int send_discover(GDHCPClient *dhcp_client, uint32_t requested)
 {
struct dhcp_packet packet;
@@ -348,6 +357,7 @@ static int send_discover(GDHCPClient *dhcp_client, uint32_t 
requested)
init_packet(dhcp_client, &packet, DHCPDISCOVER);
 
packet.xid = dhcp_client->xid;
+   packet.secs = dhcp_attempt_secs(dhcp_client);
 
if (requested)
dhcp_add_simple_option(&packet, DHCP_REQUESTED_IP, requested);
@@ -374,6 +384,7 @@ static int send_select(GDHCPClient *dhcp_client)
init_packet(dhcp_client, &packet, DHCPREQUEST);
 
packet.xid = dhcp_client->xid;
+   packet.secs = dhcp_attempt_secs(dhcp_client);
 
dhcp_add_simple_option(&packet, DHCP_REQUESTED_IP,
dhcp_client->requested_ip);
@@ -2240,7 +2251,7 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client, const 
char *last_address)
return re;
 
dhcp_client->xid = rand();
+   dhcp_client->start = time(0);
}
 
if (last_address == NULL) {
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 3/3] dhcp: request sending a RFC 1533- and 2132-compliant client-id

[Grant Erickson]

Some Internet gateways and Wi-Fi access points are unhappy when the
DHCPv4 client-id option (61) is missing and will refuse to issue a
DHCP lease. Adding the client-id option placates these devices.

This patch requests that a RFC 1533- and 2132-compliant DHCP client-id option
(61) be sent with DHCPv4 packets.

---
 src/dhcp.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 86e4261..ce1d240 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -411,6 +411,8 @@ static int dhcp_request(struct connman_dhcp *dhcp)
if (getenv("CONNMAN_DHCP_DEBUG"))
g_dhcp_client_set_debug(dhcp_client, dhcp_debug, "DHCP");
 
+   g_dhcp_client_set_id(dhcp_client);
+
hostname = connman_utsname_get_hostname();
if (hostname != NULL)
g_dhcp_client_set_send(dhcp_client, G_DHCP_HOST_NAME, hostname);
-- 
1.7.8.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 2/3] gdhcp: add RFC 1533- and 2132-compliant client-id option

[Grant Erickson]

This patch adds a function to add a RFC 1533- and 2132-compliant DHCP
client-id option (61) to sent DHCPv4 packets.

---
 gdhcp/client.c |   22 ++
 gdhcp/gdhcp.h  |2 ++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 1a7e007..7dcf40e 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2470,6 +2470,28 @@ static uint8_t *alloc_dhcp_string_option(int code, const 
char *str)
return alloc_dhcp_data_option(code, (const uint8_t *)str, strlen(str));
 }
 
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *dhcp_client)
+{
+   const unsigned maclen = 6;
+   const unsigned idlen = maclen + 1;
+   const uint8_t option_code = G_DHCP_CLIENT_ID;
+   uint8_t idbuf[idlen];
+   uint8_t *data_option;
+
+   idbuf[0] = ARPHRD_ETHER;
+
+   memcpy(&idbuf[1], dhcp_client->mac_address, maclen);
+
+   data_option = alloc_dhcp_data_option(option_code, idbuf, idlen);
+   if (data_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
+
+   g_hash_table_insert(dhcp_client->send_value_hash,
+   GINT_TO_POINTER((int) option_code), data_option);
+
+   return G_DHCP_CLIENT_ERROR_NONE;
+}
+
 /* Now only support send hostname */
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *dhcp_client,
unsigned char option_code, const char *option_value)
diff --git a/gdhcp/gdhcp.h b/gdhcp/gdhcp.h
index edebc9e..4453b26 100644
--- a/gdhcp/gdhcp.h
+++ b/gdhcp/gdhcp.h
@@ -74,6 +74,7 @@ typedef enum {
 #define G_DHCP_DOMAIN_NAME 0x0f
 #define G_DHCP_HOST_NAME   0x0c
 #define G_DHCP_NTP_SERVER  0x2a
+#define G_DHCP_CLIENT_ID   0x3d
 
 #define G_DHCPV6_CLIENTID  1
 #define G_DHCPV6_SERVERID  2
@@ -121,6 +122,7 @@ GDHCPClientError g_dhcp_client_set_request(GDHCPClient 
*client,
unsigned int option_code);
 void g_dhcp_client_clear_requests(GDHCPClient *dhcp_client);
 void g_dhcp_client_clear_values(GDHCPClient *dhcp_client);
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *client);
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *client,
unsigned char option_code,
const char *option_value);
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 1/3] gdhcp: refactor alloc_dhcp_option to accept string or raw data.

[Grant Erickson]

Support specifying NULL-terminated string or raw data for allocated
DHCP options.

---
 gdhcp/client.c |   28 +---
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 99f9951..1a7e007 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2445,19 +2445,31 @@ void g_dhcp_client_clear_values(GDHCPClient 
*dhcp_client)
g_hash_table_remove_all(dhcp_client->send_value_hash);
 }
 
-static uint8_t *alloc_dhcp_option(int code, const char *str, int extra)
+static uint8_t *alloc_dhcp_option(int code, const uint8_t *data, unsigned size)
 {
uint8_t *storage;
-   int len = strnlen(str, 255);
 
-   storage = malloc(len + extra + OPT_DATA);
+   storage = g_try_malloc(size + OPT_DATA);
+   if (storage == NULL)
+   return NULL;
+
storage[OPT_CODE] = code;
-   storage[OPT_LEN] = len + extra;
-   memcpy(storage + extra + OPT_DATA, str, len);
+   storage[OPT_LEN] = size;
+   memcpy(&storage[OPT_DATA], data, size);
 
return storage;
 }
 
+static uint8_t *alloc_dhcp_data_option(int code, const uint8_t *data, unsigned 
size)
+{
+   return alloc_dhcp_option(code, data, MIN(size, 255));
+}
+
+static uint8_t *alloc_dhcp_string_option(int code, const char *str)
+{
+   return alloc_dhcp_data_option(code, (const uint8_t *)str, strlen(str));
+}
+
 /* Now only support send hostname */
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *dhcp_client,
unsigned char option_code, const char *option_value)
@@ -2465,8 +2477,10 @@ GDHCPClientError g_dhcp_client_set_send(GDHCPClient 
*dhcp_client,
uint8_t *binary_option;
 
if (option_code == G_DHCP_HOST_NAME && option_value != NULL) {
-   binary_option = alloc_dhcp_option(option_code,
-   option_value, 0);
+   binary_option = alloc_dhcp_string_option(option_code,
+   option_value);
+   if (binary_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
 
g_hash_table_insert(dhcp_client->send_value_hash,
GINT_TO_POINTER((int) option_code), binary_option);
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 0/3] dhcp: add RFC 1533- and 2132-compliant client-id option

[Grant Erickson]

Some Internet gateways and Wi-Fi access points are unhappy when the
DHCPv4 client-id option (61) is missing and will refuse to issue a
DHCP lease. Adding the client-id option placates these devices.

This patch adds a RFC 1533- and 2132-compliant DHCP client-id option
(61) to DHCPv4 packets.

  v0: Initial submission.
  v1: Break initial patch into three parts.
  v2: Tweak comments per Daniel's feedback.

Grant Erickson (3)
  gdhcp: Refactor alloc_dhcp_option to accept string or raw data.
  gdhcp: Add free function to add client-id option to sent packets.
  dhcp: Specify the client-id option to be added to sent packets.

 gdhcp/client.c |   50 +++---
 gdhcp/gdhcp.h  |2 ++
 src/dhcp.c |2 ++
 3 files changed, 47 insertions(+), 7 deletions(-)
--
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 2/3] dhcp: add RFC 1533- and 2132-compliant client-id option

[Grant Erickson]

This patch adds a function to add a RFC 1533- and 2132-compliant DHCP
client-id option (61) to sent DHCPv4 packets.

---
 gdhcp/client.c |   22 ++
 gdhcp/gdhcp.h  |2 ++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 1a7e007..7dcf40e 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2470,6 +2470,28 @@ static uint8_t *alloc_dhcp_string_option(int code, const 
char *str)
return alloc_dhcp_data_option(code, (const uint8_t *)str, strlen(str));
 }
 
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *dhcp_client)
+{
+   const unsigned maclen = 6;
+   const unsigned idlen = maclen + 1;
+   const uint8_t option_code = G_DHCP_CLIENT_ID;
+   uint8_t idbuf[idlen];
+   uint8_t *data_option;
+
+   idbuf[0] = ARPHRD_ETHER;
+
+   memcpy(&idbuf[1], dhcp_client->mac_address, maclen);
+
+   data_option = alloc_dhcp_data_option(option_code, idbuf, idlen);
+   if (data_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
+
+   g_hash_table_insert(dhcp_client->send_value_hash,
+   GINT_TO_POINTER((int) option_code), data_option);
+
+   return G_DHCP_CLIENT_ERROR_NONE;
+}
+
 /* Now only support send hostname */
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *dhcp_client,
unsigned char option_code, const char *option_value)
diff --git a/gdhcp/gdhcp.h b/gdhcp/gdhcp.h
index edebc9e..4453b26 100644
--- a/gdhcp/gdhcp.h
+++ b/gdhcp/gdhcp.h
@@ -74,6 +74,7 @@ typedef enum {
 #define G_DHCP_DOMAIN_NAME 0x0f
 #define G_DHCP_HOST_NAME   0x0c
 #define G_DHCP_NTP_SERVER  0x2a
+#define G_DHCP_CLIENT_ID   0x3d
 
 #define G_DHCPV6_CLIENTID  1
 #define G_DHCPV6_SERVERID  2
@@ -121,6 +122,7 @@ GDHCPClientError g_dhcp_client_set_request(GDHCPClient 
*client,
unsigned int option_code);
 void g_dhcp_client_clear_requests(GDHCPClient *dhcp_client);
 void g_dhcp_client_clear_values(GDHCPClient *dhcp_client);
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *client);
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *client,
unsigned char option_code,
const char *option_value);
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 3/3] dhcp: request sending a RFC 1533- and 2132-compliant client-id

[Grant Erickson]

This patch requests that a RFC 1533- and 2132-compliant DHCP client-id option
(61) be sent with DHCPv4 packets.

---
 src/dhcp.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 86e4261..ce1d240 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -411,6 +411,8 @@ static int dhcp_request(struct connman_dhcp *dhcp)
if (getenv("CONNMAN_DHCP_DEBUG"))
g_dhcp_client_set_debug(dhcp_client, dhcp_debug, "DHCP");
 
+   g_dhcp_client_set_id(dhcp_client);
+
hostname = connman_utsname_get_hostname();
if (hostname != NULL)
g_dhcp_client_set_send(dhcp_client, G_DHCP_HOST_NAME, hostname);
-- 
1.7.8.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 0/3] dhcp: add RFC 1533- and 2132-compliant client-id option

[Grant Erickson]

Some Internet gateways and Wi-Fi access points are unhappy when the
DHCPv4 client-id option (61) is missing and will refuse to issue a
DHCP lease. Adding the client-id option placates these devices.

This patch adds a RFC 1533- and 2132-compliant DHCP client-id option
(61) to DHCPv4 packets.

  v0: Initial submission.
  v1: Break initial patch into three parts.

Grant Erickson (3)
  dhcpv4-client: Refactor alloc_dhcp_option to accept string or raw data.
  dhcpv4-client: Add free function to add client-id option to sent packets.
  dhcpv4-client: Specify the client-id option to be added to sent packets.

 gdhcp/client.c |   50 +++---
 gdhcp/gdhcp.h  |2 ++
 src/dhcp.c |2 ++
 3 files changed, 47 insertions(+), 7 deletions(-)
--
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 1/3] dhcp: refactor alloc_dhcp_option to accept string or raw data.

[Grant Erickson]

Support specifying NULL-terminated string or raw data for allocated
DHCP options.

---
 gdhcp/client.c |   28 +---
 1 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 99f9951..1a7e007 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2445,19 +2445,31 @@ void g_dhcp_client_clear_values(GDHCPClient 
*dhcp_client)
g_hash_table_remove_all(dhcp_client->send_value_hash);
 }
 
-static uint8_t *alloc_dhcp_option(int code, const char *str, int extra)
+static uint8_t *alloc_dhcp_option(int code, const uint8_t *data, unsigned size)
 {
uint8_t *storage;
-   int len = strnlen(str, 255);
 
-   storage = malloc(len + extra + OPT_DATA);
+   storage = g_try_malloc(size + OPT_DATA);
+   if (storage == NULL)
+   return NULL;
+
storage[OPT_CODE] = code;
-   storage[OPT_LEN] = len + extra;
-   memcpy(storage + extra + OPT_DATA, str, len);
+   storage[OPT_LEN] = size;
+   memcpy(&storage[OPT_DATA], data, size);
 
return storage;
 }
 
+static uint8_t *alloc_dhcp_data_option(int code, const uint8_t *data, unsigned 
size)
+{
+   return alloc_dhcp_option(code, data, MIN(size, 255));
+}
+
+static uint8_t *alloc_dhcp_string_option(int code, const char *str)
+{
+   return alloc_dhcp_data_option(code, (const uint8_t *)str, strlen(str));
+}
+
 /* Now only support send hostname */
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *dhcp_client,
unsigned char option_code, const char *option_value)
@@ -2465,8 +2477,10 @@ GDHCPClientError g_dhcp_client_set_send(GDHCPClient 
*dhcp_client,
uint8_t *binary_option;
 
if (option_code == G_DHCP_HOST_NAME && option_value != NULL) {
-   binary_option = alloc_dhcp_option(option_code,
-   option_value, 0);
+   binary_option = alloc_dhcp_string_option(option_code,
+   option_value);
+   if (binary_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
 
g_hash_table_insert(dhcp_client->send_value_hash,
GINT_TO_POINTER((int) option_code), binary_option);
-- 
1.7.8.2
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] dhcp: add RFC 1533- and 2132-compliant client-id option

[Grant Erickson]

This patch adds a RFC 1533- and 2132-compliant DHCP client-id option
(61) to DHCPv4 packets.

---
 gdhcp/client.c |   50 +++---
 gdhcp/gdhcp.h  |2 ++
 src/dhcp.c |2 ++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 99f9951..7dcf40e 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2445,19 +2445,53 @@ void g_dhcp_client_clear_values(GDHCPClient 
*dhcp_client)
g_hash_table_remove_all(dhcp_client->send_value_hash);
 }
 
-static uint8_t *alloc_dhcp_option(int code, const char *str, int extra)
+static uint8_t *alloc_dhcp_option(int code, const uint8_t *data, unsigned size)
 {
uint8_t *storage;
-   int len = strnlen(str, 255);
 
-   storage = malloc(len + extra + OPT_DATA);
+   storage = g_try_malloc(size + OPT_DATA);
+   if (storage == NULL)
+   return NULL;
+
storage[OPT_CODE] = code;
-   storage[OPT_LEN] = len + extra;
-   memcpy(storage + extra + OPT_DATA, str, len);
+   storage[OPT_LEN] = size;
+   memcpy(&storage[OPT_DATA], data, size);
 
return storage;
 }
 
+static uint8_t *alloc_dhcp_data_option(int code, const uint8_t *data, unsigned 
size)
+{
+   return alloc_dhcp_option(code, data, MIN(size, 255));
+}
+
+static uint8_t *alloc_dhcp_string_option(int code, const char *str)
+{
+   return alloc_dhcp_data_option(code, (const uint8_t *)str, strlen(str));
+}
+
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *dhcp_client)
+{
+   const unsigned maclen = 6;
+   const unsigned idlen = maclen + 1;
+   const uint8_t option_code = G_DHCP_CLIENT_ID;
+   uint8_t idbuf[idlen];
+   uint8_t *data_option;
+
+   idbuf[0] = ARPHRD_ETHER;
+
+   memcpy(&idbuf[1], dhcp_client->mac_address, maclen);
+
+   data_option = alloc_dhcp_data_option(option_code, idbuf, idlen);
+   if (data_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
+
+   g_hash_table_insert(dhcp_client->send_value_hash,
+   GINT_TO_POINTER((int) option_code), data_option);
+
+   return G_DHCP_CLIENT_ERROR_NONE;
+}
+
 /* Now only support send hostname */
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *dhcp_client,
unsigned char option_code, const char *option_value)
@@ -2465,8 +2499,10 @@ GDHCPClientError g_dhcp_client_set_send(GDHCPClient 
*dhcp_client,
uint8_t *binary_option;
 
if (option_code == G_DHCP_HOST_NAME && option_value != NULL) {
-   binary_option = alloc_dhcp_option(option_code,
-   option_value, 0);
+   binary_option = alloc_dhcp_string_option(option_code,
+   option_value);
+   if (binary_option == NULL)
+   return G_DHCP_CLIENT_ERROR_NOMEM;
 
g_hash_table_insert(dhcp_client->send_value_hash,
GINT_TO_POINTER((int) option_code), binary_option);
diff --git a/gdhcp/gdhcp.h b/gdhcp/gdhcp.h
index edebc9e..4453b26 100644
--- a/gdhcp/gdhcp.h
+++ b/gdhcp/gdhcp.h
@@ -74,6 +74,7 @@ typedef enum {
 #define G_DHCP_DOMAIN_NAME 0x0f
 #define G_DHCP_HOST_NAME   0x0c
 #define G_DHCP_NTP_SERVER  0x2a
+#define G_DHCP_CLIENT_ID   0x3d
 
 #define G_DHCPV6_CLIENTID  1
 #define G_DHCPV6_SERVERID  2
@@ -121,6 +122,7 @@ GDHCPClientError g_dhcp_client_set_request(GDHCPClient 
*client,
unsigned int option_code);
 void g_dhcp_client_clear_requests(GDHCPClient *dhcp_client);
 void g_dhcp_client_clear_values(GDHCPClient *dhcp_client);
+GDHCPClientError g_dhcp_client_set_id(GDHCPClient *client);
 GDHCPClientError g_dhcp_client_set_send(GDHCPClient *client,
unsigned char option_code,
const char *option_value);
diff --git a/src/dhcp.c b/src/dhcp.c
index 86e4261..ce1d240 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -411,6 +411,8 @@ static int dhcp_request(struct connman_dhcp *dhcp)
if (getenv("CONNMAN_DHCP_DEBUG"))
g_dhcp_client_set_debug(dhcp_client, dhcp_debug, "DHCP");
 
+   g_dhcp_client_set_id(dhcp_client);
+
hostname = connman_utsname_get_hostname();
if (hostname != NULL)
g_dhcp_client_set_send(dhcp_client, G_DHCP_HOST_NAME, hostname);
-- 
1.7.8.2

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman