[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

[PATCH] service: Fix minor memory leak on exit

[Slava Monich]

services_notify->add and services_notify->remove are always created
and have to be always destroyed.
---
 src/service.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 196f6b5..02a6844 100644
--- a/src/service.c
+++ b/src/service.c
@@ -7118,9 +7118,10 @@ void __connman_service_cleanup(void)
if (services_notify->id != 0) {
g_source_remove(services_notify->id);
service_send_changed(NULL);
-   g_hash_table_destroy(services_notify->remove);
-   g_hash_table_destroy(services_notify->add);
}
+
+   g_hash_table_destroy(services_notify->remove);
+   g_hash_table_destroy(services_notify->add);
g_free(services_notify);
 
dbus_connection_unref(connection);
-- 
1.9.1

___
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

[Sven Schwedas]

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.

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


-- 
Mit freundlichen Grüßen, / Best Regards,
Sven Schwedas
Systemadministrator
TAO Beratungs- und Management GmbH | Lendplatz 45 | A - 8020 Graz
Mail/XMPP: sven.schwe...@tao.at | +43 (0)680 301 7167
http://software.tao.at



signature.asc
Description: OpenPGP digital signature
___
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 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 6/6] service: Implement ClearProperty properly

[Marcel Holtmann]

Hi Patrik,

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

why are we doing this at all. The reason why ClearProperty exists is so that 
you can clear read-only values like "Error" That is the only thing it is 
intended for. Nothing else.

For anything that that read-write this is pointless to support. Also if I 
wanted it to reset to default values, then it would have been called 
ResetProperty and not ClearProperty. It is just for clearing an error condition 
that ConnMan entered into where is has no capability to get itself out of. So 
the one time when user interaction is needed to clear an error.

Regards

Marcel

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

Stale WPS state when authentication fails

[Juha Kuikka]

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.

Any advice on where this fix should go?

I tested this on v1.29 and v1.30, both behave the same way before (and
after) the patch.

diff --git a/external/connman-1.29/src/service.c
b/external/connman-1.29/src/service.c
index 2b2fcd5..dd58767 100644
--- a/external/connman-1.29/src/service.c
+++ b/external/connman-1.29/src/service.c
@@ -5464,6 +5464,11 @@ static int service_indicate_state(struct
connman_service *service)

case CONNMAN_SERVICE_STATE_FAILURE:

+   /* Clear WPS usage so on retry new method will be asked
from the user */
+   if (service->type == CONNMAN_SERVICE_TYPE_WIFI)
+   connman_network_set_bool(service->network,
+   "WiFi.UseWPS",
false);
+
if (service->connect_reason ==
CONNMAN_SERVICE_CONNECT_REASON_USER &&
connman_agent_report_error(service, service->path,
error2string(service->error),
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

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

[Sven Schwedas]

Hi,

On 2015-10-20 14:45, Patrik Flykt wrote:
> 
>   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?

We only change the tun-mtu value, none of the others. It seems to be
recommended to change the others too, but it fixed the problems for us,
and I didn't want to experiment more at the time. Presumably connman
would need to support all?

> 
>> (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
> 

-- 
Mit freundlichen Grüßen, / Best Regards,
Sven Schwedas
Systemadministrator
TAO Beratungs- und Management GmbH | Lendplatz 45 | A - 8020 Graz
Mail/XMPP: sven.schwe...@tao.at | +43 (0)680 301 7167
http://software.tao.at



signature.asc
Description: OpenPGP digital signature
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

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