Send connman mailing list submissions to
        connman@lists.01.org

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
        connman-requ...@lists.01.org

You can reach the person managing the list at
        connman-ow...@lists.01.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. Re: [PATCH v2] service: Add AlwaysConnectedTechnologies
      option (Patrik Flykt)
   2. Re: [PATCH] tethering: Add verification to bridge creation
      and configuration (Patrik Flykt)
   3. Re: [PATCH] main: Make -d option repeatable (Patrik Flykt)
   4. Re: Unable to setup connman using systemd (Patrik Flykt)
   5. Re: [RFC 0/4] Too many DBG() on default (Patrik Flykt)


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

Message: 1
Date: Thu, 13 Oct 2016 14:23:55 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Ioan-Adrian Ratiu <adrian.ra...@ni.com>, connman@lists.01.org
Subject: Re: [PATCH v2] service: Add AlwaysConnectedTechnologies
        option
Message-ID: <1476357835.25611.10.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"


        Hi,

Finally on to the patches that implement new features. These require a
bit more time to check as they can be a bit tricky.

On Mon, 2016-09-12 at 17:50 +0300, Ioan-Adrian Ratiu wrote:
> Add a new list option that enables auto-connecting all technologies
> which also contain AutoConnect=true regradless of the setting for
> PreferredTechnologies. The default AlwaysConnectedTechnologies value
> is empty which keeps the current behaviour intact, i.e. if a higher
> preffered technology is already connected, others will not
> autoconnect.
> 
> This setting has no effect if SingleConnectedTechnologies is enabled.
> 
> Based on work originally by Collin Richards <collin.richa...@ni.com>.
> 
> Signed-off-by: Ioan-Adrian Ratiu <adrian.ra...@ni.com>

Nitpick: we don't need/use a signed-off-by for ConnMan.

> ---
> ?src/main.c????| 18 ++++++++++++++++++
> ?src/main.conf |??7 +++++++
> ?src/service.c | 22 ++++++++++++++++++++--
> ?3 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/src/main.c b/src/main.c
> index fdb4f72..462ac15 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -67,6 +67,7 @@ static struct {
> > ?   char **pref_timeservers;
> > ?   unsigned int *auto_connect;
> > ?   unsigned int *preferred_techs;
> > +   unsigned int *always_connected_techs;
> > ?   char **fallback_nameservers;
> > ?   unsigned int timeout_inputreq;
> > ?   unsigned int timeout_browserlaunch;
> @@ -81,6 +82,7 @@ static struct {
> > ?   .pref_timeservers = NULL,
> > ?   .auto_connect = NULL,
> > ?   .preferred_techs = NULL,
> > +   .always_connected_techs = NULL,
> > ?   .fallback_nameservers = NULL,
> > ?   .timeout_inputreq = DEFAULT_INPUT_REQUEST_TIMEOUT,
> > ?   .timeout_browserlaunch = DEFAULT_BROWSER_LAUNCH_TIMEOUT,
> @@ -95,6 +97,7 @@ static struct {
> ?#define CONF_BG_SCAN????????????????????"BackgroundScanning"
> ?#define CONF_PREF_TIMESERVERS???????????"FallbackTimeservers"
> ?#define CONF_AUTO_CONNECT???????????????"DefaultAutoConnectTechnologies"
> +#define CONF_ALWAYS_CONNECTED_TECHS?????"AlwaysConnectedTechnologies"
> ?#define CONF_PREFERRED_TECHS????????????"PreferredTechnologies"
> ?#define CONF_FALLBACK_NAMESERVERS???????"FallbackNameservers"
> ?#define CONF_TIMEOUT_INPUTREQ???????????"InputRequestTimeout"
> @@ -110,6 +113,7 @@ static const char *supported_options[] = {
> > ?   CONF_BG_SCAN,
> > ?   CONF_PREF_TIMESERVERS,
> > ?   CONF_AUTO_CONNECT,
> > +   CONF_ALWAYS_CONNECTED_TECHS,
> > ?   CONF_PREFERRED_TECHS,
> > ?   CONF_FALLBACK_NAMESERVERS,
> > ?   CONF_TIMEOUT_INPUTREQ,
> @@ -295,6 +299,17 @@ static void parse_config(GKeyFile *config)
> > ?   g_clear_error(&error);
> ?
> > ?   str_list = __connman_config_get_string_list(config, "General",
> > +                   CONF_ALWAYS_CONNECTED_TECHS, &len, &error);
> +
> > +   if (!error)
> > +           connman_settings.always_connected_techs =
> > +                   parse_service_types(str_list, len);
> +
> > +   g_strfreev(str_list);
> +
> > +   g_clear_error(&error);
> +
> > +   str_list = __connman_config_get_string_list(config, "General",
> > ?                   CONF_FALLBACK_NAMESERVERS, &len, &error);
> ?
> > ?   if (!error)
> @@ -572,6 +587,9 @@ unsigned int *connman_setting_get_uint_list(const char 
> *key)
> > ?   if (g_str_equal(key, CONF_PREFERRED_TECHS))
> > ?           return connman_settings.preferred_techs;
> ?
> > +   if (g_str_equal(key, CONF_ALWAYS_CONNECTED_TECHS))
> > +           return connman_settings.always_connected_techs;
> +
> > ?   return NULL;
> ?}

The above should be patch 1/3.
?
> diff --git a/src/main.conf b/src/main.conf
> index acceda3..d619413 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -101,3 +101,10 @@
> ?# quality. See RFC6343. Default value is false (as recommended by RFC6343
> ?# section 4.1).
> ?# Enable6to4 = 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
> +# other which is already connected.
> +# This setting has no effect if SingleConnectedTechnologies is enabled.
> +# AlwaysConnectedTechnologies =

And this is patch 3/3.

> diff --git a/src/service.c b/src/service.c
> index 37af5fc..98acd14 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3750,6 +3750,19 @@ static GList *preferred_tech_list_get(void)
> > ?   return tech_data.preferred_list;
> ?}
> ?
> +bool __connman_service_always_connect(enum connman_service_type type)

Since this function is not static, gcc will bark and say:

make --no-print-directory all-am
? CC???????src/src_connmand-main.o
? CC???????src/src_connmand-service.o
src/service.c:3753:6: error: no previous declaration for 
?__connman_service_always_connect? [-Werror=missing-declarations]
?bool __connman_service_always_connect(enum connman_service_type type)
??????^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Makefile:3436: recipe for target 'src/src_connmand-service.o' failed

Just make the function static and drop the '__connman_service_' prefix
since it will not be used outside of this source file.

> +{
> > +   unsigned int *always_connected_techs =
> > +           connman_setting_get_uint_list("AlwaysConnectedTechnologies");
> > +   int i;
> +
> > +   for (i = 0; always_connected_techs && always_connected_techs[i]; i++)
> > +           if (always_connected_techs[i] == type)
> > +                   return true;
> +
> > +   return false;
> +}
> +
> ?static bool auto_connect_service(GList *services,
> > ?                           enum connman_service_connect_reason reason,
> > ?                           bool preferred)
> @@ -3757,6 +3770,7 @@ static bool auto_connect_service(GList *services,
> > ?   struct connman_service *service = NULL;
> > ?   bool ignore[MAX_CONNMAN_SERVICE_TYPES] = { };
> > ?   bool autoconnecting = false;
> +
> > ?   GList *list;
> ?
> > ?   DBG("preferred %d sessions %d reason %s", preferred, active_count,
> @@ -3776,8 +3790,12 @@ static bool auto_connect_service(GList *services,
> > ?           if (service->pending ||
> > ?                           is_connecting(service) ||
> > ?                           is_connected(service)) {
> > -                   if (!active_count)
> -                             return true;

So the above two lines stopped the autoconnect procedure once a service
was found to be connecting when there were zero Sessions set up.

> +                     if (!active_count) {
> +                             
> if(__connman_service_always_connect(service->type))
> > +                                   continue;
> +????????????????????????????????else
> > +                                   return true;
> +????????????????????????}
> ?
> > ?                   ignore[service->type] = true;
> > ?                   autoconnecting = true;

After that, the original code marked the service type to be ignored, so
that ConnMan was connecting only one service for each service type, and
would ignore the rest of the services with the same type - the Sessions
work on the granularity of service types so once one service of the
requested type was connecting, all was fine.

Now, with?_connman_service_always_connect(), the same should happen:
once the service type is found to be connecting the rest of the
services of the same type should be ignored. So the code is to continue
with the lower part setting the respective ignore[] to true.

The next time iteration stops is at 'if (autoconnecting &&
!active_sessions[service->type])' if no Sessions are active. It seems
one more condition is needed, and that is when
'__connman_service_always_connect(service->type)' is false. This in
order to proceed with connecting. The last check of 'if
(!active_count)' need also be modified so that the next services are
tried if there were still more service types to connect.

So the variables to look for are:
-?autoconnecting, another service is connecting already
- active_sessions[], if any Sessions have requested the respective
? service type to be connected
- active_count, a non-zero value means there are Sessions active,?
? shorthand in order not to iterate over the active_sessions[] array
? each time one needs to know if there are more sessions

Yes, it's a bit complicated when the Session logic is factored in.

The above then makes patch 2/3.

Are there any comments from Wagi regarding this? (I think the
active_count could be smartified so that it'd actually figure out if
has something to connect, now it brute forces itself through the list
every time if there is a non-zero number of Sessions... but that's
another can of worms and not this one.)

Cheers,

        Patrik



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

Message: 2
Date: Thu, 13 Oct 2016 14:46:02 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Jose Blanquicet <blanqui...@gmail.com>, connman@lists.01.org
Subject: Re: [PATCH] tethering: Add verification to bridge creation
        and configuration
Message-ID: <1476359162.25611.14.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"

On Thu, 2016-09-22 at 09:54 +0200, Jose Blanquicet wrote:
> In case there is an error while setting up the bridge for WiFi tethering,?
> ConnMan will anyway indicate success and the removed interface will remain
> removed and could not be use anymore. This patch aims to avoid such a 
> situation
> by adding a verification after setting up the bridge set-up and in case there 
> is
> an error stop the process and do not remove the interface.
> 
> Integration of this verification to others tethering technologies has to be
> evaluated and implemented in case it is advantageous.
> 
> ---
> ?include/technology.h |??2 +-
> ?plugins/bluetooth.c??|??5 +++++
> ?plugins/ethernet.c???|??5 +++++
> ?plugins/gadget.c?????|??5 +++++
> ?plugins/wifi.c???????| 23 ++++++++++++++++++-----
> ?src/connman.h????????|??2 +-
> ?src/technology.c?????| 19 ++++++++++++-------
> ?src/tethering.c??????| 16 +++++++++-------
> ?8 files changed, 56 insertions(+), 21 deletions(-)

Split up these changes into multiple files. Start with changing the
return value of __connman_tethering_set_enabled() to int in the .h and
.c files. Next up is?connman_technology_tethering_notify(), add it into
a patch of it's own. See that it compiles after every patch.

Then proceed with the rest split up in logical chunks. Other plugins
than wifi should also be updated if time allows. Again it would be
better with one patch per plugin. It is fine to focus only on the wifi
plugin.

> diff --git a/include/technology.h b/include/technology.h
> index d7fcdde..97db660 100644
> --- a/include/technology.h
> +++ b/include/technology.h
> @@ -36,7 +36,7 @@ extern "C" {
> ?
> ?struct connman_technology;
> ?
> -void connman_technology_tethering_notify(struct connman_technology 
> *technology,
> +int connman_technology_tethering_notify(struct connman_technology 
> *technology,
> > ?                                                   bool enabled);
> ?int connman_technology_set_regdom(const char *alpha2);
> ?void connman_technology_regdom_notify(struct connman_technology *technology,
> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
> index 401bb30..46ac7e6 100644
> --- a/plugins/bluetooth.c
> +++ b/plugins/bluetooth.c
> @@ -692,6 +692,11 @@ static void tethering_create_cb(DBusMessage *message, 
> void *user_data)
> > ?   DBG("bridge %s %s", tethering->bridge, tethering->enable ?
> > ?                   "enabled": "disabled");
> ?
> > +   /*
> > +   ?* TODO: In case of enabling, use return value to check
> > +   ?* if bridge was created and DHCP Server was
> > +   ?* configured/started successfully before continuing.
> > +   ?*/
> > ?   if (tethering->technology)
> > ?           connman_technology_tethering_notify(tethering->technology,
> > ?                           tethering->enable);
> diff --git a/plugins/ethernet.c b/plugins/ethernet.c
> index 9a4d741..942b003 100644
> --- a/plugins/ethernet.c
> +++ b/plugins/ethernet.c
> @@ -371,6 +371,11 @@ static void eth_tech_enable_tethering(struct 
> connman_technology *technology,
> > ?                           remove_network(device, ethernet);
> > ?           }
> ?
> > +           /*
> > +           ?* TODO: Use return value to check if bridge was created
> > +           ?* and DHCP Server was configured/started successfully
> > +           ?* before continuing.
> > +           ?*/
> > ?           connman_technology_tethering_notify(technology, true);
> ?
> > ?           connman_inet_ifup(index);
> diff --git a/plugins/gadget.c b/plugins/gadget.c
> index 94f6648..fbe784f 100644
> --- a/plugins/gadget.c
> +++ b/plugins/gadget.c
> @@ -265,6 +265,11 @@ static void gadget_tech_enable_tethering(struct 
> connman_technology *technology,
> > ?                           remove_network(device, gadget);
> > ?           }
> ?
> > +           /*
> > +           ?* TODO: Use return value to check if bridge was created
> > +           ?* and DHCP Server was configured/started successfully
> > +           ?* before continuing.
> > +           ?*/
> > ?           connman_technology_tethering_notify(technology, true);
> ?
> > ?           connman_inet_ifup(index);
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index 8bde9c0..1b25739 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -3138,13 +3138,14 @@ static void sta_remove_callback(int result,
> > ?   DBG("ifname %s result %d ", info->ifname, result);
> ?
> > ?   if ((result < 0) || (info->wifi->ap_supported != WIFI_AP_SUPPORTED)) {
> > -           info->wifi->tethering = true;
> > +           info->wifi->tethering = false;
> > +           connman_technology_tethering_notify(info->technology, false);
> ?
> > ?           g_free(info->ifname);
> > ?           g_free(info->ssid);
> > ?           g_free(info);
> ?
> > -           if (info->wifi->ap_supported == WIFI_AP_SUPPORTED){
> > +           if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
> > ?                   g_free(info->wifi->tethering_param->ssid);
> > ?                   g_free(info->wifi->tethering_param);
> > ?                   info->wifi->tethering_param = NULL;
> @@ -3154,8 +3155,6 @@ static void sta_remove_callback(int result,
> ?
> > ?   info->wifi->interface = NULL;
> ?
> > -   connman_technology_tethering_notify(info->technology, true);
> -
> > ?   g_supplicant_interface_create(info->ifname, driver, info->wifi->bridge,
> > ?                                           ap_create_callback,
> > ?                                                   info);
> @@ -3171,7 +3170,7 @@ static int enable_wifi_tethering(struct 
> connman_technology *technology,
> > ?   struct wifi_tethering_info *info;
> > ?   const char *ifname;
> > ?   unsigned int mode;
> > -   int err;
> > +   int err, bridge_result = 0;
> ?
> > ?   for (list = iface_list; list; list = list->next) {
> > ?           wifi = list->data;
> @@ -3230,6 +3229,11 @@ static int enable_wifi_tethering(struct 
> connman_technology *technology,
> > ?           info->wifi->tethering = true;
> > ?           info->wifi->ap_supported = WIFI_AP_SUPPORTED;
> ?
> > +           bridge_result =
> > +                   connman_technology_tethering_notify(technology, true);
> > +           if (bridge_result < 0)
> > +                   goto failed;
> +
> > ?           err = g_supplicant_interface_remove(interface,
> > ?                                           sta_remove_callback,
> > ?                                                   info);
> @@ -3244,6 +3248,15 @@ static int enable_wifi_tethering(struct 
> connman_technology *technology,
> > ?           g_free(info);
> > ?           g_free(wifi->tethering_param);
> > ?           wifi->tethering_param = NULL;
> +
> > +           if (bridge_result < 0) {
> > +                   /*
> > +                   ?* If it was an error while setting up the bridge then
> > +                   ?* do not try again on another interface, bridge set-up
> > +                   ?* does not depend on it.
> > +                   ?*/
> > +                   break;
> > +           }
> > ?   }
> ?
> > ?   return -EOPNOTSUPP;
> diff --git a/src/connman.h b/src/connman.h
> index 401e3d7..ce3ef8d 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -620,7 +620,7 @@ int __connman_tethering_init(void);
> ?void __connman_tethering_cleanup(void);
> ?
> ?const char *__connman_tethering_get_bridge(void);
> -void __connman_tethering_set_enabled(void);
> +int __connman_tethering_set_enabled(void);
> ?void __connman_tethering_set_disabled(void);
> ?
> ?int __connman_private_network_request(DBusMessage *msg, const char *owner);
> diff --git a/src/technology.c b/src/technology.c
> index 660af52..6f91af7 100644
> --- a/src/technology.c
> +++ b/src/technology.c
> @@ -211,22 +211,27 @@ static void tethering_changed(struct connman_technology 
> *technology)
> > ?   technology_save(technology);
> ?}
> ?
> -void connman_technology_tethering_notify(struct connman_technology 
> *technology,
> +int connman_technology_tethering_notify(struct connman_technology 
> *technology,
> > ?                                                   bool enabled)
> ?{
> > +   int err;
> +
> > ?   DBG("technology %p enabled %u", technology, enabled);
> ?
> > ?   if (technology->tethering == enabled)
> > -           return;
> > +           return -EALREADY;
> ?
> > -   technology->tethering = enabled;
> > +   if (enabled) {
> > +           err = __connman_tethering_set_enabled();
> > +           if (err < 0)
> > +                   return err;
> > +   } else
> > +           __connman_tethering_set_disabled();
> ?
> > +   technology->tethering = enabled;
> > ?   tethering_changed(technology);
> ?
> > -   if (enabled)
> > -           __connman_tethering_set_enabled();
> > -   else
> > -           __connman_tethering_set_disabled();
> > +   return 0;
> ?}
> ?
> ?static int set_tethering(struct connman_technology *technology,
> diff --git a/src/tethering.c b/src/tethering.c
> index 3153349..9d72bce 100644
> --- a/src/tethering.c
> +++ b/src/tethering.c
> @@ -181,7 +181,7 @@ static void tethering_restart(struct connman_ippool 
> *pool, void *user_data)
> > ?   __connman_tethering_set_enabled();
> ?}
> ?
> -void __connman_tethering_set_enabled(void)
> +int __connman_tethering_set_enabled(void)
> ?{
> > ?   int index;
> > ?   int err;
> @@ -197,12 +197,12 @@ void __connman_tethering_set_enabled(void)
> > ?   DBG("enabled %d", tethering_enabled + 1);
> ?
> > ?   if (__sync_fetch_and_add(&tethering_enabled, 1) != 0)
> > -           return;
> +             return -EACCES;

This is not an error, it means that tethering was already activated, so
it should not be activated again. return 0 is expected here.

> ?
> > ?   err = __connman_bridge_create(BRIDGE_NAME);
> > ?   if (err < 0) {
> > ?           __sync_fetch_and_sub(&tethering_enabled, 1);
> > -           return;
> > +           return -EOPNOTSUPP;
> > ?   }
> ?
> > ?   index = connman_inet_ifindex(BRIDGE_NAME);
> @@ -212,7 +212,7 @@ void __connman_tethering_set_enabled(void)
> > ?           connman_error("Fail to create IP pool");
> > ?           __connman_bridge_remove(BRIDGE_NAME);
> > ?           __sync_fetch_and_sub(&tethering_enabled, 1);
> > -           return;
> > +           return -EADDRNOTAVAIL;
> > ?   }
> ?
> > ?   gateway = __connman_ippool_get_gateway(dhcp_ippool);
> @@ -228,7 +228,7 @@ void __connman_tethering_set_enabled(void)
> > ?           __connman_ippool_unref(dhcp_ippool);
> > ?           __connman_bridge_remove(BRIDGE_NAME);
> > ?           __sync_fetch_and_sub(&tethering_enabled, 1);
> > -           return;
> > +           return -EADDRNOTAVAIL;
> > ?   }
> ?
> > ?   ns = connman_setting_get_string_list("FallbackNameservers");
> @@ -264,7 +264,7 @@ void __connman_tethering_set_enabled(void)
> > ?           __connman_ippool_unref(dhcp_ippool);
> > ?           __connman_bridge_remove(BRIDGE_NAME);
> > ?           __sync_fetch_and_sub(&tethering_enabled, 1);
> > -           return;
> > +           return -EOPNOTSUPP;
> > ?   }
> ?
> > ?   prefixlen = connman_ipaddress_calc_netmask_len(subnet_mask);
> @@ -276,7 +276,7 @@ void __connman_tethering_set_enabled(void)
> > ?           __connman_ippool_unref(dhcp_ippool);
> > ?           __connman_bridge_remove(BRIDGE_NAME);
> > ?           __sync_fetch_and_sub(&tethering_enabled, 1);
> > -           return;
> > +           return -EOPNOTSUPP;
> > ?   }
> ?
> > ?   err = __connman_ipv6pd_setup(BRIDGE_NAME);
> @@ -285,6 +285,8 @@ void __connman_tethering_set_enabled(void)
> > ?                   strerror(-err));
> ?
> > ?   DBG("tethering started");
> +
> > +   return 0;
> ?}
> ?
> ?void __connman_tethering_set_disabled(void)
> 

Cheers,

        Patrik



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

Message: 3
Date: Thu, 13 Oct 2016 14:50:34 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Slava Monich <slava.mon...@jolla.com>, connman@lists.01.org
Subject: Re: [PATCH] main: Make -d option repeatable
Message-ID: <1476359434.25611.16.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"

On Tue, 2016-10-04 at 12:31 +0300, Slava Monich wrote:
> Concatenating the patterns makes more sense than using the last
> supplied value and leaking the previous allocated patterns.
> ---
> ?src/main.c | 15 ++++++++++++---
> ?1 file changed, 12 insertions(+), 3 deletions(-)

This feature exists in a different form already. By separating the
argument strings with either ':', ',' or ' ', multiple patterns can be
specified. This happens in?__connman_log_init().

Cheers,

        Patrik



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

Message: 4
Date: Thu, 13 Oct 2016 14:56:49 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Fabio Emiliani <fabio.emili...@artgroup-spa.com>,
        connman@lists.01.org
Subject: Re: Unable to setup connman using systemd
Message-ID: <1476359809.25611.19.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"

On Thu, 2016-10-13 at 12:38 +0200, Fabio Emiliani wrote:
> Tomasz,
> you are right. I launched wpa_supplicant and then connmand. Nothing
> has appeared on wpa_supplicant stdout.
> This is my wpa_supplicant dbus configuration file:

Check the .service files and other installed files for wpa_supplicant
and connman from another distro such as Fedora or Debian? Have had zero
problems with ConnMan and systemd so far...

Cheers,

        Patrik


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

Message: 5
Date: Thu, 13 Oct 2016 15:06:01 +0300
From: Patrik Flykt <patrik.fl...@linux.intel.com>
To: Daniel Wagner <w...@monom.org>, connman@lists.01.org
Cc: Daniel Wagner <daniel.wag...@bmw-carit.de>
Subject: Re: [RFC 0/4] Too many DBG() on default
Message-ID: <1476360361.25611.21.ca...@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"

On Thu, 2016-10-06 at 14:48 +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wag...@bmw-carit.de>
> 
> Hi,
> 
> When turning on the debug option '-d *' the amount of output is just
> too much too see what's happening. Especially the dnsproxy code is
> very verbose.
> 
> There are the ones we should just remove e.g.
> 
> connmand[16807]: src/service.c:get_properties() service 0x1c72b40
> connmand[16807]:
> src/ipconfig.c:__connman_ipconfig_append_ipv4config()?
> connmand[16807]:
> src/ipconfig.c:__connman_ipconfig_append_ipv6config()?
> connmand[16807]: src/service.c:get_properties() service 0x1c72b40
> connmand[16807]:
> src/ipconfig.c:__connman_ipconfig_append_ipv4config()?
> connmand[16807]:
> src/ipconfig.c:__connman_ipconfig_append_ipv6config()?
> [...]

Yes, just remove these. There are more in src/service.c to chop.

> And there the ones which do make sense too keep, e.g. the dnsproxy
> for
> low level debugging.?
> 
> connmand[16807]: src/dnsproxy.c:ns_resolv() cache hit sync-313-us-
> west-2.sync.services.mozilla.com. type AAAA
> connmand[16807]: src/dnsproxy.c:forward_dns_reply() Received 146
> bytes (id 0x015a)
> connmand[16807]: src/dnsproxy.c:forward_dns_reply() req 0x1c81210
> dstid 0x015a altid 0x58e0 rcode 0
> connmand[16807]: src/dnsproxy.c:cache_update() offset 0 hdr
> 0x7ffc1cfb0af0 msg 0x7ffc1cfb0af0 rcode 0
> connmand[16807]: src/dnsproxy.c:parse_response() qr 1 qdcount 1
> connmand[16807]: src/dnsproxy.c:forward_dns_reply() proto 17 sent 146
> bytes to 13
> connmand[16807]: src/dnsproxy.c:forward_dns_reply() Received 146
> bytes (id 0x015a)
> connmand[16807]: src/dnsproxy.c:udp_listener_event() Received 37
> bytes (id 0x7bb2)
> c
> 
> I did a quick hack to get a discussion going on the idea to support
> debug levels? What do you think on this?

At least the aboe low-level dnsproxy ones make sense with -d
src/dnsproxy.c

> Or should I just do the same with dnsproxy as we have with these
> here:
> 
> README:????CONNMAN_DHCP_DEBUG????????DHCPv4 related debug information
> README:????CONNMAN_DHCPV6_DEBUG??????DHCPv6 related debug information
> README:????CONNMAN_IPTABLES_DEBUG????Extra information when iptables is used
> README:????CONNMAN_RESOLV_DEBUG??????Name resolver debug prints. These debug 
> prints
> README:????CONNMAN_SUPPLICANT_DEBUG??Debugging prints for communication 
> between
> README:????CONNMAN_WEB_DEBUG?????????Debug information when ConnMan does 
> Internet

The CONNMAN_*_DEBUG were used for the "external" components in ./gdhcp,
./gsupplicant and ./gweb. The rest in ./src and ./plugins follow the -d
option.


Cheers,

        Patrik



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

Subject: Digest Footer

_______________________________________________
connman mailing list
connman@lists.01.org
https://lists.01.org/mailman/listinfo/connman


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

End of connman Digest, Vol 12, Issue 14
***************************************

Reply via email to