Re: [PATCH] service: Fix service disconnection when using the same index

2014-08-06 Thread Patrik Flykt 
On Tue, 2014-08-05 at 13:12 +0300, Patrik Flykt wrote:
 When a service is to be connected, other service(s) using the same
 interface will be disconnected. Fix the already connected or connecting
 situation by ignoring the service in question when seen in the list.
 
 Simplify the index detection and signal a timeout when disconnect needs
 to happen first in order to indicate to the caller that the connect
 action needs to be redone.
 
 Thanks to Chengyi Zhao for finding the issue.

Applied.

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] service: Fix service disconnection when using the same index

2014-08-05 Thread Patrik Flykt 
When a service is to be connected, other service(s) using the same
interface will be disconnected. Fix the already connected or connecting
situation by ignoring the service in question when seen in the list.

Simplify the index detection and signal a timeout when disconnect needs
to happen first in order to indicate to the caller that the connect
action needs to be redone.

Thanks to Chengyi Zhao for finding the issue.
---
 src/service.c | 45 +++--
 1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/src/service.c b/src/service.c
index 9406bc3..294ceca 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3922,34 +3922,11 @@ static gboolean connect_timeout(gpointer user_data)
return FALSE;
 }
 
-static bool is_interface_available(struct connman_service *service,
-   struct connman_service *other_service)
-{
-   unsigned int index = 0, other_index = 0;
-
-   if (service-ipconfig_ipv4)
-   index = __connman_ipconfig_get_index(service-ipconfig_ipv4);
-   else if (service-ipconfig_ipv6)
-   index = __connman_ipconfig_get_index(service-ipconfig_ipv6);
-
-   if (other_service-ipconfig_ipv4)
-   other_index = __connman_ipconfig_get_index(
-   other_service-ipconfig_ipv4);
-   else if (other_service-ipconfig_ipv6)
-   other_index = __connman_ipconfig_get_index(
-   other_service-ipconfig_ipv6);
-
-   if (index  0  other_index != index)
-   return true;
-
-   return false;
-}
-
 static DBusMessage *connect_service(DBusConnection *conn,
DBusMessage *msg, void *user_data)
 {
struct connman_service *service = user_data;
-   int err = 0;
+   int index, err = 0;
GList *list;
 
DBG(service %p, service);
@@ -3957,27 +3934,27 @@ static DBusMessage *connect_service(DBusConnection 
*conn,
if (service-pending)
return __connman_error_in_progress(msg);
 
+   index = __connman_service_get_index(service);
+
for (list = service_list; list; list = list-next) {
struct connman_service *temp = list-data;
 
-   /*
-* We should allow connection if there are available
-* interfaces for a given technology type (like having
-* more than one wifi card).
-*/
if (!is_connecting(temp)  !is_connected(temp))
break;
 
+   if (service == temp)
+   continue;
+
if (service-type != temp-type)
continue;
 
-   if(!is_interface_available(service, temp)) {
-   if (__connman_service_disconnect(temp) == -EINPROGRESS)
-   err = -EINPROGRESS;
-   }
+   if (__connman_service_get_index(temp) == index 
+   __connman_service_disconnect(temp) == 
-EINPROGRESS)
+   err = -EINPROGRESS;
+
}
if (err == -EINPROGRESS)
-   return __connman_error_in_progress(msg);
+   return __connman_error_operation_timeout(msg);
 
service-ignore = false;
 
-- 
1.9.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

RE: [PATCH] service: Fix service disconnection when using the same index

2014-08-05 Thread Chengyi Zhao 
Hi Patrik,

 From: patrik.fl...@linux.intel.com
 To: connman@connman.net
 Subject: [PATCH] service: Fix service disconnection when using the same index
 Date: Tue, 5 Aug 2014 13:12:56 +0300
 
 When a service is to be connected, other service(s) using the same
 interface will be disconnected. Fix the already connected or connecting
 situation by ignoring the service in question when seen in the list.
 
 Simplify the index detection and signal a timeout when disconnect needs
 to happen first in order to indicate to the caller that the connect
 action needs to be redone.
 

I think whether we can fix this issue with the following patch, ConnMan only 
checks the service state when entered the function connect_service, please 
review.

 src/service.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/service.c b/src/service.c
index 9406bc3..9740738 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3957,6 +3957,16 @@ static DBusMessage *connect_service(DBusConnection *conn,
 if (service-pending)
 return __connman_error_in_progress(msg);
 
+if (is_connected(service)) {
+err = -EISCONN;
+goto done;
+}
+
+if (is_connecting(service)) {
+err = -EALREADY;
+goto done;
+}
+
 for (list = service_list; list; list = list-next) {
 struct connman_service *temp = list-data;
 
@@ -3994,6 +4004,7 @@ static DBusMessage *connect_service(DBusConnection *conn,
 service-pending = NULL;
 }
 
+done:
 if (err  0)
 return __connman_error_failed(msg, -err);

Thanks,
Chengyi
  
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] service: Fix service disconnection when using the same index

2014-08-05 Thread Patrik Flykt 

Hi,

On Tue, 2014-08-05 at 10:42 +, Chengyi Zhao wrote:
 Hi Patrik,
 
  From: patrik.fl...@linux.intel.com
  To: connman@connman.net
  Subject: [PATCH] service: Fix service disconnection when using the same 
  index
  Date: Tue, 5 Aug 2014 13:12:56 +0300
  
  When a service is to be connected, other service(s) using the same
  interface will be disconnected. Fix the already connected or connecting
  situation by ignoring the service in question when seen in the list.
  
  Simplify the index detection and signal a timeout when disconnect needs
  to happen first in order to indicate to the caller that the connect
  action needs to be redone.
  
 
 I think whether we can fix this issue with the following patch, ConnMan only 
 checks the service state when entered the function connect_service, please 
 review.
 
  src/service.c | 11 +++
  1 file changed, 11 insertions(+)
 
 diff --git a/src/service.c b/src/service.c
 index 9406bc3..9740738 100644
 --- a/src/service.c
 +++ b/src/service.c
 @@ -3957,6 +3957,16 @@ static DBusMessage *connect_service(DBusConnection 
 *conn,
  if (service-pending)
  return __connman_error_in_progress(msg);
  
 +if (is_connected(service)) {
 +err = -EISCONN;
 +goto done;
 +}
 +
 +if (is_connecting(service)) {
 +err = -EALREADY;
 +goto done;
 +}
 +

This one will work as well. But to avoid confusion, we'd like to call
__connman_service_connect() from everywhere in order to have one and
only one place in the code where all necessary checks are done.


Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

RE: [PATCH] service: Fix service disconnection when using the same index

2014-08-05 Thread Chengyi Zhao 
Hi,

  
   src/service.c | 11 +++
   1 file changed, 11 insertions(+)
  
  diff --git a/src/service.c b/src/service.c
  index 9406bc3..9740738 100644
  --- a/src/service.c
  +++ b/src/service.c
  @@ -3957,6 +3957,16 @@ static DBusMessage *connect_service(DBusConnection 
  *conn,
   if (service-pending)
   return __connman_error_in_progress(msg);
   
  +if (is_connected(service)) {
  +err = -EISCONN;
  +goto done;
  +}
  +
  +if (is_connecting(service)) {
  +err = -EALREADY;
  +goto done;
  +}
  +
 
 This one will work as well. But to avoid confusion, we'd like to call
 __connman_service_connect() from everywhere in order to have one and
 only one place in the code where all necessary checks are done.
 

Yes, agree with you.

Thanks,
Chengyi
  
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman