Re: [systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

2013-12-09 Thread Lennart Poettering
On Mon, 09.12.13 23:41, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> 
> On Mon, 2013-11-25 at 23:52 +0100, Lennart Poettering wrote:
> > Of course, timer callbacks should only be called once, ence
> > ONESHOT mode is the default -- unless you change that explicitly.
> > 
> > If this doesn't solve the issue, I wouldn't be surprised if there was
> > still a bug lurking with the sd-event code. Note that I fixed a bug
> > like this last week, did you check with currentl git of sd-event if
> > the issue persists?
> 
> Found the source of this issue. When the (monotonic) timeout function is
> called with time 0 meaning "immediately", the callback indeed gets
> called next by the event loop. But the uint64_t usec value passed to the
> timer callback is the value 0 set by the caller, not the current
> monotonic time the callback is called at. 

Yes, this is intended that way, so that for periodic usecases dispatch
latencies don't add up. Of course, in your case having the actual time
around is much more desirable.

> Using the supplied usec value,
> which is 0, as a base when calculating the next event timeout results of
> course in a time very much smaller time than the current monotonic time,
> causing the callback to be called immediately again. The value is again
> the one set by the caller and thus the calculated time is again too low.
> Rinse and repeat.
> 
> I fixed the problem by using sd_event_get_now_monotonic() instead of the
> supplied usec value as it wasn't clear what the intended behavior was.
> systemd upstream version tested was from approximately last Friday
> should something have changed recently.

Yes, sd_event_get_now_monotonic() is the right call to use here. It will
return the monotonic time of the current event loop
iteration. Alternatively you could just use now(CLOCK_MONTONIC), but
using sd_event_get_now_monotonic() helps to suppress a syscall...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

2013-12-09 Thread Patrik Flykt
On Mon, 2013-11-25 at 23:52 +0100, Lennart Poettering wrote:
> Of course, timer callbacks should only be called once, ence
> ONESHOT mode is the default -- unless you change that explicitly.
> 
> If this doesn't solve the issue, I wouldn't be surprised if there was
> still a bug lurking with the sd-event code. Note that I fixed a bug
> like this last week, did you check with currentl git of sd-event if
> the issue persists?

Found the source of this issue. When the (monotonic) timeout function is
called with time 0 meaning "immediately", the callback indeed gets
called next by the event loop. But the uint64_t usec value passed to the
timer callback is the value 0 set by the caller, not the current
monotonic time the callback is called at. Using the supplied usec value,
which is 0, as a base when calculating the next event timeout results of
course in a time very much smaller time than the current monotonic time,
causing the callback to be called immediately again. The value is again
the one set by the caller and thus the calculated time is again too low.
Rinse and repeat.

I fixed the problem by using sd_event_get_now_monotonic() instead of the
supplied usec value as it wasn't clear what the intended behavior was.
systemd upstream version tested was from approximately last Friday
should something have changed recently.

Cheers,

Patrik


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

2013-11-27 Thread Patrik Flykt

Hi,

On Mon, 2013-11-25 at 23:52 +0100, Lennart Poettering wrote:

> Of course, timer callbacks should only be called once, ence ONESHOT
> mode is the default -- unless you change that explicitly.

Event loop code said ONESHOT for timers, and I don't think I managed to
change that.

> If this doesn't solve the issue, I wouldn't be surprised if there was
> still a bug lurking with the sd-event code. Note that I fixed a bug
> like this last week, did you check with currentl git of sd-event if
> the issue persists?

I did the testing with the code from the beginning of last week, then
rebased on top of the latest so the bug may have disappeared already.
I'll take another look.

Cheers,

Patrik



___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

2013-11-25 Thread Lennart Poettering
On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> After that I noticed someting interesting when setting a monotonic timer
> event to go off at time 0. It seems to repeatedly call the timer event
> without stopping, either due to the next timer computation going wrong in
> my code or then the reshuffling of the event queue in sd-event. As I
> couldn't figure out the root cause, I left the timer setting to
> now(CLOCK_MONOTONIC) instead of the proposed value 0.

Hmm, are you aware of SD_EVENT_SOURCE_ON vs. SD_EVENT_SOURCE_ONESHOT?
The former leaves the event source on forever, and that means a timer is
dispatched over and over and over again (immediately, since the rule is
to execute a timer as soon as the configured time is passed, and that it
will be continously after it elapsed once), until it is explicitly
disabled. The latter is turned off automatically when it is first
dispatched. Of course, timer callbacks should only be called once, ence
ONESHOT mode is the default -- unless you change that explicitly.

If this doesn't solve the issue, I wouldn't be surprised if there was
still a bug lurking with the sd-event code. Note that I fixed a bug like
this last week, did you check with currentl git of sd-event if the issue
persists?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

2013-11-24 Thread Patrik Flykt
Hi,

Here is version 2 of the libsystemd-dhcp patch set fixed according to
comments. Functionality is the same as in the earlier version. I'm still
usin src/dhcp as the location for this code, but what might be the proper
directory for the code eventually? I also left it conditionally enabled
by default, should it be compiled unconditionally in the future?

I chose to drop the example DHCP client, it seems systemd-network is right
around the corner so yet another piece of code doing something very similar
is probably not very worthwhile. Thus the set shrunk with two patches.

In addition to this, I added struct sockaddr_ll to sockaddr_union in patch
11/26. It wasn't possible to use _cleanup_free_ while handling the DHCP lease
structure as freeing the structure afterwards is only to be done on error
conditions. DHCP_EVENT_NAK was renamed to the hopefully more useful
DHCP_EVENT_NO_LEASE and the public API is now in src/systemd/sd-dhcp-client.h.
I also created a function for fetching the prefix length instead of netmask.

After that I noticed someting interesting when setting a monotonic timer
event to go off at time 0. It seems to repeatedly call the timer event
without stopping, either due to the next timer computation going wrong in
my code or then the reshuffling of the event queue in sd-event. As I
couldn't figure out the root cause, I left the timer setting to
now(CLOCK_MONOTONIC) instead of the proposed value 0.



Please review,

   Patrik


Patrik Flykt (26):
  dhcp: Add DHCP protocol structures and initial defines
  dhcp: Add DHCP client initialization
  dhcp: Add test for DHCP client initialization and parameter setting
  build: Add initial build support
  dhcp: Add option appending and parsing
  dhcp: Add buffer length and invalid cookie tests for DHCP options
  build: Add DHCP option test
  dhcp: Add tests for DHCP options, file and sname fields
  dhcp: Add option append tests
  dhcp: Add test function for computing checksum
  shared: Add struct sockaddr_ll to sockaddr_union
  dhcp: Add function for sending a raw packet
  dhcp: Add DHCP discover sending
  build: Add libsystemd-dhcp
  dhcp: Add test for discover DHCP packet creation
  dhcp: Support seconds elapsed since start of DHCP negotiation
  dhcp: Add function to stop the DHCP client
  build: Add dependency on libsystemd-bus needed for main loop
  dhcp: Add timeout and main loop support
  dhcp: Handle received DHCP Offer message
  dhcp: Send DHCP Request to acquire an IP address
  dhcp: Add maximum message size option
  dhcp: Process DHCP Ack/Nak message
  dhcp: Compute expire, T1 and T2 timers
  dhcp: Add notification callback
  dhcp: Add function to free DHCP client data

 Makefile.am  |   50 +++
 configure.ac |9 +
 src/dhcp/Makefile|1 +
 src/dhcp/client.c|  981 ++
 src/dhcp/internal.h  |   41 ++
 src/dhcp/network.c   |   64 +++
 src/dhcp/option.c|  181 
 src/dhcp/protocol.h  |  119 +
 src/dhcp/test-dhcp-client.c  |  224 ++
 src/dhcp/test-dhcp-option.c  |  376 
 src/shared/socket-util.h |2 +
 src/systemd/sd-dhcp-client.h |   57 +++
 12 files changed, 2105 insertions(+)
 create mode 12 src/dhcp/Makefile
 create mode 100644 src/dhcp/client.c
 create mode 100644 src/dhcp/internal.h
 create mode 100644 src/dhcp/network.c
 create mode 100644 src/dhcp/option.c
 create mode 100644 src/dhcp/protocol.h
 create mode 100644 src/dhcp/test-dhcp-client.c
 create mode 100644 src/dhcp/test-dhcp-option.c
 create mode 100644 src/systemd/sd-dhcp-client.h

-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel