Send connman mailing list submissions to
connman@lists.01.org
To subscribe or unsubscribe via email, send a message with subject or
body 'help' to
connman-requ...@lists.01.org
You can reach the person managing the list at
connman-ow...@lists.01.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH] vpn: Do not do mixed declerations and code
(Jussi Laakkonen)
2. Re: [PATCH] vpn: Do not do mixed declerations and code
(Daniel Wagner)
3. Re: [PATCH 00/15] Support SplitRouting on vpnd, drop vpnd route management
(Daniel Wagner)
4. [PATCH] vpn: Export vpn_ipconfig_foreach as linker symbol
(Daniel Wagner)
5. Re: gdhcp: Make DHCP client timeouts suspend aware
(Holesch, Simon (GED-SDD1))
----------------------------------------------------------------------
Date: Fri, 11 Dec 2020 16:42:57 +0200
From: Jussi Laakkonen <jussi.laakko...@jolla.com>
Subject: Re: [PATCH] vpn: Do not do mixed declerations and code
To: Daniel Wagner <w...@monom.org>, connman@lists.01.org
Message-ID: <b948f576-4607-7174-2b3b-c55ec80a9...@jolla.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 12/11/20 4:11 PM, Daniel Wagner wrote:
> gcc complains 'struct vpn_route' is declared within the code section.
> By declaring it at the begin of the function we can also avoid the
> additional helper variables.
> ---
> plugins/vpn.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/plugins/vpn.c b/plugins/vpn.c
> index 94d6e8591d88..3ed9c32268b4 100644
> --- a/plugins/vpn.c
> +++ b/plugins/vpn.c
> @@ -1523,32 +1523,29 @@ static int save_route(GHashTable *routes, int family,
> const char *network,
>
> static int add_network_route(struct connection_data *data)
> {
> - char *network = NULL;
> - char *gateway = NULL;
> - char *netmask = NULL;
> - int family;
> + struct vpn_route rt = { 0, };
> int err;
>
> if (!data)
> return -EINVAL;
>
> - family = connman_provider_get_family(data->provider);
> - switch (family) {
> + rt.family = connman_provider_get_family(data->provider);
> + switch (rt.family) {
> case PF_INET:
> - err = connman_inet_get_route_addresses(data->index, &network,
> - &netmask, &gateway);
> + err = connman_inet_get_route_addresses(data->index,
> + &rt.network, &rt.netmask, &rt.gateway);
> break;
> case PF_INET6:
> err = connman_inet_ipv6_get_route_addresses(data->index,
> - &network, &netmask, &gateway);
> + &rt.network, &rt.netmask, &rt.gateway);
> break;
> default:
> - connman_error("invalid protocol family %d", family);
> + connman_error("invalid protocol family %d", rt.family);
> return -EINVAL;
> }
>
> DBG("network %s gateway %s netmask %s for provider %p",
> - network, gateway, netmask,
> + rt.network, rt.gateway,
> rt.netmask,
> data->provider);
>
> if (err) {
> @@ -1557,21 +1554,20 @@ static int add_network_route(struct connection_data
> *data)
> goto out;
> }
>
> - err = save_route(data->server_routes, family, network, netmask,
> - gateway);
> + err = save_route(data->server_routes, rt.family, rt.network, rt.netmask,
> + rt.gateway);
> if (err) {
> connman_warn("failed to add network route for provider"
> "%p", data->provider);
> goto out;
> }
>
> - struct vpn_route route = { family, network, netmask, gateway };
> - set_route(data, &route);
> + set_route(data, &rt);
>
> out:
> - g_free(network);
> - g_free(gateway);
> - g_free(netmask);
> + g_free(rt.network);
> + g_free(rt.netmask);
> + g_free(rt.gateway);
>
> return 0;
> }
>
I guess I have too old gcc (8.3.0) for these to be warned about, sorry.
Favoring stability over bleeding edge has its drawbacks. Maybe I should
setup a virtual machine with a bleeding edge, as even backports do not
give a newer gcc than the one I have.
Just out of curiosity, what version of gcc are you using with tes builds?
BR,
Jussi
------------------------------
Date: Fri, 11 Dec 2020 15:59:44 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH] vpn: Do not do mixed declerations and code
To: Jussi Laakkonen <jussi.laakko...@jolla.com>
Cc: connman@lists.01.org
Message-ID: <20201211145944.ymesm6rg3btnn...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii
On Fri, Dec 11, 2020 at 04:42:57PM +0200, Jussi Laakkonen wrote:
> I guess I have too old gcc (8.3.0) for these to be warned about, sorry.
All good. This happens sometimes. It is easy to fix.
> Favoring stability over bleeding edge has its drawbacks. Maybe I should
> setup a virtual machine with a bleeding edge, as even backports do not give
> a newer gcc than the one I have.
Actually, I do like when people use different compiler versions and even
architectures. Mono culture is usually a bad thing.
> Just out of curiosity, what version of gcc are you using with tes builds?
I'm using gcc 10.2.1. I didn't get the warning when compiling with the
default settings. I got it with my other settings I use for development.
------------------------------
Date: Fri, 11 Dec 2020 16:14:52 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH 00/15] Support SplitRouting on vpnd, drop vpnd
route management
To: Jussi Laakkonen <jussi.laakko...@jolla.com>
Cc: connman@lists.01.org
Message-ID: <20201211151452.ipqdtx2tgo535...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Fri, Dec 11, 2020 at 04:36:08PM +0200, Jussi Laakkonen wrote:
> We've had most of this running for a quite a long time and these newer
> changes have been in for some time to get any bugs squeezed out, none so far
> but if any rise I'll submit a patch.
That is really good to hear. I expected something like this.
> Some of the VPNs may be apparently
> misconfigured that they do not report proper netmask even (ifconfig verifies
> this). Not sure if it is something missing in our 1.32 based ConnMan which
> has a tonload of bandages on top and our own changes or is it just a
> (misconfigured) test VPN (IPsec VPNC) server which works in an odd
> manner.
There is this problem. If the bandages are somewhat contained and not
distributed over many files I wouldn't mind to get them. Do you have
link to it?
> I did ponder about the adding of the network route quite a long. Especially
> in that netmask being inaddr_any, what should the network be then as it
> cannot be resolved. I just thought having it as an error then would be now
> the approach until that issue pops up and gets resolved, only thing it
> affects is changing a connected misconfigured VPN to be split routed.
One thing we need to be clear to the user, that in such a scenario the
default traffic is not going over the VPN. This is more a UI thingy but
we should also have a big warning in the logs. I really want to avoid
the situation where the VPN service is at top of the service list and
it's not the default gateway.
> And about big work and other changes. At some point next year I guess it
> will be time to send our multiuser changes as an RFC for people to test and
> review, it includes having user specific WiFi and VPN (as we're working in
> mobile environment, but there is work to be done in making these
> configurable even) settings, ability to detect removed/added services at
> runtime, user change listening over systemd logind to name but a few. Plus
> simulated unit tests for user change operations in the storage.c in which
> most of the changes are and separate unit tests simulating systemd logind
> listening. These changes have been in use since summer I think and we're
> fixing small issues here and there still. But at some point I hope I can
> tweak it to patches, and hoping also to get 1.38 upgrade ongoing next year
> as well. And these are no secrets, storage.c
> https://git.sailfishos.org/mer-core/connman/blob/master/connman/src/storage.c
> and systemd_login.c
> https://git.sailfishos.org/mer-core/connman/blob/master/connman/src/systemd_login.c
> can be checked there if there is interest and/or time.
BTW, I did try to cleanup the storage.c files beginning of the year. I
really hate the mess we have in there. The mix between provisioning and
configuration for both daemons ConnMan and VPN is just mind
boggling. How could so simple get so complex. I have a very rough
prototype which makes things way simpler. The main problem I run into
was how we support the different provisioning use case. You can either
provision VPN or ConnMan independently. This makes it really messy IMO
and it's an API issue. We cannot really cleanup this mess without
touching the D-Bus API.
> There are also many other things that should be pushed to upstream for
> review.. I guess many of them require some discussion at first. But I'll get
> back at you when it is time for them.
Looking forward to it!
Thanks,
Daniel
------------------------------
Date: Fri, 11 Dec 2020 18:02:41 +0100
From: Daniel Wagner <w...@monom.org>
Subject: [PATCH] vpn: Export vpn_ipconfig_foreach as linker symbol
To: connman@lists.01.org
Cc: Daniel Wagner <w...@monom.org>
Message-ID: <20201211170241.29915-1-w...@monom.org>
Commit 95b25140bec7 ("vpn: Add WireGuard support") introduced
a plugin dependency to __vpn_ipconfig_foreach. Because the linker does
not export the prefixed functions, we need to rename it to
vpn_ipconfig_foreach in order to export the linker symbol.
---
vpn/plugins/wireguard.c | 2 +-
vpn/vpn-ipconfig.c | 2 +-
vpn/vpn-rtnl.c | 2 +-
vpn/vpn.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index a0449b36aa43..0a86acd04a0e 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -245,7 +245,7 @@ static char *get_ifname(void)
for (i = 0; i < 256; i++) {
data.ifname = g_strdup_printf("wg%d", i);
data.found = false;
- __vpn_ipconfig_foreach(ifname_check_cb, &data);
+ vpn_ipconfig_foreach(ifname_check_cb, &data);
if (!data.found)
return data.ifname;
diff --git a/vpn/vpn-ipconfig.c b/vpn/vpn-ipconfig.c
index 8b792c1b9929..825b43c40ff5 100644
--- a/vpn/vpn-ipconfig.c
+++ b/vpn/vpn-ipconfig.c
@@ -108,7 +108,7 @@ unsigned int __vpn_ipconfig_get_flags_from_index(int index)
return ipdevice->flags;
}
-void __vpn_ipconfig_foreach(void (*function) (int index,
+void vpn_ipconfig_foreach(void (*function) (int index,
void *user_data), void *user_data)
{
GList *list, *keys;
diff --git a/vpn/vpn-rtnl.c b/vpn/vpn-rtnl.c
index 6ddfd8325252..295c05ce5363 100644
--- a/vpn/vpn-rtnl.c
+++ b/vpn/vpn-rtnl.c
@@ -184,7 +184,7 @@ int vpn_rtnl_register(struct vpn_rtnl *rtnl)
rtnl_list = g_slist_insert_sorted(rtnl_list, rtnl,
compare_priority);
- __vpn_ipconfig_foreach(trigger_rtnl, rtnl);
+ vpn_ipconfig_foreach(trigger_rtnl, rtnl);
return 0;
}
diff --git a/vpn/vpn.h b/vpn/vpn.h
index fa968f602989..477cb2272c11 100644
--- a/vpn/vpn.h
+++ b/vpn/vpn.h
@@ -35,7 +35,7 @@ struct vpn_ipconfig;
struct connman_ipaddress *__vpn_ipconfig_get_address(struct vpn_ipconfig
*ipconfig);
unsigned short __vpn_ipconfig_get_type_from_index(int index);
unsigned int __vpn_ipconfig_get_flags_from_index(int index);
-void __vpn_ipconfig_foreach(void (*function) (int index,
+void vpn_ipconfig_foreach(void (*function) (int index,
void *user_data), void *user_data);
void __vpn_ipconfig_set_local(struct vpn_ipconfig *ipconfig,
const char *address);
--
2.29.2
------------------------------
Date: Fri, 11 Dec 2020 11:48:55 +0000
From: "Holesch, Simon (GED-SDD1)" <simon.hole...@bshg.com>
Subject: Re: gdhcp: Make DHCP client timeouts suspend aware
To: Daniel Wagner <w...@monom.org>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Message-ID: <20201211114855.ga22...@rbgwcw0251.bsh.corp.bshg.com>
Content-Type: text/plain; charset="us-ascii"
On Fri, Dec 11, 2020 at 12:20, Daniel Wagner wrote:
> On Fri, Dec 11, 2020 at 11:11:15AM +0100, Daniel Wagner wrote:
> > Anyway, I think the best way forward is to either revert the commit or
> > change it to use the original clock id.
>
> I've reverted the commit. There is no point in keeping the code around
> until we figure out how to address the problem this commit to tried to
> fix.
The original code used CLOCK_MONOTONIC for timeouts, which pauses during
suspend. Therefore the lease was kept long after wakeup and after the
lease was expired. To not wake up the system with CLOCK_BOOTTIME_ALARM,
we could use CLOCK_BOOTTIME instead, which at least releases the address
right after wakeup.
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- connman@lists.01.org
To unsubscribe send an email to connman-le...@lists.01.org
------------------------------
End of connman Digest, Vol 62, Issue 17
***************************************
