1:30 p.m.
From: Aleksandar Mitev <shondll(a)yahoo.com>
Hi Daniel,
So nice to hear from you :)
Ok, let me describe our use case that led me to come up with the proposed change:
We have a setup with Ethernet, Wifi and Cellular and want to have Internet access
in any case. This would mean that if an interface fails or doesn't have Internet
reachability, a switchover should be made to the next one. Of course, Ethernet
is the most preferred, falling back to Wifi, and if Wifi is not reliable source
of Internet, then go to the last resort of Cellular.
Everything is running fine with the current version of connman, except that, if,
for example, Ethernet is the current default source of Internet, and the router
we are connected to, stops routing packets for some reason, then we lose Internet
connectivity and our connection the device is no longer active.
But connman would not react and continue using the Ethernet connecion. I have seen
this on my Android phone as well: when I am connected with Wifi and the ISP
decides to cut my router's Internet access, then the phone would not switch to
cellular
and no application would recieve notifications for indefinite time.
So my proposed change is to monitor the internet reachability at regular intervals,
using the same technique that connman uses now at the connecting phase. However,
since this checking will eat some data inevitably, we would not want to run it
on the cellular connection, at least (which is last resort anyway and no fallback
can be made). That is why the config option I added is based on network interfaces,
allowing you to choose only those devices that would make sence to continuously
check for Internet reachability.
*************
On 8/7/19 2:56 PM, Aleksandar Mitev wrote:
> I would like to propose adding an additional config option to the
main
> config file of connman that would enable continuous recheck of the
> internet connectivity for the current default connection. It would solve
> the problem of losing internet connectivity after choosing the default
> connection, due to router malfunction etc., which may result in the
> system running connman in staying offline for indefinite time. The
> option defines interfaces on which such a recheck is enabled, because
> some of them may have too expensive data plans to permit for such
> regular checks.
I am bit lost, e.g. what do you mean with the last sentence. I read
checking for online connectivity is too expensive, so let's do regular
check. Can you elaborate a bit on your use case?
I hope the above info is descriptive enough. I am avaialable for more
details if needed.
> The config option goes like this:
> # List of network interfaces that will continuously check
> # for online connectivity, separated by ",".
> # Services will check whether the interface they use is
> # into this list and if so 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 of other
> # configured services to take over.
> # This feature depends on EnableOnlineCheck = true.
> # Default value is empty.
> # InterfacesPerpetualOnlineCheck = vmnet,vboxnet,virbr,ifb,ve-,vb-
Okay I might remember wrong. Currently we do only do the online check
when the link is setup, right? So adding the missing code which
constantly checks for online connectivity is just missing. With that we
would also get automatic downgrade (missing feature). IIRC this feature
is exists in Jolla version of ConnMan [1].
If possible I would like to avoid adding another config variable. Makes
it even more complex to handle it, let alone to test. It looks like I
need to understand first what you are trying to solve here.
[1] https://git.merproject.org/mer-core/connman
I was not familiar with this fork of connman. Did not test it but looked
at the code and I did not find the continuous check after the service goes to
ONLINE state. I may be missing smth here, though.
As for the config option - sure, it could be removed (meaning that all
interfaces would be treated as enabled for checks) or reduces to a
simple global switch that would enable continuous online check after
going to the ONLINE state. However, please consider the usefulness of
our use case here.
> Attached is the proposed patch (based on tag 1.37).
The patches were attached to this mail. Did you use git send-email to
send out the patches?
No, I just used my email client. Now using the git send mail. Hope this
is the right way to send patch proposals along.
> Would be happy to read your feedback for this is my first
proposal for
> change to connman :)
Welcome! I am sorry to response to many emails rather late. Not much
space between work and a lot of other hobbies :)
Thank you!
No problem, since it is holiday season, anyway. My response comes a bit
late as well, so sorry abot that as well.
Thanks again for starting the discusstion on the proposed change.
Best,
Aleksnadar
******
Aleksandar Mitev (6):
Added a config option for perpetual online check
Added check if the device is in InterfacesPerpetualOnlineCheck
Exported device getter for the network object
Added flag in service object to determinte perpetual online check
Added logic to perform perpetual online checks
Added timeout to socket connections
gweb/gweb.c | 32 +++++++++++++++++
include/device.h | 1 +
src/connman.h | 1 +
src/device.c | 27 +++++++++++++++
src/main.c | 26 ++++++++++++++
src/main.conf | 12 +++++++
src/network.c | 5 +++
src/service.c | 90 +++++++++++++++++++++++++++++++++++++++++++++---
8 files changed, 190 insertions(+), 4 deletions(-)
--
2.17.1
1:30 p.m.
New subject: [PATCH 1/6] Added a config option for perpetual online check
From: Aleksandar Mitev <shondll(a)yahoo.com>
The name is InterfacesPerpetualOnlineCheck and it is empty by
default
---
src/main.c | 26 ++++++++++++++++++++++++++
src/main.conf | 12 ++++++++++++
2 files changed, 38 insertions(+)
diff --git a/src/main.c b/src/main.c
index 2371771f..6d9e2654 100644
--- a/src/main.c
+++ b/src/main.c
@@ -69,6 +69,10 @@ static char *default_blacklist[] = {
NULL
};
+static char *default_online_check[] = {
+ NULL
+};
+
static struct {
bool bg_scan;
char **pref_timeservers;
@@ -91,6 +95,7 @@ static struct {
bool auto_connect_roaming_services;
bool acd;
bool use_gateways_as_timeservers;
+ char **interfaces_perpetual_online_check;
} connman_settings = {
.bg_scan = true,
.pref_timeservers = NULL,
@@ -113,6 +118,7 @@ static struct {
.auto_connect_roaming_services = false,
.acd = false,
.use_gateways_as_timeservers = false,
+ .interfaces_perpetual_online_check = NULL,
};
#define CONF_BG_SCAN "BackgroundScanning"
@@ -136,6 +142,7 @@ static struct {
#define CONF_AUTO_CONNECT_ROAMING_SERVICES "AutoConnectRoamingServices"
#define CONF_ACD "AddressConflictDetection"
#define CONF_USE_GATEWAYS_AS_TIMESERVERS "UseGatewaysAsTimeservers"
+#define CONF_INTERFACES_PERPETUAL_ONLINE_CHECK
"InterfacesPerpetualOnlineCheck"
static const char *supported_options[] = {
CONF_BG_SCAN,
@@ -158,6 +165,7 @@ static const char *supported_options[] = {
CONF_AUTO_CONNECT_ROAMING_SERVICES,
CONF_ACD,
CONF_USE_GATEWAYS_AS_TIMESERVERS,
+ CONF_INTERFACES_PERPETUAL_ONLINE_CHECK,
NULL
};
@@ -278,6 +286,7 @@ static void parse_config(GKeyFile *config)
bool boolean;
char **timeservers;
char **interfaces;
+ char **interfaces_online_check;
char **str_list;
char **tethering;
char *vendor_class_id;
@@ -291,6 +300,8 @@ static void parse_config(GKeyFile *config)
parse_service_types(default_favorite_techs, CONF_ARRAY_SIZE(default_favorite_techs));
connman_settings.blacklisted_interfaces =
g_strdupv(default_blacklist);
+ connman_settings.interfaces_perpetual_online_check =
+ g_strdupv(default_online_check);
return;
}
@@ -476,6 +487,17 @@ static void parse_config(GKeyFile *config)
connman_settings.use_gateways_as_timeservers = boolean;
g_clear_error(&error);
+
+ interfaces_online_check = __connman_config_get_string_list(config, "General",
+ CONF_INTERFACES_PERPETUAL_ONLINE_CHECK, &len, &error);
+
+ if (!error)
+ connman_settings.interfaces_perpetual_online_check = interfaces_online_check;
+ else
+ connman_settings.interfaces_perpetual_online_check =
+ g_strdupv(default_online_check);
+
+ g_clear_error(&error);
}
static int config_init(const char *file)
@@ -712,6 +734,9 @@ char **connman_setting_get_string_list(const char *key)
if (g_str_equal(key, CONF_TETHERING_TECHNOLOGIES))
return connman_settings.tethering_technologies;
+ if (g_str_equal(key, CONF_INTERFACES_PERPETUAL_ONLINE_CHECK))
+ return connman_settings.interfaces_perpetual_online_check;
+
return NULL;
}
@@ -919,6 +944,7 @@ int main(int argc, char *argv[])
g_strfreev(connman_settings.fallback_nameservers);
g_strfreev(connman_settings.blacklisted_interfaces);
g_strfreev(connman_settings.tethering_technologies);
+ g_strfreev(connman_settings.interfaces_perpetual_online_check);
g_free(option_debug);
g_free(option_wifi);
diff --git a/src/main.conf b/src/main.conf
index 14965e12..047851ec 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -143,3 +143,15 @@
# to an interface (in accordance with RFC 5227).
# Default value is false.
# AddressConflictDetection = false
+
+# List of network interfaces that will continuously check
+# for online connectivity, separated by ",".
+# Services will check whether the interface they use is
+# into this list and if so 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 of other
+# configured services to take over.
+# This feature depends on EnableOnlineCheck = true.
+# Default value is empty.
+# InterfacesPerpetualOnlineCheck = vmnet,vboxnet,virbr,ifb,ve-,vb-
--
2.17.1
1:30 p.m.
New subject: [PATCH 2/6] Added check if the device is in InterfacesPerpetualOnlineCheck
From: Aleksandar Mitev <shondll(a)yahoo.com>
---
include/device.h | 1 +
src/device.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/include/device.h b/include/device.h
index 0fc06bd0..c4d0fe43 100644
--- a/include/device.h
+++ b/include/device.h
@@ -122,6 +122,7 @@ void connman_device_regdom_notify(struct connman_device *device,
struct connman_device *connman_device_create_from_index(int index);
struct connman_device *connman_device_find_by_index(int index);
int connman_device_reconnect_service(struct connman_device *device);
+bool connman_device_perpetual_online_check_enabled(struct connman_device *device);
struct connman_device_driver {
const char *name;
diff --git a/src/device.c b/src/device.c
index 264c5e2d..816e5e11 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1502,3 +1502,30 @@ void __connman_device_cleanup(void)
g_strfreev(nodevice_filter);
g_strfreev(device_filter);
}
+
+bool connman_device_perpetual_online_check_enabled(struct connman_device *device)
+{
+ char **interfaces_online_check;
+ const char *dev_name;
+ char **pattern;
+
+ DBG("checking perpetual online check for device %s", device->name);
+
+ interfaces_online_check =
+ connman_setting_get_string_list("InterfacesPerpetualOnlineCheck");
+ if (!interfaces_online_check)
+ return false;
+
+ dev_name = connman_device_get_string(device, "Interface");
+ if (!dev_name)
+ return false;
+
+ for (pattern = interfaces_online_check; *pattern; pattern++) {
+ if (!g_strcmp0(dev_name, *pattern)) {
+ DBG("applying perpetual online check for device %s", dev_name);
+ return true;
+ }
+ }
+
+ return false;
+}
--
2.17.1
1:30 p.m.
New subject: [PATCH 3/6] Exported device getter for the network object
From: Aleksandar Mitev <shondll(a)yahoo.com>
---
src/connman.h | 1 +
src/network.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/src/connman.h b/src/connman.h
index 8101c7b2..17f502a9 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -610,6 +610,7 @@ const char *__connman_network_get_type(struct connman_network
*network);
const char *__connman_network_get_group(struct connman_network *network);
const char *__connman_network_get_ident(struct connman_network *network);
bool __connman_network_get_weakness(struct connman_network *network);
+struct connman_device *__connman_network_get_device(struct connman_network *network);
int __connman_config_init();
void __connman_config_cleanup(void);
diff --git a/src/network.c b/src/network.c
index 56fe24ff..35513d7d 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1465,6 +1465,11 @@ const char *__connman_network_get_ident(struct connman_network
*network)
return connman_device_get_ident(network->device);
}
+struct connman_device *__connman_network_get_device(struct connman_network *network)
+{
+ return network->device;
+}
+
bool __connman_network_get_weakness(struct connman_network *network)
{
switch (network->type) {
--
2.17.1
1:30 p.m.
New subject: [PATCH 4/6] Added flag in service object to determinte perpetual online check
From: Aleksandar Mitev <shondll(a)yahoo.com>
---
src/service.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/service.c b/src/service.c
index 3202f26c..7051165b 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);
@@ -7046,6 +7047,7 @@ static void update_from_network(struct connman_service *service,
{
uint8_t strength = service->strength;
const char *str;
+ struct connman_device *device;
DBG("service %p network %p", service, network);
@@ -7093,6 +7095,13 @@ static void update_from_network(struct connman_service *service,
if (!service->network)
service->network = connman_network_ref(network);
+ device = __connman_network_get_device(network);
+ if (device != NULL)
+ service->perpetual_online_check =
+ connman_device_perpetual_online_check_enabled(device);
+ connman_info("Perpetual online check for service %s is %s",
+ service->identifier, service->perpetual_online_check ? "Enabled" :
"Disabled");
+
service_list_sort();
}
--
2.17.1
1:30 p.m.
New subject: [PATCH 5/6] Added logic to perform perpetual online checks
From: Aleksandar Mitev <shondll(a)yahoo.com>
The feature activates when online state is reached and performs
regular checks whether online state is still maintained by the
services (default every 5 minutes). In the case then the
online check fails the serice state is downgrated to Ready
to give the chance for a switchover with another service.
---
src/service.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7051165b..c7cb0ce0 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3441,17 +3441,23 @@ int __connman_service_reset_ipconfig(struct connman_service
*service,
#define ONLINE_CHECK_INITIAL_INTERVAL 1
#define ONLINE_CHECK_MAX_INTERVAL 12
-void __connman_service_wispr_start(struct connman_service *service,
+static void reset_online_recheck_interval(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_interval_ipv4 =
ONLINE_CHECK_INITIAL_INTERVAL;
else
service->online_check_interval_ipv6 =
ONLINE_CHECK_INITIAL_INTERVAL;
+}
+
+void __connman_service_wispr_start(struct connman_service *service,
+ enum connman_ipconfig_type type)
+{
+ DBG("service %p type %s", service, __connman_ipconfig_type2string(type));
+
+ reset_online_recheck_interval(service, type);
__connman_wispr_start(service, type);
}
@@ -6040,18 +6046,64 @@ static gboolean redo_wispr_ipv6(gpointer user_data)
return FALSE;
}
+/* When the online state is once reached perform fruther
+ * checks less frequently
+ */
+#define ONLINE_CHECK_REGULAR_INTERVAL 300
+
+static int __connman_service_online_perform_regular(struct connman_service *service,
+ enum connman_ipconfig_type type)
+{
+ GSourceFunc redo_func;
+ guint retry_interval = ONLINE_CHECK_REGULAR_INTERVAL;
+
+ /* no use of double starting the check */
+ if (service->online_timeout != 0)
+ return 0;
+
+ if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
+ redo_func = redo_wispr_ipv4;
+ } else {
+ redo_func = redo_wispr_ipv6;
+ }
+ reset_online_recheck_interval(service, type);
+
+ DBG("service %p (%s) will be rechecked for internet connectivity in %ds",
+ service, service ? service->identifier : NULL, retry_interval);
+
+ service->online_timeout =
+ g_timeout_add_seconds(retry_interval, redo_func, connman_service_ref(service));
+
+ return 0;
+}
+
+static gboolean downgrade_state_handler(gpointer user_data)
+{
+ struct connman_service *service = user_data;
+
+ connman_warn("Downgrading service %s for online check did not pass",
+ service ? service->identifier : NULL);
+
+ downgrade_state(service);
+
+ return FALSE;
+}
+
int __connman_service_online_check_failed(struct connman_service *service,
enum connman_ipconfig_type type)
{
GSourceFunc redo_func;
int *interval;
+ enum connman_service_state curr_state;
if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
interval = &service->online_check_interval_ipv4;
redo_func = redo_wispr_ipv4;
+ curr_state = service->state_ipv4;
} else {
interval = &service->online_check_interval_ipv6;
redo_func = redo_wispr_ipv6;
+ curr_state = service->state_ipv6;
}
DBG("service %p type %s interval %d", service,
@@ -6065,6 +6117,12 @@ int __connman_service_online_check_failed(struct connman_service
*service,
*/
if (*interval < ONLINE_CHECK_MAX_INTERVAL)
(*interval)++;
+ else {
+ if(curr_state == CONNMAN_SERVICE_STATE_ONLINE) {
+ g_idle_add(downgrade_state_handler, service);
+ return 0;
+ }
+ }
return EAGAIN;
}
@@ -6132,8 +6190,17 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
}
/* Any change? */
- if (old_state == new_state)
+ if (old_state == new_state) {
+ /* continuous online check requires restart of the check at regular intervals */
+ 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);
+ }
+
return -EALREADY;
+ }
DBG("service %p (%s) old state %d (%s) new state %d (%s) type %d (%s)",
service, service ? service->identifier : NULL,
@@ -6162,6 +6229,12 @@ int __connman_service_ipconfig_indicate_state(struct
connman_service *service,
set_mdns(service, service->mdns_config);
break;
case CONNMAN_SERVICE_STATE_ONLINE:
+ /* will reach here once, as next time should be from
+ * ONLINE to ONLINE, there is a special check above
+ */
+ connman_info("Online state reached for %s.\n", service->name);
+ 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)
--
2.17.1
1:30 p.m.
New subject: [PATCH 6/6] Added timeout to socket connections
From: Aleksandar Mitev <shondll(a)yahoo.com>
By default, sockets timeout after the system defined time which is
too much all the timers in connman.
This patch helps make online checks in a deterministic fashion.
---
gweb/gweb.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/gweb/gweb.c b/gweb/gweb.c
index 393afe0a..5743042f 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -77,6 +77,7 @@ struct web_session {
GIOChannel *transport_channel;
guint transport_watch;
guint send_watch;
+ guint timeout_timer;
guint resolv_action;
guint address_action;
@@ -164,6 +165,8 @@ static void free_session(struct web_session *session)
web = session->web;
+ debug(web, "freeing session %p", session);
+
if (session->address_action > 0)
g_source_remove(session->address_action);
@@ -176,6 +179,9 @@ static void free_session(struct web_session *session)
if (session->send_watch > 0)
g_source_remove(session->send_watch);
+ if (session->timeout_timer > 0)
+ g_source_remove(session->timeout_timer);
+
if (session->transport_channel)
g_io_channel_unref(session->transport_channel);
@@ -1032,10 +1038,28 @@ static inline int bind_socket(int sk, int index, int family)
return err;
}
+static gboolean socket_timeout_handler(gpointer user_data)
+{
+ struct web_session *session = user_data;
+ int fd;
+
+ if (session->transport_channel) {
+ debug(session->web, "No response, closing the socket");
+ fd = g_io_channel_unix_get_fd(session->transport_channel);
+ g_io_channel_shutdown(session->transport_channel, TRUE, NULL);
+ close(fd);
+ }
+
+ session->timeout_timer = 0;
+ return FALSE;
+}
+
static int connect_session_transport(struct web_session *session)
{
GIOFlags flags;
int sk;
+ /* TODO: not sure if this is optimal enough */
+ const int socket_timeout = 20;
sk = socket(session->addr->ai_family, SOCK_STREAM | SOCK_CLOEXEC,
IPPROTO_TCP);
@@ -1090,6 +1114,14 @@ static int connect_session_transport(struct web_session *session)
G_IO_OUT | G_IO_HUP | G_IO_NVAL | G_IO_ERR,
send_data, session);
+ debug(session->web, "Setting timeout to the socket to %ds",
socket_timeout);
+ /* TODO: if this is not effective, resort to using mutexes
+ * Assign lower priority to this callback than that of the watch functions
+ * as we don't want the timeout to possibly interrupt last moment read/writes
+ */
+ session->timeout_timer = g_timeout_add_seconds_full(G_PRIORITY_DEFAULT_IDLE,
+ socket_timeout, socket_timeout_handler, session, NULL);
+
return 0;
}
--
2.17.1
8:21 a.m.
Hi Aleksandar,
On Tue, Aug 27, 2019 at 03:30:40PM +0300, aleksandar(a)analyticsfire.com wrote:
Ok, let me describe our use case that led me to come up with the
proposed change:
We have a setup with Ethernet, Wifi and Cellular and want to have Internet access
in any case. This would mean that if an interface fails or doesn't have Internet
reachability, a switchover should be made to the next one. Of course, Ethernet
is the most preferred, falling back to Wifi, and if Wifi is not reliable source
of Internet, then go to the last resort of Cellular.
Everything is running fine with the current version of connman, except that, if,
for example, Ethernet is the current default source of Internet, and the router
we are connected to, stops routing packets for some reason, then we lose Internet
connectivity and our connection the device is no longer active.
But connman would not react and continue using the Ethernet connecion. I have seen
this on my Android phone as well: when I am connected with Wifi and the ISP
decides to cut my router's Internet access, then the phone would not switch to
cellular
and no application would recieve notifications for indefinite time.
So my proposed change is to monitor the internet reachability at regular intervals,
using the same technique that connman uses now at the connecting phase. However,
since this checking will eat some data inevitably, we would not want to run it
on the cellular connection, at least (which is last resort anyway and no fallback
can be made). That is why the config option I added is based on network interfaces,
allowing you to choose only those devices that would make sence to continuously
check for Internet reachability.
Thanks for the more verbose explenation on what you want to
achieve. This sounds like a reasonable thing to expect from
ConnMan. Overall, I think we don't need to add a new config
options. Instead just depend on the default OnlineCheck option. If
that is set ConnMan should continuously do the check. Would you mind
to respin your patches withtout the InterfacesPerpetualOnlineCheck
config option?
>[1] https://git.merproject.org/mer-core/connman
I was not familiar with this fork of connman. Did not test it but looked
at the code and I did not find the continuous check after the service goes to
ONLINE state. I may be missing smth here, though.
I just might have remembered it wrong.
As for the config option - sure, it could be removed (meaning that
all
interfaces would be treated as enabled for checks) or reduces to a
simple global switch that would enable continuous online check after
going to the ONLINE state. However, please consider the usefulness of
our use case here.
>> Attached is the proposed patch (based on tag 1.37).
>
>The patches were attached to this mail. Did you use git send-email to
>send out the patches?
No, I just used my email client. Now using the git send mail. Hope this
is the right way to send patch proposals along.
Looks good now :)
Thanks,
Daniel
2:50 p.m.
Would you mind to respin your patches withtout the
InterfacesPerpetualOnlineCheck config option?
Yes, starting that now.
On Mon, Sep 2, 2019 at 10:21 AM Daniel Wagner <wagi(a)monom.org> wrote:
Hi Aleksandar,
On Tue, Aug 27, 2019 at 03:30:40PM +0300, aleksandar(a)analyticsfire.com
wrote:
> Ok, let me describe our use case that led me to come up with the
proposed change:
> We have a setup with Ethernet, Wifi and Cellular and want to have
Internet access
> in any case. This would mean that if an interface fails or doesn't have
Internet
> reachability, a switchover should be made to the next one. Of course,
Ethernet
> is the most preferred, falling back to Wifi, and if Wifi is not reliable
source
> of Internet, then go to the last resort of Cellular.
>
> Everything is running fine with the current version of connman, except
that, if,
> for example, Ethernet is the current default source of Internet, and the
router
> we are connected to, stops routing packets for some reason, then we lose
Internet
> connectivity and our connection the device is no longer active.
> But connman would not react and continue using the Ethernet connecion. I
have seen
> this on my Android phone as well: when I am connected with Wifi and the
ISP
> decides to cut my router's Internet access, then the phone would not
switch to cellular
> and no application would recieve notifications for indefinite time.
>
> So my proposed change is to monitor the internet reachability at regular
intervals,
> using the same technique that connman uses now at the connecting phase.
However,
> since this checking will eat some data inevitably, we would not want to
run it
> on the cellular connection, at least (which is last resort anyway and no
fallback
> can be made). That is why the config option I added is based on network
interfaces,
> allowing you to choose only those devices that would make sence to
continuously
> check for Internet reachability.
Thanks for the more verbose explenation on what you want to
achieve. This sounds like a reasonable thing to expect from
ConnMan. Overall, I think we don't need to add a new config
options. Instead just depend on the default OnlineCheck option. If
that is set ConnMan should continuously do the check. Would you mind
to respin your patches withtout the InterfacesPerpetualOnlineCheck
config option?
> >[1] https://git.merproject.org/mer-core/connman
>
> I was not familiar with this fork of connman. Did not test it but looked
> at the code and I did not find the continuous check after the service
goes to
> ONLINE state. I may be missing smth here, though.
I just might have remembered it wrong.
> As for the config option - sure, it could be removed (meaning that all
> interfaces would be treated as enabled for checks) or reduces to a
> simple global switch that would enable continuous online check after
> going to the ONLINE state. However, please consider the usefulness of
> our use case here.
>
> >> Attached is the proposed patch (based on tag 1.37).
> >
> >The patches were attached to this mail. Did you use git send-email to
> >send out the patches?
>
> No, I just used my email client. Now using the git send mail. Hope this
> is the right way to send patch proposals along.
Looks good now :)
Thanks,
Daniel
9:38 a.m.
From: Aleksandar Mitev <shondll(a)yahoo.com>
...
So my proposed change is to monitor the internet reachability at regular intervals,
using the same technique that connman uses now at the connecting phase. However,
since this checking will eat some data inevitably, we would not want to run it
on the cellular connection, at least (which is last resort anyway and no fallback
can be made). That is why the config option I added is based on network interfaces,
allowing you to choose only those devices that would make sence to continuously
check for Internet reachability.
IIRC, a service first becomes "ready" and then if online check succeeds,
transitions to the "online" state. Therefore, if a repeated online check
fails, the service has to become "ready" again, as if online check never
succeeded in the first place.
That's still a rather significant change, since it introduces a new
"online" -> "ready" transition in the service state machine.
Application
software may not be ready to handle that.
However it won't solve the problem of silently losing the Internet
connection, because transition to "ready" state doesn't cause reconnect.
The service just happily stays in the "ready" state and that's it.
Not to mention that a failed online check doesn't mean that you're not
connected to the Internet. It only means that the online check service
is down or unreachable (e.g. blocked by a corporate firewall or local
authorities).
Am I missing something?
-Slava
2:48 p.m.
Thanks for thinking this over :)
As of v1.37, when a service reaches the READY state, then the online check
is started. If it succeeds, the a transition to ONLINE state happens. You
are right in thinking that my proposed change will cause a downgrading to
READY again, but you see, according to the state machine, another round of
online checking will begin, possibly promoting the service back to ONLINE
it it ever succeeds (ex. the Internet disruption was temporary). In
general, this is a chance for any of the other services in READY state to
take over.
I am not sure and can never be on how applications will react to such a
transition, as I would guess it is up to, how well the application is
written, and provided it ever needs such information about which is the
default service atm (I can imagine most of the apps just need internet,
that is all). Yet, given the fact, that most applications already react to
switching of connections (i.e. the wifi goes away failing back to cellular)
I do not expect major headaches.
Finally, if a failed online check is not reliable enough to determine
Internet reachability, then it can be disabled altogether and replaced with
a simple ping check (from a separate service) as in the case of mwan3 tool
for Openwrt. However, in that case one will miss the checking of DNS
working.
Please share more details if you think I am missing smth here.
Best,
Aleksandar
On Tue, Sep 3, 2019 at 11:38 AM Slava Monich <slava.monich(a)jolla.com> wrote:
> From: Aleksandar Mitev <shondll(a)yahoo.com>
>
> ...
>
> So my proposed change is to monitor the internet reachability at regular
intervals,
> using the same technique that connman uses now at the connecting phase.
However,
> since this checking will eat some data inevitably, we would not want to
run it
> on the cellular connection, at least (which is last resort anyway and no
fallback
> can be made). That is why the config option I added is based on network
interfaces,
> allowing you to choose only those devices that would make sence to
continuously
> check for Internet reachability.
IIRC, a service first becomes "ready" and then if online check succeeds,
transitions to the "online" state. Therefore, if a repeated online check
fails, the service has to become "ready" again, as if online check never
succeeded in the first place.
That's still a rather significant change, since it introduces a new
"online" -> "ready" transition in the service state machine.
Application
software may not be ready to handle that.
However it won't solve the problem of silently losing the Internet
connection, because transition to "ready" state doesn't cause reconnect.
The service just happily stays in the "ready" state and that's it.
Not to mention that a failed online check doesn't mean that you're not
connected to the Internet. It only means that the online check service
is down or unreachable (e.g. blocked by a corporate firewall or local
authorities).
Am I missing something?
-Slava
4:22 p.m.
On 04/09/2019 16:48, Aleksandar Mitev wrote:
Thanks for thinking this over :)
As of v1.37, when a service reaches the READY state, then the online
check is started. If it succeeds, the a transition to ONLINE state
happens. You are right in thinking that my proposed change will cause
a downgrading to READY again, but you see, according to the state
machine, another round of online checking will begin, possibly
promoting the service back to ONLINE it it ever succeeds (ex. the
Internet disruption was temporary). In general, this is a chance for
any of the other services in READY state to take over.
Are you sure that another service WILL actually take over when some
other service is stuck in the READY state? That's the key point.
If such a handover doesn't happen then this transition back to READY is
useless. It will be just generating additional network traffic and
that's it.
If handover does indeed happen then it's a major change of behavior and
must be configurable. We (Sailfish OS) consider READY as of the
"connected" states. We don't need and don't want to automatically switch
from WiFi to mobile data if WiFi gets stuck in the READY state. We still
consider it connected and let the user to turn it off and switch to
mobile data, if the user notices that it's not actually working.
...
Finally, if a failed online check is not reliable enough to determine
Internet reachability, then it can be disabled altogether and replaced
with a simple ping check (from a separate service) as in the case of
mwan3 tool for Openwrt. However, in that case one will miss the
checking of DNS working.
Please share more details if you think I am missing smth here.
Have you considered the VPN scenario? When VPN is connected, you may not
be able to run online check for the transport interface - it won't have
any routes to anything other than local subnet and the VPN server.
Cheers,
-Slava
Best,
Aleksandar
2:22 p.m.
Hi,
On 9/4/19 5:22 PM, Slava Monich wrote:
> As of v1.37, when a service reaches the READY state, then the
online
> check is started. If it succeeds, the a transition to ONLINE state
> happens. You are right in thinking that my proposed change will cause
> a downgrading to READY again, but you see, according to the state
> machine, another round of online checking will begin, possibly
> promoting the service back to ONLINE it it ever succeeds (ex. the
> Internet disruption was temporary). In general, this is a chance for
> any of the other services in READY state to take over.
Are you sure that another service WILL actually take over when some
other service is stuck in the READY state? That's the key point.
I agree. There are many small problems along the road.
If such a handover doesn't happen then this transition back to
READY is
useless. It will be just generating additional network traffic and
that's it.
If handover does indeed happen then it's a major change of behavior and
must be configurable. We (Sailfish OS) consider READY as of the
"connected" states. We don't need and don't want to automatically
switch
from WiFi to mobile data if WiFi gets stuck in the READY state. We still
consider it connected and let the user to turn it off and switch to
mobile data, if the user notices that it's not actually working.
Okay, understood. If we going to mess with the current state machine we
need to keep a compact mode. I suppose compile time would be good enough?
> Finally, if a failed online check is not reliable enough to
determine
> Internet reachability, then it can be disabled altogether and replaced
> with a simple ping check (from a separate service) as in the case of
> mwan3 tool for Openwrt. However, in that case one will miss the
> checking of DNS working.
>
> Please share more details if you think I am missing smth here.
>
Have you considered the VPN scenario? When VPN is connected, you may not
be able to run online check for the transport interface - it won't have
any routes to anything other than local subnet and the VPN server.
Indeed, VPN can be pretty tricky. This needs a lot of testing.
@Slava: I highly appreciate your feedback!
Thanks,
Daniel
1025
days inactive
1035
days old
12 comments
4 participants
participants (4)
-
Aleksandar Mitev -
aleksandar＠analyticsfire.com
-
Daniel Wagner -
Slava Monich