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: gdhcp: Make DHCP client timeouts suspend aware (Daniel Wagner)
   2. Re: gdhcp: Make DHCP client timeouts suspend aware (Daniel Wagner)
   3. Re: gdhcp: Make DHCP client timeouts suspend aware (Daniel Wagner)
   4. Re: gdhcp: Make DHCP client timeouts suspend aware (Daniel Wagner)
   5. Re: [PATCH] Fix time synchronization after updating timeservers
      (Daniel Wagner)
   6. Re: [PATCH] Bugfixes (Daniel Wagner)
   7. Re: [PATCH] wispr: check service before stopping portal detection
      (Daniel Wagner)
   8. Re: [PATCH 03/15] inet: Refactor with getifaddrs() and add network route 
getter function
      (Daniel Wagner)
   9. Re: [PATCH 00/15] Support SplitRouting on vpnd, drop vpnd route management
      (Daniel Wagner)


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

Date: Fri, 11 Dec 2020 11:11:15 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: gdhcp: Make DHCP client timeouts suspend aware
To: "Holesch, Simon (GED-SDD1)" <simon.hole...@bshg.com>
Cc: "connman@lists.01.org" <connman@lists.01.org>,
        "conn...@connman.net" <conn...@connman.net>
Message-ID: <20201211101115.5wotgbolwi52e...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

On Thu, Dec 10, 2020 at 10:42:40AM +0000, Holesch, Simon (GED-SDD1) wrote:
> Here are the relevant parts of RFC 2131:
> > 4.4.5 Reacquisition and expiration
> > If no DHCPACK arrives before time T2, the client moves to REBINDING
> > state and sends (via broadcast) a DHCPREQUEST message to extend its
> > lease.
> > [...]
> > If the lease expires before the client receives a DHCPACK, the client
> > moves to INIT state, MUST immediately stop any other network
> > processing and requests network initialization parameters as if the
> > client were uninitialized.
> 
> One option would be to use CLOCK_BOOTTIME (instead of
> CLOCK_BOOTTIME_ALARM, see timer_create(2)), which continues to run
> during suspend, but doesn't wake up the system. But I'm not aware of a
> way to prevent the client to use the IP address right after wakeup.
> That's why I used the CLOCK_BOOTTIME_ALARM clock.

In this case the DHCP code needs to be made aware of the power state
transitions. In hindsight, CLOCK_BOOTTIME_ALARM was not the best
choice.

Anyway, I think the best way forward is to either revert the commit or
change it to use the original clock id. This will give some time to
figure out how to teach the DHCP the power states.

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

Date: Fri, 11 Dec 2020 11:18:12 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: gdhcp: Make DHCP client timeouts suspend aware
To: guy...@me.com
Cc: connman@lists.01.org
Message-ID: <20201211101812.w6tlrvj4zhxdp...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

On Fri, Dec 11, 2020 at 05:57:49AM -0000, guy...@me.com wrote:
> You may want to change the code to trim DHCP renew intervals to
> something that is acceptable for your application. DHCP servers can be
> configured to give very short DHCP leases which appears to be the case
> in your scenario. I have worked on connman based solutions for sleepy
> battery operated devices where we don't honor DHCP intervals less than
> one hour.  But to ignore them completely can lead to serious problems
> where another device gets assigned your IP address. In the short-term,
> perhaps your user can increase the DHCP timeout. In a study of a large
> number of DHCP servers, the bulk of them will be set to 24
> hours. Short DHCP leases are one of the evils that battery operated
> and/or sleepy devices need to account for and plan for. Hope this is
> helpful.

The short-term fix is to revert the offending commit or change to use
the old clock id until we figured out how to get it working together
with power management. The DHCP code is pretty old and so far no one
complained about the problem the commit tries to address.

BTW, the commit tries to fix the very problem with the DHCP leases
timing out while the system is in suspend state. So having short DHCP
timeouts (a couple of minutes) is actually asking for troubles in this
scenario.

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

Date: Fri, 11 Dec 2020 11:38:07 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: gdhcp: Make DHCP client timeouts suspend aware
To: "Holesch, Simon (GED-SDD1)" <simon.hole...@bshg.com>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Message-ID: <20201211103807.coeuyy3dvyrb7...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

On Thu, Dec 10, 2020 at 10:41:22AM +0000, Holesch, Simon (GED-SDD1) wrote:
> I found no other way to create a issue like on GitHub.

I've tried to access the Jira instance hosted on 01.org since a long
time and I was able to login. For a long time I was unable to
login. Anyway, I took the chance and I am about to close all bug reports
because they were all stale. No one was working on them for years. So
let's try to start from a clean state :)

In theory, you need to create an account on 01.org to be able to

   https://01.org/jira/browse/CM

Can you try this?

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

Date: Fri, 11 Dec 2020 12:20:16 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: gdhcp: Make DHCP client timeouts suspend aware
To: "Holesch, Simon (GED-SDD1)" <simon.hole...@bshg.com>
Cc: connman@lists.01.org
Message-ID: <20201211112016.gcurvmbpuzukg...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

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.

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

Date: Fri, 11 Dec 2020 12:25:25 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH] Fix time synchronization after updating
        timeservers
To: Emmanuel Vautrin <emmanuel.vaut...@cpexterne.org>
Cc: connman@lists.01.org
Message-ID: <20201211112525.xpu6ckzykgbrz...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

Hi Emmanuel,

On Fri, Dec 04, 2020 at 08:54:17AM -0000, Emmanuel Vautrin wrote:
> Hello everyone,
> 
> Please find below the patch fixing this issue:

Can you explain what this is fixing? I don't understand why you need to
check default_service there again?

If default_service is valid we use it. If not we ask
connman_service_get_default() to return a default gateway. If that fails
we return -EINVAL.

Thanks,
Daniel

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

Date: Fri, 11 Dec 2020 12:33:48 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH] Bugfixes
To: Boleslaw Tokarski <boleslaw.tokar...@jolla.com>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Message-ID: <20201211113348.k5v44z5wrd5jm...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

Hi Boleslaw,

On Mon, Dec 07, 2020 at 09:06:26AM +0000, Boleslaw Tokarski wrote:
> Hi,
> 
> Bugfix patches along with their desciprions attached,

Good job fixing those bugs. I've removed the JB reference in the commit
message, as they don't have any meaning upstream. Also please consider
to send the patches with 'git send-email' :)

Anyway, all patches applied.

Thanks,
Daniel

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

Date: Fri, 11 Dec 2020 13:08:16 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH] wispr: check service before stopping portal
        detection
To: Sergey Matyukevich <geoma...@gmail.com>
Cc: connman@lists.01.org
Message-ID: <20201211120816.jtyhwgs5pvv5z...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

Hi Sergey,

On Wed, Dec 09, 2020 at 10:09:58PM +0300, Sergey Matyukevich wrote:
> Do not stop wispr unconditionally. Make sure that wispr matches service
> before stopping it. Otherwise removal of unrelevant service may cancel
> ongoing online check. For instance, this is the case for WiFi networks,
> since their services share the same index equal to network device index.
> So when unused WiFi services are removed, online check for active WiFi
> service is canceled before completion and not rescheduled.

Patch applied (I've reformatted it slightly to match the coding style in
the file more.)

Thanks,
Daniel

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

Date: Fri, 11 Dec 2020 13:45:52 +0100
From: Daniel Wagner <w...@monom.org>
Subject: Re: [PATCH 03/15] inet: Refactor with getifaddrs() and add
        network route getter function
To: Jussi Laakkonen <jussi.laakko...@jolla.com>
Cc: connman@lists.01.org
Message-ID: <20201211124552.7glk42i2do2dg...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

On Tue, Dec 08, 2020 at 12:17:49PM +0200, Jussi Laakkonen wrote:
> +static int get_interface_addresses(struct interface_address *if_addr)
> [...]
> -     memset(&ifr, 0, sizeof(ifr));
> -     ifr.ifr_ifindex = index;
> +     for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
> +             if (!ifa->ifa_addr)
> +                     continue;
>  
> -     if (ioctl(sk, SIOCGIFNAME, &ifr) < 0) {
> -             close(sk);
> -             return false;
> +             if (g_strcmp0(ifa->ifa_name, name) ||
> +                                     ifa->ifa_addr->sa_family !=
> +                                             if_addr->family)
> +                     continue;

If I understand this correctly (g_strcmp) is to make sure we are looking
at the right interface. Do we really need to do this and why can't we
use the index for it? Generally, we try to avoid the use of the
interface name as lookup key. This avoids all sorts of problems
with renaming the interface.

I guess this snipped is just refactored from existing code. But that
doesn't mean it was correct :)

The rest looks not too scary. So let me review the rest first and if
this is the only real comment from me we fix this in tree.

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

Date: Fri, 11 Dec 2020 14:27:15 +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: <20201211132715.wcpzaxic6nak2...@beryllium.lan>
Content-Type: text/plain; charset=us-ascii

Hi Jussi,

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 is also
> 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,
> for instance.
>
> 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
> criteria.
>
> 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

  WireGuard gw
  local gw
  Splittet VPN


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.

Thanks,
Daniel

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

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 15
***************************************

Reply via email to