Re: [PATCH 1/5] dnsproxy: Rename append_domain() to append_or_remove_domain()

[Patrik Flykt]

On Thu, 2015-10-29 at 01:29 +0200, pasi.sjoh...@jolla.com wrote:
> From: Pasi Sjöholm 
> 
> A new append_domain()- and remove_domain()-functions which both
> call append_or_remove_domain()-function by setting boolean append-
> variable accordingly.
> 
> This enables a capability to remove domains from the dns-server's
> domain list.
> ---
>  src/dnsproxy.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/dnsproxy.c b/src/dnsproxy.c
> index c37eee9..1e8fcfb 100644
> --- a/src/dnsproxy.c
> +++ b/src/dnsproxy.c
> @@ -2649,7 +2649,7 @@ static bool resolv(struct request_data *req,
>   return false;
>  }
>  
> -static void append_domain(int index, const char *domain)
> +static void append_or_remove_domain(int index, const char *domain, bool 
> append)
>  {
>   GSList *list;
>  
> @@ -2680,13 +2680,26 @@ static void append_domain(int index, const char 
> *domain)
>   }
>   }
>  
> - if (!dom_found) {
> + if (!dom_found && append) {
>   data->domains =
>   g_list_append(data->domains, g_strdup(domain));
> + } else if (dom_found && !append) {
> + data->domains =
> + g_list_remove(data->domains, dom);
>   }
>   }
>  }
>  
> +static void append_domain(int index, const char *domain)
> +{
> + append_or_remove_domain(index, domain, true);
> +}
> +
> +static void remove_domain(int index, const char *domain)
> +{
> + append_or_remove_domain(index, domain, false);
> +}
> +

remove_domain() belongs to the next patch, as gcc says:

  CC   src/src_connmand-dnsproxy.o
src/dnsproxy.c:2698:13: error: ‘remove_domain’ defined but not used 
[-Werror=unused-function]
 static void remove_domain(int index, const char *domain)
 ^

At the same time, could the append_or_remove_domain() function be simply
called update_domain() ?

>  static void flush_requests(struct server_data *server)
>  {
>   GSList *list;


Cheers,

Patrik


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

Re: Ordering of NameServers is not always respected

[Patrik Flykt]

On Sat, 2015-10-31 at 17:21 +, Sam Nazarko wrote:
> I think this is non standard behaviour -- DNS servers should be
> written in the same order received from DHCP. If this is not the case,
> then DNS ordering should at least be identical regardless of whether
> DNS is being proxied or not.

Looks like a bug. ConnMan should have correct ordering internally and
then just write the entries in this order to resolv.conf.

Cheers,

Patrik

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

Re: ConnMan doesn't clear rfkill softblock when Wi-Fi plugged in after start-up

[Patrik Flykt]

On Fri, 2015-10-30 at 12:28 +1100, Craig McQueen wrote:
> Consider this pre-condition:
> 
> * ConnMan is configured for Wi-Fi enabled and tethered.
> * USB Wi-Fi device is not plugged in.
> * Linux rfkill softblock is enabled for wlan.

ConnMan follows the external softblock state. So if something else
enabled rfkill on WiFi, ConnMan should have disabled the technology at
this point. Please verify the state of WiFi technology at this point.

> Now, consider this scenario:
> 
> * ConnMan was not running.
> * USB Wi-Fi device is plugged in.
> * ConnMan is started.
> 
> In that scenario, ConnMan detects the Wi-Fi technology, enables it,
> turns off the rfkill softblock for wlan, and starts tethering. Good!

When ConnMan starts it will know that its last technology state for WiFi
was 'enabled' and therefore disables rfkill on startup.

> Now consider this alternative scenario:
> 
> * ConnMan is running.
> * USB Wi-Fi device is plugged in.
> 
> In this scenario, ConnMan detects the Wi-Fi technology, but doesn't
> enable it because of the rfkill softblock for wlan.

If this starts from the precondition above, we have all of the following
events taking place:
- ConnMan was started
- rfkill enabled for wifi by some external entity
- ConnMan updates technology to 'disabled' in response
- USB WiFi device plugged in
- Nothing happens as the technology is disabled

If this is the scenario, please check that WiFi got disabled after
rfkill was enabled for WiFi.

> In this second scenario, I would wish for ConnMan to turn off the
> rfkill softblock for wlan (since it is configured for Wi-Fi enabled
> and tethered), and start tethering. What code change would be needed
> to achieve that?

ConnMan follows external settings of rfkill events, see above.

Cheers,

Patrik


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

Re: Wi-Fi service won't auto-connect when Ethernet is already present

[Patrik Flykt]

On Fri, 2015-10-30 at 14:18 +1100, Craig McQueen wrote:
> I'm using ConnMan v1.30 on a BeagleBone Black based system. I've got a
> USB Wi-Fi device, as well as Ethernet.
> 
> Is it possible to configure ConnMan so that it will auto-connect Wi-Fi
> even if it's got an Ethernet connection? Ethernet would still be
> "preferred" in the sense of having the default route. But I want Wi-Fi
> to always connect if an auto-connect service is available, even while
> Ethernet is present.

No. ConnMan will autoconnect a newly detected WiFi only if wifi is a
more preferred technology than the already connected ethernet.

> If Wi-Fi connects while Ethernet is unplugged, then later Ethernet is
> plugged in, then they can both be connected at the same time, which is
> good.

Yes. Ethernet is more preferred than WiFi, so it will be selected as the
interface with the default route. As it is more preferred than WiFi, it
will be automatically connected.

But as it is ethernet, it's also a bit special. It will always be
connected on a cable plug-in as documented in doc/service-api.txt,
Connect() method call.

> But if Ethernet connects first, and the USB Wi-Fi device is plugged in
> later, then it won't auto-connect to the configured Wi-Fi service, and
> I wish it would.

WiFi won't autoconnect as ethernet is more preferred than wifi by
default.

> I've read the "Preferred Technologies" section of the ConnMan web site
> documentation page. It sounds as though there are twists indeed. Can
> the "Preferred Technologies" feature be disabled? I tried putting in
> my /etc/connman/main.conf:
> 
> PreferredTechnologies =
> 
> But that didn't seem to make a difference.

That restores the default behavior, which is the order in which the
services are sorted - and presented in ServicesChanged() signals -
according to connected state, favorite and technology type. With
everything being equal, the default sorting based on technologies is in
src/service.c, service_compare(), and is ethernet, wifi, cellular,
bluetooth, vpn and gadget.

Cheers,

Patrik


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

[RFC 4/5] src: Add tmpfiles.d support for resolv.conf

[Patrik Flykt]

Add tmpfiles.d support for unconditionally creating a symlink from
[/var]/run/connman/resolv.conf to /etc/resolv.conf.

To keep the same behavior as before, the configuration file is
installed to tmpfiles.d. If /etc/resolv.conf behavior needs to be
specified differently by a distribution, do not install the
tmpfiles.d configuration file.
---
 Makefile.am| 17 +
 configure.ac   |  8 
 scripts/connman_resolvconf.conf.in |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 scripts/connman_resolvconf.conf.in

diff --git a/Makefile.am b/Makefile.am
index 96abab7..95082c1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -63,9 +63,11 @@ endif
 
 if SYSTEMD
 systemdunitdir = @SYSTEMD_UNITDIR@
-
 systemdunit_DATA = src/connman.service
 
+tmpfilesdir = @SYSTEMD_TMPFILESDIR@
+nodist_tmpfiles_DATA = scripts/connman_resolvconf.conf
+
 if VPN
 systemdunit_DATA += vpn/connman-vpn.service
 endif
@@ -153,7 +155,8 @@ vpn_connman_vpnd_LDFLAGS = -Wl,--export-dynamic \
-Wl,--version-script=$(srcdir)/vpn/vpn.ver
 endif
 
-BUILT_SOURCES = $(local_headers) src/builtin.h $(service_files) scripts/connman
+BUILT_SOURCES = $(local_headers) src/builtin.h $(service_files) \
+   scripts/connman scripts/connman_resolvconf.conf
 
 if VPN
 BUILT_SOURCES += vpn/builtin.h
@@ -386,7 +389,8 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
doc/connman.8.in doc/connman-vpn.8.in
 
 EXTRA_DIST += src/main.conf \
-   src/eduroam.config
+   src/eduroam.config \
+   scripts/connman_resolvconf.conf.in
 
 MANUAL_PAGES += doc/connmanctl.1 doc/connman.conf.5 \
doc/connman-service.config.5 doc/connman-vpn.conf.5 \
@@ -457,7 +461,8 @@ do_subst = $(AM_V_GEN)$(SED) \
-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
-e 's,[@]storagedir[@],$(storagedir),g' \
-e 's,[@]vpn_storagedir[@],$(vpn_storagedir),g' \
-   -e 's,[@]localstatedir[@],$(localstatedir),g'
+   -e 's,[@]localstatedir[@],$(localstatedir),g' \
+   -e 's,[@]runstatedir[@],$(runstatedir),g'
 
 %.1 : %.1.in
$(AM_V_at)$(MKDIR_P) $(dir $@)
@@ -479,6 +484,10 @@ scripts/connman: scripts/connman.in Makefile
$(AM_V_at)$(MKDIR_P) $(dir $@)
$(do_subst) < $< > $@
 
+scripts/connman_resolvconf.conf: scripts/connman_resolvconf.conf.in
+   $(AM_V_at)$(MKDIR_P) $(dir $@)
+   $(do_subst) < $< > $@
+
 include/connman/version.h: include/version.h
$(AM_V_at)$(MKDIR_P) include/connman
$(AM_V_GEN)$(LN_S) $(abs_top_builddir)/$< $@
diff --git a/configure.ac b/configure.ac
index 51482cb..b51d6b3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -254,6 +254,14 @@ if (test -n "${path_systemdunit}"); then
 fi
 AM_CONDITIONAL(SYSTEMD, test -n "${path_systemdunit}")
 
+AC_ARG_WITH([tmpfilesdir], AC_HELP_STRING([--with-tmpfilesdir=DIR],
+   [path to systemd tmpfiles.d directory]), [path_tmpfiles=${withval}],
+   [path_tmpfiles="`$PKG_CONFIG --variable=tmpfilesdir systemd`"])
+if (test -n "${path_tmpfiles}"); then
+   SYSTEMD_TMPFILESDIR="${path_tmpfiles}"
+   AC_SUBST(SYSTEMD_TMPFILESDIR)
+fi
+
 PKG_CHECK_MODULES(XTABLES, xtables >= 1.4.11, dummy=yes,
AC_MSG_ERROR(Xtables library is required))
 AC_SUBST(XTABLES_CFLAGS)
diff --git a/scripts/connman_resolvconf.conf.in 
b/scripts/connman_resolvconf.conf.in
new file mode 100644
index 000..e47dc07
--- /dev/null
+++ b/scripts/connman_resolvconf.conf.in
@@ -0,0 +1 @@
+L+ /etc/resolv.conf- - - - @runstatedir@/connman/resolv.conf
-- 
2.1.4

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

[RFC 2/5] build: Use runstatedir instead of deriving it from localstatedir

[Patrik Flykt]

With $(runstatedir) being defined, use it instead of $(localstatedir)/run.
---
 Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 4319c4c..96abab7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -161,8 +161,8 @@ endif
 
 CLEANFILES = src/connman.conf $(BUILT_SOURCES) $(service_files)
 
-statedir = $(localstatedir)/run/connman
-vpn_statedir = $(localstatedir)/run/connman-vpn
+statedir = $(runstatedir)/connman
+vpn_statedir = $(runstatedir)/connman-vpn
 
 if VPN
 vpn_plugindir = $(libdir)/connman/plugins-vpn
-- 
2.1.4

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

[RFC 0/5] Move resolv.conf to /run/connman

[Patrik Flykt]

Hi,

One of the features asked every now and then is how to keep ConnMan from
always writing resolv.conf. This RFC patch set makes ConnMan write its
resolv.conf file into [/var]/run/connman and unconditionally set up links
by default to this file from /etc/resolv.conf.

Patch 3/5 changes ConnMan to create the [/var]/run/connman directory and
always write resolv.conf to this location. In order to keep compatibility
with previous versions, patch 4/5 implements tmpfiles.d support to
unconditionally create the symlink to /etc when using systemd. Likewise,
the init script is updated in patch 5/5 with similar functionality, should
someone use that startup method.

In order to be a bit future-proof, the first patch introduces the
$(runstatedir) variable in a compatible manner, as the variable will
appear starting with automake 2.70. The two invocations of
$(localstatedir)/run are updated to use $(runstatedir) in patch 2/5.


Please review and comment,

   Patrik



Patrik Flykt (5):
  build: Define runstatedir
  build: Use runstatedir instead of deriving it from localstatedir
  resolver: Create STATEDIR and use it when writing resolv.conf
  src: Add tmpfiles.d support for resolv.conf
  scripts: Handle link to resolv.conf in init script

 .gitignore |  2 ++
 Makefile.am| 21 +++--
 configure.ac   | 12 
 m4/configmake.m4   | 15 +++
 scripts/connman.in |  4 
 scripts/connman_resolvconf.conf.in |  1 +
 src/main.c |  6 ++
 src/resolver.c |  2 +-
 8 files changed, 56 insertions(+), 7 deletions(-)
 create mode 100644 m4/configmake.m4
 create mode 100644 scripts/connman_resolvconf.conf.in

-- 
2.1.4

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

[RFC 3/5] resolver: Create STATEDIR and use it when writing resolv.conf

[Patrik Flykt]

Create STATEDIR [/var]/run/connman and unconditionally write resolv.conf
to this directory.
---
 src/main.c | 6 ++
 src/resolver.c | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/main.c b/src/main.c
index e46fa7b..6cf6bc8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -623,6 +623,12 @@ int main(int argc, char *argv[])
perror("Failed to create storage directory");
}
 
+   if (mkdir(STATEDIR, S_IRUSR | S_IWUSR | S_IXUSR |
+   S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH) < 0) {
+   if (errno != EEXIST)
+   perror("Failed to create storage directory");
+   }
+
umask(0077);
 
main_loop = g_main_loop_new(NULL, FALSE);
diff --git a/src/resolver.c b/src/resolver.c
index 6a64938..9db2756 100644
--- a/src/resolver.c
+++ b/src/resolver.c
@@ -130,7 +130,7 @@ static int resolvfile_export(void)
 
old_umask = umask(022);
 
-   fd = open("/etc/resolv.conf", O_RDWR | O_CREAT | O_CLOEXEC,
+   fd = open(STATEDIR"/resolv.conf", O_RDWR | O_CREAT | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd < 0) {
err = -errno;
-- 
2.1.4

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

[RFC 1/5] build: Define runstatedir

[Patrik Flykt]

Provide an m4 macro defining runstatedir as $(localstatedir)/var. This
applies to automake versions < 2.70.
---
 .gitignore   |  2 ++
 configure.ac |  4 
 m4/configmake.m4 | 15 +++
 3 files changed, 21 insertions(+)
 create mode 100644 m4/configmake.m4

diff --git a/.gitignore b/.gitignore
index bbb44c3..74acd7c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -24,6 +24,8 @@ missing
 stamp-h1
 autom4te.cache
 test-driver
+m4/
+!m4/configmake.m4
 
 connman.pc
 include/connman
diff --git a/configure.ac b/configure.ac
index 69c0eeb..51482cb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,8 @@
 AC_PREREQ(2.60)
 AC_INIT(connman, 1.30)
 
+AC_CONFIG_MACRO_DIR([m4])
+
 AM_INIT_AUTOMAKE([foreign subdir-objects color-tests])
 AC_CONFIG_HEADERS([config.h])
 
@@ -31,6 +33,8 @@ m4_ifdef([AC_LIBTOOL_TAGS], [AC_LIBTOOL_TAGS([])])
 AC_DISABLE_STATIC
 AC_PROG_LIBTOOL
 
+gl_CONFIGMAKE_PREP
+
 AC_ARG_ENABLE(optimization, AC_HELP_STRING([--disable-optimization],
[disable code optimization through compiler]), [
if (test "${enableval}" = "no"); then
diff --git a/m4/configmake.m4 b/m4/configmake.m4
new file mode 100644
index 000..ef78ebe
--- /dev/null
+++ b/m4/configmake.m4
@@ -0,0 +1,15 @@
+# configmake.m4 serial 2
+dnl Copyright (C) 2010-2015 Free Software Foundation, Inc.
+dnl This file is free software; the Free Software Foundation
+dnl gives unlimited permission to copy and/or distribute it,
+dnl with or without modifications, as long as this notice is preserved.
+
+# gl_CONFIGMAKE_PREP
+# --
+AC_DEFUN([gl_CONFIGMAKE_PREP],
+[
+  dnl Added in autoconf 2.70
+  if test "x$runstatedir" = x; then
+AC_SUBST([runstatedir], ['${localstatedir}/run'])
+  fi
+])
-- 
2.1.4

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

[RFC 5/5] scripts: Handle link to resolv.conf in init script

[Patrik Flykt]

Add init script functionality to create a symlink by default from
[/var]/run/connman/resolv.conf to /etc/resolv.conf.
---
 scripts/connman.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/scripts/connman.in b/scripts/connman.in
index 1692b95..9cc5895 100644
--- a/scripts/connman.in
+++ b/scripts/connman.in
@@ -9,6 +9,10 @@ if [ -f @sysconfdir@/default/connman ] ; then
. @sysconfdir@/default/connman
 fi
 
+if [ "HANDLE_RESOLVCONF" != "no" ] ; then
+ln -sf @runstatedir@/connman/resolv.conf /etc/
+fi
+
 set -e
 
 do_start() {
-- 
2.1.4

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

Re: connmanctl quitting before multiple return functions called

[Patrik Flykt]

Hi,

On Thu, 2015-10-15 at 15:41 +1100, Craig McQueen wrote:
> I agree, it would be best if connmanctl exits after all the method
> calls have completed. I'd be happy to do a patch in principle, however
> I'm new to this D-Bus stuff and unfamiliar with the connman codebase,
> so at this point I'm not sure what is an architecturally elegant way
> to fix this. E.g., would it a counter which increments for each method
> call, and decrements when its response is received

Yes, that should work.

>  (and if so, does it need locks to be thread safe)?

This is entirely single threaded using a main loop. No locking
structures needed.

>  Or, is there some existing D-Bus code that already provides such
> functionality, e.g. returning true when there are no method calls
> waiting for a response?

Currently not. Looks like the 'config' command has the same problem
here. But that is a different "bug"...

Cheers,

Patrik


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

Re: [PATCH] connmanctl: Implement clock API with new clock commands

[Patrik Flykt]

Hi,

On Thu, 2015-10-15 at 15:07 +1100, Craig McQueen wrote:
> ---
> 
> I'm not sure if you would like this use of commands. I'm interested not 
> only in _setting_ clock settings, but also in reading them conveniently. 
> The precedent in connmanctl is that there is one command to configure a 
> service ('config'), and a separate command to see its properties 
> ('services'). So for net.connman.Clock, the analagous commands are 
> 'configclock' to configure, and 'clock' to read. But 'clock' also takes 
> parameters which allow individual properties to be read.

Here I'd go for the simpler option of just having a 'clock' command that
shows all Clock properties. This way it is similar to 'technologies' and
'state' commands.

> My earlier e-mail--
> "connmanctl quitting before multiple return functions called" --
> can be seen with the 'clock' command, reading multiple items. E.g.:
> 
> connmanctl clock time timeupdates timezone
> 
> which on my ARM embedded Linux based system, sometimes gets all three 
> responses, but often only two.
> 
>  client/commands.c | 273 
> ++
>  client/dbus_helpers.c |   6 ++
>  2 files changed, 279 insertions(+)
> 
> diff --git a/client/commands.c b/client/commands.c
> index 3bfdfd7..8b56965 100644
> --- a/client/commands.c
> +++ b/client/commands.c
> @@ -291,6 +291,75 @@ static int peers_list(DBusMessageIter *iter,
>   return 0;
>  }
>  
> +static int one_object_properties(DBusMessageIter *iter,
> + const char *error, void *user_data)
> +{
> + DBusMessageIter dict;
> +
> + if (!error) {
> + dbus_message_iter_recurse(iter, );
> + __connmanctl_dbus_print(, "", " = ", "\n");
> +
> + fprintf(stdout, "\n");
> +
> + } else {
> + fprintf(stderr, "Error: %s\n", error);
> + }
> +
> + return 0;
> +}
> +
> +static int one_object_one_property(DBusMessageIter *iter,
> + const char *error, void *user_data)
> +{
> + char *property = user_data;
> + int arg_type;
> + DBusMessageIter dict;
> + DBusMessageIter entry;
> + DBusMessageIter iter2;
> + char *str;
> +
> + if (error) {
> + fprintf(stderr, "Error: %s\n", error);
> + return 0;
> + }
> +
> + dbus_message_iter_recurse(iter, );
> + while (dbus_message_iter_get_arg_type() == DBUS_TYPE_DICT_ENTRY) {
> + dbus_message_iter_recurse(, );
> + arg_type = dbus_message_iter_get_arg_type();
> + if (arg_type == DBUS_TYPE_STRING) {
> + dbus_message_iter_get_basic(, );
> + } else {
> + str = "";
> + }
> + if (g_strcmp0(str, property) == 0) {
> + dbus_message_iter_next();
> + while (dbus_message_iter_get_arg_type() == 
> DBUS_TYPE_VARIANT) {
> + dbus_message_iter_recurse(, );
> + entry = iter2;
> + }
> + switch (dbus_message_iter_get_arg_type()) {
> + case DBUS_TYPE_STRUCT:
> + case DBUS_TYPE_ARRAY:
> + case DBUS_TYPE_DICT_ENTRY:
> + dbus_message_iter_recurse(, );
> + __connmanctl_dbus_print(, "", "=", " ");
> + break;
> + default:
> + __connmanctl_dbus_print(, "", " = ", " = 
> ");
> + break;
> + }
> + break;
> + }
> + dbus_message_iter_next();
> + }
> +
> + fprintf(stdout, "\n");
> +
> + return 0;
> +}
> +
>  static int object_properties(DBusMessageIter *iter,
>   const char *error, void *user_data)
>  {
> @@ -1188,6 +1257,189 @@ static int cmd_config(char *args[], int num, struct 
> connman_option *options)
>   return result;
>  }
>  
> +static int configclock_return(DBusMessageIter *iter, const char *error,
> + void *user_data)
> +{
> + char *property = user_data;
> +
> + if (error)
> + fprintf(stderr, "Error %s: %s\n", property, error);
> +
> + g_free(user_data);
> +
> + return 0;
> +}
> +
> +static int cmd_clock(char *args[], int num, struct connman_option *options)
> +{
> + int result = 0, res = 0, index = 1, oldindex = 0;
> + int c;
> + char *path;
> + char **opt_start;
> + struct config_append append;
> +
> + if (num == 1) {
> + return __connmanctl_dbus_method_call(connection, 
> CONNMAN_SERVICE, "/",
> + "net.connman.Clock", "GetProperties",
> + one_object_properties, NULL, NULL, NULL);
> + }
> +
> + while (index < num && args[index]) {
> + c = 

Re: [PATCH 0/8] Man page overhaul

[Patrik Flykt]

On Mon, 2015-10-26 at 16:39 +0200, Jaakko Hannikainen wrote:
> This patchset rewrites half of the existing man pages and adds 4
> new ones, since the old ones were either badly formatted, incomplete
> or simply wrong. It also moves them under automake so that end users
> will see proper paths to connman's files instead of suggesting that
> they are probably under /var/lib/connman/.
> 
> Not many changes from the RFC version, mostly typos here and there.
> Mainly changed the build system parts, now only the last patch
> touches the build system. If/when the discussion about OpenVPN's MTUs
> reaches some end, the appropriate documentation should be updated.

Applied with v2 for patch 8/8, thanks!

Patrik

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

Re: [PATCH] Set exit value in connmanctl

[Patrik Flykt]

Hi,

On Tue, 2015-10-27 at 09:58 +0200, Mikko Tuumanen wrote:
> ---
>  client/agent.c|  2 +-
>  client/commands.c | 27 +--
>  client/dbus_helpers.c |  5 +++--
>  client/input.c|  6 --
>  client/input.h|  2 +-
>  5 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/client/agent.c b/client/agent.c
> index d020889..b87d5bb 100644
> --- a/client/agent.c
> +++ b/client/agent.c
> @@ -235,7 +235,7 @@ static DBusMessage *agent_release(DBusConnection 
> *connection,
>   "VPNd\n");
>  
>   if (__connmanctl_is_interactive() == false)
> - __connmanctl_quit();
> + __connmanctl_quit(0);
>  
>   return dbus_message_new_method_return(message);
>  }
> diff --git a/client/commands.c b/client/commands.c
> index 3bfdfd7..ee56d46 100644
> --- a/client/commands.c
> +++ b/client/commands.c
> @@ -153,14 +153,21 @@ static int enable_return(DBusMessageIter *iter, const 
> char *error,
>   else
>   str = tech;
>  
> - if (!error)
> + int ret;

int ret = 0; should be declared together with the other variables. As a
nitpick the convention has been that variables with values were declared
first, then uninitialized variables.

> + if (!error) {
> + ret = 0;
>   fprintf(stdout, "Enabled %s\n", str);
> - else
> + } else {
> + if (!strncmp("Already ",error,8))
> + ret = -2;
> + else
> + ret = -1;

I don't think one can rely on the string here. It's a semi-descriptive
one that may be used for logging or other purposes, but not much else. 

The code is looking for -EINPROGRESS in dbus_method_reply(), so "proper"
values from errno*.h shall be used. What if this just sets the return
value to -EOPNOTSUPP?

>   fprintf(stderr, "Error %s: %s\n", str, error);
> + }
>  
>   g_free(user_data);
>  
> - return 0;
> + return ret;
>  }
>  
>  static int cmd_enable(char *args[], int num, struct connman_option *options)
> @@ -202,14 +209,22 @@ static int disable_return(DBusMessageIter *iter, const 
> char *error,
>   else
>   str = tech;
>  
> - if (!error)
> + int ret;
> + if (!error) {
> + ret = 0;
>   fprintf(stdout, "Disabled %s\n", str);
> - else
> + } else {
> + if (!strncmp("Already ", error, 8))
> + ret = -2;
> + else
> + ret = -1;
> +
>   fprintf(stderr, "Error %s: %s\n", str, error);
> + }

Same here.

>   g_free(user_data);
>  
> - return 0;
> + return ret;
>  }
>  
>  static int cmd_disable(char *args[], int num, struct connman_option *options)
> diff --git a/client/dbus_helpers.c b/client/dbus_helpers.c
> index 9c4cfee..fe55abf 100644
> --- a/client/dbus_helpers.c
> +++ b/client/dbus_helpers.c
> @@ -141,6 +141,7 @@ static void dbus_method_reply(DBusPendingCall *call, void 
> *user_data)
>   int res = 0;
>   DBusMessage *reply;
>   DBusMessageIter iter;
> + int exit_value = 0;
>  
>   __connmanctl_save_rl();
>  
> @@ -152,7 +153,7 @@ static void dbus_method_reply(DBusPendingCall *call, void 
> *user_data)
>   dbus_error_init();
>   dbus_set_error_from_message(, reply);
>  
> - callback->cb(NULL, err.message, callback->user_data);
> + exit_value = callback->cb(NULL, err.message, 
> callback->user_data);
>  
>   dbus_error_free();
>   goto end;
> @@ -164,7 +165,7 @@ static void dbus_method_reply(DBusPendingCall *call, void 
> *user_data)
>  end:
>   __connmanctl_redraw_rl();
>   if (__connmanctl_is_interactive() == false && res != -EINPROGRESS)
> - __connmanctl_quit();
> + __connmanctl_quit(exit_value);
>  
>   g_free(callback);
>   dbus_message_unref(reply);
> diff --git a/client/input.c b/client/input.c
> index e59df8a..f310d15 100644
> --- a/client/input.c
> +++ b/client/input.c
> @@ -42,9 +42,11 @@ static bool interactive = false;
>  static bool save_input;
>  static char *saved_line;
>  static int saved_point;
> +static int exit_status = 0;
>  
> -void __connmanctl_quit(void)
> +void __connmanctl_quit(int status)
>  {
> + exit_status = status;
>   if (main_loop)
>   g_main_loop_quit(main_loop);
>  }
> @@ -264,7 +266,7 @@ int __connmanctl_input_init(int argc, char *argv[])
>   main_loop = g_main_loop_new(NULL, FALSE);
>   g_main_loop_run(main_loop);
>  
> - err = 0;
> + err = exit_status;
>   }
>  
>   if (interactive) {
> diff --git a/client/input.h b/client/input.h
> index c7256a4..2136d13 100644
> --- a/client/input.h
> +++ b/client/input.h
> @@ -29,7 +29,7 @@
>  extern "C" {
>  #endif
>  
> -void __connmanctl_quit(void);
> +void __connmanctl_quit(int status);
>  bool 

Re: [PATCH] Set exit value in connmanctl

[Patrik Flykt]

On Tue, 2015-10-27 at 09:58 +0200, Mikko Tuumanen wrote:
> ---
>  client/agent.c|  2 +-
>  client/commands.c | 27 +--
>  client/dbus_helpers.c |  5 +++--
>  client/input.c|  6 --
>  client/input.h|  2 +-
>  5 files changed, 30 insertions(+), 12 deletions(-)

And a bit more description on the idea behind the change in the commit
message, if I may.

Cheers,

Patrik

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

Re: [PATCH] unit: IP pools exist as long as they are referenced

[Patrik Flykt]

On Fri, 2015-10-23 at 14:59 +0300, Patrik Flykt wrote:
> An IP pool exists after it has been created until it is unreferenced.
> Make no assumptions that the ip pool address range will not be
> re-assigned after it has ben unreferenced.
> ---
> 
> This behavior became obvious after changing the ip pool allocation
> with the previous patches. Nevertheless, the unit test assumes things
> that cannot be guaranteed and needs fixing.

Applied.

Patrik

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

Re: [PATCH 0/2] Keep same subnet when enabling/disabling tethering

[Patrik Flykt]

On Fri, 2015-10-23 at 14:38 +0300, Patrik Flykt wrote:
>   Hi,
> 
> Here is a simple fix for reusing the same IPv4 subnet when tethering.
> As before, the subnet is not guaranteed to always stay the same, but
> with this change its previous value should be reused if nothing else
> causes it to be changed.
> 
> The second patch removes an unused hash from ippool.c

Applied.

Patrik

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

Re: [PATCH] wifi: Reset device->scanning if scan has not returned in 15 secs

[Patrik Flykt]

Hi,

On Fri, 2015-10-23 at 00:47 +0300, pasi.sjoh...@jolla.com wrote:
> From: Pasi Sjöholm 
> 
> Due unknown reason sometimes device->scanning is not set to false after
> wifi scanning (connman 1.30 and wpa_supplicant 2.5).
> This is probably due callback-function not being called after wifi scan
> and therefore it needs to have a timer to prevent deadlock.
> ---

You probably have evidence that the callback is not called, right?
Please state that in the commit message.

What will happen if wpa_supplicant will call the callback after 16
seconds, i.e. after ConnMan has triggered scan_fail_timeout? Will there
be any artefacts if this happens?

Cheers,

Patrik

>  plugins/wifi.c | 50 +-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/plugins/wifi.c b/plugins/wifi.c
> index dfe849f..35d4fe6 100644
> --- a/plugins/wifi.c
> +++ b/plugins/wifi.c
> @@ -61,6 +61,7 @@
>  
>  #define CLEANUP_TIMEOUT   8  /* in seconds */
>  #define INACTIVE_TIMEOUT  12 /* in seconds */
> +#define SCAN_FAIL_TIMEOUT 15 /* in seconds */
>  #define FAVORITE_MAXIMUM_RETRIES 2
>  
>  #define BGSCAN_DEFAULT "simple:30:-45:300"
> @@ -131,6 +132,7 @@ struct wifi_data {
>   struct hidden_params *hidden;
>   bool postpone_hidden;
>   struct wifi_tethering_info *tethering_param;
> + unsigned int scan_fail_timeout;
>   /**
>* autoscan "emulation".
>*/
> @@ -817,9 +819,25 @@ static void reset_autoscan(struct connman_device *device)
>   connman_device_unref(device);
>  }
>  
> +static gboolean scan_fail_timeout(gpointer data)
> +{
> + struct connman_device *device = data;
> + struct wifi_data *wifi = connman_device_get_data(device);
> +
> + DBG("");
> +
> + if (!wifi)
> + return FALSE;
> +
> + connman_device_set_scanning(device, CONNMAN_SERVICE_TYPE_WIFI, false);
> + wifi->scan_fail_timeout = 0;
> +
> + return FALSE;
> +}
> +
>  static void stop_autoscan(struct connman_device *device)
>  {
> - const struct wifi_data *wifi = connman_device_get_data(device);
> + struct wifi_data *wifi = connman_device_get_data(device);
>  
>   if (!wifi || !wifi->autoscan)
>   return;
> @@ -827,6 +845,11 @@ static void stop_autoscan(struct connman_device *device)
>   reset_autoscan(device);
>  
>   connman_device_set_scanning(device, CONNMAN_SERVICE_TYPE_WIFI, false);
> +
> + if (wifi->scan_fail_timeout) {
> + g_source_remove(wifi->scan_fail_timeout);
> + wifi->scan_fail_timeout = 0;
> + }
>  }
>  
>  static void check_p2p_technology(void)
> @@ -876,6 +899,10 @@ static void wifi_remove(struct connman_device *device)
>   if (wifi->p2p_connection_timeout)
>   g_source_remove(wifi->p2p_connection_timeout);
>  
> + if (wifi->scan_fail_timeout) {
> + g_source_remove(wifi->scan_fail_timeout);
> + }
> +
>   remove_networks(device, wifi);
>  
>   connman_device_set_powered(device, false);
> @@ -1193,6 +1220,11 @@ static int throw_wifi_scan(struct connman_device 
> *device,
>   if (ret == 0) {
>   connman_device_set_scanning(device,
>   CONNMAN_SERVICE_TYPE_WIFI, true);
> +
> + wifi->scan_fail_timeout = g_timeout_add_seconds(
> + SCAN_FAIL_TIMEOUT,
> + scan_fail_timeout,
> + device);
>   } else
>   connman_device_unref(device);
>  
> @@ -1262,6 +1294,11 @@ static void scan_callback(int result, 
> GSupplicantInterface *interface,
>   if (scanning) {
>   connman_device_set_scanning(device,
>   CONNMAN_SERVICE_TYPE_WIFI, false);
> +
> + if (wifi && wifi->scan_fail_timeout) {
> + g_source_remove(wifi->scan_fail_timeout);
> + wifi->scan_fail_timeout = 0;
> + }
>   }
>  
>   if (result != -ENOLINK)
> @@ -1516,6 +1553,11 @@ static int wifi_disable(struct connman_device *device)
>   connman_device_unref(wifi->device);
>   }
>  
> + if (wifi->scan_fail_timeout) {
> + g_source_remove(wifi->scan_fail_timeout);
> + wifi->scan_fail_timeout = 0;
> + }
> +
>   /* In case of a user scan, device is still referenced */
>   if (connman_device_get_scanning(device)) {
>   connman_device_set_scanning(device,
> @@ -1864,6 +1906,12 @@ static int wifi_scan(enum connman_service_type type,
>   if (ret == 0) {
>   connman_device_set_scanning(device,
>   CONNMAN_SERVICE_TYPE_WIFI, true);
> +
> + wifi->scan_fail_timeout = g_timeout_add_seconds(
> + SCAN_FAIL_TIMEOUT,
> + 

Re: [PATCH 8/8] doc: Move man pages under automake

[Patrik Flykt]

Hi,

On Mon, 2015-10-26 at 16:40 +0200, Jaakko Hannikainen wrote:
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -248,8 +248,6 @@ include Makefile.plugins
>  if CLIENT
>  bin_PROGRAMS += client/connmanctl
>  
> -MANUAL_PAGES += doc/connmanctl.1
> -
>  client_connmanctl_SOURCES = client/dbus_helpers.h client/dbus_helpers.c \
> client/services.h client/services.c \
> client/commands.h client/commands.c \
> @@ -385,7 +383,10 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
>  EXTRA_DIST += src/main.conf \
> src/eduroam.config
>  
> -MANUAL_PAGES += doc/connman.8 doc/connman.conf.5
> +MANUAL_PAGES += doc/connmanctl.1 doc/connman.conf.5 \
> +   doc/connman-service.config.5 doc/connman-vpn.conf.5 \
> +   doc/connman-vpn-provider.config.5 \
> +   doc/connman.8 doc/connman-vpn.8
>  
>  dist_man_MANS = $(MANUAL_PAGES)

Here we need to allow the user to set a different prefix
with ./configure. This causes the following changes to this patch:

--- a/Makefile.am
+++ b/Makefile.am
@@ -378,7 +378,12 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
doc/vpn-config-format.txt \
doc/vpn-connection-api.txt \
doc/vpn-manager-api.txt doc/vpn-overview.txt \
-   doc/session-policy-format.txt
+   doc/session-policy-format.txt \
+   doc/connmanctl.1.in doc/connman.conf.5.in \
+   doc/connman-service.config.5.in \
+   doc/connman-vpn.conf.5.in \
+   doc/connman-vpn-provider.config.5.in \
+   doc/connman.8.in doc/connman-vpn.8.in
 
 EXTRA_DIST += src/main.conf \
src/eduroam.config
@@ -388,7 +393,7 @@ MANUAL_PAGES += doc/connmanctl.1 doc/connman.conf.5 \
doc/connman-vpn-provider.config.5 \
doc/connman.8 doc/connman-vpn.8
 
-dist_man_MANS = $(MANUAL_PAGES)
+nodist_man_MANS = $(MANUAL_PAGES)
 
 pkgconfigdir = $(libdir)/pkgconfig
 
Notice the 'nodist_man_MANS', which causes the generated man pages not
to end up in the dist tarball. As 'EXTRA_DIST' now contains the man .in
files, running ./configure will create the man pages using the provided
prefix. I'll squash up those changes into this patch and apply it all.

Cheers,

Patrik


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

Re: [PATCH v2 2/2] Documentation change

[Patrik Flykt]

On Mon, 2015-10-26 at 20:41 -0700, Naveen Singh wrote:
> From: nasingh 
> 
> Change the overview-api.txt to do the service transition from
> failure state to idle.

Applied, thanks!

Patrik

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

Re: [PATCH v2 1/2] Clear the Error property

[Patrik Flykt]

On Mon, 2015-10-26 at 20:41 -0700, Naveen Singh wrote:
> From: nasingh 
> 
> It is been seen that if the service state has transitioned to failure
> there is no way for it to get it back to idle. This fix allows the
> state to be transitioned back to idle as part of handling clear_property
> handler for error event.
> Refer Patrik's commit 251d95755dd144c8bd6d3e3bd5d6a47f891f938f which
> fixes the documentation for transitioning out of failure state.

Applied, thanks!

The nasingh-ym... email address did not resolve to a existing host, so I
had to remove the From: from the commit message.

Patrik

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

Re: [PATCH 8/8] doc: Move man pages under automake

[Patrik Flykt]

On Tue, 2015-10-27 at 15:21 +0200, Patrik Flykt wrote:
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -378,7 +378,12 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
> doc/vpn-config-format.txt \
> doc/vpn-connection-api.txt \
> doc/vpn-manager-api.txt doc/vpn-overview.txt \
> -   doc/session-policy-format.txt
> +   doc/session-policy-format.txt \
> +   doc/connmanctl.1.in doc/connman.conf.5.in \
> +   doc/connman-service.config.5.in \
> +   doc/connman-vpn.conf.5.in \
> +   doc/connman-vpn-provider.config.5.in \
> +   doc/connman.8.in doc/connman-vpn.8.in
>  
>  EXTRA_DIST += src/main.conf \
> src/eduroam.config
> @@ -388,7 +393,7 @@ MANUAL_PAGES += doc/connmanctl.1 doc/connman.conf.5 \
> doc/connman-vpn-provider.config.5 \
> doc/connman.8 doc/connman-vpn.8
>  
> -dist_man_MANS = $(MANUAL_PAGES)
> +nodist_man_MANS = $(MANUAL_PAGES)
>  
>  pkgconfigdir = $(libdir)/pkgconfig
>  
> Notice the 'nodist_man_MANS', which causes the generated man pages not
> to end up in the dist tarball. As 'EXTRA_DIST' now contains the man .in
> files, running ./configure will create the man pages using the provided
> prefix. I'll squash up those changes into this patch and apply it all.

And then we also need to do a $(AM_V_at)$(MKDIR_P) $(dir $@) in all the
%.X : %.X.in rules to create the proper directory...

Phew.

Patrik

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

[PATCH 8/8 v2] doc: Move man pages under automake

[Patrik Flykt]

From: Jaakko Hannikainen 

This change prettifies man pages with  and /var/lib so
that it will show the actual value connman was compiled with, rather
than just hinting at some magical compile time variable.

Add only the man page autoconf .in files to the dist tar ball so
that running ./configure with a different prefix generates the man
pages using the given prefix. As the autoconf .in is not created by
standard rules, ensure also that the target directory is created.
---
 .gitignore   |   8 +-
 Makefile.am  |  32 ++-
 doc/connman-service.config.5 | 197 
 doc/connman-service.config.5.in  | 197 
 doc/connman-vpn-provider.config.5| 424 ---
 doc/connman-vpn-provider.config.5.in | 424 +++
 doc/connman-vpn.8|  63 --
 doc/connman-vpn.8.in |  62 +
 doc/connman-vpn.conf.5   |  48 
 doc/connman-vpn.conf.5.in|  42 
 doc/connman.8|  98 
 doc/connman.8.in |  97 
 doc/connman.conf.5   | 144 
 doc/connman.conf.5.in| 138 
 doc/connmanctl.1 | 282 ---
 doc/connmanctl.1.in  | 282 +++
 16 files changed, 1274 insertions(+), 1264 deletions(-)
 delete mode 100644 doc/connman-service.config.5
 create mode 100644 doc/connman-service.config.5.in
 delete mode 100644 doc/connman-vpn-provider.config.5
 create mode 100644 doc/connman-vpn-provider.config.5.in
 delete mode 100644 doc/connman-vpn.8
 create mode 100644 doc/connman-vpn.8.in
 delete mode 100644 doc/connman-vpn.conf.5
 create mode 100644 doc/connman-vpn.conf.5.in
 delete mode 100644 doc/connman.8
 create mode 100644 doc/connman.8.in
 delete mode 100644 doc/connman.conf.5
 create mode 100644 doc/connman.conf.5.in
 delete mode 100644 doc/connmanctl.1
 create mode 100644 doc/connmanctl.1.in

diff --git a/.gitignore b/.gitignore
index 9c22e4a..bbb44c3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -64,8 +64,12 @@ unit/test-nat
 doc/*.bak
 doc/*.stamp
 doc/connman.*
-!doc/connman.8
-!doc/connman.conf.5
+doc/*.1
+doc/*.5
+doc/*.8
+!doc/*.1.in
+!doc/*.5.in
+!doc/*.8.in
 doc/connman-*.txt
 
 vpn/builtin.h
diff --git a/Makefile.am b/Makefile.am
index 3d0645e..4319c4c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -248,8 +248,6 @@ include Makefile.plugins
 if CLIENT
 bin_PROGRAMS += client/connmanctl
 
-MANUAL_PAGES += doc/connmanctl.1
-
 client_connmanctl_SOURCES = client/dbus_helpers.h client/dbus_helpers.c \
client/services.h client/services.c \
client/commands.h client/commands.c \
@@ -380,14 +378,22 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
doc/vpn-config-format.txt \
doc/vpn-connection-api.txt \
doc/vpn-manager-api.txt doc/vpn-overview.txt \
-   doc/session-policy-format.txt
+   doc/session-policy-format.txt \
+   doc/connmanctl.1.in doc/connman.conf.5.in \
+   doc/connman-service.config.5.in \
+   doc/connman-vpn.conf.5.in \
+   doc/connman-vpn-provider.config.5.in \
+   doc/connman.8.in doc/connman-vpn.8.in
 
 EXTRA_DIST += src/main.conf \
src/eduroam.config
 
-MANUAL_PAGES += doc/connman.8 doc/connman.conf.5
+MANUAL_PAGES += doc/connmanctl.1 doc/connman.conf.5 \
+   doc/connman-service.config.5 doc/connman-vpn.conf.5 \
+   doc/connman-vpn-provider.config.5 \
+   doc/connman.8 doc/connman-vpn.8
 
-dist_man_MANS = $(MANUAL_PAGES)
+nodist_man_MANS = $(MANUAL_PAGES)
 
 pkgconfigdir = $(libdir)/pkgconfig
 
@@ -449,8 +455,22 @@ do_subst = $(AM_V_GEN)$(SED) \
-e 's,[@]prefix[@],$(prefix),g' \
-e 's,[@]sbindir[@],$(sbindir),g' \
-e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+   -e 's,[@]storagedir[@],$(storagedir),g' \
+   -e 's,[@]vpn_storagedir[@],$(vpn_storagedir),g' \
-e 's,[@]localstatedir[@],$(localstatedir),g'
 
+%.1 : %.1.in
+   $(AM_V_at)$(MKDIR_P) $(dir $@)
+   $(do_subst) < $< > $@
+
+%.5 : %.5.in
+   $(AM_V_at)$(MKDIR_P) $(dir $@)
+   $(do_subst) < $< > $@
+
+%.8 : %.8.in
+   $(AM_V_at)$(MKDIR_P) $(dir $@)
+   $(do_subst) < $< > $@
+
 %.service: %.service.in Makefile
$(AM_V_at)$(MKDIR_P) $(dir $@)
$(do_subst) < $< > $@
@@ -468,4 +488,4 @@ include/connman/%.h: $(abs_top_srcdir)/include/%.h
$(AM_V_GEN)$(LN_S) $< $@
 
 clean-local:
-   @$(RM) -rf include/connman
+   @$(RM) -rf 

Re: [PATCH] Clear the Error property

[Patrik Flykt]

Hi,

On Thu, 2015-10-22 at 22:30 -0700, Naveen Singh wrote:
> From: nasingh 
> 
> It is been seen that if the service state has transitioned to failure
> there is no way for it to get it back to idle. This fix allows the
> state to be transitioned back to idle as part of handling clear_property
> handler for error event.
> Refer Patrik's commit 251d95755dd144c8bd6d3e3bd5d6a47f891f938f which
> fixes the documentation for transitioning out of failure state.
> ---
>  src/service.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/service.c b/src/service.c
> index 02a6844..dfeac1b 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3532,6 +3532,7 @@ static DBusMessage *clear_property(DBusConnection *conn,
>  
>   g_get_current_time(>modified);
>   service_save(service);
> + __connman_service_clear_error(service);

__connman_service_clear_error() is the right choice here. But one needs
to be careful when using that function, it resets the service state to
idle, but does not inform any pending clients what happened. In the two
cases it is used, ConnMan is starting to connect the service in
__connman_service_connect() and not asked to retry when the Agent
replies in report_error_cb().

So after calling __connman_service_clear_error() one needs to call
service_complete() as well to inform any pending clients that the
operation was interrupted. This because the service will be in the
middle of a connect for example. service_complete() does already
g_get_current_time() and service_save(), so those two lines shall be
removed as well from this code in clear_property().

>   } else
>   return __connman_error_invalid_property(msg);
>  

Thanks!

Patrik


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

[PATCH 0/2] Keep same subnet when enabling/disabling tethering

[Patrik Flykt]

Hi,

Here is a simple fix for reusing the same IPv4 subnet when tethering.
As before, the subnet is not guaranteed to always stay the same, but
with this change its previous value should be reused if nothing else
causes it to be changed.

The second patch removes an unused hash from ippool.c

Cheers,

Patrik


Patrik Flykt (2):
  ippool: Try to assign the same subnet while tethering
  ippool: Remove unused hash table

 src/ippool.c | 45 +++--
 1 file changed, 15 insertions(+), 30 deletions(-)

-- 
2.1.4

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

[PATCH 1/2] ippool: Try to assign the same subnet while tethering

[Patrik Flykt]

When enabling and disabling tethering, the next available class C
IPv4 subnet will be assigned and not the same one as last time.

Fix this by starting with the last allocated block instead of
unconditionally with the next free block. This way the subnet used
for tethering will be the same as the previous time unless the subnet
block is unavailable due to other reasons. Note that even with this
change the subnet user for tethering is not guaranteed to always
stay the same.

Fixes CM-666.
---
 src/ippool.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/ippool.c b/src/ippool.c
index bb8568d..06f3762 100644
--- a/src/ippool.c
+++ b/src/ippool.c
@@ -181,10 +181,10 @@ static uint32_t get_free_block(unsigned int size)
 * To only thing we have to make sure is that we terminated if
 * there is no block left.
 */
-   if (last_block == 0)
-   block = block_16_bits;
+   if (last_block)
+   block = last_block;
else
-   block = next_block(last_block);
+   block = block_16_bits;
 
do {
collision = false;
-- 
2.1.4

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

[PATCH 2/2] ippool: Remove unused hash table

[Patrik Flykt]

As IP pool structures were only added to and removed from the
ippool_hash hash table, remove it as being unnecessary. Move
freeing of pool data to the unref function.
---
 src/ippool.c | 39 ---
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/src/ippool.c b/src/ippool.c
index 06f3762..cea1dcc 100644
--- a/src/ippool.c
+++ b/src/ippool.c
@@ -58,7 +58,6 @@ struct connman_ippool {
 };
 
 GSList *allocated_blocks;
-GHashTable *pool_hash;
 
 static uint32_t last_block;
 static uint32_t block_16_bits;
@@ -90,7 +89,18 @@ void __connman_ippool_unref_debug(struct connman_ippool 
*pool,
if (__sync_fetch_and_sub(>refcount, 1) != 1)
return;
 
-   g_hash_table_remove(pool_hash, pool);
+   if (pool->info) {
+   allocated_blocks = g_slist_remove(allocated_blocks, pool->info);
+   g_free(pool->info);
+   }
+
+   g_free(pool->gateway);
+   g_free(pool->broadcast);
+   g_free(pool->start_ip);
+   g_free(pool->end_ip);
+   g_free(pool->subnet_mask);
+
+   g_free(pool);
 }
 
 static char *get_ip(uint32_t ip)
@@ -393,7 +403,6 @@ struct connman_ippool *__connman_ippool_create(int index,
pool->end_ip = get_ip(block + start + range);
 
allocated_blocks = g_slist_prepend(allocated_blocks, info);
-   g_hash_table_insert(pool_hash, pool, pool);
 
return pool;
 }
@@ -423,24 +432,6 @@ const char *__connman_ippool_get_subnet_mask(struct 
connman_ippool *pool)
return pool->subnet_mask;
 }
 
-static void pool_free(gpointer data)
-{
-   struct connman_ippool *pool = data;
-
-   if (pool->info) {
-   allocated_blocks = g_slist_remove(allocated_blocks, pool->info);
-   g_free(pool->info);
-   }
-
-   g_free(pool->gateway);
-   g_free(pool->broadcast);
-   g_free(pool->start_ip);
-   g_free(pool->end_ip);
-   g_free(pool->subnet_mask);
-
-   g_free(pool);
-}
-
 int __connman_ippool_init(void)
 {
DBG("");
@@ -450,9 +441,6 @@ int __connman_ippool_init(void)
block_24_bits = ntohl(inet_addr("10.0.0.0"));
subnet_mask_24 = ntohl(inet_addr("255.255.255.0"));
 
-   pool_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
-   pool_free);
-
return 0;
 }
 
@@ -460,9 +448,6 @@ void __connman_ippool_cleanup(void)
 {
DBG("");
 
-   g_hash_table_destroy(pool_hash);
-   pool_hash = NULL;
-
g_slist_free_full(allocated_blocks, g_free);
last_block = 0;
allocated_blocks = NULL;
-- 
2.1.4

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

[PATCH] unit: IP pools exist as long as they are referenced

[Patrik Flykt]

An IP pool exists after it has been created until it is unreferenced.
Make no assumptions that the ip pool address range will not be
re-assigned after it has ben unreferenced.
---

This behavior became obvious after changing the ip pool allocation
with the previous patches. Nevertheless, the unit test assumes things
that cannot be guaranteed and needs fixing.

 unit/test-ippool.c | 70 ++
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/unit/test-ippool.c b/unit/test-ippool.c
index e8d077a..17fac9d 100644
--- a/unit/test-ippool.c
+++ b/unit/test-ippool.c
@@ -253,7 +253,7 @@ static void test_case_4(void)
 
 static void test_case_5(void)
 {
-   struct connman_ippool *pool;
+   struct connman_ippool *pool1, *pool2;
const char *gateway;
const char *broadcast;
const char *subnet_mask;
@@ -271,14 +271,14 @@ static void test_case_5(void)
g_assert(flag == 0);
 
/* pool should return 192.168.0.1 now */
-   pool = __connman_ippool_create(26, 1, 100, collision_cb, );
-   g_assert(pool);
+   pool1 = __connman_ippool_create(26, 1, 100, collision_cb, );
+   g_assert(pool1);
 
-   gateway = __connman_ippool_get_gateway(pool);
-   broadcast = __connman_ippool_get_broadcast(pool);
-   subnet_mask = __connman_ippool_get_subnet_mask(pool);
-   start_ip = __connman_ippool_get_start_ip(pool);
-   end_ip = __connman_ippool_get_end_ip(pool);
+   gateway = __connman_ippool_get_gateway(pool1);
+   broadcast = __connman_ippool_get_broadcast(pool1);
+   subnet_mask = __connman_ippool_get_subnet_mask(pool1);
+   start_ip = __connman_ippool_get_start_ip(pool1);
+   end_ip = __connman_ippool_get_end_ip(pool1);
 
g_assert(gateway);
g_assert(broadcast);
@@ -296,8 +296,6 @@ static void test_case_5(void)
"\tgateway %s broadcast %s mask %s", start_ip, end_ip,
gateway, broadcast, subnet_mask);
 
-   __connman_ippool_unref(pool);
-
/*
 * Now create the pool again, we should not get collision
 * with existing allocated address.
@@ -305,14 +303,14 @@ static void test_case_5(void)
 
/* pool should return 192.168.2.1 now */
flag = 0;
-   pool = __connman_ippool_create(23, 1, 100, collision_cb, );
-   g_assert(pool);
+   pool2 = __connman_ippool_create(23, 1, 100, collision_cb, );
+   g_assert(pool2);
 
-   gateway = __connman_ippool_get_gateway(pool);
-   broadcast = __connman_ippool_get_broadcast(pool);
-   subnet_mask = __connman_ippool_get_subnet_mask(pool);
-   start_ip = __connman_ippool_get_start_ip(pool);
-   end_ip = __connman_ippool_get_end_ip(pool);
+   gateway = __connman_ippool_get_gateway(pool2);
+   broadcast = __connman_ippool_get_broadcast(pool2);
+   subnet_mask = __connman_ippool_get_subnet_mask(pool2);
+   start_ip = __connman_ippool_get_start_ip(pool2);
+   end_ip = __connman_ippool_get_end_ip(pool2);
 
g_assert(gateway);
g_assert(broadcast);
@@ -332,14 +330,15 @@ static void test_case_5(void)
 
g_assert(flag == 0);
 
-   __connman_ippool_unref(pool);
+   __connman_ippool_unref(pool1);
+   __connman_ippool_unref(pool2);
 
__connman_ippool_cleanup();
 }
 
 static void test_case_6(void)
 {
-   struct connman_ippool *pool;
+   struct connman_ippool *pool1, *pool2;
const char *gateway;
const char *broadcast;
const char *subnet_mask;
@@ -362,14 +361,14 @@ static void test_case_6(void)
g_assert(flag == 0);
 
/* pool should return 192.168.2.1 now */
-   pool = __connman_ippool_create(26, 1, 100, collision_cb, );
-   g_assert(pool);
+   pool1 = __connman_ippool_create(26, 1, 100, collision_cb, );
+   g_assert(pool1);
 
-   gateway = __connman_ippool_get_gateway(pool);
-   broadcast = __connman_ippool_get_broadcast(pool);
-   subnet_mask = __connman_ippool_get_subnet_mask(pool);
-   start_ip = __connman_ippool_get_start_ip(pool);
-   end_ip = __connman_ippool_get_end_ip(pool);
+   gateway = __connman_ippool_get_gateway(pool1);
+   broadcast = __connman_ippool_get_broadcast(pool1);
+   subnet_mask = __connman_ippool_get_subnet_mask(pool1);
+   start_ip = __connman_ippool_get_start_ip(pool1);
+   end_ip = __connman_ippool_get_end_ip(pool1);
 
g_assert(gateway);
g_assert(broadcast);
@@ -387,8 +386,6 @@ static void test_case_6(void)
"\tgateway %s broadcast %s mask %s", start_ip, end_ip,
gateway, broadcast, subnet_mask);
 
-   __connman_ippool_unref(pool);
-
/*
 * Now create the pool again, we should not get collision
 * with existing allocated address.
@@ -396,14 +393,14 @@ static void test_case_6(void)
 
/* pool should return 192.168.3.1 now */
flag = 0;
-   pool = 

Re: [PATCH 0/4] Added captive portal possibility

[Patrik Flykt]

On Tue, 2015-10-20 at 14:14 +1100, Craig McQueen wrote:
> The following patches are related to the captive portal code originally 
> provided by Alexandre Chataignon. The first patch is a copy of the 
> original patch, provided again as a reference. The 3 following patches 
> are improvements I made following a review of Alexandre's patch, to 
> improve the following:
> 
> * Check that the query first question is for an A record, and only 
> send an A record if so.
> * Handle the case of a query containing additional records (don't 
> send a corrupt response).
> * For queries from a loopack IP address, don't return the captive 
> address, but do the normal DNS handling.

So this is a special case of a special case.

The general case to solve is to add local entries to ConnMan's resolver,
i.e. entries in /etc/hosts should be supported. As should the hostname
handled by systemd-hostnamed. A and  records as well as the IP4.ARPA
and IP6.ARPA domains need to be added automatically.

After this comes the first special case: what, if any, should be
configured for hosts in the tethered network. The second special case is
the wildcard catch-all for the portal case specified above. This seems
to point to a wildcard entry in the file mapping all other names. I'm
not very sure how both of these cases will work out in the end.

Plan B for handling this special case is to set up unbound or similar,
start ConnMan with --nodnsproxy while adding the IP address of the
tethering interface to FallbackNameservers. The nameservers in
FallbackNameservers will be sent out via DHCP if ConnMan is not proxying
DNS requests. In this particular special case there were no uplink
connections so the DHCP server IP address is always the same - minus any
bugs of course.

Plan C is to use mDNS from Avahi, but that seems to be outside of the
original solution unless short names are always used.

Cheers,

Patrik

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

Re: Stale WPS state when authentication fails

[Patrik Flykt]

On Tue, 2015-10-20 at 16:06 -0700, Juha Kuikka wrote:
> Hello,
> 
> If I attempt to connect to a network using WPS (pin code or push button)
> and the authentication fails it seems the "WiFi.UseWPS" flag never gets
> reset.
> 
> This causes the next connection attempt to default to the WPS mode (and
> fail) and not use the Agent API to request new authentication information.
> 
> I did a quick patch that fixes this behavior but I'm not sure if there is a
> better way to do this, maybe in the wifi plugin code by indicating key
> error if wps fails.

Please send this as a patch email.

> Any advice on where this fix should go?

By quickly looking and not thinking about it, it looks fine. Tomasz?

Patrik



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

Re: DHCP & hostname faceplant

[Patrik Flykt]

Hi,

On Thu, 2015-10-08 at 01:31 -0600, Daniel Wagner wrote:
> I have uploaded two traces. Both created on my Linux machine.
> 
> - This one is with an unmodified ConnMan:
> 
>http://www.monom.org/connman/dhcp-not-working.pcapng
> 
> - And this one is with a modified ConnMan which doesn't sent its
>hostname:
> 
>http://www.monom.org/connman/dhcp-working.pcapng

Looks like there are two ways to tame this: have a main.conf entry or
retry without sending a hostname. In the latter case people expecting
their host name to be added to DNS will be surprised if there is
something else going wrong in the network at the same time and their
hostname is therefore not sent to the server.

The first option will be clearer (will it?) to the user that no
hostnames are sent and DNS therefore cannot be expected to work
automatically. But of course this is now an option that hits all DHCP
connection, not the presumably "faulty" one. OTOH with this main.conf
one can prevent hostnames to be sent altogether.

Cheers,

Patrik

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

Re: [PATCH v2] openvpn: Add MTU related options

[Patrik Flykt]

Hi,

On Wed, 2015-10-21 at 12:12 +0200, Daniel Wagner wrote:

> The use of the different MTU options depend on you server and link
> configuration. I don't think you can derive it. OpenVPN seems to offer
> an option to learn the MTU size by doing some measurements but that
> takes around 3 minutes according documentation. I recommend to just
> expose those options and let the user decide what he needs.

Well, now the problem is that I have no idea what to add as MTU values
if I need to... Sven's "documented" problem and solution was to use
--tun-mtu, if that is the max sized packet that can go through
unfragmented it looks like the only option one should specify. And
--fragment seems to be needed always in addition to any of the MTUs?
Unless it's the default behavior?

Cheers,

Patrik

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

Re: [PATCH] gdbus: move typedefs for interwork with strict compilers

[Patrik Flykt]

On Tue, 2015-10-20 at 23:44 -0700, Grant Erickson wrote:
> Move enumeration type defintions AFTER the enumerations themselves are 
> declared 
> and defined such that the header works with strict compilers.

Applied, thanks!

Patrik

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

Re: [PATCH v2] doc: Fix ClearProperty documentation

[Patrik Flykt]

On Wed, 2015-10-21 at 11:19 +0300, Patrik Flykt wrote:
> ClearProperty applies only on the Error property and causes the
> service to go to idle state.
> ---
> 
> Updated the text and noted that InvalidProperty is the only error that
> can be returned.

Applied. Now the implementation is wrong and needs to be fixed according
to the corrected documentation.

Patrik

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

Re: [PATCH] service: Fix minor memory leak on exit

[Patrik Flykt]

On Tue, 2015-10-20 at 17:07 +0300, Slava Monich wrote:
> services_notify->add and services_notify->remove are always created
> and have to be always destroyed.

Applied, thanks!

Patrik

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

Re: WPA2 connection problem

[Patrik Flykt]

On Wed, 2015-10-21 at 09:55 +, Wawrzek Niewodniczanski wrote:
> On 19 October 2015 at 10:45, Patrik Flykt <patrik.fl...@linux.intel.com> 
> wrote:
> >
> > Hi,
> >
> > On Mon, 2015-10-19 at 09:57 +, Wawrzek Niewodniczanski wrote:
> >> I try to connected to Enterprise WPA2 and what I see in console is:
> >> 'Could not connect net.connman.Error.InvalidArguments: Invalid
> >> arguments'
> >
> > WPA2 EAP networks need to be configured using doc/config-format.txt.
> > After that ConnMan may ask a username/passphrase so and Agent is advised
> > to be running. The simplest Agent is connmanctl run with 'agent on'.
> >
> 
> Hi,
> 
> I'm using connmanctl, Agent is one. I configured my WPA2 Enterprise in
> /var/lib/network_name.config but I still get 'Invalid arguments' after
> connection attempt to the network.

/var/lib/connman/network_name.config

> My configuration is:
> 
> [service service_name]

[service_] here, see doc/config-format.txt, "Service sections
[service_*]" part.

HTH,

Patrik


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

[PATCH v2] doc: Fix ClearProperty documentation

[Patrik Flykt]

ClearProperty applies only on the Error property and causes the
service to go to idle state.
---

Updated the text and noted that InvalidProperty is the only error that
can be returned.

doc/service-api.txt | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/doc/service-api.txt b/doc/service-api.txt
index 88816d8..e2e9e84 100644
--- a/doc/service-api.txt
+++ b/doc/service-api.txt
@@ -30,13 +30,12 @@ Methods dict GetProperties()  [deprecated]
 
void ClearProperty(string name)
 
-   Clears the value of the specified property.
+   Clears the value of the specified property. Only
+   the readonly Error property can be cleared using
+   this method call. When cleared the service is reset
+   to the idle state.
 
-   Properties cannot be cleared for hidden WiFi service
-   entries or provisioned services.
-
-   Possible Errors: [service].Error.InvalidArguments
-[service].Error.InvalidProperty
+   Possible Errors: [service].Error.InvalidProperty
 
void Connect()
 
-- 
2.1.4

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

[PATCH] doc: Fix ClearProperty documentation

[Patrik Flykt]

ClearProperty applies only on the Error property and causes the
service to go to idle state.
---
 doc/service-api.txt | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/doc/service-api.txt b/doc/service-api.txt
index 88816d8..dca97cd 100644
--- a/doc/service-api.txt
+++ b/doc/service-api.txt
@@ -30,10 +30,9 @@ Methods  dict GetProperties()  [deprecated]
 
void ClearProperty(string name)
 
-   Clears the value of the specified property.
-
-   Properties cannot be cleared for hidden WiFi service
-   entries or provisioned services.
+   Clears the value of the specified property. Only
+   the readonly Error property can be cleared. When
+   cleared the service is reset to the idle state.
 
Possible Errors: [service].Error.InvalidArguments
 [service].Error.InvalidProperty
-- 
2.1.4

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

Re: [RFC 4/8] doc: Add connman-vpn.8

[Patrik Flykt]

Hi,

This fails distcheck with:

make[2]: *** No rule to make target 'doc/connman-vpn.8', needed by 'all-am'.  
Stop.
Makefile:1570: recipe for target 'all' failed
make[1]: *** [all] Error 2
Makefile:5020: recipe for target 'distcheck' failed
make: *** [distcheck] Error 1
result 2

On Mon, 2015-10-19 at 13:49 +0300, Jaakko Hannikainen wrote:
> ---
>  Makefile.am   |  6 --
>  doc/connman-vpn.8 | 62 
> +++
>  doc/connman.8 |  2 +-
>  doc/connmanctl.1  |  2 +-
>  4 files changed, 68 insertions(+), 4 deletions(-)
>  create mode 100644 doc/connman-vpn.8
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3d0645e..9193231 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -123,6 +123,8 @@ src_connmand_LDADD = gdbus/libgdbus-internal.la 
> $(builtin_libadd) \
>  src_connmand_LDFLAGS = -Wl,--export-dynamic \
>   -Wl,--version-script=$(srcdir)/src/connman.ver
>  
> +MANUAL_PAGES += doc/connman.conf.5 doc/connman.8
> +

These pages are not part of this patch...

>  if VPN
>  vpn_plugin_LTLIBRARIES =
>  
> @@ -151,6 +153,8 @@ vpn_connman_vpnd_LDADD = gdbus/libgdbus-internal.la 
> $(builtin_vpn_libadd) \
>  
>  vpn_connman_vpnd_LDFLAGS = -Wl,--export-dynamic \
>   -Wl,--version-script=$(srcdir)/vpn/vpn.ver
> +
> +MANUAL_PAGES += doc/connman-vpn.8
>  endif

This file is part of this patch, but I prefer having patches for the
manual pages separate from changes to automake.
 
>  BUILT_SOURCES = $(local_headers) src/builtin.h $(service_files) 
> scripts/connman
> @@ -385,8 +389,6 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
>  EXTRA_DIST += src/main.conf \
>   src/eduroam.config
>  
> -MANUAL_PAGES += doc/connman.8 doc/connman.conf.5
> -
>  dist_man_MANS = $(MANUAL_PAGES)
>  
>  pkgconfigdir = $(libdir)/pkgconfig
> diff --git a/doc/connman-vpn.8 b/doc/connman-vpn.8
> new file mode 100644
> index 000..889faff
> --- /dev/null
> +++ b/doc/connman-vpn.8
> @@ -0,0 +1,62 @@
> +.\" connman-vpn(8) manual page
> +.\"
> +.\" Copyright (C) 2015 Intel Corporation
> +.\"
> +.TH CONNMAN-VPN "8" "2015-10-15"
> +.SH NAME
> +ConnMan-VPN \- VPN management daemon
> +.SH SYNOPSIS
> +.B connman-vpnd
> +.RB [\| \-\-version \||\| \-\-help \|]
> +.PP
> +.B connman-vpnd
> +.RB [\| \-c
> +.IR file \|]
> +.RB [\| \-d\  [\c
> +.IR file [,...]\|]\|]
> +.RB [\| \-p
> +.IR plugin [,...]\|]
> +.RB [\| \-P
> +.IR plugin [,...]\|]
> +.RB [\| \-n \|]
> +.RB [\| \-r \|]
> +.SH DESCRIPTION
> +The \fIConnMan-VPN\fP provides a daemon for managing vpn connections together
> +with \fBconnmand\fP(8). The Connection Manager is designed to be slim and to
> +use as few resources as possible. The VPN daemon supports 
> \fBopenconnect\fP(8),
> +\fBopenvpn\fP(8), \fBvpnc\fP(8) and L2TP/PPTP (\fBxl2tpd.conf\fP(5),
> +\fBpptp\fP(8), \fBpppd\fP(8)).
> +.P
> +.SH OPTIONS
> +The following options are supported:
> +.TP
> +.BR \-v ", " \-\-version
> +Print the ConnMan-VPN software version and exit.
> +.TP
> +.BR \-h ", " \-\-help
> +Print ConnMan-VPN's available options and exit.
> +.TP
> +.BI \-c\  file\fR,\ \fB\-\-config= \fIfile
> +Specify configuration file to set up various settings for ConnMan.  If not
> +specified, the default value of \fI/connman/connman-vpn.conf\fP
> +is used; where \fI\fP is dependent on your distribution (usually
> +it's \fI/etc\fP).  See \fBconnman-vpn.conf\fP(5) for more information on
> +configuration file. The use of config file is optional and sane default 
> values
> +are used if config file is missing.
> +.TP
> +.BR \-d\  [ \fIfile [,...]],\  \-\-debug [= \fIfile [,...]]
> +Sets how much information ConnMan-VPN sends to the log destination (usually
> +syslog's "daemon" facility).  If the file options are omitted, then debugging
> +information from all the source files are printed. If file options are
> +present, then only debug prints from that source file are printed. Example:
> +.PP
> +   connman-vpnd --debug=vpn/vpn-provider.c,vpn/vpn-config.c
> +.TP
> +.BR \-n ", " \-\-nodaemon
> +Do not daemonize. This is useful for debugging, and directs log output to
> +the controlling terminal in addition to syslog.
> +.TP
> +.BR \-r ", " \-\-routes
> +Manage VPN routes instead of telling \fBconnmand\fP(8) to do it.
> +.SH SEE ALSO
> +.BR connmanctl (1), \ connmand (8)
> diff --git a/doc/connman.8 b/doc/connman.8
> index 51c77e3..7ae9ff5 100644
> --- a/doc/connman.8
> +++ b/doc/connman.8
> @@ -94,4 +94,4 @@ If this option is used, then ConnMan is not able to cache 
> the DNS queries
>  because the DNS traffic is not going through ConnMan and that can cause
>  some extra network traffic.
>  .SH SEE ALSO
> -.BR connmanctl (1), \ connman.conf (5)
> +.BR connmanctl (1), \ connman.conf (5), \ connman-vpn (8)
> diff --git a/doc/connmanctl.1 b/doc/connmanctl.1
> index c3d4803..470fae7 100644
> --- a/doc/connmanctl.1
> +++ b/doc/connmanctl.1
> @@ -272,4 +272,4 @@ Setting multiple proxy 

Re: [PATCH v2] openvpn: Add MTU related options

[Patrik Flykt]

Hi,

On Tue, 2015-10-20 at 14:26 +0200, Sven Schwedas wrote:
> On 2015-10-20 14:04, Patrik Flykt wrote:
> > Are any of these necessary for any use cases? If they are needed for
> > something, could the currently specified OpenVPN.MTU be used to derive
> > proper MTUs for the devices involved?
> 
> They are necessary for some mobile carriers. We're fixing the tun MTU to
> 576 so OpenVPN doesn't die on (I think) Vodafone Germany's networks.

Which of the mtu, fragment and/or mssfix options do you end up using and
with what value?

> (Though not yet with Connman, it's legacy deployments with
> NetworkManager/manual OpenVPN, and some special industrial modems from
> Vodafone. I can't say just how widespread the issue is otherwise.)

Cheers,

Patrik

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

Re: [RFC 8/8] doc: Move man pages under automake

[Patrik Flykt]

Hi,

On Mon, 2015-10-19 at 13:50 +0300, Jaakko Hannikainen wrote:
> This change prettifies man pages with  and /var/lib
> so that it will show the actual value connman was compiled with,
> rather than just hinting at some magical compile time variable.
> ---
>  .gitignore  |  8 ++--
>  Makefile.am | 17 
> -
>  ...man-service.config.5 => connman-service.config.5.in} |  8 
>  ...ovider.config.5 => connman-vpn-provider.config.5.in} |  6 +++---
>  doc/{connman-vpn.8 => connman-vpn.8.in} |  3 +--
>  doc/{connman-vpn.conf.5 => connman-vpn.conf.5.in}   |  8 +---
>  doc/{connman.8 => connman.8.in} |  3 +--
>  doc/{connman.conf.5 => connman.conf.5.in}   |  8 +---
>  doc/{connmanctl.1 => connmanctl.1.in}   |  0
>  9 files changed, 33 insertions(+), 28 deletions(-)
>  rename doc/{connman-service.config.5 => connman-service.config.5.in} (96%)
>  rename doc/{connman-vpn-provider.config.5 => 
> connman-vpn-provider.config.5.in} (98%)
>  rename doc/{connman-vpn.8 => connman-vpn.8.in} (93%)
>  rename doc/{connman-vpn.conf.5 => connman-vpn.conf.5.in} (90%)
>  rename doc/{connman.8 => connman.8.in} (95%)
>  rename doc/{connman.conf.5 => connman.conf.5.in} (97%)
>  rename doc/{connmanctl.1 => connmanctl.1.in} (100%)
> 
> diff --git a/.gitignore b/.gitignore
> index 9c22e4a..bbb44c3 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -64,8 +64,12 @@ unit/test-nat
>  doc/*.bak
>  doc/*.stamp
>  doc/connman.*
> -!doc/connman.8
> -!doc/connman.conf.5
> +doc/*.1
> +doc/*.5
> +doc/*.8
> +!doc/*.1.in
> +!doc/*.5.in
> +!doc/*.8.in
>  doc/connman-*.txt
>  
>  vpn/builtin.h
> diff --git a/Makefile.am b/Makefile.am
> index 8312a66..0887831 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -390,6 +390,21 @@ EXTRA_DIST += doc/overview-api.txt doc/behavior-api.txt \
>  EXTRA_DIST += src/main.conf \
>   src/eduroam.config
>  
> +%.1 : %.1.in
> + sed -e 's||$(sysconfdir)|' \
> + -e 's||$(storagedir)|' \
> + -e 's||$(vpn_storagedir)|' $< > $@
> +
> +%.5 : %.5.in
> + sed -e 's||$(sysconfdir)|' \
> + -e 's||$(storagedir)|' \
> + -e 's||$(vpn_storagedir)|' $< > $@
> +
> +%.8 : %.8.in
> + sed -e 's||$(sysconfdir)|' \
> + -e 's||$(storagedir)|' \
> + -e 's||$(vpn_storagedir)|' $< > $@
> +

As this is a .in file, I'd rather like @sysconfdir@ etc. being used
since @variablename@ is the common syntax here. At the bottom of
Makefile.am, there is do_subst that does the same thing.

>  dist_man_MANS = $(MANUAL_PAGES)
>  
>  pkgconfigdir = $(libdir)/pkgconfig
> @@ -471,4 +486,4 @@ include/connman/%.h: $(abs_top_srcdir)/include/%.h
>   $(AM_V_GEN)$(LN_S) $< $@
>  
>  clean-local:
> - @$(RM) -rf include/connman
> + @$(RM) -rf include/connman $(MANUAL_PAGES)
> diff --git a/doc/connman-service.config.5 b/doc/connman-service.config.5.in
> similarity index 96%
> rename from doc/connman-service.config.5
> rename to doc/connman-service.config.5.in
> index e1ee753..e932fe7 100644
> --- a/doc/connman-service.config.5
> +++ b/doc/connman-service.config.5.in
> @@ -6,11 +6,11 @@
>  .SH NAME
>  service-name.config \- ConnMan service provisioning file
>  .SH SYNOPSIS
> -.B /var/lib/connman/\fIservice-name\fB.config
> +.B /\fIservice-name\fB.config
>  .SH DESCRIPTION
>  .P
>  \fIConnMan\fP's services are configured with so called
> -"\fBprovisioning files\fP" which reside under \fI/var/lib/connman/\fP.
> +"\fBprovisioning files\fP" which reside under \fI/\fP.
>  The files can be named anything, as long as they end in \fB.config\fP.
>  The provisioning files can be used to configure for example secured
>  wireless access points which need complex authentication, for example
> @@ -125,7 +125,7 @@ method (should only be used with \fBEAP=ttls\fP).
>  .SH "EXAMPLE"
>  .SS Eduroam
>  This is a configuration file for eduroam networks. This file could for
> -example be in /var/lib/connman/eduroam.config. Your university's exact
> +example be in /eduroam.config. Your university's exact
>  settings might be different.
>  .PP
>  .nf
> @@ -139,7 +139,7 @@ CACertFile = /home/user/UNIV_CA.crt
>  .SS Complex networking
>  This is a configuration file for a network providing EAP-TLS, EAP-TTLS and
>  EAP-PEAP services. The respective SSIDs are tls_ssid, ttls_ssid and peap_ssid
> -and the file name is /var/lib/connman/example.config.
> +and the file name is /example.config.
>  .PP
>  Please note that the SSID entry is for hexadecimal encoded SSID (e.g. "SSID =
>  746c735f73736964"). If your SSID does not contain any exotic character then
> diff --git a/doc/connman-vpn-provider.config.5 
> b/doc/connman-vpn-provider.config.5.in
> similarity index 98%
> rename from doc/connman-vpn-provider.config.5
> rename to doc/connman-vpn-provider.config.5.in
> index 615b4c4..71e958c 100644
> --- 

Re: [PATCH] vpn: Fix wrong VPN parameters

[Patrik Flykt]

On Mon, 2015-10-19 at 13:16 +0300, Jaakko Hannikainen wrote:
> Some VPN parameters are mistyped in the source, fix them
> and update documentation. Add deprecated-label to
> OpenVPN.TLSRemote.
> ---
> I went through all of the VPN parameters, and I believe the parameters are
> correct now (I don't have means to properly test them). Turns out 
> xl2tpd.conf's
> man page is actually incorrect about itself; the client or whatever it is
> parses more options than mentioned in the man page and for example, the man
> page says the option is 'flow bits' but the program parses the option 'flow 
> bit'...
> 
> Also worth of notice is that there is an option called 'PPPD.UseAccomp' which
> when set to true disables accomp.

Applied, thanks!

Patrik

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

Re: [PATCH] ipconfig: Ensure ifname is not null when setting ipv6 state

[Patrik Flykt]

On Sun, 2015-10-18 at 09:07 -0700, Abtin Keshavarzian wrote:
> From: Abtin Keshavarzian 
> 
> This is a quick fix for the issue where removal of an interface may
> cause IPv6 to be disabled/enabled on other interfaces.
> When removing an ipdevice in free_ipdevice(), it is ensured that the
> ifname (which we get from connman_inet_ifname()) is not null,
> before invoking set_ipv6_state(). If a null ifname is passed into
> set_ipv6_state(), it will apply the change to all interfaces instead
> of the intended one.

Applied, thanks!

Patrik

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

Re: Enable/disable Wi-Fi tethering when Wi-Fi dongle is not plugged in

[Patrik Flykt]

Hi,

On Thu, 2015-10-15 at 13:37 +1100, Craig McQueen wrote:
> I'm setting PersistentTetheringMode = true in /etc/connman/main.conf.
> I'm also providing an initial /var/lib/connman/settings which contains
> parameters for WiFi tethering enabled. So when a user plugs in a Wi-Fi
> USB dongle, it goes into tethering mode automatically.
> 
> However, when the Wi-Fi dongle is not plugged in, the D-Bus interface
> gives no indication that persistent tethering is enabled, and gives no
> ability to disable it.

There are exactly zero devices supporting WiFi, and there is also zero
knowledge of any possibly appearing at a later point either. So when
there are no devices of a certain technology, there is no point in
showing a technology that would somehow indicate something else...

> Perhaps if persistent tethering mode is saved, then the technology
> should continue to be visible, albeit with some property that
> indicates hardware isn't available.  E.g. Tethering = True, Powered =
> False, Hardware = False. Or something like that. Then it should be
> possible to at least disable the persistent tethering.

Sorry, no. It will make everybody believe that hardware is
malfunctioning although it will never be present. A technology that
exists implies Hardware = true.

> Similarly, I reckon it should be possible to see that configuration
> exists for a Wi-Fi network, and to remove it ("forget network"), even
> when the hardware isn't currently present or the network isn't
> currently detected. That is, all previously configured services should
> always appear in the list of services, with some property that
> indicates it isn't connected. E.g. Strength = 0, State = absent. Or
> something like that.

Sorry, no. We have had this discussion before, and the Service API is by
definition showing existing services. It has been repeatedly asked from
people needing this kind of a features to specify a configuration API
where all the configured services can be listed. So far no proposals
have been received.

> Another note: I notice that Connected = False when tethering is
> enabled and operational. I'm not sure what the rationale is for that,
> but perhaps that could be changed to Connected = True to clearly
> indicate when tethering is both enabled and operational.

Connected is defined to indicate that there is a service connected.

Cheers,

Patrik



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

Re: [PATCH] Added captive portal possibility

[Patrik Flykt]

Hi,

On Wed, 2015-09-23 at 11:20 +0200, Alexandre Chataignon wrote:
> This patch adds the possibility to enable a captive portal while connman 
> is in tethering mode by sending its own IP via the DNS proxy (which acts 
> as a DNS server more than a proxy in this case).
> 
> I needed this usage for an IoT device, in which the tethering mode is in 
> fact a hotspot mode. With a light http server and patched connman, it 
> offers the possibility to have a light and easy-to-use captive portal 
> for example to diagnostic device.

A captive portal is easier to implement by setting a NAT rule that
redirects incoming connections to a desired port on the local x.x.x.1
tethering interface. Once the captive portal stuff is cleared out, the
redirecting NAT rule for that host can be modified not to capture
connections. It's much simpler than having to rewrite DNS replies from
ConnMan. Your patch enforces captive portal implementation for each and
every device all the time, for sure other use cases include letting
hosts through that have completed the steps enforced by the captive
portal.

Cheers,

Patrik

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

Re: [PATCH] Added captive portal possibility

[Patrik Flykt]

On Fri, 2015-10-16 at 16:52 +1100, Craig McQueen wrote:
> So the patch would need to be improved as follows:
> 
> * Check that the query first question is for an A record, and only
> send an A record if so.
> * Calculate the correct length of the query header and question
> record(s), in order to position the answer in the correct location in
> the response.
> * Calculate the correct total length of the response.
> * Consider whether an "OPT" or other additional record should be
> copied into the answer
> * Consider how to handle a query with more than one question.

Let's attack the captive portal situation by redirecting traffic from
the hosts to local ports using netfilter. It's a much more elegant and
modular solution than to modify DNS replies from ConnMan.

Cheers,

Patrik


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

Re: How to provide DHCP on a technology

[Patrik Flykt]

On Thu, 2015-10-15 at 09:26 +, John Ernberg wrote:

> > USB Host USB Gadget
> > +--+  +--+
> > |  eth0|  |  |
> > |ES|USB cable |Dev   |
> > |  usb0|--|usb0  |
> > +--+  +--+
> >cdc_ether  g_ether
> >DHCP ServerDHCP Client
> >
> > Interface usb0   Always has usb0
> > created when DEVTYPE=gadget
> > gadget is connected
> > by USB cable.
> > DEVTYPE=gadget
> > in uevent file in
> > /sys/class/net/usb0

The above looks fishy. You now have a gadget-gadget connection over USB,
which is something I didn't knew existed. Which end runs ConnMan and in
which direction is the USB cable connected? Is this an USB OTG device?

> > I should have tried this much earlier, I started an udhcpc instance on the 
> > USB Gadget side,
> > and that gave the interface an IP almost immediately.

On the gadget side you also run 'connmanctl enable gadget' and
'connmanctl connect gadget_' or something 100% compatible?

> > This makes me think there is something with the DHCP Client that ConnMan 
> > starts on the
> > USB gadget side.

It's the same DHCP client as for all other connections. Please check
that you don't have other dhcp, ntp or dns clients running. If you have,
ConnMan's clients won't start.

> # ./tcpdump -i usb0 &
> [ 3026.487670] device usb0 entered promiscuous mode
> tcpdump: WARNING: usb0: no IPv4 address assigned
> tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> listening on usb0, link-type EN10MB (Ethernet), capture size 65535 bytes
> [ 3061.146682] g_ether gadget: high-speed config #1: CDC Ethernet (ECM)
> [ 3061.196880] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready
> 02:02:35.646716 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 1 
> group record(s), length 28
> 02:02:36.526704 IP6 :: > ff02::1:ff00:1011: ICMP6, neighbor solicitation, who 
> has fe80::ff:fe00:1011, length 24
> 02:02:37.526725 IP6 fe80::ff:fe00:1011 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:02:41.536686 IP6 fe80::ff:fe00:1011 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:02:43.866697 IP6 fe80::ff:fe00:1011 > ff02::16: HBH ICMP6, multicast 
> listener report v2, 1 group record(s), length 28
> 02:02:45.546687 IP6 fe80::ff:fe00:1011 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:02:51.224212 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 1 
> group record(s), length 28
> 02:02:52.254225 IP6 fe80::ff:fe02:1201 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:02:52.304195 IP6 fe80::ff:fe02:1201 > ff02::16: HBH ICMP6, multicast 
> listener report v2, 1 group record(s), length 28
> 02:02:56.264202 IP6 fe80::ff:fe02:1201 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:03:00.274210 IP6 fe80::ff:fe02:1201 > ff02::2: ICMP6, router solicitation, 
> length 16
> 02:03:01.534275 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 2 
> group record(s), length 48
> 02:03:01.754226 IP6 :: > ff02::1:ff7c:e91f: ICMP6, neighbor solicitation, who 
> has fe80::886c:91ff:fe7c:e91f, length 24
> 02:03:02.754255 IP6 fe80::886c:91ff:fe7c:e91f > ff02::2: ICMP6, router 
> solicitation, length 16
> 02:03:06.764221 IP6 fe80::886c:91ff:fe7c:e91f > ff02::2: ICMP6, router 
> solicitation, length 16
> 02:03:09.834262 IP6 fe80::886c:91ff:fe7c:e91f > ff02::16: HBH ICMP6, 
> multicast listener report v2, 1 group record(s), length 28
> 02:03:10.774317 IP6 fe80::886c:91ff:fe7c:e91f > ff02::2: ICMP6, router 
> solicitation, length 16

>From the above one does not know whether the device is tethering or not.
If tethering is enabled, the interface to tcpdump is 'tether'. Which
device is this, USB "host" which now is a gadget according to the above
or the USB gadget device...? And why are you backgrounding the tcpdump
command?

> # udhcpc -i usb0

If this is the next command, you're on the same device. I don't think
this is the case, but there is nothing indicating otherwise either.

> udhcpc (v1.22.1) started
> Sending discover...
> Sending select for 192.168.0.2...
> Lease of 192.168.0.2 obtained, lease time 86400
> /etc/udhcpc.d/50default: Adding DNS 192.168.0.1
> 02:04:48.286721 IP 0.0.0.0.bootpc > 255.255.255.255.bootps: BOOTP/DHCP, 
> Request from 02:00:00:00:10:11 (oui Unknown), length 300
> 02:04:48.324435 IP 192.168.0.1.bootps > 255.255.255.255.bootpc: BOOTP/DHCP, 
> Reply, length 548
> 02:04:48.366696 IP 0.0.0.0.bootpc > 255.255.255.255.bootps: BOOTP/DHCP, 
> Request from 02:00:00:00:10:11 (oui Unknown), length 300
> 02:04:48.404440 IP 192.168.0.1.bootps > 255.255.255.255.bootpc: BOOTP/DHCP, 
> Reply, length 548
> 02:04:48.733158 ARP, Request who-has 192.168.0.1 tell 192.168.0.2, length 28
> 02:04:48.733448 ARP, Reply 192.168.0.1 is-at 02:00:00:02:12:01 (oui Unknown), 
> length 28
> 02:04:48.733486 IP 

Re: [RFC 1/3] ofono: create a list of contexts per modem

[Patrik Flykt]

Hi,

On Fri, 2015-10-09 at 08:37 +0200, Mylene JOSSERAND wrote:
> This is a first patch of a serie to implement a simultaneous APNs context 
> activation.
> 
> The current commit removes the previous implementation of one context per 
> modem and
> adds a list of contexts instead.

As this patch is really quite long, it looks it would be possible to add
GSList *context_list without removing struct network_context *context
immediately. So the code would function as before, but the contexts are
added/removed to/from the list. The next patch could then remove struct
network_context *context and change the functionality to always use the
list.

Instead of looking up the context with g_slist_nth_data and a
context_id, can't the code simply pass around a struct network_context *
all the time? For example in network_connect() the corresponding call
would then look like 'context_set_active(modem, context, TRUE)'. It's
much simpler that way and avoids iterating over the list twice. Right
now it is not immediately clear what a context_id with values 0 or -1
means.

Also, if you have a singly-linked list, please always prepend items as
the appending function has to always reach the end of the list before an
item is added. Since the order makes no difference here, prepeding is a
much more elegant (and faster) solution.

> Some modifications are necessary : some properties in the modem structure are 
> moved
> to the context structure (such as connman_network, valid_apn, etc) as they 
> are properties
> specifics to context.

Ok.

> Some functions are implemented to search for a specific context in the list 
> by his path
> or by his network. A function to remove all the context of one modem
> is also implemented.

Yes, those need to be implemented. The other two patches are really nice
and small.

Cheers,

Patrik

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

Re: [PATCH] gdhcp: Remove unnecessary operation on lease seconds

[Patrik Flykt]

On Thu, 2015-10-08 at 16:39 +0300, Patrik Flykt wrote:
> Lease seconds will not overflow, they are specified as a delta from
> now to a point in the future. Therefore no truncation of the
> received value is necessary.

Applied.

Patrik

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

Re: WPA2 connection problem

[Patrik Flykt]

Hi,

On Mon, 2015-10-19 at 09:57 +, Wawrzek Niewodniczanski wrote:
> I try to connected to Enterprise WPA2 and what I see in console is:
> 'Could not connect net.connman.Error.InvalidArguments: Invalid
> arguments'

WPA2 EAP networks need to be configured using doc/config-format.txt.
After that ConnMan may ask a username/passphrase so and Agent is advised
to be running. The simplest Agent is connmanctl run with 'agent on'.

Cheers,

Patrik


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

Re: [PATCH] Added captive portal possibility

[Patrik Flykt]

Hi,

On Mon, 2015-10-19 at 10:26 +0200, Alexandre Chataignon wrote:

> This solution was the first I implemented (no need to modify connman 
> source code).
> 
> The problem is that in your case, DNS requests are answered by a remote 
> DNS server. However, as I said, the tethering mode here is in fact a « 
> hotspot » mode, i.e. we don't have internet connection on the other end.
>
> So your solution will work if we want to access device via any IP 
> address (for exemple http://1.2.3.4) but not via a DNS request (for 
> example http://device.config or even http://www.google.fr), which is 
> much more practical.

I was thinking of any connection going to e.g. port 80 would be forced
to the locally used x.x.x.1 address on the tether interface. Like an
automatic proxy solution. This can then be refined to work only for a
subset of hosts, by looking up e.g. www.google.fr and setting up a
transparent proxy/captive portal only for those. Besides, using
http://1.2.3.4 would go through unnoticed, as it is already an IP
address :-). device.config is not a proper host name as it does not
exist and ConnMan should respond properly to DNS lookups...

That said, reading the contents of e.g. /run/connman/hosts and replying
with those to DNS requests sounds like a decent solution to me. But
still that would not capture http://216.34.181.45.

Cheers,

Patrik

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

Re: Hidden network issue

[Patrik Flykt]

On Mon, 2015-10-19 at 11:14 +0100, Wawrzek Niewodniczanski wrote:
> Hello,
> 
> Some time ago I asked about connection to hidden network. One of the
> information I remember was that following following identifier is
> wrong:
>wifi_70188ba36ecd_hidden_managed_psk
> 
> One of suggestion that time was to ancient version of connman. So now
> I retry is something newer (1.30 from Arch), but have the same
> problem. I cannot access this hidden network - I got Input/Output
> error.
> 
> What should I do to debug?

You need to have an Agent running, the simplest one is connmanctl with
the 'agent on' command. Connecting to
wifi_70188ba36ecd_hidden_managed_psk requires ConnMan to ask the SSID
and passphrase from the user.

Cheers,

Patrik

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

Re: Timezone doesn't update when /etc/localtime symlink changes

[Patrik Flykt]

Hi,

On Thu, 2015-10-15 at 11:28 +1100, Craig McQueen wrote:

> That doesn't sound right -- it looks as though there's code in
> timezone.c to watch the file for changes. __connman_timezone_init()
> sets up an inotify on /etc. When inotify triggers it calls
> inotify_data(). If the file that changed is localtime, then it calls
> __connman_clock_update_timezone().

I'm corrected, which means we can have this fixed with a decent amount
of effort, cool! Looking at other code IN_MODIFY | IN_MOVED_TO is
handled. Now some figuring out is needed to see whether /etc/localtime
is a symlink and if yes, what needs to be done to detect that the entity
it points to has changed. Or something even more simpler than that which
is just broken.

Cheers,

Patrik

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

Re: [PATCH 6/6] service: Implement ClearProperty properly

[Patrik Flykt]

Hi,

Thanks for these patches. I'm going to add text from the cover letter to
this patch and nitpick on clearing AutoConnect below.

On Thu, 2015-10-15 at 10:41 +0300, Jaakko Hannikainen wrote:
> The previous implementation only cleared Error, which as per
> documentation is wrong since Error is readonly. Add sane
> defaults to all readwrite fields.
> 
> Reported by Francesco Bruno.
> ---
>  src/service.c | 31 ++-
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index e9107c2..fe20763 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -3582,11 +3582,32 @@ static DBusMessage *clear_property(DBusConnection 
> *conn,
>   dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, ,
>   DBUS_TYPE_INVALID);
>  
> - if (g_str_equal(name, "Error")) {
> - set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
> -
> - g_get_current_time(>modified);
> - service_save(service);
> + if (service->hidden) {
> + return __connman_error_not_supported(msg);
> + } else if (g_str_equal(name, "AutoConnect")) {
> + __connman_service_set_autoconnect(service, true);

Here I'd say clearing means to unset the value 'true'. I.e. the result
is not that one gets a default value but the cleared one. Nitpicking on
this means that the default value of AutoConnect is really false, it's
only the DefaultAutoConnectTechnologies ones that are set to true.

I'll change this to avoid an extra round-trip.

Cheers,

Patrik

> + } else if (service->immutable) {
> + return __connman_error_not_supported(msg);
> + } else if (g_str_equal(name, "Nameservers.Configuration")) {
> + __connman_service_set_nameservers_conf(service, NULL);
> + } else if (g_str_equal(name, "Timeservers.Configuration")) {
> + __connman_service_set_timeservers_conf(service, NULL);
> + } else if (g_str_equal(name, "Domains.Configuration")) {
> + __connman_service_set_domains_conf(service, NULL);
> + } else if (g_str_equal(name, "Proxy.Configuration")) {
> + __connman_service_set_proxy_conf(service,
> + CONNMAN_SERVICE_PROXY_METHOD_AUTO,
> + NULL, NULL, NULL);
> + } else if (g_str_equal(name, "IPv4.Configuration")) {
> + __connman_service_reset_ipconfig(service,
> + CONNMAN_IPCONFIG_TYPE_IPV4, NULL, NULL);
> + __connman_ipconfig_set_method(service->ipconfig_ipv4,
> + CONNMAN_IPCONFIG_METHOD_DHCP);
> + __connman_network_enable_ipconfig(service->network,
> + service->ipconfig_ipv4);
> + } else if (g_str_equal(name, "IPv6.Configuration")) {
> + __connman_service_reset_ipconfig(service,
> + CONNMAN_IPCONFIG_TYPE_IPV6, NULL, NULL);
>   } else
>   return __connman_error_invalid_property(msg);
>  


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

Re: service state transition to failure

[Patrik Flykt]

On Tue, 2015-10-13 at 14:42 -0700, Naveen Singh wrote:

> There is nothing that application knows that connman does not know.
> In fact application gets to know through connman that connection did
> not go through. The way application gets connected back is to initiate
> a scan and hoping that one of these scan would find the AP (or
> services) and then run autoconnect would trigger and get device
> connected. But in this case run autoconnect would not attempt
> connection because service state was left to failure.

This approach won't work at all. When ConnMan receives a Scan() over
D-Bus, it will scan immediately and reset its own autoscan timer. Which
means that if the application issues a scan after less than ~6 minutes,
it causes ConnMan to scan so often that wpa_s never times out its WiFi
networks. So if the service has failed, it has failed for all eternity.

The solution here is very simple. Get rid of the application assisted
scan and let ConnMan handle it instead.

Cheers,

Patrik


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

Re: service state transition to failure

[Patrik Flykt]

Hi,

On Tue, 2015-10-13 at 23:24 -0700, Grant Erickson wrote:
> Due to these constraints, the application knows best when to scan and
> does so at its discretion. Neither connman nor wpa_s autoscan
> infrastructure scan at the application-desired times. When they do
> scan automatically, it typically results in one or both of an
> undesirable expenditure of power or a momentary and inopportune move
> off-channel.

Fair enough. There is always the BackgroundScanning option in main.conf
that you then should set to false in order not to have ConnMan
restarting autoscans for no apparent benefit and an undesired increase
in at least power consumption.

Notice that support for wpa_supplicant's internal autoscan mechanism was
removed a while back in version 1.28, as it interfered with hidden SSID
detection. Nowadays only ConnMan's internal autoscan implementation is
being used.

> Absent the proposed change, per doc/overview-api.txt, with a WiFi
> network continually refreshed, there is no exit path from the Failure
> service state and, consequently, no unattended means by which the
> application can recover WiFi connectivity under these circumstances in
> a deterministic and application-controlled way.

Since the application has full knowledge of what the suitable scan
interval is for this device, there is really no reason why the
application cannot issue a Connect() as well after a finished scan. This
way the application can gracefully get away from situations where
previous service connect attempts failed. This also works should the
application have had a need to disconnect the service in between.

> The proposed change offers, for unattended applications that wish to
> avail themselves of it, an exit gate to effectively acknowledge a
> service error by clearing it. There is no downside or consequence to
> interactive applications that wish to continue to operate as they do
> today.

If the application already knows better when to scan, it also knows
better when to issue a reconnect. Opening up service state transitions
to user or UI interference is a can of worms I cannot maintain in any
sensible manner. Autoscan backoff strategy and wifi network expiry are
up for tweaking to better values, but here one needs to come with an
explanation how everything functions smoother afterwards.

Cheers,

Patrik

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

Re: Enable/disable Wi-Fi tethering when Wi-Fi dongle is not plugged in

[Patrik Flykt]

On Wed, 2015-10-14 at 17:53 +1100, Craig McQueen wrote:
> I am working on a system where a USB dongle is used for Wi-Fi
> functionality. The Wi-Fi can be used in client or access point mode,
> configurable. However, it seems it's not possible to enable/disable
> tethering mode while the USB Wi-Fi device is not currently plugged in.
> E.g.:
> 
> # connmanctl tether wifi off
> Error disabling wifi tethering: Method "SetProperty" with
> signature "sv" on interface "net.connman.Technology" doesn't exist
> 
> What options would there be for enabling/disabling tethering, while
> the Wi-Fi device is not currently plugged in?

If this dongle is the only WiFi device, removing it will cause the
technology to disappear as there are no devices this specific technology
is able to control. So no options for this, but see
TetheringTechnologies and PersistentTetheringMode in main.conf if the
use case is such that tethering state needs to be remembered.

Cheers,

Patrik


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

Re: How to provide DHCP on a technology

[Patrik Flykt]

On Wed, 2015-10-14 at 06:40 +, John Ernberg wrote:

> I made a ramdisk out of /var/ when I made the file system read only.
> Nothing is however preserved after a reboot, and that has not been
> problem for me yet.

That's actually a problem, as /var (especially /var/lib) is the only
location that per definition can hold application state over reboot. In
this case it presents a problem, as ConnMan will now not know its state
over reboot. /var/run (or /run) is the proper location for run-time
data, which can be cleared.


> If I have other services that reach the internet, and possibly want to
> tether on these in the future, but want to keep this link private (not
> share it with the other connections). Would this then be out of scope
> for what ConnMan is designed for?

What do you mean by keeping the link private?


> I had to obfuscate the MAC addresses, but it should still give a
> picture of what the setup looks like. Our cdc_ether driver publishes
> the Ethernet-over-USB interface as usb0, and shows up in ConnMan as a
> gadget technology.

> USB Host side:
> eth0  Link encap:Ethernet  HWaddr yy:yy:yy:yy:yy:yy
>UP BROADCAST MULTICAST  MTU:1500  Metric:1
>RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>collisions:0 txqueuelen:1000
>RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)
> 
> tetherLink encap:Ethernet  HWaddr xx:xx:xx:xx:xx:xx
>inet addr:192.168.0.1  Bcast:192.168.0.255  Mask:255.255.255.0
>inet6 addr: fe80::d838:94ff:fe48:389b/64 Scope:Link
>UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>RX packets:5 errors:0 dropped:0 overruns:0 frame:0
>TX packets:8 errors:0 dropped:0 overruns:0 carrier:0
>collisions:0 txqueuelen:0
>RX bytes:364 (364.0 B)  TX bytes:620 (620.0 B)
>
> usb0  Link encap:Ethernet  HWaddr xx:xx:xx:xx:xx:xx
>inet6 addr: fe80::ff:fe02:1201/64 Scope:Link
>UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>RX packets:11 errors:0 dropped:0 overruns:0 frame:0
>TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
>collisions:0 txqueuelen:1000
>RX bytes:748 (748.0 B)  TX bytes:132 (132.0 method)

Thethering has been enabled on the USB host. If you enabled 'ethernet'
technology tethering, eth0 will be tethered. If you enabled 'gadget'
tethering usb0 will be tethered. The fact that usb0 shows up with an
IPv6 address makes me think that ethernet tethering is enabled.

So your USB host end also has a gadget connector??

 
> return sender=:1.2 -> dest=:1.22 reply_serial=2
> array [
>struct {
>   object path "/net/connman/technology/gadget"
>   array [
>  dict entry(
> string "Name"
> variant   string "Gadget"
>  )
>  dict entry(
> string "Type"
> variant   string "gadget"
>  )
>  dict entry(
> string "Powered"
> variant   boolean true
>  )
>  dict entry(
> string "Connected"
> variant   boolean false
>  )
>  dict entry(
> string "Tethering"
> variant   boolean true
>  )
>   ]
>}

Gadget is tethering. Which means this is the USB client end. If it's the
same situation as above, there is a bug somewhere as usb0 should not
have an IP address.

>struct {
>   object path "/net/connman/technology/ethernet"
>   array [
>  dict entry(
> string "Name"
> variant   string "Wired"
>  )
>  dict entry(
> string "Type"
> variant   string "ethernet"
>  )
>  dict entry(
> string "Powered"
> variant   boolean true
>  )
>  dict entry(
> string "Connected"
> variant   boolean false
>  )
>  dict entry(
> string "Tethering"
> variant   boolean false
>  )
>   ]
>}

Ok, ethernet is not tethering. Is this the same situation as above where
you listed the interfaces? Networks created in the USB host end of
things will show up as 'ethernet' devices.

> USB gadget side:
> usb0  Link encap:Ethernet  HWaddr xx:xx:xx:xx:xx:xx
>inet6 addr: fe80::ff:fe00:1011/64 Scope:Link
>UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>RX packets:2 

Re: Timezone doesn't update when /etc/localtime symlink changes

[Patrik Flykt]

Hi,

On Wed, 2015-10-14 at 17:42 +1100, Craig McQueen wrote:
> > In my target system, /etc/localtime is a symlink into the
> appropriate file
> > under /usr/share/zoneinfo.
> 
> I've tried making /etc/localtime an ordinary file, rather than a
> symlink. But still ConnMan doesn't notice when it changes. Is Timezone
> update working for others? 
> 
> My target system is an ARM embedded system built with Yocto. I've
> verified that inotify is enabled in my system's kernel.

Currently ConnMan only writes to /etc/localtime when the experimental
Clock API Timezone property is updated. Nothing in ConnMan watches this
file for changes. You are free to send a patch for this, see
include/inotify.h for support functions.

Cheers,

Patrik


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

Re: connmanctl quitting before multiple return functions called

[Patrik Flykt]

Hi,

On Wed, 2015-10-14 at 17:49 +1100, Craig McQueen wrote:
> I've been looking into extending connmanctl's functionality with
> configuration for the net.connman.Clock API. I've been developing a
> command that can set clock properties, similar to the existing
> "config" command.
> 
> I've noticed that when multiple parameters are set, then the 'return'
> function is often not called. E.g.:
> 
> connmanctl config ethernet_4a4a5a6a3a0a_cable --ipv6 off
> --autoconnect yes --timeservers 0.pool.ntp.org
> 
> In this case, it sends 3 D-Bus commands, and config_return() should be
> called for each response. But it is likely to shutdown before calling
> config_return() all three times.
> 
> I see that dbus_method_reply() calls __connmanctl_quit(). That seems
> to be implicated in this. It seems the logic is to initiate shutdown
> after the response is received for the first D-Bus message sent.

The logic has a bug here. It would be best if connmanctl exits after all
the method calls have completed. You are always free to send a patch for
this or implement it properly for Clock API commands, if you intend to
send patches for supporting that.

Cheers,

Patrik

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

Re: How to provide DHCP on a technology

[Patrik Flykt]

Hi,

On Wed, 2015-10-14 at 13:09 +, John Ernberg wrote:
> All I know is that cdc_ether interfaces shows up as usb0, the eth0
> ethernet is a real ethernet port. When looking in /sys/class/net/usb0
> it indeed shows up as a gadget in the uevent file.

This now looks like you are connecting an USB client end on the USB host
system to a USB client on the USB client system. A USB client - USB
client connection between two devices won't work. If this is a correct
interpretation of your setup, the USB controller on the USB host system
needs to be configured to support USB host and not USB client as it
looks like its doing now.

I think we need to start drawing figures in ascii art to get the picture
complete, there are too many USBs in the explanation above...

Cheers,

Patrik


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

Re: service state transition to failure

[Patrik Flykt]

Hi,

On Mon, 2015-10-12 at 21:46 -0700, Naveen Singh wrote:
> In my previous email when I meant *"**User could be initiating scan" *I
> actually meant "*Application could be initiating scan".*

Well, don't. Now the responsibility of correct behavior is taken away
from ConnMan and placed on the application. What does the application
know about network connectivity that ConnMan doesn't?

If some other entity is requesting sudden irregular scans, it is a sign
for ConnMan that fresh information is needed; for sure the networks
available are not the ones desired and therefore there is even less
point in trying to connect to a service that has already failed...


Cheers,

Patrik

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

Re: net.connman.Service setProperty

[Patrik Flykt]

On Tue, 2015-10-13 at 04:22 +, francobrun...@hotmail.com wrote:
> Thanks.
> 
> Are you able to change IPv4 configuration? I'm getting not supported error.

'IPv4.Configuration' is the property to change. On low-level the
attribute is an 'sv' with the variant 'v' containing a dict which is of
the form 'a{sv}'. Look at the python example code in
test/set-ipv4-method and perhaps connmanctl C code in client/commands.c.

> It doesn't work even with python test application. The service stucks
> in configuraton state and the method cannot be changed in anything but
> off.

If a service looks like it is stuck in configuration state, start by
checking that a DHCP server is running in the network as the default
IPv4 configuration mechanism tries to use DHCP and will timeout in a bit
over a minute if none is found. If a service looks like it is stuck when
changing IPv4 configuration on a connected service, it sounds like a
bug.

HTH,

Patrik

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

Re: How to provide DHCP on a technology

[Patrik Flykt]

Hi,

On Mon, 2015-10-12 at 14:26 +, John Ernberg wrote:

> I'm using Connman in an embedded environment where a device will connect 
> to us over USB, this device will provide an ethernet interface on the 
> USB port, and the device will be in gadget mode.
> Thus when they connect to us we load the cdc_ether driver and create an 
> Ethernet interface over the USB port.
> 
> Being completely new to tethering in Connman and how it works, I'm 
> having some issues setting up Connman to provide a DHCP server on this 
> interface so the other device will receive an IP automatically once it 
> starts a DHCP client.

If I understood the above thing correctly, ConnMan runs on a device that
has a USB client and ConnMan needs to set up a DHCP server on it? This
is achieved by ensuring that both 'gadget' technology and 'gadget'
tethering are enabled. Also verify that the interface is not listed
in /etc/connman/main.conf NetworkInterfaceBlacklist option and that
'gadget' is either listed in TetheringTechnologies or
TetheringTechnologies is unset.

Now that the 'gadget' technology has been enable once, ConnMan will
remember its enabled state over reboots. With that in mind, one can then
set PersistentTetheringMode to true in /etc/connman/main.conf. This
preserves tethering state for all technologies over reboot, including
'gadget'.

If it is the opposite, i.e. ConnMan runs on the USB host end, the
procedure is the same, but the technology is instead 'ethernet'. This
affects the all ethernet devices on the machine, as USB ethernet dongles
are indistinguishable from internal PCI connected ones.

The networks that show up as services are the ones where ConnMan is a
end host/client. Tethering creates a local network bridge, 'tether', and
starts a DHCP server on it. The tethered network does not show up as a
service, as it is not an upstream connection.

> I have googled and read the documentation but I cannot figure out how to 
> do this properly.
> The current method on the server side is to enable the gadget-technology 
> when it shows up, configure an IP, netmask and gateway, and then enable 
> tethering on it. Which results in the services not being listable, and 
> no IP being given to the other side. If I run ifconfig on my side, the 
> interface is up, but unconfigured.

Here I lost track of which device is which. Please check the above and
see if that fixes your problem.

Cheers,

Patrik

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

Re: How to provide DHCP on a technology

[Patrik Flykt]

Hi,

On Tue, 2015-10-13 at 08:49 +, John Ernberg wrote:

> ConnMan runs on the embedded system, which is the USB host side.
> And when an USB gadget connects, I want to start a DHCP server on the
> interface provided by cdc_ether so that the USB gadget that connects
> can receive an IP by using a DHCP client.

USB host side is counted as a regular ethernet. So 'ethernet' tethering
is the one you want to configure.

> I have parts of the file system as read only, so ConnMan does not
> remember things between reboots.

ConnMan stores its technology state into /var/lib/connman/settings, if
this file is not perserved over reboots maybe tmpfiles from systemd can
create a suitable one into that directory? Or some other shell scripting
during boot.

> > The networks that show up as services are the ones where ConnMan is a
> > end host/client. Tethering creates a local network bridge, 'tether', and
> > starts a DHCP server on it. The tethered network does not show up as a
> > service, as it is not an upstream connection.
> 
> Ok, got it. I found the tether interface, the USB gadget side does not
> receive an IP however.

In the USB client/gadget end, especially when running Linux also here,
it usually is a matter of being able to (automatically) load the proper
drivers for the gadget device. It also is a matter of USB client mode
setting, the USB client/gadget is probably registered automatically for
some other (Linux kernel module) profile instead of networking.

> Could this be caused by that we do not always have any other active
> connections?

The upstream connections in ConnMan, i.e. services, do not need to be
enabled for thethering to work. Only routing/internet connectivity is
affected by the presence or absence of a connected service.

>  I.e. should this be done differently if I want to have a connection
> that is end-to-end, without bridging?

The connection between the USB host device running ConnMan and the other
device with the USB client connection already provide an end-to-end
connection over IP. If ConnMan has an service connected providing an
upstream internet connection, ConnMan sets up a NAT to get the tethered
devices connecting to the internet. The brigde interface comes into play
as it is the smartest way of connecting all tethering technologies and
interfaces into one network served by a single DHCP server. On the
winning side all devices tethered can now connect to each other via IP,
be they connected via Bluetooth, USB, ethernet or wifi.

Cheers,

Patrik

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

Re: service state transition to failure

[Patrik Flykt]

Hi,

On Fri, 2015-10-09 at 10:52 -0700, Naveen Singh wrote:
> This may not happen at all. My understanding is that wpa_supplicant would
> time out only if the AP is not seen in subsequent scans. But the AP is
> always found in scan as there is nothing wrong at 802.11 level. The user
> found that WAN cable was not connected so he went ahead and fixed it. And
> now the connection to DHCP server is established but connection will still
> not happen. Is user supposed to power off the AP so that it disappears from
> scan list.

wpa_s will time out wifi networks in 2 minutes if no scans have been
done to refresh them. For ConnMan it will take ~6 min 20 sec to get
fresh results with a new scan after the 2 minute expiry time.

If the user is connected to the system, nothing prevents the user from
connecting manually and immediately after said WAN cable is reattached.

Cheers,

Patrik

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

Re: service state transition to failure

[Patrik Flykt]

Hi,

On Thu, 2015-10-08 at 13:25 -0700, Naveen Singh wrote:

> I looked into the code and the documentation and I do not see any way for
> service to go back to idle (even clearing the error property). I am not
> sure how devices which are supposed to auto-connect handle this?

This all works as intended. ConnMan will run autoconnect internally, and
if a service fails to connect it is excluded the next time autoconnect
is run. Internally from ConnMan's perspective, what would be the use of
retrying a service if it is already known to fail?

That the service is being excluded from autoconnect is a temporary
setting and will be cleared once the service is connected successfully
or the service disappears and reappears again. As autoconnect is not
reconnecting a failed service, it means that an outside action needs to
call Connect() via D-Bus. The rationale here is that at this point the
user may have corrected something or knows otherwise that a connection
will now succeed. ConnMan will retry to connect the service via its
autoconnect mechanism if the service has been removed by wpa_supplicant
and re-detected later on; as said the failure state has thereby been
forgotten. So yes, ConnMan will retry every now and then even when only
relying on its autoconnect mechanism.

> Only the error property is allowed to clear which does not change the
> state. Even the state is specified read only in the documentation.

Correct.

Patrik

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

Re: service state transition to failure

[Patrik Flykt]

On Thu, 2015-10-08 at 13:30 -0700, Naveen Singh wrote:
> 5. Now it is in failure state and will never be able to connect.

It will be autoconnected once its state is no longer failure. The
failure state is cleared once the service is removed. The service is
removed once wpa_supplicant times out the wifi network. The next time
the wifi network is discovered via autoconnect wifi scan, it is created
with state 'idle' and is therefore a candidate for autoconnection.

Cheers,

Patrik


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

Re: [PATCH] Resetting the service state to idle on Error event

[Patrik Flykt]

On Thu, 2015-10-08 at 18:33 -0700, Naveen Singh wrote:
> From: Naveen Singh 
> 
> It is been seen that if the service state has transitioned to failure
> there is no way for it to get it back to idle. This fix allows the state
> to be transitioned back to idle as part of handling clear_property handler
> for error event.

NACK. Service states are internal to ConnMan and not resettable by the
user.

Patrik

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

Re: [PATCH] gdhcp:Remove unnecessary check

[Patrik Flykt]

On Mon, 2015-10-05 at 16:05 +0530, Maneesh Jain wrote:
> Signed-off-by: Maneesh Jain 

Please explain in the commit message *why* the client_id check is
unnecessary. Quickly looking at the code doesn't immediately reveal the
reasoning behind this check.

No signed-off-bys either.

> ---
>  gdhcp/client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdhcp/client.c b/gdhcp/client.c
> index 3c11957..cb4bcb4 100755
> --- a/gdhcp/client.c
> +++ b/gdhcp/client.c
> @@ -2349,7 +2349,7 @@ static gboolean listener_event(GIOChannel *channel, 
> GIOCondition condition,
>   return TRUE;
>   }
>  
> - if (!message_type && !client_id)
> + if (!message_type)
>   /* No message type / client id option, ignore package */
>   return TRUE;
>  

Patrik

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

Re: [PATCH] supplicant.c: Fixed Potential Crash issue

[Patrik Flykt]

Hi,

On Mon, 2015-10-05 at 14:31 +0530, Maneesh Jain wrote:
> Signed-off-by: Maneesh Jain 
> ---
>  gsupplicant/supplicant.c | 6 ++
>  1 files changed, 6 insertions(+)
> 
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index 98ca94b..9506f02
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -2804,6 +2804,9 @@ static void signal_peer_found(const char *path, 
> DBusMessageIter *iter)
>   g_hash_table_replace(peer_mapping, peer->path, interface);
>  
>   property_data = dbus_malloc0(sizeof(struct peer_property_data));
> + if (!property_data)
> + return;
> +

NACK. With the above code there will be a peer object without any of its
associated data. Not a good thing. Please do the needed allocations
first, and if either fail, don't add a new peer.
 
>   property_data->peer = peer;
>  
>   dbus_message_iter_next(iter);
> @@ -2861,6 +2864,9 @@ static void signal_peer_changed(const char *path, 
> DBusMessageIter *iter)
>   }
>  
>   property_data = dbus_malloc0(sizeof(struct peer_property_data));
> + if (!property_data)
> + return;
> +

Same here. This time the peer is not updated, though.

>   property_data->peer = peer;
>  
>   supplicant_dbus_property_foreach(iter, peer_property, property_data);


Patrik

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

Re: [PATCH] gsupplicant:Fix possible memory leak in signal_peer_found function

[Patrik Flykt]

On Mon, 2015-10-05 at 15:35 +0530, Maneesh Jain wrote:
> Signed-off-by: Maneesh Jain 

We don't do/need signed-off-bys in this project. Short description here
of the cause of the fix like "If supplicant_dbus_property_get_all()
returns and error, the callback function will not be called and
property_data will not be freed" or such, so that the next person's
attention is focused on the fix and need not necessarily look up the
function to understand if this really was necessary.

> ---
>  gsupplicant/supplicant.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
> index 6a62026..ad53a56 100755
> --- a/gsupplicant/supplicant.c
> +++ b/gsupplicant/supplicant.c
> @@ -2779,7 +2779,8 @@ static void signal_peer_found(const char *path, 
> DBusMessageIter *iter)
>   GSupplicantInterface *interface;
>   const char *obj_path = NULL;
>   GSupplicantPeer *peer;
> -
> + int ret;
> + 

NACK. Trailing whitespace. Please don't send MSDOS formatted patches,
all lines in the patch end with ^M.

>   SUPPLICANT_DBG("");
>  
>   interface = g_hash_table_lookup(interface_table, path);
> @@ -2816,10 +2817,15 @@ static void signal_peer_found(const char *path, 
> DBusMessageIter *iter)
>   peer_property(NULL, NULL, property_data);
>   return;
>   }
> + 
> + ret = supplicant_dbus_property_get_all(obj_path,
> + SUPPLICANT_INTERFACE ".Peer",
> + peer_property, property_data, 
> NULL);
> + if (ret < 0) {
> + dbus_free(property_data);
> + return;
> + }
>  
> - supplicant_dbus_property_get_all(obj_path,
> - SUPPLICANT_INTERFACE ".Peer",
> - peer_property, property_data, NULL);
>  }
>  
>  static void signal_peer_lost(const char *path, DBusMessageIter *iter)


Patrik

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

Re: [PATCH v3] IPv6 timeserver for NTP

[Patrik Flykt]

Hi,

On Tue, 2015-10-06 at 23:50 -0700, Naveen Singh wrote:
> From: Naveen Singh 
> 
> Current NTP code is written with an assumption that timeserver is
> always an IPv4 address. If there is an IPv6 timeserver then the socket
> operation would fail with error as Permission denied (13). This change in
> ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> ---
>  src/ntp.c | 139 
> ++
>  1 file changed, 103 insertions(+), 36 deletions(-)
> 
> diff --git a/src/ntp.c b/src/ntp.c
> index 2c313a4..7858633 100644
> --- a/src/ntp.c
> +++ b/src/ntp.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -117,12 +118,12 @@ static struct timespec mtx_time;
>  static int transmit_fd = 0;
>  
>  static char *timeserver = NULL;
> -static struct sockaddr_in timeserver_addr;
> +static struct sockaddr_in6 timeserver_addr;

Yes, this should be large enough.

>  static gint poll_id = 0;
>  static gint timeout_id = 0;
>  static guint retries = 0;
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout);
> +static void send_packet(int fd, struct sockaddr *server, uint32_t timeout);
>  
>  static void next_server(void)
>  {
> @@ -143,17 +144,18 @@ static gboolean send_timeout(gpointer user_data)
>   if (retries++ == NTP_SEND_RETRIES)
>   next_server();
>   else
> - send_packet(transmit_fd, timeserver, timeout << 1);
> + send_packet(transmit_fd, (struct sockaddr *)_addr, 
> timeout << 1);
>  
>   return FALSE;
>  }
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout)
> +static void send_packet(int fd, struct sockaddr *server, uint32_t timeout)
>  {
>   struct ntp_msg msg;
> - struct sockaddr_in addr;
>   struct timeval transmit_timeval;
>   ssize_t len;
> + int size;
> + char ipaddrstring[28];

The size is not correct. Use INET6_ADDRSTRLEN + 1 as it wants to be null
terminated also.

>  
>   /*
>* At some point, we could specify the actual system precision with:
> @@ -168,10 +170,14 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.poll = 10;  // max
>   msg.precision = NTP_PRECISION_S;
>  
> - memset(, 0, sizeof(addr));
> - addr.sin_family = AF_INET;
> - addr.sin_port = htons(123);
> - addr.sin_addr.s_addr = inet_addr(server);
> + if (server->sa_family == AF_INET) {
> + size = sizeof(struct sockaddr_in);
> + } else if (server->sa_family == AF_INET6){
> + size = sizeof(struct sockaddr_in6);
> + } else {
> + connman_error("Family is neither ipv4 nor ipv6");
> + return;
> + }
>  
>   gettimeofday(_timeval, NULL);
>   clock_gettime(CLOCK_MONOTONIC, _time);
> @@ -180,10 +186,18 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
>  
>   len = sendto(fd, , sizeof(msg), MSG_DONTWAIT,
> - , sizeof(addr));
> + server, size);
> +
> + if ((len < 0) || (len != sizeof(msg))) {
> + inet_ntop(server->sa_family,
> + (server->sa_family == AF_INET)?(void *)&(((struct sockaddr_in 
> *)_addr)->sin_addr):(void *)_addr.sin6_addr,
> + ipaddrstring,
> + 28);

This very complicated. In the previous block server->sa_family is
already investigated, why not record the address of sin_addr or
sin6_addr into a pointer already there? inet_ntop wants a void * anyway.
inet_ntop is not needed right away, because...

> + }
> +
>   if (len < 0) {
>   connman_error("Time request for server %s failed (%d/%s)",
> - server, errno, strerror(errno));
> + ipaddrstring, errno, strerror(errno));

...it can be used here in the log print, as it returns a non-null
pointer as well. So we would have connman_error(...,
inet_ntop(server->sa_family, addr, , sizeof(ipaddrstr)), ...);

>  
>   if (errno == ENETUNREACH)
>   __connman_timeserver_sync_next();
> @@ -192,7 +206,7 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   }
>  
>   if (len != sizeof(msg)) {
> - connman_error("Broken time request for server %s", server);
> + connman_error("Broken time request for server %s", 
> ipaddrstring);

Same here. Yes, it duplicates the print but makes the logic a bit easier
to follow.

>   return;
>   }
>  
> @@ -213,7 +227,7 @@ static gboolean next_poll(gpointer user_data)
>   if (!timeserver || transmit_fd == 0)
>   return FALSE;
>  
> - send_packet(transmit_fd, timeserver, NTP_SEND_TIMEOUT);
> + send_packet(transmit_fd, (struct sockaddr *)_addr, 
> 

Re: [PATCH V2] add management of dsa interface

[Patrik Flykt]

On Tue, 2015-10-06 at 10:51 +0200, Laurent Vaudoit wrote:
> ---

Added a commit message in similar style as the VLAN patch. In the future
please describe the changes in the commit message!

Applied,

Patrik

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

Re: [PATCH] gsupplicant:Fix Potential crash issue in case dbus_malloc() fail

[Patrik Flykt]

On Mon, 2015-10-05 at 15:24 +0530, Maneesh Jain wrote:
> Signed-off-by: Maneesh Jain 

Added a shor commit message and removed the signed off by. Applied!

Patrik

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

Re: service state transition to failure

[Patrik Flykt]

Hi,

On Wed, 2015-10-07 at 21:21 -0700, Naveen Singh wrote:
> I have few questions about service state transition to failure:
> 
> 1. Can service state transition to Failure w/o generating an error event? I
> am seeing that WiFi connection is not going through even if the AP is found
> in scan. I dumped the service properties and I see that state is Failure
> but Event is a NULL string.

When a service goes to state 'failure', it's an indication that none of
the other properties for that service should be considered valid. It may
happen that the current implementation does not send updates for any of
its properties after a transition to 'failure', not even the Error
property. Report to the mailing list if this happens and the level of
frustration associated so we'll take a look.

> 2. We are handling the error property event by clearing the error property.
> This clears the error property and service state to Idle

The Error property is actually specified read only in
doc/service-api.txt. The current code in ConnMan allows the service
property to be cleared, which is a bug. But the state is still 'failure'
for the service after clearing the property, so no state transitions
will take place.

> 3. Even after the error event is cleared we do see a service property
> change event for State (transitioning to failure).

Yes. Only the 'Error' property got cleared, not the state transition.

> 4. Is it advisable to handle the service property change event for state
> (transitioning to failure) same way as we handle the service property
> change event for error.

State transition to failure causes all properties to become invalid and
should be the one to follow. The 'Error' property may try to explain
what happened, but usually the information is not very well suited for
UIs. ConnMan might not announce the 'Error' property explicitly in all
cases of failure; see above.

HTH,

Patrik

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

[PATCH] gdhcp: Remove unnecessary operation on lease seconds

[Patrik Flykt]

Lease seconds will not overflow, they are specified as a delta from
now to a point in the future. Therefore no truncation of the
received value is necessary.
---
 gdhcp/client.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdhcp/client.c b/gdhcp/client.c
index 3c11957..f9cba89 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -1625,8 +1625,7 @@ static uint32_t get_lease(struct dhcp_packet *packet)
return 3600;
 
lease_seconds = get_be32(option);
-   /* paranoia: must not be prone to overflows */
-   lease_seconds &= 0x0fff;
+
if (lease_seconds < 10)
lease_seconds = 10;
 
-- 
2.1.4

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

Re: clamping issue in dhcp lease

[Patrik Flykt]

On Wed, 2015-10-07 at 00:33 -0700, Naveen Singh wrote:
> lease_seconds &= 0x0fff;

This seems to be a bug and at the same time very old code. I'll try to
find time to fix this.

Patrik

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

Re: Fail to call SetProperty

[Patrik Flykt]

Hi,

On Mon, 2015-10-05 at 14:50 +, Francesco Bruno wrote:
> I'm trying to clear properties using the net.connman.Service
> For instance in order to clear out the AutoConnected flag, I've
> written the following code:
> g_dbus_proxy_call_sync( myProxy, "ClearProperty",
> g_variant_new( "(s)", "AutoConnect"), G_DBUS_CALL_FLAGS_NONE,  -1,
> NULL, NULL );

Your code is correct. Unfortunately ConnMan's ClearProperty is not
implemented for quite a few properties. I can ask jhannika to fix this
on Thursday.

Meanwhile you can set AutoConnect to false instead of using the
currently not implemented ClearProperty.

Cheers,

Patrik

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

Re: [PATCH] IPv6 Timeserver for NTP

[Patrik Flykt]

On Mon, 2015-10-05 at 10:49 +0300, Jukka Rissanen wrote:
> > Exactly this is what I am saying.My concern is for the type of
> > timeserver_addr which is used for storing the IP address of the
> > server. It cannot be sockaddr. It needs to be sockaddr_storage.
> Where
> > we know what IP address we are using we can use sockaddr_in or
> > sockaddr_in6 and hen cast it to sockaddr *. 
> 
> 
> Normally in socket programming you instantiate either sockaddr_in or
> sockaddr_in6 variable, and then cast sockaddr * to that variable and
> pass that pointer around. At least I would prefer that way instead of
> creating sockaddr_storage variable for this purpose. So in practice
> there is no need to create sockaddr_storage variable.

And as we know that a sockaddr_in6 has the space to hold also
sockaddr_in, sockaddr_in6 is a decently sized data structure to hold all
addresses. Provide this sockaddr_in6 as a sockaddr * to the functions.
They can cast it back to the needed type.

Cheers,

Patrik

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

Re: Glib main loop stops dispatching

[Patrik Flykt]

On Thu, 2015-10-01 at 20:13 -0700, Naveen Singh wrote:
> It does look like somehow glib main loop is not dispatching to correct
> handlers and then it stops dispatching causing connman to hang.
> 
> Has anyone seen any problem like this. I could provide complete logs
> for this. Just before this happens there is a pending
> wispr_portal_request.

I have never seen nor heard about such behavior of the glib main loop.
Check with your OS distributor if the problem persists.

Cheers,

Patrik


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

Re: [PATCH] add management of dsa interface

[Patrik Flykt]

Hi,

git am complains of trailing white space errors on lines 47 and 56.
While looking at the patch, all lines end with ^M, please fix.

On Mon, 2015-09-28 at 14:48 +0200, Laurent Vaudoit wrote:
> ---
>  plugins/ethernet.c | 60 
> +++---
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/plugins/ethernet.c b/plugins/ethernet.c
> index d176508..456d3cc 100644
> --- a/plugins/ethernet.c
> +++ b/plugins/ethernet.c
> @@ -32,6 +32,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #ifndef IFF_LOWER_UP
>  #define IFF_LOWER_UP 0x1
> @@ -83,6 +84,54 @@ static int get_vlan_vid(const char *ifname)
>   return vid;
>  }
>  
> +static int get_dsa_port(const char *ifname)
> +{
> + int sk;
> + int dsaport=-1;

Spaces before and after =

> + struct ifreq ifr;
> + struct ethtool_cmd cmd;
> + struct ethtool_drvinfo drvinfocmd;
> + struct vlan_ioctl_args vifr;
> +
> + sk = socket(AF_INET, SOCK_STREAM, 0);
> + if (sk < 0)
> + return -errno;
> +
> + memset(, 0, sizeof(ifr));
> + strcpy(ifr.ifr_name, ifname);

ifr.ifr_name probably has a max lenght and I don't know if it must end
in a \0. If it does not defined to end with a \0 or max length is
specified, a strncpy should be used instead.

> +
> + /* check if it is a vlan and get physical interface name*/
> + vifr.cmd = GET_VLAN_REALDEV_NAME_CMD;
> + strncpy(vifr.device1, ifname, sizeof(vifr.device1));
> +
> + if(ioctl(sk, SIOCSIFVLAN, ) >= 0)
> + strcpy(ifr.ifr_name, vifr.u.device2);

If this failed, can we return here? Is strcpy safe here for all string
lengths as above?

> +
> + /* get driver info */
> + drvinfocmd.cmd =  ETHTOOL_GDRVINFO;
> + ifr.ifr_data = (caddr_t)
> + 
> + /*  ioctl failed:   */
> + if (ioctl(sk, SIOCETHTOOL, ) != 0)
> + DBG("Cannot get driver infos\n");

Could this be turned around without the debug, as the more likely case
is that this always works, and with the error returned in the return
value. One can later on see that it worked, as the identifier is
different. And the comment is also a bit unnecessary then.

Compare zero with '!'.

> + else {
> + if(strcmp(drvinfocmd.driver,"dsa") == 0) {
> + /* get dsa port*/
> + cmd.cmd =  ETHTOOL_GSET;
> + ifr.ifr_data = (caddr_t)
> + 
> + /*  ioctl failed:   */
> + if (ioctl(sk, SIOCETHTOOL, ) != 0)
> + DBG("Cannot get device settings\n");

Same here.

> + else
> + dsaport = cmd.phy_address;
> + }
> + }
> + close(sk);
> +
> + return dsaport;
> +}
> +
>  static int eth_network_probe(struct connman_network *network)
>  {
>   DBG("network %p", network);
> @@ -126,7 +175,7 @@ static void add_network(struct connman_device *device,
>   struct ethernet_data *ethernet)
>  {
>   struct connman_network *network;
> - int index, vid;
> + int index, vid,dsaport;

Nitpick: space after comma.

>   char *ifname;
>  
>   network = connman_network_create("carrier",
> @@ -140,6 +189,7 @@ static void add_network(struct connman_device *device,
>   if (!ifname)
>   return;
>   vid = get_vlan_vid(ifname);
> + dsaport = get_dsa_port(ifname);
>  
>   connman_network_set_name(network, "Wired");
>  
> @@ -149,14 +199,18 @@ static void add_network(struct connman_device *device,
>   }
>  
>   if (!eth_tethering) {
> - char group[10] = "cable";
> + char group[16] = "cable";
>   /*
>* Prevent service from starting the reconnect
>* procedure as we do not want the DHCP client
>* to run when tethering.
>*/
> - if (vid >= 0)
> + if((vid >= 0) && (dsaport >= 0))

dsaport need only be assigned here, can you move it down here please. I
just noticed that then vid is also misplaced, but don't worry about
that.

> + snprintf(group, sizeof(group), "p%02x_%03x_cable", 
> dsaport, vid);
> + else if (vid >= 0)
>   snprintf(group, sizeof(group), "%03x_cable", vid);
> + else if (dsaport >= 0)
> + snprintf(group, sizeof(group), "p%02x_cable", dsaport);
>  
>   connman_network_set_group(network, group);
>   }


Patrik


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

Re: [PATCH] IPv6 Timeserver for NTP

[Patrik Flykt]

Hi,

On Wed, 2015-09-30 at 22:08 -0700, Naveen Singh wrote:
> From: Naveen Singh 
> 
> Current NTP code is written with an assumption that timeserver is
> always an IPv4 address. If there is an IPv6 timeserver then the socket
> operation would fail with error as Permission denied (13). This change in
> ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> ---
>  src/ntp.c | 129 
> --
>  1 file changed, 101 insertions(+), 28 deletions(-)

This was patch version 2, so the next one is version 3. Add the version
to the Subject: line as [PATCH v3], this can be done with
'git ... --subject-prefix="PATCH v3" ...'.

> diff --git a/src/ntp.c b/src/ntp.c
> index 2c313a4..a55365d 100644
> --- a/src/ntp.c
> +++ b/src/ntp.c
> @@ -18,7 +18,6 @@
>   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
> USA
>   *
>   */
> -
>  #ifdef HAVE_CONFIG_H
>  #include 
>  #endif
> @@ -34,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -117,12 +117,12 @@ static struct timespec mtx_time;
>  static int transmit_fd = 0;
>  
>  static char *timeserver = NULL;
> -static struct sockaddr_in timeserver_addr;
> +static struct sockaddr_storage timeserver_addr;
>  static gint poll_id = 0;
>  static gint timeout_id = 0;
>  static guint retries = 0;
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout);
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family);
>  
>  static void next_server(void)
>  {
> @@ -143,17 +143,21 @@ static gboolean send_timeout(gpointer user_data)
>   if (retries++ == NTP_SEND_RETRIES)
>   next_server();
>   else
> - send_packet(transmit_fd, timeserver, timeout << 1);
> + send_packet(transmit_fd, timeserver, timeout << 1, 
> timeserver_addr.ss_family);
>  
>   return FALSE;
>  }
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout)
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family)

Instead of supplying a char *server, why don't we give a struct
sockaddr* instead? What's the purpose of carrying the server
"name" (nah, the IP address as a string), if it is already resolved in
the calling function? BTW, it is available as timeserver_addr, so do we
really need any of this information here?

>  {
>   struct ntp_msg msg;
> - struct sockaddr_in addr;
> + struct sockaddr_in in4addr;
> + struct sockaddr_in6 in6addr;
> + struct sockaddr * addr;
>   struct timeval transmit_timeval;
>   ssize_t len;
> + int size;
> + unsigned char * addrptr;
>  
>   /*
>* At some point, we could specify the actual system precision with:
> @@ -168,10 +172,29 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.poll = 10;  // max
>   msg.precision = NTP_PRECISION_S;
>  
> - memset(, 0, sizeof(addr));
> - addr.sin_family = AF_INET;
> - addr.sin_port = htons(123);
> - addr.sin_addr.s_addr = inet_addr(server);
> + if (family == AF_INET) {
> + memset(, 0, sizeof(in4addr));
> + in4addr.sin_family = AF_INET;
> + in4addr.sin_port = htons(123);
> + size = sizeof(in4addr);
> + addrptr = (unsigned char *)_addr.s_addr;
> + addr = (struct sockaddr *)
> + } else if (family == AF_INET6){
> + memset(, 0, sizeof(in6addr));
> + in6addr.sin6_family = AF_INET6;
> + in6addr.sin6_port = htons(123);
> + size = sizeof(in6addr);
> + addrptr = in6addr.sin6_addr.__in6_u.__u6_addr8;
> + addr = (struct sockaddr *)
> + } else {
> + DBG("wrong family type");
> + return;
> + }
> + if (inet_pton(family, server, addrptr) == 0)
> + {
> + DBG("cannot convert ip address string");
> + return;
> + }

The above is unnecessary. You already have timeserver_addr set or the
function gets a sockaddr * to use.

>   gettimeofday(_timeval, NULL);
>   clock_gettime(CLOCK_MONOTONIC, _time);
> @@ -180,7 +203,7 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
>  
>   len = sendto(fd, , sizeof(msg), MSG_DONTWAIT,
> - , sizeof(addr));
> + addr, size);

Use the stored timeserver_addr or supplied sockaddr * family information
to set the size to either sizeof(struct sockaddr_in) or sizeof(struct
sockaddr_in6).

>   if (len < 0) {
>   connman_error("Time request for server %s failed (%d/%s)",
>   server, errno, strerror(errno));
> @@ -213,7 +236,7 @@ static gboolean next_poll(gpointer user_data)
>   if (!timeserver || 

Re: [help] Implementation of 2 internet contexts

[Patrik Flykt]

Hi,

On Wed, 2015-09-30 at 17:13 +0200, Mylene Josserand wrote:
> Hi everyone,
> 
> 
> > On 05/13/2015 01:10 PM, Tomasz Bursztyka wrote:
> >> Hi Mylene,
> >>
> >>> I understand and see (in outline)  how adapt the code.
> >>> Thank you for the tips, it will help me a lot if the Frederik solution's 
> >>> did not
> >>> work for me.
> >>
> >> Looks like Frederik's solution works with either context A or context B, 
> >> not
> >> both at same time.
> >>
> >> Anyway, if you get a patch that works, you will be welcomed to send it on 
> >> this
> >> ML.
> > 
> > Okay, I will let you know if I managed to get it work.
> > 
> 
> 
> Sorry for my (very) late answer.

We're fine with a bit longer real world schedules, no worries here.

> I have managed to handle two (or more) internet contexts at the same
> time, 2-3 days after my last post but I was busy so I did not have
> time to send a patch.
> 
> I have implemented it (and tested it) with version 1.29. I am
> currently merging it to the last version of connman.
> 
> There are many modifications and I do not really know how I should
> categorize them into patches series. As I removed the "network"
> property in modem_data and added it into context_data, there are many
> functions that are impacted.

Ideally it should be be possible to present this kind of major
structural change with moving data from one structure to another by
creating a bigger patch that does only that with no loss of
functionality. Out of necessity this patch then touches quite many
places at the same time. Ideally the next (or previous) patch(es) in the
series would then add the desired feature change(s). But as you propose
below,...

>  If it is okay for you, I thought I could send one patch with all
> modifications and if there is a way to categorize, you could report it
> to me.

...you can also send one big patch where we can help you categorizing
and splitting up the functionality into smaller patches. To help us
remember this still next week, add 'RFC' to the patch subject and write
the commit or cover letter message to indicate that you are asking for
input in splitting up this patch.

> I tried to follow the coding style and I checked the code with
> checkpatch.pl script from Linux kernel. I hope it will be okay.

Kernel coding style is mostly ok, we'll do any additional nitpicking
after or during split up. I'd concentrate more on the patch splitting
first with nitpicking later. Causes one more round, but is probably less
confusing this way.

> I will send the patch in next few days. It will be the first version
> so do not hesitate for any comments. I would send a version 2 with
> pleasure.

Thanks!

Patrik

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

Re: How to set preferred services

[Patrik Flykt]

Hi,

On Wed, 2015-09-30 at 07:59 +, Francesco Bruno wrote:
> this is my first drop in this mailing list so please forgive me if I'm
> going to ask something already answered.
> I've seen that Connman allows to set the list of preferred
> technologies and it works as espected.Now I'm looking for a method, if
> any, to set the list of preferred services given a technology.
> For instance I've two modems (mod1 and mod2) and I'd like to give mod2
> high priority.
> Does Connman support this?

Unfortunately for your specific use case, ConnMan supports only
user-tunable ordering between technologies, not services. So far ConnMan
users have coped with this by having the more important service
autoconnectable with the less important ones configured, but with an
unset autoconnect property.


Cheers,

Patrik


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

Re: [PATCH] IPv6 Timeserver for NTP

[Patrik Flykt]

On Thu, 2015-09-24 at 15:38 -0700, Naveen Singh wrote:
> Isn't the IPv4 or IPv6 address passed as an argument to
> __connman_ntp_start a character array? We would still need to convert
> this character array to the IP addresses and for that we would need to
> know the family. Do you think there is any other way to do this
> without using inet_pton (which requires the family knowledge). Let me
> know.

Since the string is always an address, a shortcut would be to use
getaddrinfo() directly with hints.ai_flags = AI_NUMERICHOST. This is
what connman_inet_check_ipaddress() does, but in start_ntp() the address
is also needed, not only the address family.

The point is that the conversion from string to addres and address
family need to be done only when needed to make the patch smaller and
less intrusive.

Cheers,

Patrik



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

Re: [PATCH] IPv6 Timeserver for NTP

[Patrik Flykt]

On Thu, 2015-09-24 at 11:14 +0300, Patrik Flykt wrote:
> > +
> > + if (ret) {
> > + connman_error("cannot get the server info");
> > + return -1;
> > + }
> > +
> > + family = info->ai_family;
> > + if (family == AF_INET) {
> > + if (inet_pton(family, server, &(((struct sockaddr_in 
> > *)_addr)->sin_addr.s_addr)) == 0) {
> > + connman_error("cannot convert ip address string");
> > + return -1;
> > + }
> > + } else if (family == AF_INET6) {
> > + if (inet_pton(family, server, ((struct sockaddr_in6 
> > *)_addr)->sin6_addr.__in6_u.__u6_addr8) == 0) {
> > + connman_error("cannot convert ipv6 address string");
> > + return -1;
> > + }
> > + } else {
> > + connman_error("Neither IPv4 nor IPv6 type");
> > + return -1;
> 
> This check has already happened in src/service.c, set_property() and
> thus need not be re-done here. It is not done in src/clock.c
> set_property(), neither for the main.conf FallbackTimeservers, so those
> parts need a new patch each.

Let me correct my comment above. Timeservers can also be specified using
host names, so no IP address checks need to be done in the above places.

src/timeservers.c has the code to figure out whether name resolution is
needed or whether the given name is an IP address. So
__connman_ntp_start() can therefore always expect to get either an IPv4
or an IPv6 address.

Cheers,

Patrik

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

Re: [PATCH] IPv6 Timeserver for NTP

[Patrik Flykt]

Hi,

On Wed, 2015-09-23 at 21:22 -0700, Naveen Singh wrote:
> From: Naveen Singh 
> 
> Current NTP code is written with an assumption that timeserver is
> always an IPv4 address. If there is an IPv6 timeserver then the socket
> operation would fail with error as Permission denied (13). This change in
> ntp.c ensures that code works fine with both IPv4 and IPv6 address.
> ---
>  src/ntp.c | 144 
> --
>  1 file changed, 113 insertions(+), 31 deletions(-)
> 
> diff --git a/src/ntp.c b/src/ntp.c
> index 2c313a4..5c135ce 100644
> --- a/src/ntp.c
> +++ b/src/ntp.c
> @@ -18,7 +18,6 @@
>   *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
> USA
>   *
>   */
> -
>  #ifdef HAVE_CONFIG_H
>  #include 
>  #endif
> @@ -34,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -117,12 +117,12 @@ static struct timespec mtx_time;
>  static int transmit_fd = 0;
>  
>  static char *timeserver = NULL;
> -static struct sockaddr_in timeserver_addr;
> +static struct sockaddr_storage timeserver_addr;
>  static gint poll_id = 0;
>  static gint timeout_id = 0;
>  static guint retries = 0;
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout);
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family);
>  
>  static void next_server(void)
>  {
> @@ -143,17 +143,21 @@ static gboolean send_timeout(gpointer user_data)
>   if (retries++ == NTP_SEND_RETRIES)
>   next_server();
>   else
> - send_packet(transmit_fd, timeserver, timeout << 1);
> + send_packet(transmit_fd, timeserver, timeout << 1, 
> timeserver_addr.ss_family);
>  
>   return FALSE;
>  }
>  
> -static void send_packet(int fd, const char *server, uint32_t timeout)
> +static void send_packet(int fd, const char *server, uint32_t timeout, int 
> family)
>  {
>   struct ntp_msg msg;
> - struct sockaddr_in addr;
> + struct sockaddr_in in4addr;
> + struct sockaddr_in6 in6addr;
> + struct sockaddr * addr;
>   struct timeval transmit_timeval;
>   ssize_t len;
> + int size;
> + unsigned char * addrptr;
>  
>   /*
>* At some point, we could specify the actual system precision with:
> @@ -168,10 +172,29 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.poll = 10;  // max
>   msg.precision = NTP_PRECISION_S;
>  
> - memset(, 0, sizeof(addr));
> - addr.sin_family = AF_INET;
> - addr.sin_port = htons(123);
> - addr.sin_addr.s_addr = inet_addr(server);
> + if (family == AF_INET) {
> + memset(, 0, sizeof(in4addr));
> + in4addr.sin_family = AF_INET;
> + in4addr.sin_port = htons(123);
> + size = sizeof(in4addr);
> + addrptr = (unsigned char *)_addr.s_addr;
> + addr = (struct sockaddr *)
> + } else if (family == AF_INET6){
> + memset(, 0, sizeof(in6addr));
> + in6addr.sin6_family = AF_INET6;
> + in6addr.sin6_port = htons(123);
> + size = sizeof(in6addr);
> + addrptr = in6addr.sin6_addr.__in6_u.__u6_addr8;
> + addr = (struct sockaddr *)
> + } else {
> + DBG("wrong family type");
> + return;
> + }
> + if (inet_pton(family, server, addrptr) == 0)
> + {
> + DBG("cannot convert ip address string");
> + return;
> + }
>  
>   gettimeofday(_timeval, NULL);
>   clock_gettime(CLOCK_MONOTONIC, _time);
> @@ -180,7 +203,7 @@ static void send_packet(int fd, const char *server, 
> uint32_t timeout)
>   msg.xmttime.fraction = htonl(transmit_timeval.tv_usec * 1000);
>  
>   len = sendto(fd, , sizeof(msg), MSG_DONTWAIT,
> - , sizeof(addr));
> + addr, size);
>   if (len < 0) {
>   connman_error("Time request for server %s failed (%d/%s)",
>   server, errno, strerror(errno));
> @@ -213,7 +236,7 @@ static gboolean next_poll(gpointer user_data)
>   if (!timeserver || transmit_fd == 0)
>   return FALSE;
>  
> - send_packet(transmit_fd, timeserver, NTP_SEND_TIMEOUT);
> + send_packet(transmit_fd, timeserver, NTP_SEND_TIMEOUT, 
> timeserver_addr.ss_family);
>  
>   return FALSE;
>  }
> @@ -363,7 +386,7 @@ static gboolean received_data(GIOChannel *channel, 
> GIOCondition condition,
>   gpointer user_data)
>  {
>   unsigned char buf[128];
> - struct sockaddr_in sender_addr;
> + struct sockaddr_storage sender_addr;
>   struct msghdr msg;
>   struct iovec iov;
>   struct cmsghdr *cmsg;
> @@ -396,9 +419,18 @@ static gboolean received_data(GIOChannel *channel, 
> GIOCondition condition,
>   if (len < 0)
>

Re: Can connman send local hostname on dhcp request?

[Patrik Flykt]

Hi,

On Mon, 2015-09-21 at 21:34 +0200, Bert Haverkamp wrote:

> I'm struggling to find an answer to this question.
> I am running a raspberry with osmc, a distro that includes connman for 
> managing the connection.
> I would like to have the hostname be sent in the dhcp request, so my 
> local dns can be updated.
> This does not seem to work, but I don't know if it is not implemented in 
> connman, or if I am missing a configuration.
> The only thing I can find is the option AllowHostnameUpdates, but this 
> is for hostname updates from the dhcp-server to the client, while I am 
> looking for the opposite.
> 
> My question therefore is: Do I overlook something, is it possible to 
> send the hostname in connman? If so, how do I enable this?

ConnMan already sends the system hostname in its DHCP client messages,
so this is handled automatically. The host name is fetched in
plugins/loopback.c. If the DHCP based dns name setting does not work,
please check the DHCP server configuration and/or ask the server
administrator if there are restrictions on which hostnames to accept or
if your machine is allowed to have its name updated in dns etc.

But according to the mail from Daniel Wagner on Friday, not all DHCP
servers will provide an IP address lease if the host name is sent...

Cheers,

Patrik

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

Re: DHCP & hostname faceplant

[Patrik Flykt]

Hi,

On Fri, 2015-09-18 at 02:50 +, Daniel Wagner wrote:

> The WiFi APs provided by the local ISP here in Utah does not like the 
> hostname in the DHCP Request packet. Without it the hostname I get the 
> ACK instead of the NACK as respond. Any ideas how we could handle this 
> problem? E.g. stop sending the hostname after the first round of retries 
> have failed?

Hm, sounds like very strange way of not handing out DHCP leases. This is
definitely a first for ConnMan, congratulations! Isn't this funny, the
expected use cases are for DHCP updating DNS information but sabotaged
by this kind of interesting network setup ;-)

There of course aren't any instructions from the ISP not to send
hostnames on the network? Do you have a tcpdump of the exchange to
share?

How does a normal Windows machine cope with this? Any possibilities to
get a packet dump and check how those machines work as sending a host
name is quite a normal operation...

Cheers,

Patrik


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

Re: Any known Connman Vulnerabilities ?

[Patrik Flykt]

Hi,

On Mon, 2015-09-21 at 10:42 +, Lamsoge, Abhijit wrote:

> Any currently known and fixed vulnerability in connman ?

None reported/known since that very old version 0.85.

> How does connman handle exploitation currently, if any ?

If there are any exploits I hope we have a chance to fix them before
they get any extended publicity. As we have a rolling release schedule,
using the latest relase(s) is a very good option as the later releases
always have more issues fixed than their predecessors.

Cheers,

Patrik


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

Re: [PATCH] doc: Reformat connman.8

[Patrik Flykt]

On Thu, 2015-09-17 at 16:40 +0300, Jaakko Hannikainen wrote:
> Reformat the man page so that it follows the convention of
> common man pages (see: manual page of man); bold text
> should be copied exactly as shown, italic text should be
> replaced with the appropriate argument and arguments inside
> [] are optional.

Applied, thanks!

Patrik

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

[PATCH 2/3] connman.service: Fix dependencies for early boot

[Patrik Flykt]

Unset default dependencies in order to properly run at early boot but set
a requirement for the save directory to be mounted. See the systemd.unit
man page, Debian's wiki page
https://wiki.debian.org/Teams/pkg-systemd/rcSMigration and the upstream
systemd-networkd.service file for details.
---
 src/connman.service.in | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/connman.service.in b/src/connman.service.in
index 8f7f342..d96ea39 100644
--- a/src/connman.service.in
+++ b/src/connman.service.in
@@ -1,7 +1,10 @@
 [Unit]
 Description=Connection service
+DefaultDependencies=no
+Conflicts=shutdown.target
+RequiresMountsFor=@localstatedir@/lib/connman
 After=dbus.service network-pre.target
-Before=network.target remote-fs-pre.target
+Before=network.target shutdown.target remote-fs-pre.target
 Wants=network.target remote-fs-pre.target
 
 [Service]
-- 
2.1.4

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

[PATCH 3/3] connman.service: Prevent access to home and /run/user directories

[Patrik Flykt]

---
 src/connman.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/connman.service.in b/src/connman.service.in
index d96ea39..76ef968 100644
--- a/src/connman.service.in
+++ b/src/connman.service.in
@@ -13,6 +13,7 @@ BusName=net.connman
 Restart=on-failure
 ExecStart=@sbindir@/connmand -n
 StandardOutput=null
+ProtectHome=true
 
 [Install]
 WantedBy=multi-user.target
-- 
2.1.4

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

[PATCH 1/3] build: Add localstatedir to variable substitutions

[Patrik Flykt]

---
 Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 886cace..3d0645e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -448,7 +448,8 @@ EXTRA_DIST += vpn/connman-task.te
 do_subst = $(AM_V_GEN)$(SED) \
-e 's,[@]prefix[@],$(prefix),g' \
-e 's,[@]sbindir[@],$(sbindir),g' \
-   -e 's,[@]sysconfdir[@],$(sysconfdir),g'
+   -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+   -e 's,[@]localstatedir[@],$(localstatedir),g'
 
 %.service: %.service.in Makefile
$(AM_V_at)$(MKDIR_P) $(dir $@)
-- 
2.1.4

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

[PATCH 0/3] Systemd .service fixes

[Patrik Flykt]

Hi,

Here are a few modifications to the ConnMan systemd .service file. In the
future once resolv.conf file is written to /run, even more of the filesystem
can be made read-only.

Cheers,

Patrik


Patrik Flykt (3):
  build: Add localstatedir to variable substitutions
  connman.service: Fix dependencies for early boot
  connman.service: Prevent access to home and /run/user directories

 Makefile.am| 3 ++-
 src/connman.service.in | 6 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.1.4

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

Re: [PATCH] service: clear credentials on manual connect failure

[Patrik Flykt]

Hi,

On Tue, 2015-09-15 at 17:19 -0700, Adam Moore wrote:
> Poor RF environments can cause connect-failed regardless
> of password correctness.  When performing a manual connection,
> the correctness of the password has not been proved. Clear the
> credentials in this case so they have an opportunity to be
> corrected if necessary.

Interesting. So wpa_s always reports that the network connect has failed
and never that a key is wrong in this situation? But is this due to the
RF environment beeing so poor that nothing goes through or is wpa_s
sending the wrong reason repeatedly when operating in bad RF conditions?

After applying this patch, ConnMan managed to connect in the same RF
conditions? If yes, wpa_s is a bit broken for this use case.

> ---
>  src/service.c | 40 ++--
>  1 file changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index 196f6b5..568300c 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -4048,6 +4048,24 @@ static DBusMessage *disconnect_service(DBusConnection 
> *conn,
> return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>  }
> 
> +static void clear_credentials(struct connman_service *service)
> +{
> +   g_free(service->passphrase);
> +   service->passphrase = NULL;
> +
> +   g_free(service->identity);
> +   service->identity = NULL;
> +
> +   g_free(service->anonymous_identity);
> +   service->anonymous_identity = NULL;
> +
> +   g_free(service->agent_identity);
> +   service->agent_identity = NULL;
> +
> +   g_free(service->eap);
> +   service->eap = NULL;
> +}
> +
>  bool __connman_service_remove(struct connman_service *service)
>  {
> if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET ||
> @@ -4064,20 +4082,7 @@ bool __connman_service_remove(struct connman_service 
> *service)
> 
> __connman_service_disconnect(service);
> 
> -   g_free(service->passphrase);
> -   service->passphrase = NULL;
> -
> -   g_free(service->identity);
> -   service->identity = NULL;
> -
> -   g_free(service->anonymous_identity);
> -   service->anonymous_identity = NULL;
> -
> -   g_free(service->agent_identity);
> -   service->agent_identity = NULL;
> -
> -   g_free(service->eap);
> -   service->eap = NULL;
> +   clear_credentials(service);
> 
> service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;
> 
> @@ -5500,6 +5505,13 @@ int __connman_service_indicate_error(struct 
> connman_service *service,
> __connman_service_ipconfig_indicate_state(service,
> CONNMAN_SERVICE_STATE_FAILURE,
> CONNMAN_IPCONFIG_TYPE_IPV6);
> +
> +   if (!service->favorite) {
> +   if (error == CONNMAN_SERVICE_ERROR_CONNECT_FAILED) {
> +   clear_credentials(service);
> +   }
> +   }
> +

Here I rather like to have the error forcefully set to authentication
failed, as the code nowadays treats an authentication error identical to
an unset passphrase. This so that there is less confusion about special
cases where passphrases are cleared.

The poor RF conditions must also hit an already saved service. In the
patch above the idea seems to be that wpa_s will then report the error
properly, as it is known that the service was saved only if the
passphrase was correct. Unfortunately there seems to be a corner case
with this one also, in poor RF conditions when trying to connect to an
access point away from home with an identical SSID and security setup
but different passphrase (as it's a different network altogether),
ConnMan is now unable to connect as wpa_s always reports the connection
attempt having failed and not authentication error as it should...

I have to re-read the previous shortish mail thread again with this in
mind and think about how to minimize any brokenness. Ideas welcome!

Cheers,

Patrik

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

Re: problem after suspend/resume using connman and switch

[Patrik Flykt]

On Tue, 2015-09-15 at 08:25 +0200, laurent vaudoit wrote:
> After some analysis, it seems that ipv4ll is launch only on one
> service if dhcp timeout.
> So when we have more than one services, only the first one get an ip
> from link local.

The current implementation starts IPv4 link local after a DHCPv4
timeout.

As IPv4 link local addressing is reusing subnet addressing per
interface, it's not a reliable means of identifying hosts by routing and
is therefore implemented only for one interface at a time in ConnMan.
IPv4 link local addressing is no substitute for networks lacking DHCP or
static configuration.

> Is there any reason for this behaviour?
> It seems not to be linked to our "DSA patch", who is in attachment.

Please send patches as inline mails. Use git send-email for example.

The numerical dsa port id you proposed in the patch will get mixed with
VLAN ids, so you will have to add something more to identify the network
being a dsa one and not a VLAN. According to the coding style, else is
on the same line as the end of the last code block, i.e. '} else {'.

Cheers,

Patrik


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

Re: Wi-Fi access point use case - tether DNS and IP address

[Patrik Flykt]

Hi,

On Tue, 2015-09-15 at 15:40 +1000, Craig McQueen wrote:
> I'd like to use ConnMan in a device that connects to an Ethernet
> network, and also (optionally) can operate Wi-Fi in access point mode
> to provide technician type access to itself.
> 
> So it looks like "tethering" is the way to do a Wi-Fi access point.
> But in this use case, I don't want Wi-Fi tethering to permit access to
> upstream connections (the wired Ethernet). I can easily achieve that
> by adding a firewall with FORWARD rule set to DROP.

By default ConnMan does not touch /proc/sys/net/ipv4/conf/*/forwarding,
so whatever is the system default forwarding/routing policy will be
followed. Whether this is a good or bad idea depends on the used case,
but happens to be the current status quo.

> However, I also want to lock down DNS. Even with forwarding stopped, a
> Wi-Fi client can still do DNS look-ups through the upstream
> connection, thus providing a back channel of communication.

The sane default is to provide the tethered networks with DNS services,
so blocking the DNS port in your case sounds like applying an extra
iptables rule.

> Secondly, I'd also like Wi-Fi clients to be able to access the device
> via a DNS name, such as my-serial-number.lan. I'm not sure how to
> configure a DNS server for the tether interface.

That is currently not implemented. Patches for reading e.g. /etc/hosts
or some other (symlinked) file like /var/lib/connman/hosts can be
considered.

> Related to this: maybe I could run a DNS server, getting the tether
> interface's IP address updates through D-Bus. But as I've seen with
> connmanctl monitor, tether IP address doesn't seem to be notified on
> D-Bus.

As a hackish solution one can always disable dns proxying in ConnMan,
but that usually creates more problem than it solves. So a patch for
reading a file of host names looks like a more attractive option here.

> In summary, these questions:
> 
> 1) How could the tether interface's DNS look-ups through upstream be
> restricted?
> 2) How could a DNS server be provided for the tether interface, which
>responds to my-serial-number.lan with the tether interface's current IP
>address?

It won't be easy to do selective DNS lookups in ConnMan, but some file
could be read to provide local DNS services. But before doing thta, it
should be investigated how or if systemd-resolved can be exploited to
handle name lookups in the future, especially if it gains a D-Bus API.

> 3) How can notifications of tether interface's IP address changes be
> obtained via D-Bus?

As tethering is considered to happen automatically without needing any
user intervention, there hasn't been any real need to expose tethered IP
address information either. Although ConnMan is light-weight, it's still
not a generic embedded routing platform.

Cheers,

Patrik

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

Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

[Patrik Flykt]

Hi,

On Tue, 2015-09-15 at 11:29 +, Sam Nazarko wrote:
> >But also scripts/connman.in should be modified to create the
> >needed symlink.
> 
> Are you saying that you would like to create the symlink as part of
> the packaging? I'm not sure this is necessarily a good idea,
> particularly when packaging with Debian. This means that connman would
> take 'ownership' of /etc/resolv.conf which is not necessarily a good
> idea.

That connman.in file is a generic init script. Need it work for Debian,
it needs LSB fields added in a patch or otherwise anyway. The init
script in the upstream tar ball properly sources /etc/default/connman,
so one can always hide updating of resolv.conf behind a variable.

Having ConnMan write directly into /etc/resolv.conf is probably not what
Debian would like to happen either, but for better or worse it's the
current behavior. The idea here is that from one version to another
there should be a very high probability of things working exactly as
before, also when using init scripts.

Cheers,

Patrik


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