Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH] vpn: Handle stale task completions (Daniel Wagner)
2. Re: NTP failures on RaspberryPi (Daniel Wagner)
3. Re: [PATCH] inet: Remove error message if /proc/net/pnp file
is missing (Daniel Wagner)
4. Re: [PATCH] gdhcp: Fix wrong initialization of listener_watch
(Daniel Wagner)
5. Re: Retry NTP servers periodically on startup (Daniel Wagner)
6. Re: Problem when creating a connman-vpn provider with dbus
net.connman.vpn.Manager.Create (Daniel Wagner)
7. Re: [PATCH 2/2] doc: Update Security values for services
(Robert Tiemann)
8. Re: [PATCH 2/2] doc: Update Security values for services
(Daniel Wagner)
9. [PATCH] service: retry online check for IPv4 (Daniel Mack)
----------------------------------------------------------------------
Message: 1
Date: Wed, 18 Apr 2018 07:59:39 +0200
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] vpn: Handle stale task completions
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Slava,
On 04/17/2018 01:31 AM, Slava Monich wrote:
> On 16/04/18 23:15, Daniel Wagner wrote:
>> Hi Slava,
>>
>> On 04/13/2018 05:11 PM, Slava Monich wrote:
>>> The task may die after we have already started the new one
>>
>> Can elaborate a bit here? It is really hard to figure out what is
>> happening. I suppose it is a race somewhere.
>>
>
> Here are the events leading to the crash:
>
> ...
> connman-vpnd[8015]: vpn/plugins/vpn.c:vpn_disconnect() disconnect provider
> 0x5694638:
> ...
> connman-vpnd[8015]: src/task.c:connman_task_stop() task 0x5b2bdd0
> connman-vpnd[8015]: src/task.c:check_kill() pid 11723 was not killed,
> retrying after 2 sec
> connman-vpnd[8015]: vpn/vpn-provider.c:do_connect() conn 0x50f0018 provider
> 0x5694638
> connman-vpnd[8015]: vpn/vpn-provider.c:__vpn_provider_connect() provider
> 0x5694638
> connman-vpnd[8015]: vpn/plugins/vpn.c:vpn_connect() data 0x5b1e7d0 state 4
> ...
> connman-vpnd[8015]: src/task.c:connman_task_create()
> connman-vpnd[8015]: src/task.c:connman_task_create() task 0x5f21a08
> connman-vpnd[8015]: src/task.c:connman_task_set_notify() task 0x5f21a08
> ...
> connman-vpnd[8015]: src/task.c:task_died() task 0x5b2bdd0 exit status 0
> connman-vpnd[8015]: src/agent.c:connman_agent_cancel() context 0x5b326e0
> connman-vpnd[8015]: vpn/plugins/vpn.c:vpn_died() provider 0x5694638 data
> 0x5b1e7d0
> ...
> connman-vpnd[8015]: src/task.c:connman_task_destroy() task 0x5b2bdd0
> connman-vpnd[8015]: src/task.c:free_task() task 0x5b2bdd0
> ...
> connman-vpnd[8015]: vpn/plugins/vpn.c:vpn_notify() provider 0x5694638 driver
> openvpn state 2
>
> ==8015== Invalid read of size 4
> ==8015== at 0x1C7C6: vpn_notify (vpn.c:279)
> ==8015== by 0x2152D: task_filter (task.c:452)
> ==8015== by 0x49B402F: dbus_connection_dispatch (dbus-connection.c:4703)
> ==8015== by 0x2E721: message_dispatch (mainloop.c:72)
> ==8015== by 0x4C6306F: g_main_dispatch (gmain.c:3154)
> ==8015== by 0x4C6306F: g_main_context_dispatch (gmain.c:3769)
> ==8015== by 0x4C6331D: g_main_context_iterate.isra.4 (gmain.c:3840)
> ==8015== by 0x4C6358F: g_main_loop_run (gmain.c:4034)
> ==8015== by 0x188E3: main (main.c:274)
> ==8015== Address 0x10 is not stack'd, malloc'd or (recently) free'd
>
>
> In other words, we are stopping task A (0x5b2bdd0), then starting new
> task B (0x5f21a08) reusing vpn_data allocated for task A (in
> vpn/plugins/vpn.c:vpn_connect), then task A finally dies, which
> deallocates vpn_data and only then we get a notification for task B but
> vpn_data is already gone. Bummer!
I added the stacktrace and analisys to the commit message.
Thanks a lot!
Daniel
------------------------------
Message: 2
Date: Wed, 18 Apr 2018 08:05:53 +0200
From: Daniel Wagner <[email protected]>
To: Rudi <[email protected]>
Cc: [email protected]
Subject: Re: NTP failures on RaspberryPi
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Rudi,
On 04/15/2018 07:52 AM, Rudi wrote:
> Hi folks,
>
> the NTP on any version of any connman newer than 1.33 (including latest git
> version) does bizarre things here on RaspberryPi
> devices. The time gets set completely wrong. Yesterday, I've found out that
> this seems to be related to the presence/absence of a
> real time clock. If I add a small I2C RTC module, everything works as
> expected. But if this module is not installed, the
> misbehaviour shows up. It appears the newer time adjustment mechanism via
> adjtimex() is causing this. When reverting to the old
> adjtime()/settimeofday() approach as used in 1.33, it works without RTC
> hardware installed.
>
> Any thoughts?
That sounds really strange behavior. I don't think that ConnMan behaves
differently with or without the RTC module. I don't know enough about
the timer internals of the kernel, but I would try to start debugging
there. There are some debug/statistics interfaces (proc, sysfs) to see
what's going on in the kernel.
Probably your kernel is not really a fresh one. So also check if there
are upstream commit in the timer subsystem describing your problem.
Thanks,
Daniel
------------------------------
Message: 3
Date: Wed, 18 Apr 2018 08:06:34 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Subject: Re: [PATCH] inet: Remove error message if /proc/net/pnp file
is missing
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/16/2018 06:05 PM, Daniel Wagner wrote:
> Commit 8810e72176f8 ("inet: Return error if pnp_file doesn't exists")
> introduced the error message. For system without an NFS root this
> is not an error. Avoid printing this message on every bootup.
Patch applied.
------------------------------
Message: 4
Date: Wed, 18 Apr 2018 08:07:18 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Subject: Re: [PATCH] gdhcp: Fix wrong initialization of listener_watch
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/16/2018 09:49 PM, Daniel Wagner wrote:
> listener_watch is of type unsigned integer. So initializing it
> to -1 means we always try to remove the source and we get a
> warning by Glib
Patch applied.
------------------------------
Message: 5
Date: Wed, 18 Apr 2018 08:09:27 +0200
From: Daniel Wagner <[email protected]>
To: Craig McQueen <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: Retry NTP servers periodically on startup
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/17/2018 09:48 AM, Craig McQueen wrote:
>> Can you try this one here?
>>
>> diff --git a/src/ntp.c b/src/ntp.c
>> index 3c40f6210b81..335065f813e3 100644
>> --- a/src/ntp.c
>> +++ b/src/ntp.c
>> @@ -531,7 +531,9 @@ static void start_ntp(struct ntp_data *nd)
>> nd->transmit_fd = socket(family, SOCK_DGRAM | SOCK_CLOEXEC,
>> 0);
>>
>> if (nd->transmit_fd <= 0) {
>> - connman_error("Failed to open time server socket");
>> + if (errno != EAFNOSUPPORT)
>> + connman_error("Failed to open time server socket");
>> + nd->cb(false, nd->user_data);
>> return;
>> }
>
> Yes, that seems to fix the issue, based on my testing today. I'll give it
> more testing in the next few days.
Thanks for confirming. That means we need to fixup all error paths in
this file.
Will spin a patch in the coming days. Though weather is reported to be
really good so it will take a while :)
Thanks,
Daniel
------------------------------
Message: 6
Date: Wed, 18 Apr 2018 08:24:10 +0200
From: Daniel Wagner <[email protected]>
To: Thomas Achleitner <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: Problem when creating a connman-vpn provider with dbus
net.connman.vpn.Manager.Create
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Thomas,
On 04/17/2018 10:58 AM, Thomas Achleitner wrote:
> Hi Daniel,
>
> Thank you for your response!
>
> On 04/16/2018 10:06 PM, Daniel Wagner wrote:
>> Hi Thomas,
>>
>> On 04/12/2018 08:54 AM, Thomas Achleitner wrote:
>>> I may have found a bug when creating a vpn provider with DBus.
>>>
>>> The first time i create a connection the provider will not contain the
>>> vpn type specific configuration (e.g. OpenVPN.ConfigFile)
>>
>> Not sure if I understand you right. What do you mean with 'vpn type'?
>> Do you mean the OpenVPN.DeviceType (tap or tun)? Or is
>> OpenVPN.ConfigFile missing?
>
> With vpn type i meant OpenVPN, VPNC etc.
>
> I set 'OpenVPN.ConfigFile' and? 'OpenVPN.DeviceType', but the first time
> they won't appear in the provider file that is created by connman.
Okay, got it.
Looking at the code how ConfigFile and DeviceType are parsed I see that
we do anything when ConfigFile is set:
static int ov_connect(struct vpn_provider *provider,
struct connman_task *task, const char *if_name,
vpn_provider_connect_cb_t cb, const char *dbus_sender,
void *user_data)
{
const char *option;
int stdout_fd, stderr_fd;
int err = 0;
option = vpn_provider_get_string(provider, "Host");
if (!option) {
connman_error("Host not set; cannot enable VPN");
return -EINVAL;
}
task_append_config_data(provider, task);
option = vpn_provider_get_string(provider, "OpenVPN.ConfigFile");
if (!option) {
/*
* Set some default options if user has no config file.
*/
option = vpn_provider_get_string(provider, "OpenVPN.TLSAuth");
if (option) {
connman_task_add_argument(task, "--tls-auth", option);
option = vpn_provider_get_string(provider,
"OpenVPN.TLSAuthDir");
if (option)
connman_task_add_argument(task, option, NULL);
}
connman_task_add_argument(task, "--nobind", NULL);
connman_task_add_argument(task, "--persist-key", NULL);
connman_task_add_argument(task, "--client", NULL);
}
...
}
Not sure if that is a problem or not. Just looks suspicious.
> The connection works as expected and
> 'net.connman.vpn.Connection.GetProperties' returns the expected
> configuration, but the 'OpenVPN' values don't get saved in the provider
> file.
>
> So if i restart connman i can't connect the VPN
>
> If i do the same call a second time the values are saved though. My
> current fix is to sent the creation call twice.
>
> I can see in the connman-vpn debug output that it behaves different if
> the provider file is already present
>
> If not present there are no calls to 'vpn_provider_get_string()' after
> 'vpn_provider_save()'
> If the provider file is present the values for 'OpenVPN' are read
vpn_provider_save() is called once from __vpn_provider_create() and
from __vpn_provider_create_from_config(). If I read this correctly,
than __vpn_provider_create() is handling the ConfigFile option
differently than __vpn_provider_create_from_config().
Sorry running out of time to look at this right now. Maybe you can
give it a shot at look at those two function and see the
problem.
Thanks,
Daniel
------------------------------
Message: 7
Date: Wed, 18 Apr 2018 09:25:45 +0200
From: Robert Tiemann <[email protected]>
To: Daniel Wagner <[email protected]>, Jose Blanquicet
<[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 2/2] doc: Update Security values for services
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
On 01/24/2018 10:09 AM, Daniel Wagner wrote:
> This looks quite sane to me. Jose, if don't have any objections, I am
> going to apply both patches.
Hi,
these patches have not been merged yet. Is there anything left I need
to do about them?
> Thanks,
> Daniel
Cheers,
Robert
------------------------------
Message: 8
Date: Wed, 18 Apr 2018 09:58:18 +0200
From: Daniel Wagner <[email protected]>
To: Robert Tiemann <[email protected]>, Jose Blanquicet
<[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 2/2] doc: Update Security values for services
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Robert,
On 04/18/2018 09:25 AM, Robert Tiemann wrote:
> On 01/24/2018 10:09 AM, Daniel Wagner wrote:
>
>> This looks quite sane to me. Jose, if don't have any objections, I am
>> going to apply both patches.
>
> Hi,
>
> these patches have not been merged yet. Is there anything left I need
> to do about them?
Yes, you need to remind me to apply them :)
I forgot about them. I'll marked them to apply them later. Sorry about it.
Thanks,
Daniel
Daniel
------------------------------
Message: 9
Date: Wed, 18 Apr 2018 11:55:03 +0200
From: Daniel Mack <[email protected]>
To: [email protected]
Cc: [email protected], Daniel Mack <[email protected]>
Subject: [PATCH] service: retry online check for IPv4
Message-ID: <[email protected]>
Occasionally. wacky wifi connections might drop the first packets after
they got their DHCP lease, but work stable afterwards. Be a bit more
patient and retry the IPv4 online check in case it failed in the first
attempt.
The change is straight forward. A new function named
__connman_service_wispr_start() is added to initialize the retry
counters for both v4 and v6, and all former call sites of
__connman_wispr_start() are now routed through
__connman_service_wispr_start().
---
src/connman.h | 2 ++
src/service.c | 83 ++++++++++++++++++++++++++++++++++++++++++++---------------
src/wispr.c | 2 +-
src/wpad.c | 4 +--
4 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index 6d1831b1..82e77d37 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -689,6 +689,8 @@ struct connman_ipconfig *__connman_service_get_ipconfig(
struct connman_service *service, int family);
void __connman_service_notify_ipv4_configuration(
struct connman_service *service);
+void __connman_service_wispr_start(struct connman_service *service,
+ enum connman_ipconfig_type type);
bool __connman_service_is_connected_state(struct connman_service *service,
enum connman_ipconfig_type type);
const char *__connman_service_get_ident(struct connman_service *service);
diff --git a/src/service.c b/src/service.c
index 529be38c..23dcfbb0 100644
--- a/src/service.c
+++ b/src/service.c
@@ -127,7 +127,8 @@ struct connman_service {
char *pac;
bool wps;
bool wps_advertizing;
- int online_check_count;
+ int online_check_count_ipv4;
+ int online_check_count_ipv6;
bool do_split_routing;
bool new_service;
bool hidden_service;
@@ -3401,6 +3402,19 @@ int __connman_service_reset_ipconfig(struct
connman_service *service,
return err;
}
+void __connman_service_wispr_start(struct connman_service *service,
+ enum connman_ipconfig_type type)
+{
+ DBG("service %p type %s", service,
__connman_ipconfig_type2string(type));
+
+ if (type == CONNMAN_IPCONFIG_TYPE_IPV4)
+ service->online_check_count_ipv4 = 1;
+ else
+ service->online_check_count_ipv6 = 1;
+
+ __connman_wispr_start(service, type);
+}
+
static DBusMessage *set_property(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -3518,13 +3532,11 @@ static DBusMessage *set_property(DBusConnection *conn,
if (__connman_service_is_connected_state(service,
CONNMAN_IPCONFIG_TYPE_IPV4))
- __connman_wispr_start(service,
- CONNMAN_IPCONFIG_TYPE_IPV4);
+ __connman_service_wispr_start(service,
CONNMAN_IPCONFIG_TYPE_IPV4);
if (__connman_service_is_connected_state(service,
CONNMAN_IPCONFIG_TYPE_IPV6))
- __connman_wispr_start(service,
- CONNMAN_IPCONFIG_TYPE_IPV6);
+ __connman_service_wispr_start(service,
CONNMAN_IPCONFIG_TYPE_IPV6);
service_save(service);
} else if (g_str_equal(name, "Timeservers.Configuration")) {
@@ -5918,7 +5930,7 @@ static void check_proxy_setup(struct connman_service
*service)
return;
done:
- __connman_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV4);
+ __connman_service_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV4);
}
/*
@@ -5974,18 +5986,38 @@ static void service_rp_filter(struct connman_service
*service,
connected_networks_count, original_rp_filter);
}
-static gboolean redo_wispr(gpointer user_data)
+static void redo_wispr(struct connman_service *service,
+ enum connman_ipconfig_type type)
{
- struct connman_service *service = user_data;
int refcount = service->refcount - 1;
connman_service_unref(service);
if (refcount == 0) {
DBG("Service %p already removed", service);
- return FALSE;
+ return;
}
- __connman_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV6);
+ DBG("Retrying %s WISPr for %p %s",
+ __connman_ipconfig_type2string(type),
+ service, service->name);
+
+ __connman_wispr_start(service, type);
+}
+
+static gboolean redo_wispr_ipv4(gpointer user_data)
+{
+ struct connman_service *service = user_data;
+
+ redo_wispr(service, CONNMAN_IPCONFIG_TYPE_IPV4);
+
+ return FALSE;
+}
+
+static gboolean redo_wispr_ipv6(gpointer user_data)
+{
+ struct connman_service *service = user_data;
+
+ redo_wispr(service, CONNMAN_IPCONFIG_TYPE_IPV6);
return FALSE;
}
@@ -5993,25 +6025,35 @@ static gboolean redo_wispr(gpointer user_data)
int __connman_service_online_check_failed(struct connman_service *service,
enum connman_ipconfig_type type)
{
- DBG("service %p type %d count %d", service, type,
- service->online_check_count);
+ GSourceFunc redo_func;
+ int *count;
+
+ if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
+ count = &service->online_check_count_ipv4;
+ redo_func = redo_wispr_ipv4;
+ } else {
+ count = &service->online_check_count_ipv6;
+ redo_func = redo_wispr_ipv6;
+ }
+
+ DBG("service %p type %s count %d", service,
+ __connman_ipconfig_type2string(type), *count);
- /* currently we only retry IPv6 stuff */
- if (type == CONNMAN_IPCONFIG_TYPE_IPV4 ||
- service->online_check_count != 1) {
- connman_warn("Online check failed for %p %s", service,
- service->name);
+ if (*count == 0) {
+ connman_warn("%s online check failed for %p %s",
+ __connman_ipconfig_type2string(type),
+ service, service->name);
return 0;
}
- service->online_check_count = 0;
+ *count -= 1;
/*
* We set the timeout to 1 sec so that we have a chance to get
* necessary IPv6 router advertisement messages that might have
* DNS data etc.
*/
- g_timeout_add_seconds(1, redo_wispr, connman_service_ref(service));
+ g_timeout_add_seconds(1, redo_func, connman_service_ref(service));
return EAGAIN;
}
@@ -6089,8 +6131,7 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
check_proxy_setup(service);
} else {
- service->online_check_count = 1;
- __connman_wispr_start(service, type);
+ __connman_service_wispr_start(service, type);
}
else
connman_info("Online check disabled. "
diff --git a/src/wispr.c b/src/wispr.c
index 03b38bb8..473c0e03 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -568,7 +568,7 @@ static void wispr_portal_browser_reply_cb(struct
connman_service *service,
}
/* Restarting the test */
- __connman_wispr_start(service, wp_context->type);
+ __connman_service_wispr_start(service, wp_context->type);
}
static void wispr_portal_request_wispr_login(struct connman_service *service,
diff --git a/src/wpad.c b/src/wpad.c
index f066feee..54084ee8 100644
--- a/src/wpad.c
+++ b/src/wpad.c
@@ -87,7 +87,7 @@ static void wpad_result(GResolvResultStatus status,
g_free(url);
- __connman_wispr_start(wpad->service,
+ __connman_service_wispr_start(wpad->service,
CONNMAN_IPCONFIG_TYPE_IPV4);
return;
@@ -119,7 +119,7 @@ failed:
connman_service_set_proxy_method(wpad->service,
CONNMAN_SERVICE_PROXY_METHOD_DIRECT);
- __connman_wispr_start(wpad->service,
+ __connman_service_wispr_start(wpad->service,
CONNMAN_IPCONFIG_TYPE_IPV4);
}
--
2.14.3
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 30, Issue 21
***************************************
