Re: [PATCH] clock: Add TimeSynced signal emitted when the system
time has been synced
by Daniel Wagner
Hi,
Could you please send your patches directly and not as attachment? I
have to copy the patch over to for the review. The simplest way is to
use 'git send-email' directly.
On Wed, Dec 23, 2020 at 03:19:30PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> As our system depends on https requests, and for boot performance
> purposes, we need to be informed when a time synchronization (via the
> ntp) is performed.
>
> In this aim, please find the attached patch, adding the TimeSynced
> signal emitted when the system time has been synced.
> From cf311e998c85b18703886b49bc98c06840612c22 Mon Sep 17 00:00:00 2001
> From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org>
> Date: Wed, 23 Dec 2020 12:06:04 +0100
> Subject: [PATCH] clock: Add TimeSynced signal emitted when the system time has
> been synced.
>
>
Please add a commit message explaining what problem it solves.
> ---
> doc/clock-api.txt | 4 ++++
> include/dbus.h | 2 ++
> src/clock.c | 2 ++
> src/dbus.c | 30 ++++++++++++++++++++++++++++++
> src/timeserver.c | 3 +++
> 5 files changed, 41 insertions(+)
>
> diff --git a/doc/clock-api.txt b/doc/clock-api.txt
> index 6818f5a8..2e368d0a 100644
> --- a/doc/clock-api.txt
> +++ b/doc/clock-api.txt
> @@ -27,6 +27,10 @@ Signals PropertyChanged(string name, variant value) [experimental]
> This signal indicates a changed value of the given
> property.
>
> + TimeSynced(uint64 Time) [experimental]
> +
> + This signal indicates that the current system time
> + has been synced.
I think it would make more sense to just emit a bool and send out the
Time event along side. In this case it would be the same event for auto
or manual mode. This would be more consistent in my opinion.
Also, TimeserverSynced would be also be better name for it in this case.
> Properties uint64 Time [readonly or readwrite] [experimental]
>
> diff --git a/include/dbus.h b/include/dbus.h
> index bcab4189..3208adc5 100644
> --- a/include/dbus.h
> +++ b/include/dbus.h
> @@ -85,6 +85,8 @@ dbus_bool_t connman_dbus_setting_changed_array(const char *owner,
> const char *path, const char *key, int type,
> connman_dbus_append_cb_t function,
> void *user_data);
> +dbus_bool_t connman_dbus_time_synced(const char *path,
> + const char *interface);
>
> static inline void connman_dbus_dict_open(DBusMessageIter *iter,
> DBusMessageIter *dict)
> diff --git a/src/clock.c b/src/clock.c
> index 0fde2c34..315e908e 100644
> --- a/src/clock.c
> +++ b/src/clock.c
> @@ -381,6 +381,8 @@ static const GDBusMethodTable clock_methods[] = {
> static const GDBusSignalTable clock_signals[] = {
> { GDBUS_SIGNAL("PropertyChanged",
> GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
> + { GDBUS_SIGNAL("TimeSynced",
> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
> { },
> };
>
> diff --git a/src/dbus.c b/src/dbus.c
> index d80a46ce..73d1bad6 100644
> --- a/src/dbus.c
> +++ b/src/dbus.c
> @@ -23,6 +23,8 @@
> #include <config.h>
> #endif
>
> +#include <sys/time.h>
> +
> #include <string.h>
> #include <errno.h>
> #include <gdbus.h>
> @@ -382,6 +384,34 @@ dbus_bool_t connman_dbus_setting_changed_array(const char *owner,
> return TRUE;
> }
>
> +dbus_bool_t connman_dbus_time_synced(const char *path, const char *interface)
> +{
> + DBusMessage *signal;
> + DBusMessageIter iter;
> + struct timeval tv;
> + dbus_uint64_t val = 0;
> +
> + if (!path)
> + return FALSE;
> +
> + signal = dbus_message_new_signal(path, interface, "TimeSynced");
> + if (!signal)
> + return FALSE;
> +
> + dbus_message_iter_init_append(signal, &iter);
> +
> + if (gettimeofday(&tv, NULL) == 0) {
> + val = tv.tv_sec;
> + }
> +
> + connman_dbus_property_append_basic(&iter, "Time",
> + DBUS_TYPE_UINT64, &val);
> +
> + g_dbus_send_message(connection, signal);
> +
> + return TRUE;
> +}
> +
> dbus_bool_t __connman_dbus_append_objpath_dict_array(DBusMessage *msg,
> connman_dbus_append_cb_t function, void *user_data)
> {
> diff --git a/src/timeserver.c b/src/timeserver.c
> index decca153..ebd1fab3 100644
> --- a/src/timeserver.c
> +++ b/src/timeserver.c
> @@ -57,6 +57,9 @@ static void ntp_callback(bool success, void *user_data)
>
> if (!success)
> sync_next();
> + else
> + connman_dbus_time_synced(CONNMAN_MANAGER_PATH,
> + CONNMAN_CLOCK_INTERFACE);
You should also add it to get_properties.
Daniel
1 month, 2 weeks
Re: [PATCH] clock: Add functions to trigger time synchronization via
the clock API
by Daniel Wagner
Hi,
On Wed, Dec 23, 2020 at 04:14:52PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> As our system is based on mandatory https requests, in some degraded
> cases, we need to trigger a time synchronization. For example, when
> booting with the clock TimeUpdates equal to manual, changing it to
> auto does not suffy to update the system time.
Instead adding a API as workaround, why not fixing the problem you
describe? When changing from manual to auto, try to sync?
> Please find the attached patch, adding a clock API providing a way to
> trigger a time synchronization.
I don't like this at all. This is a really ad-hock API for something we
just need to fix internally.
Daniel
1 month, 3 weeks
Unreliable 4G LTE interface IP address
by Jupiter
Hi,
I am running 4G LTE managed by ofono version 1.30 and connman version
1.37, it works well most of the time, but occasionally, the 4G modem
is connected, there is no IP address so I have to run systemctl
restart connman to bring the IP address back. As it is intermittent
and random, it is hard to debug, any insight advice what I could be
missing, why couldn't connman pick up the 4G LTE IP reliably?
Thank you.
Kind regards,
Jupiter
2 months
[PATCH] timeserver: Split new service and configuration update
by Daniel Wagner
Allow to update the existing used service with a new time server
configuration. Instead overloading the __connman_timeservice_sync()
function introduce a new function for updating the currently used
service for time synchronization. By this we document clearly in the
code what is supposed to be updated.
---
src/connman.h | 3 ++-
src/service.c | 4 +---
src/timeserver.c | 53 +++++++++++++++++++++++++++---------------------
3 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index 12e9407f261e..17452a76567f 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -451,7 +451,8 @@ char **__connman_timeserver_system_get();
GSList *__connman_timeserver_add_list(GSList *server_list,
const char *timeserver);
GSList *__connman_timeserver_get_all(struct connman_service *service);
-int __connman_timeserver_sync(struct connman_service *service);
+void __connman_timeserver_sync(struct connman_service *service);
+void __connman_timeserver_conf_update(struct connman_service *service);
enum __connman_dhcpv6_status {
CONNMAN_DHCPV6_STATUS_FAIL = 0,
diff --git a/src/service.c b/src/service.c
index f8ea8ebbe9a4..2c01447e4673 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3746,9 +3746,7 @@ static DBusMessage *set_property(DBusConnection *conn,
service_save(service);
timeservers_configuration_changed(service);
-
- if (service == connman_service_get_default())
- __connman_timeserver_sync(service);
+ __connman_timeserver_conf_update(service);
} else if (g_str_equal(name, "Domains.Configuration")) {
DBusMessageIter entry;
diff --git a/src/timeserver.c b/src/timeserver.c
index 7e4f88ae849a..9e221a26bfc8 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -272,6 +272,7 @@ GSList *__connman_timeserver_get_all(struct connman_service *service)
static gboolean ts_recheck(gpointer user_data)
{
+ struct connman_service *service;
GSList *ts;
ts = __connman_timeserver_get_all(connman_service_get_default());
@@ -287,7 +288,8 @@ static gboolean ts_recheck(gpointer user_data)
g_slist_free_full(ts, g_free);
- __connman_timeserver_sync(NULL);
+ service = connman_service_get_default();
+ __connman_timeserver_sync(service);
return FALSE;
}
@@ -327,29 +329,14 @@ static void ts_recheck_enable(void)
NULL);
}
-/*
- * This function must be called every time the default service changes, the
- * service timeserver(s) or gateway changes or the global timeserver(s) changes.
- */
-int __connman_timeserver_sync(struct connman_service *default_service)
+static void ts_reset(struct connman_service *service)
{
- struct connman_service *service;
char **nameservers;
int i;
- if (default_service)
- service = default_service;
- else
- service = connman_service_get_default();
-
- if (!service)
- return -EINVAL;
-
- if (service == ts_service)
- return -EALREADY;
-
if (!resolv)
- return 0;
+ return;
+
/*
* Before we start creating the new timeserver list we must stop
* any ongoing ntp query and server resolution.
@@ -380,17 +367,32 @@ int __connman_timeserver_sync(struct connman_service *default_service)
if (!timeservers_list) {
DBG("No timeservers set.");
- return 0;
+ return;
}
ts_recheck_enable();
ts_service = service;
timeserver_sync_start();
+}
- return 0;
+void __connman_timeserver_sync(struct connman_service *service)
+{
+ if (!service || service == ts_service)
+ return;
+
+ ts_reset(service);
}
+void __connman_timeserver_conf_update(struct connman_service *service)
+{
+ if (!service || service != ts_service)
+ return;
+
+ ts_reset(service);
+}
+
+
static int timeserver_start(struct connman_service *service)
{
char **nameservers;
@@ -430,7 +432,9 @@ static int timeserver_start(struct connman_service *service)
g_strfreev(nameservers);
}
- return __connman_timeserver_sync(service);
+ __connman_timeserver_sync(service);
+
+ return 0;
}
static void timeserver_stop(void)
@@ -457,9 +461,12 @@ static void timeserver_stop(void)
int __connman_timeserver_system_set(char **servers)
{
+ struct connman_service *service;
+
save_timeservers(servers);
- __connman_timeserver_sync(NULL);
+ service = connman_service_get_default();
+ __connman_timeserver_conf_update(service);
return 0;
}
--
2.29.2
2 months
[PATCH] services: Escape passphrase string
by Daniel Wagner
The WiFi passphrase might use chars which need escaping because
g_key_file_get_string() is trying to decode the strings it reads.
---
src/service.c | 11 ++++++++---
src/technology.c | 13 +++++++++----
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/service.c b/src/service.c
index f8ea8ebbe9a4..a83f98757cf1 100644
--- a/src/service.c
+++ b/src/service.c
@@ -572,8 +572,10 @@ static int service_load(struct connman_service *service)
str = g_key_file_get_string(keyfile,
service->identifier, "Passphrase", NULL);
if (str) {
+ char *dec = g_strcompress(str);
+ g_free(str);
g_free(service->passphrase);
- service->passphrase = str;
+ service->passphrase = dec;
}
if (service->ipconfig_ipv4)
@@ -736,9 +738,12 @@ static int service_save(struct connman_service *service)
g_free(str);
}
- if (service->passphrase && strlen(service->passphrase) > 0)
+ if (service->passphrase && strlen(service->passphrase) > 0) {
+ char *enc = g_strescape(service->passphrase, NULL);
g_key_file_set_string(keyfile, service->identifier,
- "Passphrase", service->passphrase);
+ "Passphrase", enc);
+ g_free(enc);
+ }
if (service->ipconfig_ipv4)
__connman_ipconfig_save(service->ipconfig_ipv4, keyfile,
diff --git a/src/technology.c b/src/technology.c
index 4e053fc9caa9..672d6ea81990 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -185,10 +185,12 @@ static void technology_save(struct connman_technology *technology)
"Tethering.Identifier",
technology->tethering_ident);
- if (technology->tethering_passphrase)
+ if (technology->tethering_passphrase) {
+ char *enc = g_strescape(technology->tethering_passphrase, NULL);
g_key_file_set_string(keyfile, identifier,
- "Tethering.Passphrase",
- technology->tethering_passphrase);
+ "Tethering.Passphrase", enc);
+ g_free(enc);
+ }
done:
g_free(identifier);
@@ -390,6 +392,7 @@ static void technology_load(struct connman_technology *technology)
gchar *identifier;
GError *error = NULL;
bool enable, need_saving = false;
+ char *enc;
DBG("technology %p", technology);
@@ -436,8 +439,10 @@ static void technology_load(struct connman_technology *technology)
technology->tethering_ident = g_key_file_get_string(keyfile,
identifier, "Tethering.Identifier", NULL);
- technology->tethering_passphrase = g_key_file_get_string(keyfile,
+ enc = g_key_file_get_string(keyfile,
identifier, "Tethering.Passphrase", NULL);
+ if (enc)
+ technology->tethering_passphrase = g_strcompress(enc);
done:
g_free(identifier);
--
2.29.2
2 months
Re: Strange connection problem
by Daniel Wagner
On Mon, Dec 21, 2020 at 08:06:42PM +0000, Thomas Green wrote:
> Has anyone been able to look at this or my logfile? I'm still in the dark as to what is going on.
Sorry, I haven't found time to look at it.
2 months, 1 week
Re: [PATCH] Force BSS expiration
by Daniel Wagner
Hi Emmanuel,
On Tue, Dec 22, 2020 at 03:21:21PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> By our side, with wpa_supplicant, we are facing the minimal network
> list (only ethernet and connected wifi) during 2 minutes every 3
> minutes.
Thanks for adding the commit message. The explanation makes totally
sense. I am surprised no one was providing a fix long before. So thank
you!
> Please find my attached patch, taking into account your remarks.
I've reformatted it slightly, so it matches a bit more the code style in
the file. Also I changed the connman_error() into a connman_warn() as
this is not a real error IMO.
Patch applied.
Thanks,
Daniel
2 months, 1 week
[PATCH] clock: Add functions to trigger time synchronization via the
clock API
by VAUTRIN Emmanuel (Canal Plus Prestataire)
Hello,
As our system is based on mandatory https requests, in some degraded cases, we need to trigger a time synchronization. For example, when booting with the clock TimeUpdates equal to manual, changing it to auto does not suffy to update the system time.
Please find the attached patch, adding a clock API providing a way to trigger a time synchronization.
Best Regards,
Emmanuel
2 months, 1 week
[PATCH] clock: Add TimeSynced signal emitted when the system time has
been synced
by VAUTRIN Emmanuel (Canal Plus Prestataire)
Hello,
As our system depends on https requests, and for boot performance purposes, we need to be informed when a time synchronization (via the ntp) is performed.
In this aim, please find the attached patch, adding the TimeSynced signal emitted when the system time has been synced.
Best Regards,
Emmanuel
2 months, 1 week
connmanctl fails to connect to WiFi network having passwords ending
with a backslash character
by * *
Hello,
I ran into the following issue with connman recently. I would have prefered to open a Jira ticket for that but I don't seem to be able to request access to https://01.org/jira/projects/CM which is rather expected though. I hope this is the right place to post this in this context, please someone tell me otherwise.
* Description of the issue:
connmanctl fails to connect to WiFi network having passwords ending with a backslash character (i.e. '\').
See the following commands entered consecutively in bash on my machine (observe the Passphrase value in both cat invocation on my network "settings" file):
LibreElec:~/.cache/connman/wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk # cat settings
[wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk]
Name=My wifi network
SSID=50726574747920666c7920666f7220612057694669
Frequency=5180
Favorite=true
AutoConnect=true
Modified=2020-12-15T22:28:21Z
Passphrase="kjHTJ235nnM!0\
IPv4.method=dhcp
IPv4.DHCP.LastAddress=192.168.87.29
IPv6.method=off
IPv6.privacy=disabled
LibreElec:~/.cache/connman/wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk # connmanctl connect wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk
Error /net/connman/service/wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk: Input/output error
LibreElec:~/.cache/connman/wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk # cat settings
[wifi_dca632348f72_50726574747920666c7920666f7220612057694669_managed_psk]
Name=My wifi network
SSID=50726574747920666c7920666f7220612057694669
Frequency=5180
Favorite=true
AutoConnect=true
Modified=2020-12-15T22:28:21Z
Passphrase="kjHTJ235nnM!0
IPv4.method=dhcp
IPv4.DHCP.LastAddress=192.168.87.29
IPv6.method=off
IPv6.privacy=disabled
The content of the setting file shown with cat differs between the 2 invocations as the ending backslash char has been removed from the passphrase value in the second instance, just after trying to connect the network with connman.
Please note that connman connects seamlessly on another network with a simpler password on the very same WiFi router. Other Linux devices can connect to the network with a password ending with a backslash as long as they don't rely on connman.
Also note that most modern WiFi tools (e.g. wpa_supplicant) offer the possibility to store hashed passphrases rather than clear ones in their config files, for 2 good reasons:
1. It is safer and good practice to store a hashed password rather than a clear one;
2. a hash will never have any special character issue such as the one described in this ticket.
For the story, the reason why I fiddled with connmanctl is that the LibreElec OS provides a GUI for the WiFi settings (e.g. as a connectivity wizard on Kodi 1st start) which initially failed on my network having a password ending with a backslash character, whereas another network on the same router with a very simple password connected successfuly.
As a side note, my WiFi password do start with a double quote character as well as ending witht a backslash. I was not specifically looking for trouble but it seems I have been very "unlucky" with the password generator I used at the time I setup the network.
I have since plugged my rPi to my network via ethernet rather than WiFi for bandwidth reasons so it is not a blocker to me anymore, but I believe I may not be the only one hitting this wall so it may worth a look into.
* Additional info:
** package version(s)
1.38 (shipped on latest LibreElec distro for raspberry pi 4)
** Steps to reproduce:
Create a WiFi configuration for a network with a password ending with a backslash character (i.e. Passphrase=<whatever_you_fancy>\) and try the commands listed above.
Cheers
2 months, 1 week