Hi Aleksandar,
On Mon, Sep 09, 2019 at 03:43:55PM +0300, aleksandar(a)analyticsfire.com wrote:
From: Aleksandar Mitev <shondll(a)yahoo.com>
New flag is
--enable-perpetual-online-check
and is disabled by default, leaving the original behaviour
in place. Even if enabled the regular check relies on:
EnableOnlineCheck = true
Otherwise it will have no effect.
---
README | 10 ++++++++++
configure.ac | 6 ++++++
src/service.c | 12 ++++++++++--
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/README b/README
index f16b9ec0..1ccf766c 100644
--- a/README
+++ b/README
@@ -231,6 +231,16 @@ For a working system, certain configuration options need to be
enabled:
is selected, ConnMan configures systemd-resolved to do DNS
resolving. The default value is "internal".
+ --enable-perpetual-online-check
I am not an native English speaker and it took a few seconds to get to
the meaning of perpetual. But that could also be that I am constantly
in fight with languages :)
+
+ Enable continuously checking for Internet connection.
'continuously' is more catching in my ears. Though, not sure about
it. If no one objects we go with it.
+
+ If enabled, services will conduct online state check
+ regularly once Online state is reached. When in Online state,
+ but due to lack of internet connectivity, the service is
+ downgraded to a Ready state giving the chance for other
+ configured services to take over.
+ This feature depends on EnableOnlineCheck = true.
Activating debugging
====================
diff --git a/configure.ac b/configure.ac
index ee49a22c..a6868b32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -323,6 +323,12 @@ AC_ARG_ENABLE(selinux, AC_HELP_STRING([--enable-selinux],
[enable_selinux=${enableval}], [enable_selinux="no"])
AM_CONDITIONAL(SELINUX, test "${enable_selinux}" != "no")
+AC_ARG_ENABLE(perpetual-online-check, AC_HELP_STRING([--enable-perpetual-online-check],
+ [enable perpetual online check in the online state]),
+ [enable_perp_check="true"], [enable_perp_check="false"])
+AC_DEFINE_UNQUOTED([PERPETUAL_ONLINE_CHECK_ENABLED], ${enable_perp_check},
+ [enable perpetual online check during the online state])
+
AC_ARG_ENABLE(loopback, AC_HELP_STRING([--disable-loopback],
[disable loopback support]),
[enable_loopback=${enableval}])
diff --git a/src/service.c b/src/service.c
index f86baaa6..128ec1d8 100644
--- a/src/service.c
+++ b/src/service.c
@@ -135,6 +135,7 @@ struct connman_service {
bool hidden_service;
char *config_file;
char *config_entry;
+ bool perpetual_online_check;
};
static bool allow_property_changed(struct connman_service *service);
@@ -6191,7 +6192,8 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
/* Any change? */
if (old_state == new_state) {
/* continuous online check requires restart of the check at regular intervals */
- if (new_state == CONNMAN_SERVICE_STATE_ONLINE) {
+ if ((new_state == CONNMAN_SERVICE_STATE_ONLINE) &&
+ service->perpetual_online_check ) {
connman_info("Online check continues for %s.\n", service->name);
__connman_service_online_perform_regular(service, type);
service_indicate_state(service);
@@ -6231,7 +6233,8 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
* ONLINE to ONLINE, there is a special check above
*/
connman_info("Online state reached for %s.\n", service->name);
- __connman_service_online_perform_regular(service, type);
+ if (service->perpetual_online_check)
+ __connman_service_online_perform_regular(service, type);
break;
case CONNMAN_SERVICE_STATE_DISCONNECT:
if (service->state == CONNMAN_SERVICE_STATE_IDLE)
@@ -7164,6 +7167,11 @@ static void update_from_network(struct connman_service *service,
if (!service->network)
service->network = connman_network_ref(network);
+ /* TODO: Perform runtime/config file setting of this flag if needed */
+ service->perpetual_online_check = PERPETUAL_ONLINE_CHECK_ENABLED;
I think it would be better at just to move the 'continuously' is more catching in
my ears. Though, not sure about
it. If no one objects we go with it.
+
+ If enabled, services will conduct online state check
+ regularly once Online state is reached. When in Online state,
+ but due to lack of internet connectivity, the service is
+ downgraded to a Ready state giving the chance for other
+ configured services to take over.
+ This feature depends on EnableOnlineCheck = true.
Activating debugging
====================
diff --git a/configure.ac b/configure.ac
index ee49a22c..a6868b32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -323,6 +323,12 @@ AC_ARG_ENABLE(selinux, AC_HELP_STRING([--enable-selinux],
[enable_selinux=${enableval}], [enable_selinux="no"])
AM_CONDITIONAL(SELINUX, test "${enable_selinux}" != "no")
+AC_ARG_ENABLE(perpetual-online-check, AC_HELP_STRING([--enable-perpetual-online-check],
+ [enable perpetual online check in the online state]),
+ [enable_perp_check="true"], [enable_perp_check="false"])
+AC_DEFINE_UNQUOTED([PERPETUAL_ONLINE_CHECK_ENABLED], ${enable_perp_check},
+ [enable perpetual online check during the online state])
+
AC_ARG_ENABLE(loopback, AC_HELP_STRING([--disable-loopback],
[disable loopback support]),
[enable_loopback=${enableval}])
diff --git a/src/service.c b/src/service.c
index f86baaa6..128ec1d8 100644
--- a/src/service.c
+++ b/src/service.c
@@ -135,6 +135,7 @@ struct connman_service {
bool hidden_service;
char *config_file;
char *config_entry;
+ bool perpetual_online_check;
};
static bool allow_property_changed(struct connman_service *service);
@@ -6191,7 +6192,8 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
/* Any change? */
if (old_state == new_state) {
/* continuous online check requires restart of the check at regular intervals */
- if (new_state == CONNMAN_SERVICE_STATE_ONLINE) {
+ if ((new_state == CONNMAN_SERVICE_STATE_ONLINE) &&
+ service->perpetual_online_check ) {
connman_info("Online check continues for %s.\n", service->name);
__connman_service_online_perform_regular(service, type);
service_indicate_state(service);
@@ -6231,7 +6233,8 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
* ONLINE to ONLINE, there is a special check above
*/
connman_info("Online state reached for %s.\n", service->name);
- __connman_service_online_perform_regular(service, type);
+ if (service->perpetual_online_check)
+ __connman_service_online_perform_regular(service, type);
break;
case CONNMAN_SERVICE_STATE_DISCONNECT:
if (service->state == CONNMAN_SERVICE_STATE_IDLE)
@@ -7164,6 +7167,11 @@ static void update_from_network(struct connman_service *service,
if (!service->network)
service->network = connman_network_ref(network);
+ /* TODO: Perform runtime/config file setting of this flag if needed */
+ service->perpetual_online_check = PERPETUAL_ONLINE_CHECK_ENABLED;
I think it would be better at just to move the
PERPETUAL_ONLINE_CHECK_ENABLED right to the check above. With this the
compiler should be able to remove the code completely if it is
disabled. We can still add the runtime config part if needed. Or
turned into a main.conf config option. This is not too important right
now. The important thing is that we don't change the old behaviour.
+ connman_info("Perpetual online check for service %s is
%s",
+ service->identifier, service->perpetual_online_check ? "Enabled" :
"Disabled");
Please remove this one as well.
BTW, could you also update the doc accordingly? The state diagram
would need an update for this.
Overall, I think this goes into the right direction.
Thanks for your patience!
Daniel