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 3/3] Defined a compile time config of perpetual
online check (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Tue, 24 Sep 2019 09:00:30 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: [email protected], Aleksandar Mitev <[email protected]>
Subject: Re: [PATCH 3/3] Defined a compile time config of perpetual
online check
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Aleksandar,
On Mon, Sep 09, 2019 at 03:43:55PM +0300, [email protected] wrote:
> From: Aleksandar Mitev <[email protected]>
>
> 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
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 47, Issue 13
***************************************
