connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Disable online check for real
@ 2021-08-29 19:53 Daniel Wagner
  2021-08-29 19:53 ` [PATCH v1 1/2] service: Move wispr start code into helper Daniel Wagner
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Daniel Wagner @ 2021-08-29 19:53 UTC (permalink / raw)
  To: connman
  Cc: Daniel Wagner,
	Страхиња
	Радић

Страхиња reported we still do some online checks even though
EnableOnlineCheck is set to false. Let's move the
__connman_service_wispr_start() code to helper and add a check there.

Cc: Страхиња Радић <contact@strahinja.org>

Daniel Wagner (2):
  service: Move wispr start code into helper
  service: Do not trigger wispr start when EnableOnlineCheck is disabled

 src/service.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 1/2] service: Move wispr start code into helper
  2021-08-29 19:53 [PATCH v1 0/2] Disable online check for real Daniel Wagner
@ 2021-08-29 19:53 ` Daniel Wagner
  2021-08-29 19:53 ` [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled Daniel Wagner
  2021-08-30 16:32 ` [PATCH v1 0/2] Disable online check for real Daniel Wagner
  2 siblings, 0 replies; 9+ messages in thread
From: Daniel Wagner @ 2021-08-29 19:53 UTC (permalink / raw)
  To: connman; +Cc: Daniel Wagner

We want to be able to extend the functionally around the
__connman_service_wispr_start() code. Instead having to update the
open code part for IPv4 and IPv6 handling, let's move into a small
helper.
---
 src/service.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/service.c b/src/service.c
index a5a5cfde30fd..237c88e15506 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1595,6 +1595,19 @@ bool __connman_service_index_is_default(int index)
 	return __connman_service_get_index(service) == index;
 }
 
+static void start_wispr_when_connected(struct connman_service *service)
+{
+	if (__connman_service_is_connected_state(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_service_wispr_start(service,
+					CONNMAN_IPCONFIG_TYPE_IPV6);
+}
+
 static void default_changed(void)
 {
 	struct connman_service *service = connman_service_get_default();
@@ -1619,15 +1632,7 @@ static void default_changed(void)
 				connman_setting_get_bool("AllowDomainnameUpdates"))
 			__connman_utsname_set_domainname(service->domainname);
 
-		if (__connman_service_is_connected_state(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_service_wispr_start(service,
-						CONNMAN_IPCONFIG_TYPE_IPV6);
+		start_wispr_when_connected(service);
 
 		/*
 		 * Connect VPN automatically when new default service
@@ -3726,13 +3731,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		nameserver_add_all(service, CONNMAN_IPCONFIG_TYPE_ALL);
 		dns_configuration_changed(service);
 
-		if (__connman_service_is_connected_state(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_service_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV6);
+		start_wispr_when_connected(service);
 
 		service_save(service);
 	} else if (g_str_equal(name, "Timeservers.Configuration")) {
-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-29 19:53 [PATCH v1 0/2] Disable online check for real Daniel Wagner
  2021-08-29 19:53 ` [PATCH v1 1/2] service: Move wispr start code into helper Daniel Wagner
@ 2021-08-29 19:53 ` Daniel Wagner
  2021-08-29 20:08   ` Страхиња Радић
  2021-08-30 16:32 ` [PATCH v1 0/2] Disable online check for real Daniel Wagner
  2 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-08-29 19:53 UTC (permalink / raw)
  To: connman; +Cc: Daniel Wagner

Do not try to use wispr when the user has disabled the online check.

Reported by Страхиња Радић
---
 src/service.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/service.c b/src/service.c
index 237c88e15506..8097f5373250 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1597,6 +1597,12 @@ bool __connman_service_index_is_default(int index)
 
 static void start_wispr_when_connected(struct connman_service *service)
 {
+	if (!connman_setting_get_bool("EnableOnlineCheck")) {
+		connman_info("Online check disabled. "
+			"Default service remains in READY state.");
+		return;
+	}
+
 	if (__connman_service_is_connected_state(service,
 			CONNMAN_IPCONFIG_TYPE_IPV4))
 		__connman_service_wispr_start(service,
-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-29 19:53 ` [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled Daniel Wagner
@ 2021-08-29 20:08   ` Страхиња Радић
  2021-08-30  6:44     ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: Страхиња Радић @ 2021-08-29 20:08 UTC (permalink / raw)
  To: connman

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

On 21/08/29 09:53, Daniel Wagner wrote:
> Do not try to use wispr when the user has disabled the online check.

The patch you proposed seems fine to me. The gist is to let the user control if
the online check happens or not.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-29 20:08   ` Страхиња Радић
@ 2021-08-30  6:44     ` Daniel Wagner
  2021-08-30 10:34       ` Страхиња Радић
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-08-30  6:44 UTC (permalink / raw)
  To: Страхиња
	Радић
  Cc: connman

On Sun, Aug 29, 2021 at 10:08:49PM +0200, Страхиња Радић wrote:
> On 21/08/29 09:53, Daniel Wagner wrote:
> > Do not try to use wispr when the user has disabled the online check.
> 
> The patch you proposed seems fine to me. The gist is to let the user control if
> the online check happens or not.

Could you give them a quick test if they work for you? Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-30  6:44     ` Daniel Wagner
@ 2021-08-30 10:34       ` Страхиња Радић
  2021-08-30 11:02         ` Страхиња Радић
  0 siblings, 1 reply; 9+ messages in thread
From: Страхиња Радић @ 2021-08-30 10:34 UTC (permalink / raw)
  To: connman

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On 21/08/30 08:44, Daniel Wagner wrote:
> On Sun, Aug 29, 2021 at 10:08:49PM +0200, Страхиња Радић wrote:
> > On 21/08/29 09:53, Daniel Wagner wrote:
> > > Do not try to use wispr when the user has disabled the online check.
> > 
> > The patch you proposed seems fine to me. The gist is to let the user control if
> > the online check happens or not.
> 
> Could you give them a quick test if they work for you? Thanks!

I tested the patch with tcpdump, but unfortunately, it seems the connections are
still being made, even with EnableOnlineCheck=false, Enable6to4=false and
EnableOnlineToReadyTransition=false in /etc/connman/main.conf.

I first uninstalled and pkilled any existing versions of connman, then ensured I
applied both of your patches with git apply, ran make clean && make and make
install as root. Then I ran connmand as root and inside connmanctl:

connmanctl> enable wifi
connmanctl> agent on
connmanctl> connect <my_network>
<entered passphrase>

at the same time, I used tcpdump -i wlan0 > log.txt

and in the log I got a number of lines containing ipv4.connman.net.

Then I tried to apply my patch on top of your patches and repeat the process,
but got the same result.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-30 10:34       ` Страхиња Радић
@ 2021-08-30 11:02         ` Страхиња Радић
  2021-08-30 14:29           ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: Страхиња Радић @ 2021-08-30 11:02 UTC (permalink / raw)
  To: connman

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On 21/08/30 12:34, Страхиња Радић wrote:
> I tested the patch with tcpdump, but unfortunately, it seems the connections are
> still being made, even with EnableOnlineCheck=false, Enable6to4=false and
> EnableOnlineToReadyTransition=false in /etc/connman/main.conf.

I found the culprit -- when built from source, default prefix is /usr/local, so
connman was looking for main.conf in /usr/local/etc/connman instead of
/etc/connman. When I symlinked /etc/connman/main.conf to
/usr/local/etc/connman/main.conf and repeated the test, there were no
occurrences of ipv4.connman.net in the log. I also repeated the test using only
your two patches, without my patch, and got the same result.

Your two patches combined look ok.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled
  2021-08-30 11:02         ` Страхиња Радић
@ 2021-08-30 14:29           ` Daniel Wagner
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Wagner @ 2021-08-30 14:29 UTC (permalink / raw)
  To: Страхиња
	Радић
  Cc: connman

On Mon, Aug 30, 2021 at 01:02:19PM +0200, Страхиња Радић wrote:
> On 21/08/30 12:34, Страхиња Радић wrote:
> > I tested the patch with tcpdump, but unfortunately, it seems the connections are
> > still being made, even with EnableOnlineCheck=false, Enable6to4=false and
> > EnableOnlineToReadyTransition=false in /etc/connman/main.conf.
> 
> I found the culprit -- when built from source, default prefix is /usr/local, so
> connman was looking for main.conf in /usr/local/etc/connman instead of
> /etc/connman. When I symlinked /etc/connman/main.conf to
> /usr/local/etc/connman/main.conf and repeated the test, there were no
> occurrences of ipv4.connman.net in the log. I also repeated the test using only
> your two patches, without my patch, and got the same result.
> 
> Your two patches combined look ok.

Perfect! Thanks for the testing. In this case I'll queue them up this
evening.

Daniel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 0/2] Disable online check for real
  2021-08-29 19:53 [PATCH v1 0/2] Disable online check for real Daniel Wagner
  2021-08-29 19:53 ` [PATCH v1 1/2] service: Move wispr start code into helper Daniel Wagner
  2021-08-29 19:53 ` [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled Daniel Wagner
@ 2021-08-30 16:32 ` Daniel Wagner
  2 siblings, 0 replies; 9+ messages in thread
From: Daniel Wagner @ 2021-08-30 16:32 UTC (permalink / raw)
  To: connman
  Cc: Страхиња
	Радић

On Sun, Aug 29, 2021 at 09:53:19PM +0200, Daniel Wagner wrote:
> Страхиња reported we still do some online checks even though
> EnableOnlineCheck is set to false. Let's move the
> __connman_service_wispr_start() code to helper and add a check there.
> 
> Cc: Страхиња Радић <contact@strahinja.org>

Patches applied.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-30 16:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29 19:53 [PATCH v1 0/2] Disable online check for real Daniel Wagner
2021-08-29 19:53 ` [PATCH v1 1/2] service: Move wispr start code into helper Daniel Wagner
2021-08-29 19:53 ` [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled Daniel Wagner
2021-08-29 20:08   ` Страхиња Радић
2021-08-30  6:44     ` Daniel Wagner
2021-08-30 10:34       ` Страхиња Радић
2021-08-30 11:02         ` Страхиња Радић
2021-08-30 14:29           ` Daniel Wagner
2021-08-30 16:32 ` [PATCH v1 0/2] Disable online check for real Daniel Wagner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).