Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via email, send a message with subject or
body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

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

Today's Topics:

   1. Re: [PATCH 00/15] Support SplitRouting on vpnd, drop vpnd route management
      (Jussi Laakkonen)


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

Date: Wed, 16 Dec 2020 16:29:12 +0200
From: Jussi Laakkonen <[email protected]>
Subject: Re: [PATCH 00/15] Support SplitRouting on vpnd, drop vpnd
        route management
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed

Hi Daniel,

Sorry for a bit late reply, been busy.

On 12/11/20 5:14 PM, Daniel Wagner wrote:
> 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 think I have sometimes hard time in keeping up with these as some are 
there for historic reasons, some are for our different devices (we have 
developer mode over USB that tends to be working differently on 
different devices because of different setups and there is different 
types of internet sharing and...), and some are just simply on top of 
our own changes based on our historic changes and... urgh, I think you 
get the point.

We do have a plan to upstream/RFC our changes in our fork. I did an 
overhaul of the differences some time ago, now it is just matter of 
getting the different tasks into the working queue. I think we'll at 
least attempt to push these bandages, fixes and improvements then. I 
think the inet_pton() use was one of the items on the list and I guess 
half of them was fixed quite a while ago.

And there are of course then those that have been fixed 
differently/require time to solve the conflict.. All of them are in our 
fork directly as commits.

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

Well that case I did not see at all. All I saw that when misconfigured 
VPNC was put as split routed it did not have any routes set and network 
route couldn't be enabled. In this case connmand then refused the change 
and signaled connman-vpnd about the current split routing value (false), 
which was then propagated to the client using the VPN D-Bus API.

Now to think of it, would it be reasonable to have the VPN D-Bus API 
SetProperty() call wait until connmand replies to the call whether the 
option could be changed or not or does this signaling approach be 
enough, as it follows the way how the rest of the VPN values are changed 
via VPN D-Bus API? I chose the current approach based on that, and the 
simplicity, although multiple signaling in case of error but in that 
case the error is caused by a misconfigured VPN..

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

Well, if you look that storage.c of ours it gets quite complex. We've 
had a change to put all VPN related content to connman-vpn (so vpn_ and 
provider_), and support for manual removal and adding of services and 
having support for adding services over D-Bus as well there are some 
other changes needed. Our storage.c also has that user change logic 
embedded as of now. It was a design decision to avoid same content on 
both connmand and vpnd side, hence both use the same. But unit testing 
that whole process was a big challenge, however, almost all of it is now 
simulated with unit tests.

I think we've had to amend the API in the old days. I'm not still aware 
of all of them we have.

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

Cheers,
  Jussi

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

Subject: Digest Footer

_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]


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

End of connman Digest, Vol 62, Issue 22
***************************************

Reply via email to