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. [PATCH v2] service: Add EnableOnlineCheck config option
      (Lukasz Nowak)
   2. [PATCH v2] service: Add EnableOnlineCheck config option
      (Lukasz Nowak)
   3. [PATCH v2] ofono: Fix segfault during set_property
      (Scott Valentine)
   4. Re: [PATCH] ofono: Fix segfault during set_property
      (Scott Valentine)


----------------------------------------------------------------------

Message: 1
Date: Mon, 13 Mar 2017 15:14:22 +0000
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH v2] service: Add EnableOnlineCheck config option
Message-ID: <[email protected]>

From: Lukasz Nowak <[email protected]>

Changes from v1:
- added info printouts reminding that the online check is disabled
  at main.conf parse time, and every time a service would go ONLINE
  but is kept in READY.

Lukasz Nowak (1):
  service: Add EnableOnlineCheck config option

 doc/connman.conf.5.in |  9 +++++++++
 src/main.c            | 17 +++++++++++++++++
 src/main.conf         |  9 +++++++++
 src/service.c         | 18 +++++++++++-------
 4 files changed, 46 insertions(+), 7 deletions(-)

-- 
2.7.4



------------------------------

Message: 2
Date: Mon, 13 Mar 2017 15:14:23 +0000
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH v2] service: Add EnableOnlineCheck config option
Message-ID: <[email protected]>

From: Lukasz Nowak <[email protected]>

Global config option, which allows to enable/disable (enabled by default)
use of http get in wispr to transition a default service from READY to
ONLINE state.
---
 doc/connman.conf.5.in |  9 +++++++++
 src/main.c            | 17 +++++++++++++++++
 src/main.conf         |  9 +++++++++
 src/service.c         | 18 +++++++++++-------
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/doc/connman.conf.5.in b/doc/connman.conf.5.in
index c113ac3..9c99910 100644
--- a/doc/connman.conf.5.in
+++ b/doc/connman.conf.5.in
@@ -136,6 +136,15 @@ See RFC6343.  Default value is false (as recommended by 
RFC6343 section 4.1).
 Set DHCP option 60 (Vendor Class ID) to the given string. This option can
 be used by DHCP servers to identify specific clients without having to
 rely on MAC address ranges, etc
+.TP
+.BI EnableOnlineCheck=true\ \fR|\fB\ false
+Enable or disable use of http get as on online status check.
+When a service is in a READY state, and is selected as default,
+ConnMan will issue an HTTP GET request to verify that end-to-end
+connectivity is successful. Only then the service will be
+transitioned to ONLINE state.
+If this setting is false, the default service will remain in READY state.
+Default value is true.
 .SH "EXAMPLE"
 The following example configuration disables hostname updates and enables
 ethernet tethering.
diff --git a/src/main.c b/src/main.c
index 915c17e..38023fa 100644
--- a/src/main.c
+++ b/src/main.c
@@ -78,6 +78,7 @@ static struct {
        bool persistent_tethering_mode;
        bool enable_6to4;
        char *vendor_class_id;
+       bool enable_online_check;
 } connman_settings  = {
        .bg_scan = true,
        .pref_timeservers = NULL,
@@ -94,6 +95,7 @@ static struct {
        .persistent_tethering_mode = false,
        .enable_6to4 = false,
        .vendor_class_id = NULL,
+       .enable_online_check = true,
 };
 
 #define CONF_BG_SCAN                    "BackgroundScanning"
@@ -111,6 +113,7 @@ static struct {
 #define CONF_PERSISTENT_TETHERING_MODE  "PersistentTetheringMode"
 #define CONF_ENABLE_6TO4                "Enable6to4"
 #define CONF_VENDOR_CLASS_ID            "VendorClassID"
+#define CONF_ENABLE_ONLINE_CHECK        "EnableOnlineCheck"
 
 static const char *supported_options[] = {
        CONF_BG_SCAN,
@@ -128,6 +131,7 @@ static const char *supported_options[] = {
        CONF_PERSISTENT_TETHERING_MODE,
        CONF_ENABLE_6TO4,
        CONF_VENDOR_CLASS_ID,
+       CONF_ENABLE_ONLINE_CHECK,
        NULL
 };
 
@@ -394,6 +398,16 @@ static void parse_config(GKeyFile *config)
                connman_settings.vendor_class_id = vendor_class_id;
 
        g_clear_error(&error);
+
+       boolean = __connman_config_get_bool(config, "General",
+                                       CONF_ENABLE_ONLINE_CHECK, &error);
+       if (!error) {
+               connman_settings.enable_online_check = boolean;
+               if (!boolean)
+                       connman_info("Online check disabled by main config.");
+       }
+
+       g_clear_error(&error);
 }
 
 static int config_init(const char *file)
@@ -574,6 +588,9 @@ bool connman_setting_get_bool(const char *key)
        if (g_str_equal(key, CONF_ENABLE_6TO4))
                return connman_settings.enable_6to4;
 
+       if (g_str_equal(key, CONF_ENABLE_ONLINE_CHECK))
+               return connman_settings.enable_online_check;
+
        return false;
 }
 
diff --git a/src/main.conf b/src/main.conf
index d619413..68870b2 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -102,6 +102,15 @@
 # section 4.1).
 # Enable6to4 = false
 
+# Enable use of http get as on online status check.
+# When a service is in a READY state, and is selected as default,
+# ConnMan will issue an HTTP GET request to verify that end-to-end
+# connectivity is successful. Only then the service will be
+# transitioned to ONLINE state.
+# If this setting is false, the default service will remain in READY state.
+# Default value is true.
+# EnableOnlineCheck = false
+
 # List of technologies with AutoConnect = true which are always connected
 # regardless of PreferredTechnologies setting. Default value is empty and
 # will connect a technology only if it is at a higher preference than any
diff --git a/src/service.c b/src/service.c
index 4e97a15..0d08b5c 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5981,13 +5981,17 @@ int __connman_service_ipconfig_indicate_state(struct 
connman_service *service,
        case CONNMAN_SERVICE_STATE_CONFIGURATION:
                break;
        case CONNMAN_SERVICE_STATE_READY:
-               if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
-                       check_proxy_setup(service);
-                       service_rp_filter(service, true);
-               } else {
-                       service->online_check_count = 1;
-                       __connman_wispr_start(service, type);
-               }
+               if (connman_setting_get_bool("EnableOnlineCheck"))
+                       if (type == CONNMAN_IPCONFIG_TYPE_IPV4) {
+                               check_proxy_setup(service);
+                               service_rp_filter(service, true);
+                       } else {
+                               service->online_check_count = 1;
+                               __connman_wispr_start(service, type);
+                       }
+               else
+                       connman_info("Online check disabled. "
+                               "Default service remains in READY state.");
                break;
        case CONNMAN_SERVICE_STATE_ONLINE:
                break;
-- 
2.7.4



------------------------------

Message: 3
Date: Mon, 13 Mar 2017 08:51:20 -1000
From: Scott Valentine <[email protected]>
To: [email protected]
Subject: [PATCH v2] ofono: Fix segfault during set_property
Message-ID: <2883395.AGqk9ia47S@localhost>
Content-Type: text/plain; charset="us-ascii"

If the SIM card is ejected or the modem is reset / removed between the 
set_property call and the set_property_reply callback, connmand will generally 
abort, as the remove_all_contexts will get called, and the callback accesses a 
reference to a stale context via the property_info pointer.

The following patch adds reference counting to the network_context struct to 
prevent the segfault, and has been modified slightly after review.

diff -uNrp old/plugins/ofono.c new/plugins/ofono.c
--- old/plugins/ofono.c 2016-04-17 21:04:30.000000000 -1000
+++ new/plugins/ofono.c 2017-03-13 08:49:02.416476829 -1000
@@ -143,6 +143,8 @@ struct network_context {
        struct connman_ipaddress *ipv6_address;
        char *ipv6_nameservers;
 
+       int refcount;
+
        bool active;
        bool valid_apn; /* APN is 'valid' if length > 0 */
 };
@@ -271,11 +273,23 @@ static struct network_context *network_c
        context->ipv6_address = NULL;
        context->ipv6_nameservers = NULL;
 
+       context->refcount = 1;
+
        return context;
 }
 
-static void network_context_free(struct network_context *context)
+static void network_context_ref(struct network_context *context)
+{
+       __sync_fetch_and_add(&context->refcount, 1);
+       DBG("%p ref %d", context, context->refcount);
+}
+
+static void network_context_unref(struct network_context *context)
 {
+       DBG("%p ref %d", context, context->refcount - 1);
+       if (__sync_fetch_and_sub(&context->refcount, 1) != 1)
+               return;
+
        g_free(context->path);
 
        connman_ipaddress_free(context->ipv4_address);
@@ -389,6 +403,15 @@ struct property_info {
        get_properties_cb get_properties_cb;
 };
 
+static void free_property_info(void * memory)
+{
+       struct property_info * info = memory;
+       if (info->context)
+               network_context_unref(info->context);
+
+       g_free(info);
+}
+
 static void set_property_reply(DBusPendingCall *call, void *user_data)
 {
        struct property_info *info = user_data;
@@ -476,8 +499,11 @@ static int set_property(struct modem_dat
        info->property = property;
        info->set_property_cb = notify;
 
+       if (info->context)
+               network_context_ref(info->context);
+
        dbus_pending_call_set_notify(modem->call_set_property,
-                                       set_property_reply, info, g_free);
+                               set_property_reply, info, free_property_info);
 
        dbus_message_unref(message);
 
@@ -1228,7 +1254,7 @@ static int add_cm_context(struct modem_d
        }
 
        if (g_strcmp0(context_type, "internet") != 0) {
-               network_context_free(context);
+               network_context_unref(context);
                return -EINVAL;
        }
 
@@ -1261,7 +1287,7 @@ static void remove_cm_context(struct mod
                remove_network(modem, context);
        modem->context_list = g_slist_remove(modem->context_list, context);
 
-       network_context_free(context);
+       network_context_unref(context);
        context = NULL;
 }
 

Note: There is a blank line at the end of the patch.

Mahalo,
-Scott V.


------------------------------

Message: 4
Date: Mon, 13 Mar 2017 08:52:56 -1000
From: Scott Valentine <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] ofono: Fix segfault during set_property
Message-ID: <2408673.nai0LQQfP0@localhost>
Content-Type: text/plain; charset="us-ascii"

On Sunday, March 12, 2017 8:44:46 PM HST Daniel Wagner wrote:
> Hi Scott,
> 
> Only a couple of nitpicks.
> 

Hi Daniel,

I just sent up a slightly modified patch under a new thread that addresses your 
comments.

Mahalo,
-Scott V.

> On 03/06/2017 08:58 PM, Scott Valentine wrote:
> > If the SIM card is ejected or the modem is reset / removed between the
> > set_property call and the set_property_reply callback, connmand will
> > generally abort, as the remove_all_contexts will get called, and the
> > callback accesses a reference to a stale context via the property_info
> > pointer.
> > 
> > The following patch adds reference counting to the network_context struct
> > to
> > prevent the segfault:
> Do you want to list something here? The colon indicates this :)
> 
> > diff -uNrp old/plugins/ofono.c new/plugins/ofono.c
> > --- old/plugins/ofono.c     2016-04-17 21:04:30.000000000 -1000
> > +++ new/plugins/ofono.c     2017-03-03 17:39:02.922101490 -1000
> > @@ -143,6 +143,8 @@ struct network_context {
> > 
> >     struct connman_ipaddress *ipv6_address;
> >     char *ipv6_nameservers;
> > 
> > +   int refcount;
> > +
> > 
> >     bool active;
> >     bool valid_apn; /* APN is 'valid' if length > 0 */
> >  
> >  };
> > 
> > @@ -271,11 +273,24 @@ static struct network_context *network_c
> > 
> >     context->ipv6_address = NULL;
> >     context->ipv6_nameservers = NULL;
> > 
> > +   context->refcount = 1;
> > +
> > 
> >     return context;
> >  
> >  }
> > 
> > -static void network_context_free(struct network_context *context)
> > +static void network_context_ref(struct network_context *context)
> > +{
> > +   __sync_fetch_and_add(&context->refcount, 1);
> > +   DBG("refcount = %d", context->refcount);
> > +}
> 
> We have a bunch of ref log in the code base and most of them use for the
> DBG() something like  DBG("%p ref %d", ...) I suggest you do it alike.
> 
> > +static void network_context_unref(struct network_context *context)
> > 
> >  {
> > 
> > +   DBG("refcount = %d", context->refcount);
> > +   if (__sync_fetch_and_sub(&context->refcount, 1) != 1)
> > +           return;
> > +
> > +   DBG("free");
> 
> This one it not really necessary.
> 
> The rest of the patch looks good.
> 
> Thanks,
> Daniel




------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 17, Issue 13
***************************************

Reply via email to