connman.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH] service: Ignore state information in service reordering
@ 2021-06-23 14:58 VAUTRIN Emmanuel (Canal Plus Prestataire)
  2021-07-02  7:53 ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: VAUTRIN Emmanuel (Canal Plus Prestataire) @ 2021-06-23 14:58 UTC (permalink / raw)
  To: connman

The states of the default and new services shall not impact
the preferred service reordering.
---
 src/service.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 13f619be7066..222f811c2690 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5894,8 +5894,7 @@ static int service_update_preferred_order(struct connman_service *default_servic
 	unsigned int *tech_array;
 	int i;
 
-	if (!default_service || default_service == new_service ||
-			default_service->state != new_state)
+	if (!default_service || default_service == new_service)
 		return 0;
 
 	tech_array = connman_setting_get_uint_list("PreferredTechnologies");
-- 
2.25.1


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

* Re: [PATCH] service: Ignore state information in service reordering
  2021-06-23 14:58 [PATCH] service: Ignore state information in service reordering VAUTRIN Emmanuel (Canal Plus Prestataire)
@ 2021-07-02  7:53 ` Daniel Wagner
  2021-07-02  8:13   ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-07-02  7:53 UTC (permalink / raw)
  To: VAUTRIN Emmanuel (Canal Plus Prestataire); +Cc: connman

Hi Emmanuel,

On Wed, Jun 23, 2021 at 02:58:00PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> The states of the default and new services shall not impact
> the preferred service reordering.

I suspect this one is needed to the moving of the
update_default(). Let's first figure out why you need the first patch.

Thanks,
Daniel

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

* RE: [PATCH] service: Ignore state information in service reordering
  2021-07-02  7:53 ` Daniel Wagner
@ 2021-07-02  8:13   ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  2021-07-02  8:37     ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: VAUTRIN Emmanuel (Canal Plus Prestataire) @ 2021-07-02  8:13 UTC (permalink / raw)
  To: Daniel Wagner; +Cc: connman

> I suspect this one is needed to the moving of the
> update_default(). Let's first figure out why you need the first patch.
Daniel,

The involved states are "online" and "ready", so it is irrelevant
to take them into account here.

Best Regards,

Emmanuel

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

* Re: [PATCH] service: Ignore state information in service reordering
  2021-07-02  8:13   ` VAUTRIN Emmanuel (Canal Plus Prestataire)
@ 2021-07-02  8:37     ` Daniel Wagner
  2021-07-02 15:19       ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-07-02  8:37 UTC (permalink / raw)
  To: VAUTRIN Emmanuel (Canal Plus Prestataire); +Cc: connman

On Fri, Jul 02, 2021 at 08:13:27AM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> The involved states are "online" and "ready", so it is irrelevant
> to take them into account here.

So you are saying we should check the order independent of the
states. Hmm, without VPN Services I'd say you are right, but let me
think this through with VPN in mind.

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

* RE: [PATCH] service: Ignore state information in service reordering
  2021-07-02  8:37     ` Daniel Wagner
@ 2021-07-02 15:19       ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  2021-07-06  7:19         ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: VAUTRIN Emmanuel (Canal Plus Prestataire) @ 2021-07-02 15:19 UTC (permalink / raw)
  To: Daniel Wagner; +Cc: connman

> So you are saying we should check the order independent of the
> states. Hmm, without VPN Services I'd say you are right, but let me
> think this through with VPN in mind.
Indeed. I have not checked the VPN part.
I will develop my point.
With the following context:
* PreferredTechnologies=ethernet,wifi
* Ethernet (E) unplugged,
* Wifi (W) network connected and online.
when plugging the Ethernet, we have
default_service: W, default_service->state: ONLINE.
new_service: E, new_state: READY
So default_service->state != new_state, the service list
is not updated, as required.
If the state still matters for VPN part, maybe it will be cleaner
to call is_connected() instead of checking the state itself.


Best Regards,

Emmanuel

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

* Re: [PATCH] service: Ignore state information in service reordering
  2021-07-02 15:19       ` VAUTRIN Emmanuel (Canal Plus Prestataire)
@ 2021-07-06  7:19         ` Daniel Wagner
  2021-08-25 12:30           ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-07-06  7:19 UTC (permalink / raw)
  To: VAUTRIN Emmanuel (Canal Plus Prestataire); +Cc: connman

Hi Emmanuel,

On Fri, Jul 02, 2021 at 03:19:28PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> If the state still matters for VPN part, maybe it will be cleaner
> to call is_connected() instead of checking the state itself.

Thanks for the explanation. Indeed if the service needs to be connected
then using is_connected() will clearly document this. I need do to test
this a bit first though.

Thannks,
Daniel

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

* RE: [PATCH] service: Ignore state information in service reordering
  2021-07-06  7:19         ` Daniel Wagner
@ 2021-08-25 12:30           ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  2021-08-29 18:48             ` Daniel Wagner
  0 siblings, 1 reply; 9+ messages in thread
From: VAUTRIN Emmanuel (Canal Plus Prestataire) @ 2021-08-25 12:30 UTC (permalink / raw)
  To: Daniel Wagner; +Cc: connman

> I need do to test
> this a bit first though.
Hi Daniel,

Was there any progress on this subject? Does it still require some clarification?


Best Regards,

Emmanuel

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

* Re: [PATCH] service: Ignore state information in service reordering
  2021-08-25 12:30           ` VAUTRIN Emmanuel (Canal Plus Prestataire)
@ 2021-08-29 18:48             ` Daniel Wagner
  2021-08-30  7:26               ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Wagner @ 2021-08-29 18:48 UTC (permalink / raw)
  To: VAUTRIN Emmanuel (Canal Plus Prestataire); +Cc: connman

Hi Emmanuel,

On Wed, Aug 25, 2021 at 12:30:27PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> > I need do to test
> > this a bit first though.
> 
> Was there any progress on this subject? Does it still require some clarification?

Terrible sorry for the long delay. Anyway, I tested this change and it
all looks good with VPN. I've applied the patch

Thanks,
Daniel

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

* RE: [PATCH] service: Ignore state information in service reordering
  2021-08-29 18:48             ` Daniel Wagner
@ 2021-08-30  7:26               ` VAUTRIN Emmanuel (Canal Plus Prestataire)
  0 siblings, 0 replies; 9+ messages in thread
From: VAUTRIN Emmanuel (Canal Plus Prestataire) @ 2021-08-30  7:26 UTC (permalink / raw)
  To: Daniel Wagner; +Cc: connman

Hi Daniel,

> Terrible sorry for the long delay. Anyway, I tested this change and it
> all looks good with VPN. I've applied the patch

Great news, thank you!

Emmanuel

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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 14:58 [PATCH] service: Ignore state information in service reordering VAUTRIN Emmanuel (Canal Plus Prestataire)
2021-07-02  7:53 ` Daniel Wagner
2021-07-02  8:13   ` VAUTRIN Emmanuel (Canal Plus Prestataire)
2021-07-02  8:37     ` Daniel Wagner
2021-07-02 15:19       ` VAUTRIN Emmanuel (Canal Plus Prestataire)
2021-07-06  7:19         ` Daniel Wagner
2021-08-25 12:30           ` VAUTRIN Emmanuel (Canal Plus Prestataire)
2021-08-29 18:48             ` Daniel Wagner
2021-08-30  7:26               ` VAUTRIN Emmanuel (Canal Plus Prestataire)

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).