Re: [RFC 0/1] dhcp: add notifier event when dhcp is renewed

2014-01-08 Thread Patrik Flykt
On Tue, 2014-01-07 at 14:29 -0800, Andrew LeCain wrote:

> I agree that really all we need is the next major event (T1 or T2
> expiring) The plugin is pretty simple and just chooses the earliest
> expiration to wake up. I included all of these statistics for
> completeness, but if you think it is better to only provide the next
> wakeup then that will make the plugin even simpler. However, we will
> also need some indication that it is okay to sleep. For example, when
> we send our renewal packet at T1 expiration, we need to keep the
> system awake for a while to wait for the timeout and and recalculate
> T1, set T2 timeout, etc.

Indeed. Only when the T1 or T2 lease reacquisition has happened, should
whatever system be notified with the new timeout.

> I would suggest then that we need two changes to the API:
> 
> 
> - void (*wakeup_changed) (struct connman_ipconfig *ipconfig); --
> notifier called whenever get_next_wakeup will return a valid sleep
> value. This will happen whenever we can safely enter sleep, such as a
> successful renew, successful rebind, or expired lease.
> 
> 
> - time_t connman_ipconfig_get_next_wakeup() -- added to ipconfig.h to
> provide the next wakeup time (min(T1, T1 retry, T2,
> lease_expiration);) Because of the accounting of T1 and T2 in gdhcp,
> it would be easiest to return an absolute time. If the time returned
> is in the past then assume we should remain awake until we get another
> wakeup_changed notifier event.

No ipconfig involvement here, please. Just something simple indicated in
a generic way after a new lease when the next time of an important event
will happen. Internal DHCP timers are not interesting to anyone.
Link-local is also of concern, with link-local the device needs to be
able to defend its autogenerated IP address. That situation must be
addressed too, and also the case with IPv6 with or without DHCP.

The other obvious catch is to figure out what needs to be done with more
than one service connecting at the same time, perhaps wait for all
connecting services? At that point let's hope the other network
technologies also can wake up the device, otherwise we have a guaranteed
weird situation with other devices knowing our IPv4 DHCP, IPv4
link-local, IPv6 or IPv6 DHCP address trying unsuccessfully to connect.
Yet another catch is how to wake up on IP packets destined to us via all
the other non-WiFi network technologies.

Cheers,

Patrik


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


Re: [RFC 0/1] dhcp: add notifier event when dhcp is renewed

2014-01-07 Thread Andrew LeCain
Thanks for the great feedback Patrik!

I agree that really all we need is the next major event (T1 or T2 expiring)
The plugin is pretty simple and just chooses the earliest expiration to
wake up. I included all of these statistics for completeness, but if you
think it is better to only provide the next wakeup then that will make the
plugin even simpler. However, we will also need some indication that it is
okay to sleep. For example, when we send our renewal packet at T1
expiration, we need to keep the system awake for a while to wait for the
timeout and and recalculate T1, set T2 timeout, etc.

I would suggest then that we need two changes to the API:

- void (*wakeup_changed) (struct connman_ipconfig *ipconfig); -- notifier
called whenever get_next_wakeup will return a valid sleep value. This will
happen whenever we can safely enter sleep, such as a successful renew,
successful rebind, or expired lease.

- time_t connman_ipconfig_get_next_wakeup() -- added to ipconfig.h to
provide the next wakeup time (min(T1, T1 retry, T2, lease_expiration);)
Because of the accounting of T1 and T2 in gdhcp, it would be easiest to
return an absolute time. If the time returned is in the past then assume we
should remain awake until we get another wakeup_changed notifier event.

(I have not sent the plugin because it's mostly system specific code, aside
from registering itself as a plugin, subscribing to the settings_changed
notifier, and the above mentioned logic to select the shortest timeout from
all ipconfigs it gets notifications from. If you'd like I can strip out our
system specific code and provide a bare bones example)

I will make these changes and submit a more functionally split patch set.
-Andrew


On Tue, Jan 7, 2014 at 1:19 AM, Patrik Flykt
wrote:

>
> Hi,
>
> On Mon, 2014-01-06 at 18:09 -0800, Andrew LeCain wrote:
> >
> > As mentioned in discussion of RFC "Publish DHCP statistics to dbus" on
> > 10/31, we have a usecase that requires knowledge of dhcp statistics
> > (specifically lease duration and lease time) to make power management
> > decisions for the system. As suggested by Marcel, we've implemented
> > this as a plugin, however there are a few changes to the core and api
> > needed to expose this data to the plugin.
> >
> > Specifically, we need accessors to request the statistics about the
> > lease and a notifier event to trigger every time the lease changes
> > (renew, rebind, etc) I have implemented this as generically as
> > possible to allow other plugins that need information about the lease
> > to use this information as well.
>
> Exposing DHCP data will not be necessary.
>
> Your system wants to sleep as long as possible. Thus ConnMan needs to
> inform the system of the next time when an event of importance takes
> place. From the DHCP perspective, this is when ConnMan needs to get back
> to the server to renew it's lease. Only the time of the next event needs
> to be communicated, not the full spectrum of related data.
>
> Start with the plugin itself. It surely communicates over D-Bus to the
> rest of the system and is quite simple. As input for the D-Bus
> communication the time of the event is needed, either absolute or
> relative.
>
> DHCP is one source of such information, but the individual DHCP timer
> values are not. Starting off with an acquired lease, the system can
> sleep until Renewing timeout T1 minus the time it takes to wake up from
> whatever deep sleep the system is in. That computation is part of the
> system and not any of ConnMan's concern, though.
>
> If reacquiring a DHCP lease after waking up is successful, T1 gets
> recomputed and the system can sleep until T1 again. If it didn't
> reacquire the DHCP lease, either a newly recomputed T1 timeout, T2
> timeout or expiry timeout is next. Thus the plugin needs to be informed
> of the minimum time of these three events and report that over D-Bus.
> Same thing if T2 timer hits, at that point T1 is of course cancelled. So
> the full set of DHCP values is not needed, only the time of the next
> event.
>
> The other funny thing is that some fixes may be needed for all the T1,
> T2 handling, at least the code does not take the T1 or T2 timeouts into
> account.
>
> So perhaps you could send the plugin first and then work backwards from
> there to extracting the next timeout for any DHCP event?
>
> Cheers,
>
> Patrik
>
>
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [RFC 0/1] dhcp: add notifier event when dhcp is renewed

2014-01-07 Thread Patrik Flykt

Hi,

On Mon, 2014-01-06 at 18:09 -0800, Andrew LeCain wrote:
> 
> As mentioned in discussion of RFC "Publish DHCP statistics to dbus" on
> 10/31, we have a usecase that requires knowledge of dhcp statistics
> (specifically lease duration and lease time) to make power management
> decisions for the system. As suggested by Marcel, we've implemented
> this as a plugin, however there are a few changes to the core and api
> needed to expose this data to the plugin.
> 
> Specifically, we need accessors to request the statistics about the
> lease and a notifier event to trigger every time the lease changes
> (renew, rebind, etc) I have implemented this as generically as
> possible to allow other plugins that need information about the lease
> to use this information as well.

Exposing DHCP data will not be necessary.

Your system wants to sleep as long as possible. Thus ConnMan needs to
inform the system of the next time when an event of importance takes
place. From the DHCP perspective, this is when ConnMan needs to get back
to the server to renew it's lease. Only the time of the next event needs
to be communicated, not the full spectrum of related data.

Start with the plugin itself. It surely communicates over D-Bus to the
rest of the system and is quite simple. As input for the D-Bus
communication the time of the event is needed, either absolute or
relative.

DHCP is one source of such information, but the individual DHCP timer
values are not. Starting off with an acquired lease, the system can
sleep until Renewing timeout T1 minus the time it takes to wake up from
whatever deep sleep the system is in. That computation is part of the
system and not any of ConnMan's concern, though.

If reacquiring a DHCP lease after waking up is successful, T1 gets
recomputed and the system can sleep until T1 again. If it didn't
reacquire the DHCP lease, either a newly recomputed T1 timeout, T2
timeout or expiry timeout is next. Thus the plugin needs to be informed
of the minimum time of these three events and report that over D-Bus.
Same thing if T2 timer hits, at that point T1 is of course cancelled. So
the full set of DHCP values is not needed, only the time of the next
event.

The other funny thing is that some fixes may be needed for all the T1,
T2 handling, at least the code does not take the T1 or T2 timeouts into
account.

So perhaps you could send the plugin first and then work backwards from
there to extracting the next timeout for any DHCP event?

Cheers,

Patrik


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


[RFC 0/1] dhcp: add notifier event when dhcp is renewed

2014-01-06 Thread Andrew LeCain
Hi,

As mentioned in discussion of RFC "Publish DHCP statistics to dbus" on 10/31,
we have a usecase that requires knowledge of dhcp statistics (specifically
lease duration and lease time) to make power management decisions for the
system. As suggested by Marcel, we've implemented this as a plugin, however
there are a few changes to the core and api needed to expose this data to
the plugin.

Specifically, we need accessors to request the statistics about the lease
and a notifier event to trigger every time the lease changes (renew, rebind,
etc) I have implemented this as generically as possible to allow other
plugins that need information about the lease to use this information as well.

Please let me know if there are any changes you'd like to see made before
accepting these changes!

Cheers,
Andrew LeCain

Andrew LeCain (1):
  dhcp: add notifier event when dhcp is renewed

 gdhcp/client.c |  158 +---
 gdhcp/gdhcp.h  |6 ++
 include/ipconfig.h |   18 ++
 src/connman.h  |6 ++
 src/dhcp.c |   75 +
 src/dhcpv6.c   |   93 +++
 src/ipconfig.c |   35 
 src/service.c  |6 ++
 8 files changed, 366 insertions(+), 31 deletions(-)

-- 
1.7.9.5

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