On Tue, Dec 08, 2020 at 12:17:46PM +0200, Jussi Laakkonen wrote:
SplitRouting is a VPN only setting that was managed by connmand and
saved along the vpn_*/settings. This change exposes SplitRouting value also to
vpnd, making it accessible over VPN D-Bus API as well. This allows to control
the split routing value also on VPNs that are not connected yet. And being a
VPN only value it seems natural to have it controllable over VPN D-Bus API.
This does not affect the service moving, but extends it for internal use.
In order to make this happen a lot of changes were required. First of all it
is required to have synchronization of the value changes between connmand and
vpnd. This is achieved by using service specific property changed signals sent
always when the value changes, or is being set (e.g., loading the service), on
both sides. This will result in vpnd informing about the change twice when the
SplitRouting value is changed over VPN D-Bus API. This happens because VPN
changes to connmand are propagated using property changed signals and after
connmand has processed the SplitRouting signal connmand will notify the value
again with a property changed signal to keep vpnd in sync in case of an error,
Since service moving is utilized here for making the SplitRouting value changes
on a connected VPN the route checks in plugins/vpn.c is enhanced to handle the
routes better. If a VPN that is being set as a split routed does not have any
other than the default route a network route will be attempted to be added. If
that fails an error is printed and the resulting property changed signal will
contain the current, not affected value of SplitRouting But if a VPN has a
default route configured it will be prevented for SplitRouted VPNs. Also to
make sure that a non-SplitRouted VPN will have a default route set it will be
added if missing.
And to make this happen inet.c was amended with new functionality to get route
addresses. This resulted in some other changes to clean up the code and fix
the issue of getifaddrs() not being able to interpret the destination address
if the P-t-P connection has broadcast address also set. Combined many different
getifaddrs() uses behind one function to have it a multi-purpose function for
different uses. The added, inet.c internal get_interface_addresses() uses
struct interface_address that allows AF specific addresses to be retrieved to
their respective by copying the struct ifaddrs content that matches the given
Also some minor change to service.c was required in order to propagate the
SplitRouting value at all times. When loading a service it is imperative to
apply the service settings after registering the D-Bus service because
otherwise any signals cannot be sent about the loaded settings, if needed. This
boils down to the fact that the internal gdbus implementation does not allow
signals to be sent if the path given is not registered.
Route management is also dropped from vpnd as a separate commit so it may be
reverted easily if need arises. This was deemed to be unnecessary and confusing
in discussions with Daniel Wagner on the ConnMan mailing list.
W00t, this is a lot of work! I looked through the series and all looks
reasonable sane. I spotted a few nitpicks, which I'll fix up directly
before applying it.
My not so trivial test setup is
working for IPv4. Now we just have to fix the IPv6 routing which is
completely broken. But this is something I try to address in my
'Set IPv6 default route' series.