Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[Tollef Fog Heen]

]] Dax Kelson 

> On Nov 13, 2013 8:03 AM, "Lennart Poettering" 
> wrote:
> >
> > I also have the suspicion that the best strategy for handling degraded
> > arrays is to timeout and not assemble them but rather put the system in
> > a state where the admin has to become active. Auto-assembling degraded
> > arrays has the feel of taping over issues. If the admin chooses to
> > boot-up with degraded disks then that's ok, but I am pretty sure this is
> > something the admin should explicitly decide.
> 
> As an experienced admin, I disagree with this. If I've gone to the effort
> to setup a RAID volume obviously I value high availability.

That's not obvious.  You might value data integrity.  You might value
the ability to have file systems larger than a single disk which has
some resilence against single- or double disk failures.

The problem with just continuing with a degraded RAID is for those who
run without monitoring turned on: How is the admin to know that the
system's RAID is degraded?  (Think simple home server without working
outgoing email for mdadm, no monitoring, no desktop style login where
you can pop up a notification.)

Sadly, this means that us experienced admins have to flip the defaults
because we have working email from mdadm and monitoring and alerts and
we would rather the volume be available and degraded than not available
at all.

-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journal: timestamp support on console messages

[Umut Tezduyar Lindskog]

Hi,

Thanks for taking in the patch. Using _likely_ and economic size buffer are 
correct calls. 

> -Original Message-
> From: Zbigniew Jędrzejewski-Szmek [mailto:zbys...@in.waw.pl]
> Sent: den 14 november 2013 05:28
> To: Umut Tezduyar Lindskog
> Cc: systemd-devel@lists.freedesktop.org; Umut Tezduyar Lindskog
> Subject: Re: [systemd-devel] [PATCH] journal: timestamp support on console
> messages
> 
> On Wed, Nov 13, 2013 at 03:27:19PM +0100, Umut Tezduyar Lindskog wrote:
> > ---
> >  src/journal/journald-console.c |   33
> ++---
> >  1 files changed, 30 insertions(+), 3 deletions(-)
> Applied, but changed prefix_timestamp to be more like on_tty(), and make
> use of DECIMAL_STR_MAX. Please verify that I didn't screw up your patch :)
> 
> Zbyszek
> 
> 
> 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Fix PAM module to not clobber XDG_RUNTIME_DIR with su

[Martin Pitt]

Hello all,

pam_systemd currently causes some havoc when you run programs or
shells with su: it passes on the $XDG_RUNTIME_DIR from the original
user session, so that programs like pulseaudio or dconf end up
scribbling into the original user's runtime dir. This has been
discussed at length at [1][2] and is leading people to consider
workarounds like [3].

It seems Lennart is against giving the new user a new logind session
and runtime dir; I think it would be right to give it a fresh (or an
already existing one for the target user) runtime dir, but in either
case passing it the original user's runtime dir is actively wrong and
harmful.

Until then I recommend applying this patch (or something equivalent)
which at least stops destroying existing runtime dirs and makes it
compliant to the spec [4]. With that, things like pulse, dconf, or
dbus will still need to keep their internal fallback if there is no
runtime dir, but that's a less pressing matter.

Thanks for considering,

Martin

[1] https://bugzilla.redhat.com/show_bug.cgi?id=753882
[2] https://launchpad.net/bugs/1197395
[3] 
http://lists.freedesktop.org/archives/pulseaudio-discuss/2013-November/019121.html
[4] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From f33647a5370ef2dc55f8e077c545adf937686f10 Mon Sep 17 00:00:00 2001
From: Martin Pitt 
Date: Wed, 13 Nov 2013 13:02:28 +0100
Subject: [PATCH] pam: Check $XDG_RUNTIME_DIR owner

http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html requires
that $XDG_RUNTIME_DIR "MUST be owned by the user, and he MUST be the only one
having read and write access to it.".

Don't set an existing $XDG_RUNTIME_DIR in the PAM module if it isn't owned by
the session user. Otherwise su sessions get a runtime dir from a different user
which leads to either permission errors or scribbling over the other user's
files.

https://bugzilla.redhat.com/show_bug.cgi?id=753882
https://launchpad.net/bugs/1197395
---
 src/login/pam-module.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/login/pam-module.c b/src/login/pam-module.c
index 1975d80..d80967c 100644
--- a/src/login/pam-module.c
+++ b/src/login/pam-module.c
@@ -194,6 +194,7 @@ _public_ PAM_EXTERN int pam_sm_open_session(
 uint32_t uid, pid, vtnr = 0;
 bool debug = false, remote;
 struct passwd *pw;
+struct stat st;
 
 assert(handle);
 
@@ -385,10 +386,24 @@ _public_ PAM_EXTERN int pam_sm_open_session(
 return r;
 }
 
-r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", runtime_path, 0);
-if (r != PAM_SUCCESS) {
-pam_syslog(handle, LOG_ERR, "Failed to set runtime dir.");
-return r;
+/* only set $XDG_RUNTIME_DIR if it is owned by the target user, as per
+ * XDG basedir-spec; this avoids su sessions to scribble over a runtime
+ * dir of a different user */
+r = lstat(runtime_path, &st);
+if (r != 0) {
+pam_syslog(handle, LOG_ERR, "Failed to stat runtime dir: %s", strerror(errno));
+return PAM_SYSTEM_ERR;
+}
+
+if (st.st_uid == uid) {
+r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", runtime_path, 0);
+if (r != PAM_SUCCESS) {
+pam_syslog(handle, LOG_ERR, "Failed to set runtime dir.");
+return r;
+}
+} else {
+pam_syslog(handle, LOG_DEBUG, "Runtime dir %s is not owned by the target uid %u, ignoring.",
+   runtime_path, uid);
 }
 
 if (!isempty(seat)) {
-- 
1.8.4.3



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

Re: [systemd-devel] [PATCH] Add bootctl man page, zsh completion

[Marko Myllynen]

Hi,

On 2013-11-14 06:30, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Nov 13, 2013 at 11:06:13AM +0200, Marko Myllynen wrote:
>>
>> please consider the attached bootctl man page to fix RHBZ#1014303. As a
>> bonus I'm also attaching zsh completion rules for bootctl.
> Applied, thanks.
> 
> In the future, please provide patches separately, and after adding
> man pages, generate them with 'make man/whatever.x' and then run
> 'make update-man-list' and include the changes to Makefile-man.am.

sure, thanks for your prompt response and the finishing touches!

Cheers,

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

Re: [systemd-devel] [systemd-bugs] Russian translation for systemd

[Zbigniew Jędrzejewski-Szmek]

On Thu, Nov 14, 2013 at 08:44:57AM +0400, Juliette Tux wrote:
> Hello gentlemen,
> Whom should I contact in order to add Russian localization to systemd?
> Thank you.
Just prepare a patch adding po/ru.po (like the stub po/pl.po)
and send it to systemd-devel@lists.freedesktop.org. AFAIK,
nobody has been working on this. The plan has been to support translations
all along, but just nobody has done the work. I'm not even sure if the
template file is up-to-date... you should verify that first.

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

Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[Alexander E. Patrakov]

NeilBrown пишет:

On Wed, 13 Nov 2013 22:11:27 +0600 "Alexander E. Patrakov"
 wrote:


2013/11/13 NeilBrown :

On Tue, 12 Nov 2013 19:01:49 +0400 Andrey Borzenkov 
wrote:


Something like

mdadm-last-resort@.timer

[Timer]
OnCalendar=+5s

mdadm-last-resort@.service

[Service]
Type=oneshot
ExecStart=/sbin/mdadm -IRs %n

udev rule

... SYSTEMD_WANTS=mdadm-last-resort@$ENV{SOMETHING_UNIQUE}.timer


Thanks.  This certainly looks interesting and might be part of a solution.
However it gets the timeout test backwards.

I don't want to set the timeout when the array starts to appear.  I want to
set the time out when someone wants to use the array.
If no-one is waiting for the array device, then there is no point forcing it.

That's why I want to plug into the timeout that systemd already has.

Maybe that requirement isn't really necessary though.  I'll experiment with
your approach.

It is useless to even try to plug into the existing systemd timeout,
for a very simple reason. in the setups where your RAID array is not
on the top of the storage device hierarchy, systemd does not know that
it wants your RAID array to appear.

So the statement "If no-one is waiting for the array device, then
there is no point forcing it" is false, because there is no way to
know that no-one is waiting.


"useless" seems a bit harsh.  "not-optimal" may be true.

If systemd was waiting for a device, then it is clear that something was
waiting for something.  In this case it might be justified to activate as
much as possible in the hope that the important things will get activated.
This is what "mdadm -IRs" does.  It activates all arrays that are still
inactive but have enough devices to become active (though degraded).  It
isn't selective.
If they are deep in some hierarchy, then udev will pull it all together and
the root device will appear.

If systemd is not waiting for a device, then there is no justification for
prematurely starting degraded arrays.


Maybe I could get emergency.service to run "mdadm -IRs" and if that actually
started anything, then to somehow restart local-fs.target.  Might that be
possible?


If this is the way forward, then we, obviously, should think about a 
general mechanism that is useful not only for mdadm, but also to other 
layered storage implementations such as dm-raid, or maybe multi-device 
btrfs, and that is useful if more than one of these technologies are 
used on top of each other. This by necessity leads to multiple emergency 
missing-device handlers. And then a question immediately appears, in 
which order the emergency handlers should be tried, because all that is 
known at the time of emergency is that some device listed in /etc/fstab 
is missing. I suspect that the answer is "in arbitrary order" or even 
"in parallel", but then there is a chance that one run of all of them 
will not be enough.


This is not a criticism, just something to be fully thought out before 
starting an implementation.


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

[systemd-devel] [PATCHv2 2/2] zsh-completion: Function declarations follow style

[William Giokas]

Follow the instructions in CODING_STYLE for the zsh completion
functions.
---
 shell-completion/zsh/_hostnamectl |  3 ++-
 shell-completion/zsh/_journalctl  | 12 
 shell-completion/zsh/_kernel-install  |  6 --
 shell-completion/zsh/_localectl   | 12 
 shell-completion/zsh/_sd_machines |  3 ++-
 shell-completion/zsh/_systemctl   |  9 ++---
 shell-completion/zsh/_systemd-analyze |  6 --
 shell-completion/zsh/_systemd-coredumpctl |  3 ++-
 shell-completion/zsh/_systemd-delta   |  3 ++-
 shell-completion/zsh/_systemd-inhibit |  6 --
 shell-completion/zsh/_systemd-nspawn  |  3 ++-
 shell-completion/zsh/_systemd-run |  9 ++---
 shell-completion/zsh/_timedatectl | 15 ++-
 shell-completion/zsh/_udevadm | 27 ++-
 14 files changed, 78 insertions(+), 39 deletions(-)

diff --git a/shell-completion/zsh/_hostnamectl 
b/shell-completion/zsh/_hostnamectl
index 7d7baeb..c7a5ec0 100644
--- a/shell-completion/zsh/_hostnamectl
+++ b/shell-completion/zsh/_hostnamectl
@@ -1,6 +1,7 @@
 #compdef hostnamectl
 
-_hostnamectl_command() {
+_hostnamectl_command()
+{
 local -a _hostnamectl_cmds
 _hostnamectl_cmds=(
 "status:Show current hostname settings"
diff --git a/shell-completion/zsh/_journalctl b/shell-completion/zsh/_journalctl
index b518e35..fcb7874 100644
--- a/shell-completion/zsh/_journalctl
+++ b/shell-completion/zsh/_journalctl
@@ -1,6 +1,7 @@
 #compdef journalctl
 
-_list_fields() {
+_list_fields()
+{
 local -a journal_fields
 journal_fields=(MESSAGE{,_ID} PRIORITY CODE_{FILE,LINE,FUNC}
 ERRNO SYSLOG_{FACILITY,IDENTIFIER,PID}
@@ -20,7 +21,8 @@ _list_fields() {
 esac
 }
 
-_journal_none() {
+_journal_none()
+{
 local -a _commands _files _jrnl_none
 # Setting use-cache will slow this down considerably
 _commands=( ${"$(_call_program commands "$service" -F _EXE 
2>/dev/null)"} )
@@ -31,7 +33,8 @@ _journal_none() {
 'fields:fields:_list_fields'
 }
 
-_journal_fields() {
+_journal_fields()
+{
 local -a _fields cmd
 cmd=("journalctl" "-F ${@[-1]}" "2>/dev/null" )
 _fields=( ${(f)"$(_call_program fields $cmd[@])"} )
@@ -39,7 +42,8 @@ _journal_fields() {
 _describe 'possible values' _fields
 }
 
-_journal_boots() {
+_journal_boots()
+{
 local -a _bootid _previousboots
 _bootid=( ${(fao)"$(_call_program bootid "$service -F _BOOT_ID")"}  )
 _previousboots=( -{1..${#_bootid}} )
diff --git a/shell-completion/zsh/_kernel-install 
b/shell-completion/zsh/_kernel-install
index 3bc29fd..2f1adb5 100644
--- a/shell-completion/zsh/_kernel-install
+++ b/shell-completion/zsh/_kernel-install
@@ -1,6 +1,7 @@
 #compdef kernel-install
 
-_images(){
+_images()
+{
 if [[ "$words[2]" == "remove" ]]; then
 _message 'No more options'
 else
@@ -8,7 +9,8 @@ _images(){
 fi
 }
 
-_kernels(){
+_kernels()
+{
 read _MACHINE_ID < /etc/machine-id
 _kernel=( /lib/modules/[0-9]* )
 if [[ "$cmd" == "remove" && -n "$_MACHINE_ID" ]]; then
diff --git a/shell-completion/zsh/_localectl b/shell-completion/zsh/_localectl
index 1e3cf03..b94c9d3 100644
--- a/shell-completion/zsh/_localectl
+++ b/shell-completion/zsh/_localectl
@@ -1,6 +1,7 @@
 #compdef localectl
 
-_localectl_set-locale() {
+_localectl_set-locale()
+{
 local -a _confs _locales
 local expl suf
 _locales=( ${(f)"$(_call_program locales "$service" list-locales)"} )
@@ -15,7 +16,8 @@ _localectl_set-locale() {
 fi
 }
 
-_localectl_set-keymap() {
+_localectl_set-keymap()
+{
 local -a _keymaps
 _keymaps=( ${(f)"$(_call_program locales "$service" list-keymaps)"} )
 if (( CURRENT <= 3 )); then
@@ -25,7 +27,8 @@ _localectl_set-keymap() {
 fi
 }
 
-_localectl_set-x11-keymap() {
+_localectl_set-x11-keymap()
+{
 if (( $+commands[pkg-config] )); then
 local -a _file _layout _model _variant _options
 local _xorg_lst
@@ -50,7 +53,8 @@ _localectl_set-x11-keymap() {
 fi
 }
 
-_localectl_command() {
+_localectl_command()
+{
 local -a _localectl_cmds
 _localectl_cmds=(
 'status:Show current locale settings'
diff --git a/shell-completion/zsh/_sd_machines 
b/shell-completion/zsh/_sd_machines
index df2f462..e2e5c04 100644
--- a/shell-completion/zsh/_sd_machines
+++ b/shell-completion/zsh/_sd_machines
@@ -1,5 +1,6 @@
 #autoload
-__get_machines () {
+__get_machines()
+{
 machinectl --full --no-pager list | {while read -r a b; do echo $a; 
done;};
 }
 
diff --git a/shell-completion/zsh/_systemctl b/shell-completion/zsh/_systemctl
index 133a550..1ab66ce 100644
--- a/shell-completion/zsh/_systemctl
+++ b/shell-completion/zsh/_systemctl
@@ -121,7 +121,8 @@ _systemctl_really_all_units()

[systemd-devel] [PATCHv2 0/2] zsh-completion style fixes

[William Giokas]

Just style changes to the zsh completion. If either of these patches
are too large, you can view them at

  http://git.kaictl.net/wgiokas/systemd.git/?h=zsh-stylefix

v2: Added fixes to _bootctl

William Giokas (2):
  zsh-completion: Use same tab-width for all files
  zsh-completion: Function declarations follow style

 shell-completion/zsh/_bootctl  |  33 +-
 shell-completion/zsh/_hostnamectl  |  52 +--
 shell-completion/zsh/_journalctl   | 169 -
 shell-completion/zsh/_kernel-install   |  38 +-
 shell-completion/zsh/_localectl| 143 
 shell-completion/zsh/_loginctl | 133 +++
 shell-completion/zsh/_machinectl   |  71 ++--
 shell-completion/zsh/_sd_hosts_or_user_at_host |   5 +-
 shell-completion/zsh/_sd_machines  |   4 +-
 shell-completion/zsh/_sd_outputmodes   |   1 +
 shell-completion/zsh/_systemctl| 466 +
 shell-completion/zsh/_systemd  | 154 
 shell-completion/zsh/_systemd-analyze  |  71 ++--
 shell-completion/zsh/_systemd-coredumpctl  |  60 ++--
 shell-completion/zsh/_systemd-delta|  22 +-
 shell-completion/zsh/_systemd-inhibit  |  53 +--
 shell-completion/zsh/_systemd-nspawn   |  40 ++-
 shell-completion/zsh/_systemd-run  |  10 +-
 shell-completion/zsh/_systemd-tmpfiles |  15 +-
 shell-completion/zsh/_timedatectl  | 106 +++---
 shell-completion/zsh/_udevadm  | 234 +++--
 21 files changed, 969 insertions(+), 911 deletions(-)

-- 
1.8.5.rc1.17.g0ecd94d

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

Re: [systemd-devel] [PATCH 0/3] Fix issues re: visibility of status messages

[Zbigniew Jędrzejewski-Szmek]

On Fri, Sep 20, 2013 at 10:18:27PM +0200, Olivier Brunel wrote:
> Hi,
> 
> I'm running Arch Linux, have been using systemd-204, and recently tried the 
> new
> 207 release, and I have been having some issues with it. One was that status
> messages would just stop at some point near the end of the boot process, and
> also that I wouldn't get any during a shutdown/reboot.
> 
> It might be useful to note that I don't start a getty on tty1, which is why I
> expect to see all status messages until default target is reached, even after
> the getty/login has been started (which happens on tty2).
> 
> After looking into it, I came up with the following patches to fix the issue.
> The reason status messages would stop was that the getty was started, and
> systemd then stopped using the console to avoid "collisions" w/ gettys.
> 
> However, as I said I don't have a getty started on tty1 so for me that is a 
> bug,
> as there's no reason not to keep printing status messages on tty1.
> 
> The lack of messages on shutdown/reboot was also linked to this, because if
> no_console_output was set to true during boot, it'd stay there and prevent
> messages to show up on shutdown.
> 
> To fix this (in the event it was set to true on boot) a patch simply resets it
> to false on job_shutdown_magic(), but I'm not exactly sure if that's the right
> way to do this.
All 3 patches applied. I *think* they are all correct, but this code
has so many corner cases that it's hard to be sure. I made some
tweaks, please check that it still works. Sorry for the delay. In the
future, if you don't get an answer within a week or two, please holler :)
Patches do sometimes slip through, especially when there are a lot
of changes like recently, and a ping to the ml will help to bring the
thread to the bottom. 

> FYI I should add that in a similar setup as the one I described, this will not
> be enough to keep messages on tty1, since fsck's units are now RemainAfterExit
> (see https://bugs.freedesktop.org/show_bug.cgi?id=66784), which means they're
> seen by systemd as "owning" the console (as far as outputing messages there is
> concerned I mean), and it will therefore stop printing status messages.
> 
> I'm not sure you want to "fix" this, as it might be only a cosmetic issue for 
> a
> small usecase hence not worth the trouble, so I've simply "undone" it using a
> .conf file on my end, figured I should mention it though.
Hm, we could detect this case by looking at services in the SERVICE_EXITED
substate. It might actually be worth fixing, since almost everything now
is RemainAfterExit=true.

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

Re: [systemd-devel] [PATCH] Add bootctl man page, zsh completion

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:06:13AM +0200, Marko Myllynen wrote:
> Hi,
> 
> please consider the attached bootctl man page to fix RHBZ#1014303. As a
> bonus I'm also attaching zsh completion rules for bootctl.
Applied, thanks.

In the future, please provide patches separately, and after adding
man pages, generate them with 'make man/whatever.x' and then run
'make update-man-list' and include the changes to Makefile-man.am.

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

Re: [systemd-devel] [PATCH] journal: timestamp support on console messages

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 03:27:19PM +0100, Umut Tezduyar Lindskog wrote:
> ---
>  src/journal/journald-console.c |   33 ++---
>  1 files changed, 30 insertions(+), 3 deletions(-)
Applied, but changed prefix_timestamp to be more like on_tty(),
and make use of DECIMAL_STR_MAX. Please verify that I didn't
screw up your patch :)

Zbyszek


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

Re: [systemd-devel] [PATCH] nspawn does not correctly handle I/O redirection

[Zbigniew Jędrzejewski-Szmek]

On Tue, Nov 05, 2013 at 07:59:46PM +0100, Lennart Poettering wrote:
> On Mon, 04.11.13 11:05, Luke T. Shumaker (luke...@sbcglobal.net) wrote:
> 
> > A couple of weeks ago, I reported a bug that systemd-nspawn does not
> > correctly handle I/O redirection[1].
> > 
> > I described in detail the several smaller bugs that lead up to both
> > stdin and stdout redirection being broken, and uploaded a patch that
> > fixes stdout redirection.  That patch is attached here.  Stdin
> > redirection is more involved to fix.
> > 
> > It has been several weeks, and no one has acknowledged the bug
> > report, so I figured I would try another channel.
> 
> Hmm, can you rebase on current git please?
Ping?

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

Re: [systemd-devel] [PATCH 08/28] dhcp: Add tests for DHCP options, file and sname fields

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:36PM +0200, Patrik Flykt wrote:
> Add a structure describing the DHCP file, sname and trailing options
> fields. Create a messge holding these fields and call the internal
> option parsing function.
> 
> In the test callback function verify that only regular options are
> passed and figure out which part of the DHCP message is the one that
> is being processed. As the test program knows the full contents of
> the test options in the test structure, skip all non-regular fields
> and verify that the option provided to the callback indeed is the
> one expected. Check also if non-regular option fields are to be
> ignored in the end of the option field as the callback is not called
> again and the final check when the whole message has been processed
> needs to be successful.
> 
> Add a boolean flag for pretty-printing, anticipate there will be a
> nice option to toggle it in the future.
> ---
>  src/dhcp/test-dhcp-option.c |  258 
> +++
>  1 file changed, 258 insertions(+)
> 
> diff --git a/src/dhcp/test-dhcp-option.c b/src/dhcp/test-dhcp-option.c
> index 9302fd6..a2b79ce 100644
> --- a/src/dhcp/test-dhcp-option.c
> +++ b/src/dhcp/test-dhcp-option.c
> @@ -10,6 +10,68 @@
>  #include "protocol.h"
>  #include "internal.h"
>  
> +struct option_desc {
> +uint8_t sname[64];
> +int snamelen;
> +uint8_t file[128];
> +int filelen;
> +uint8_t options[128];
> +int len;
> +bool success;
> +int filepos;
> +int snamepos;
> +int pos;
> +};
> +
> +static bool verbose = false;
> +
> +static struct option_desc option_tests[] = {
> +{ {}, 0, {}, 0, { 42, 5, 65, 66, 67, 68, 69 }, 7, false, },
> +{ {}, 0, {}, 0, { 42, 5, 65, 66, 67, 68, 69, 0, 0,
> +  DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK }, 12, true, 
> },
> +{ {}, 0, {}, 0, { 8, 255, 70, 71, 72 }, 5, false, },
> +{ {}, 0, {}, 0, { 0x35, 0x01, 0x05, 0x36, 0x04, 0x01, 0x00, 0xa8,
> +  0xc0, 0x33, 0x04, 0x00, 0x01, 0x51, 0x80, 0x01,
> +  0x04, 0xff, 0xff, 0xff, 0x00, 0x03, 0x04, 0xc0,
> +  0xa8, 0x00, 0x01, 0x06, 0x04, 0xc0, 0xa8, 0x00,
> +  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, },
> +  40, true, },
> +{ {}, 0, {}, 0, { DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_OFFER,
> +  42, 3, 0, 0, 0 }, 8, true, },
> +{ {}, 0, {}, 0, { 42, 2, 1, 2, 44 }, 5, false, },
> +
> +{ {}, 0,
> +  { 222, 3, 1, 2, 3, DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_NAK }, 8,
> +  { DHCP_OPTION_OVERLOAD, 1, DHCP_OVERLOAD_FILE }, 3, true, },
> +
> +{ { 1, 4, 1, 2, 3, 4, DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK }, 9,
> +  { 222, 3, 1, 2, 3 }, 5,
> +  { DHCP_OPTION_OVERLOAD, 1,
> +DHCP_OVERLOAD_FILE|DHCP_OVERLOAD_SNAME }, 3, true, },
> +};
> +
> +static const char *dhcp_type(int type)
> +{
> +switch(type) {
> +case DHCP_DISCOVER:
> +return "DHCPDISCOVER";
> +case DHCP_OFFER:
> +return "DHCPOFFER";
> +case DHCP_REQUEST:
> +return "DHCPREQUEST";
> +case DHCP_DECLINE:
> +return "DHCPDECLINE";
> +case DHCP_ACK:
> +return "DHCPACK";
> +case DHCP_NAK:
> +return "DHCPNAK";
> +case DHCP_RELEASE:
> +return "DHCPRELEASE";
> +default:
> +return "unknown";
> +}
> +}
> +
>  static void test_invalid_buffer_length(void)
>  {
>  DHCPMessage message;
> @@ -43,10 +105,206 @@ static void test_cookie(void)
>  free(message);
>  }
>  
> +static DHCPMessage *create_message(uint8_t *options, uint16_t optlen,
> +uint8_t *file, uint8_t filelen,
> +uint8_t *sname, uint8_t snamelen)
> +{
> +DHCPMessage *message;
> +int len = sizeof(DHCPMessage) + 4 + optlen;
> +uint8_t *opt;
> +
> +message = malloc0(len);
> +opt = (uint8_t *)(message + 1);
> +
> +opt[0] = 99;
> +opt[1] = 130;
> +opt[2] = 83;
> +opt[3] = 99;
> +
> +if (options && optlen)
> +memcpy(&opt[4], options, optlen);
> +
> +if (file && filelen <= 128)
> +memcpy(&message->file, file, filelen);
> +
> +if (sname && snamelen <= 64)
> +memcpy(&message->sname, sname, snamelen);
> +
> +return message;
> +}
> +
> +static void test_ignore_opts(uint8_t *descoption, int *descpos, int *desclen)
> +{
> +while (*descpos < *desclen) {
> +switch(descoption[*descpos]) {
> +case DHCP_OPTION_PAD:
> +*descpos += 1;
> +break;
> +
> +case DHCP_OPTION_MESSAGE_TYPE:
> +case DHCP_OPTION_OVERLOAD:
> + 

Re: [systemd-devel] [PATCH 10/28] dhcp: Add test function for computing checksum

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:38PM +0200, Patrik Flykt wrote:
> ---
>  src/dhcp/test-dhcp-client.c |   40 
>  1 file changed, 40 insertions(+)
> 
> diff --git a/src/dhcp/test-dhcp-client.c b/src/dhcp/test-dhcp-client.c
> index 26857b6..94f576a 100644
> --- a/src/dhcp/test-dhcp-client.c
> +++ b/src/dhcp/test-dhcp-client.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "protocol.h"
>  #include "client.h"
> @@ -73,9 +74,48 @@ static void test_request_basic(void)
>  assert(dhcp_client_set_request_option(client, 44) == 0);
>  }
>  
> +static uint16_t client_checksum(void *buf, int len)
> +{
> +uint32_t sum;
   = 0;
> +uint16_t *check;
= buf;
> +int i;
> +uint8_t *odd;
> +
> +sum = 0;
> +check = buf;
> +
> +for (i = 0; i < len / 2 ; i++)
> +sum += check[i];
> +
> +if (len & 0x01) {
> +odd = buf;
> +sum += odd[len];
> +}
> +
> +return ~((sum & 0x) + (sum >> 16));
> +}
> +
> +static void test_checksum(void)
> +{
> +uint8_t buf[20] = {
> +0x45, 0x00, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00,
> +0x40, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +0xff, 0xff, 0xff, 0xff
> +};
> +
> +uint8_t check[2] = {
> +0x78, 0xae
> +};
> +
> +uint16_t *val = (uint16_t *)check;
No need to cast here.

> +
> +assert(client_checksum(&buf, 20) == *val);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  test_request_basic();
> +test_checksum();
>  
>  return 0;
>  }

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

Re: [systemd-devel] [PATCH 28/28] dhcp: Use callback in test client

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:56PM +0200, Patrik Flykt wrote:
> Print out the event received and possibly also IP related data.
> ---
>  src/dhcp/dhcp-example-client.c |   53 
> 
>  1 file changed, 53 insertions(+)
> 
> diff --git a/src/dhcp/dhcp-example-client.c b/src/dhcp/dhcp-example-client.c
> index afb7102..3635bd8 100644
> --- a/src/dhcp/dhcp-example-client.c
> +++ b/src/dhcp/dhcp-example-client.c
> @@ -24,6 +24,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  
>  #include "client.h"
>  
> @@ -79,6 +82,52 @@ static int get_index(char *ifname)
>  return ifr.ifr_ifindex;
>  }
>  
> +static const char *print_event(int event)
Can this be called event_to_string?

> +{
> +if (event < 0)
> +return strerror(-event);
> +
> +switch (event) {
> +case DHCP_EVENT_STOP:
> +return "Stopped";
> +case DHCP_EVENT_NAK:
> +return "DHCP NAK";
> +case DHCP_EVENT_IP_ACQUIRE:
> +return "DHCP address acquired";
> +case DHCP_EVENT_IP_CHANGE:
> +return "DHCP address changed";
> +case DHCP_EVENT_EXPIRED:
> +return "DHCP lease expired";
> +default:
> +break;
> +}
> +
> +return "unknown";
> +}
> +
> +static void print_state(DHCPClient *client, int event, void *userdata)
> +{
> +sd_event *e = userdata;
> +struct in_addr addr;
> +
> +if (event < 0) {
> +printf("Error %d %s\n", event, print_event(event));
> +sd_event_unref(e);
> +return;
> +}
So e is unrefed only sometimes... This seems confusing.

> +
> +printf("Event %s\n", print_event(event));
> +
> +if (dhcp_client_get_address(client, &addr) >= 0)
> +printf("Address %s\n", inet_ntoa(addr));
> +
> +if (dhcp_client_get_netmask(client, &addr) >= 0)
> +printf("Netmask %s\n", inet_ntoa(addr));
> +
> +if (dhcp_client_get_router(client, &addr) >= 0)
> +printf("Default router %s\n", inet_ntoa(addr));
> +}
> +
>  int main(int argc, char **argv)
>  {
>  DHCPClient *client;
> @@ -109,9 +158,13 @@ int main(int argc, char **argv)
>  printf("Interface %s index %d\n", argv[1], index);
>  dhcp_client_set_index(client, index);
>  dhcp_client_set_mac(client, &mac);
> +dhcp_client_set_callback(client, print_state, &event);
>  
>  dhcp_client_start(client);
>  sd_event_loop(event);
>  
> +dhcp_client_free(client);
> +printf("Exit\n");
> +
>  return 0;
>  }
Hm, since this is just example code, then probably it doesn't matter that
much :)

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

Re: [systemd-devel] [PATCH 24/28] dhcp: Process DHCP Ack/Nak message

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:52PM +0200, Patrik Flykt wrote:
> Process a DHCP Ack/Nak in much the same way as an DHCP Offer. Factor
> out header verification and process options sent. Add notification
> functionality with discrete values for the outcome of the DHCP Ack/
> Nak processing.
> ---
>  src/dhcp/client.c |  145 
> +
>  src/dhcp/client.h |4 ++
>  2 files changed, 128 insertions(+), 21 deletions(-)
> 
> diff --git a/src/dhcp/client.c b/src/dhcp/client.c
> index 45c62f3..669804a 100644
> --- a/src/dhcp/client.c
> +++ b/src/dhcp/client.c
> @@ -151,6 +151,11 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
> ether_addr *addr)
>  return 0;
>  }
>  
> +static int client_notify(DHCPClient *client, int event)
> +{
> +return 0;
> +}
> +
>  static int client_stop(DHCPClient *client, int error)
>  {
>  if (!client)
> @@ -178,6 +183,7 @@ static int client_stop(DHCPClient *client, int error)
>  
>  case DHCP_STATE_INIT:
>  case DHCP_STATE_SELECTING:
> +case DHCP_STATE_REQUESTING:
>  
>  client->start_time = 0;
>  client->state = DHCP_STATE_INIT;
> @@ -185,7 +191,6 @@ static int client_stop(DHCPClient *client, int error)
>  
>  case DHCP_STATE_INIT_REBOOT:
>  case DHCP_STATE_REBOOTING:
> -case DHCP_STATE_REQUESTING:
>  case DHCP_STATE_BOUND:
>  case DHCP_STATE_RENEWING:
>  case DHCP_STATE_REBINDING:
> @@ -499,41 +504,52 @@ static int client_parse_offer(uint8_t code, uint8_t 
> len, uint8_t *option,
>  return 0;
>  }
>  
> -static int client_receive_offer(DHCPClient *client,
> -DHCPPacket *offer, int len)
> +static int client_verify_headers(DHCPClient *client,
> + DHCPPacket *message, int len)
>  {
>  int hdrlen;
> -DHCPLease *lease;
>  
>  if (len < (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
>  return -EINVAL;
>  
> -hdrlen = offer->ip.ihl * 4;
> -if (hdrlen < 20 || hdrlen > len || client_checksum(&offer->ip,
> +hdrlen = message->ip.ihl * 4;
> +if (hdrlen < 20 || hdrlen > len || client_checksum(&message->ip,
> hdrlen))
>  return -EINVAL;
>  
> -offer->ip.check = offer->udp.len;
> -offer->ip.ttl = 0;
> +message->ip.check = message->udp.len;
> +message->ip.ttl = 0;
>  
> -if (hdrlen + ntohs(offer->udp.len) > len ||
> -client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
> +if (hdrlen + ntohs(message->udp.len) > len ||
> +client_checksum(&message->ip.ttl, ntohs(message->udp.len) + 12))
>  return -EINVAL;
>  
> -if (ntohs(offer->udp.source) != DHCP_PORT_SERVER ||
> -ntohs(offer->udp.dest) != DHCP_PORT_CLIENT)
> +if (ntohs(message->udp.source) != DHCP_PORT_SERVER ||
> +ntohs(message->udp.dest) != DHCP_PORT_CLIENT)
>  return -EINVAL;
>  
> -if (offer->dhcp.op != BOOTREPLY)
> +if (message->dhcp.op != BOOTREPLY)
>  return -EINVAL;
>  
> -if (ntohl(offer->dhcp.xid) != client->xid)
> +if (ntohl(message->dhcp.xid) != client->xid)
>  return -EINVAL;
>  
> -if (memcmp(&offer->dhcp.chaddr[0], 
> &client->mac_addr.ether_addr_octet,
> +if (memcmp(&message->dhcp.chaddr[0], 
> &client->mac_addr.ether_addr_octet,
>  ETHER_ADDR_LEN))
>  return -EINVAL;
>  
> +return 0;
> +}
> +
> +static int client_receive_offer(DHCPClient *client, DHCPPacket *offer, int 
> len)
> +{
> +int err;
> +DHCPLease *lease;
> +
> +err = client_verify_headers(client, offer, len);
> +if (err < 0)
> +return err;
> +
>  lease = new0(DHCPLease, 1);
>  if (!lease)
>  return -ENOBUFS;
> @@ -561,6 +577,64 @@ error:
>  return -ENOMSG;
>  }
>  
> +static int client_receive_ack(DHCPClient *client, DHCPPacket *offer, int len)
> +{
> +int err = -ENOMSG;
Since err is not an error always, but a sucess return value too,
it would be nicer to call it r. Also this value is override right below.

You might consider the following pattern:
  _cleanup_free_  DHCPLease *lease = NULL;

and ...

> +DHCPLease *lease;
> +int type;
> +
> +err = client_verify_headers(client, offer, len);
> +if (err < 0)
> +return err;
> +
> +lease = new0(DHCPLease, 1);
> +if (!lease)
> +return -ENOBUFS;
> +
> +len = len - DHCP_IP_UDP_SIZE;
> +type = __dhcp_option_parse(&offer->dhcp, len, client_parse_offer,
> +   lease);
> +
> +if (type == DHCP_NAK) {
> +err = DHCP_EVENT_NAK;
> +

Re: [systemd-devel] [PATCH 25/28] dhcp: Compute expire, T1 and T2 timers

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:53PM +0200, Patrik Flykt wrote:
> Compute the default T1 and T2 timer values if they were not set by
> the DHCP server. Verify that the values are reasonable.
> ---
>  src/dhcp/client.c   |  123 
> ++-
>  src/dhcp/protocol.h |2 +
>  2 files changed, 124 insertions(+), 1 deletion(-)
> 
> diff --git a/src/dhcp/client.c b/src/dhcp/client.c
> index 669804a..50008a9 100644
> --- a/src/dhcp/client.c
> +++ b/src/dhcp/client.c
> @@ -34,6 +34,8 @@
>  
>  struct DHCPLease {
>  uint32_t lifetime;
> +uint32_t t2;
> +uint32_t t1;
Can we have the order t1, t2 everywhere?

>  struct in_addr address;
>  struct in_addr server_address;
>  struct in_addr subnet_mask;
> @@ -57,6 +59,10 @@ struct DHCPClient {
>  uint32_t xid;
>  uint64_t start_time;
>  unsigned int attempt;
> +uint64_t request_sent;
> +sd_event_source *timeout_expire;
> +sd_event_source *timeout_t1;
> +sd_event_source *timeout_t2;
>  DHCPLease *lease;
>  };
>  
> @@ -177,6 +183,16 @@ static int client_stop(DHCPClient *client, int error)
>  client->timeout_resend =
>  sd_event_source_unref(client->timeout_resend);
>  
> +if (client->timeout_expire)
> +client->timeout_expire =
> +sd_event_source_unref(client->timeout_expire);
sd_event_source_unref accepts NULL, so those if's are not necessary.
I think that this is not performance trical code, so it's better to 
simplify.

> +if (client->timeout_t2)
> +client->timeout_t2 =
> +sd_event_source_unref(client->timeout_t2);
> +if (client->timeout_t1)
> +client->timeout_t1 =
> +sd_event_source_unref(client->timeout_t1);
> +
>  client->attempt = 1;
>  
>  switch (client->state) {
> @@ -184,6 +200,7 @@ static int client_stop(DHCPClient *client, int error)
>  case DHCP_STATE_INIT:
>  case DHCP_STATE_SELECTING:
>  case DHCP_STATE_REQUESTING:
> +case DHCP_STATE_BOUND:
>  
>  client->start_time = 0;
>  client->state = DHCP_STATE_INIT;
> @@ -191,7 +208,6 @@ static int client_stop(DHCPClient *client, int error)
>  
>  case DHCP_STATE_INIT_REBOOT:
>  case DHCP_STATE_REBOOTING:
> -case DHCP_STATE_BOUND:
>  case DHCP_STATE_RENEWING:
>  case DHCP_STATE_REBINDING:
>  
> @@ -447,6 +463,8 @@ static int client_timeout_resend(sd_event_source *s, 
> uint64_t usec,
>  if (err < 0 && client->attempt >= 64)
>  goto error;
>  
> +client->request_sent = usec;
> +
>  break;
>  
>  case DHCP_STATE_INIT_REBOOT:
> @@ -466,6 +484,22 @@ error:
>  return 0;
>  }
>  
> +static int client_timeout_expire(sd_event_source *s, uint64_t usec,
> + void *userdata)
> +{
> +return 0;
> +}
> +
> +static int client_timeout_t2(sd_event_source *s, uint64_t usec, void 
> *userdata)
> +{
> +return 0;
> +}
> +
> +static int client_timeout_t1(sd_event_source *s, uint64_t usec, void 
> *userdata)
> +{
> +return 0;
> +}
> +
>  static int client_parse_offer(uint8_t code, uint8_t len, uint8_t *option,
>void *user_data)
>  {
> @@ -499,6 +533,22 @@ static int client_parse_offer(uint8_t code, uint8_t len, 
> uint8_t *option,
>  memcpy(&lease->router.s_addr, option, 4);
>  
>  break;
> +
> +case DHCP_OPTION_RENEWAL_T1_TIME:
> +if (len == 4) {
> +memcpy(&val, option, 4);
> +lease->t1 = ntohl(val);
> +}
> +
> +break;
> +
> +case DHCP_OPTION_REBINDING_T2_TIME:
> +if (len == 4) {
> +memcpy(&val, option, 4);
> +lease->t2 = ntohl(val);
> +}
> +
> +break;
>  }
>  
>  return 0;
> @@ -635,6 +685,72 @@ error:
>  return err;
>  }
>  
> +static uint64_t client_compute_timeout(uint64_t request_sent,
> +   uint32_t lifetime)
> +{
> +return request_sent + (lifetime - 3) * USEC_PER_SEC +
> ++ (random_u() & 0x1f);
> +}
> +
> +static int client_set_lease_timeouts(DHCPClient *client, uint64_t usec)
> +{
> +int err;
> +uint64_t next_timeout;
> +
> +if (client->lease->lifetime < 10)
> +return -EINVAL;
> +
> +if (!client->lease->t1)
> +client->lease->t1 = client->lease->lifetime / 2;
> +
> +next_timeout = client_compute_timeout(client->request_sent,
> +  client->lease->t1);
> +if (next

Re: [systemd-devel] [PATCH 01/28] dhcp: Add DHCP protocol structures and initial defines

[Marcel Holtmann]

Hi Lennart,

>>> +#define BOOTREQUEST 1
>>> +#define BOOTREPLY   2
>>> +
>>> +#define DHCP_DISCOVER   1
>>> +#define DHCP_OFFER  2
>>> +#define DHCP_REQUEST3
>>> +#define DHCP_DECLINE4
>>> +#define DHCP_ACK5
>>> +#define DHCP_NAK6
>>> +#define DHCP_RELEASE7
>>> +
>>> +#define DHCP_OVERLOAD_FILE  1
>>> +#define DHCP_OVERLOAD_SNAME 2
>>> +
>>> +#define DHCP_OPTION_PAD 0
>>> +#define DHCP_OPTION_SUBNET_MASK 1
>>> +#define DHCP_OPTION_ROUTER  3
>>> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
>>> +#define DHCP_OPTION_HOST_NAME   12
>>> +#define DHCP_OPTION_DOMAIN_NAME 15
>>> +#define DHCP_OPTION_NTP_SERVER  42
>>> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
>>> +#define DHCP_OPTION_OVERLOAD52
>>> +#define DHCP_OPTION_MESSAGE_TYPE53
>>> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
>>> +#define DHCP_OPTION_END 255
>> 
>> For defines like these I'd really suggest using anonymous enums. It's a
>> good thing if the compiler knows these things, not just the
>> pre-processor...
> 
> these are wire protocol definitions. What benefit do you gain if the
> compiler knows them. You always have to handle invalid cases anyway
> since malicious servers are a reality.
 
 For example, it's nicer to work with gdb if it can resolve them...
>>> You'll also get an error if you use an enum value of one kind
>>> in a call requiring an enum of different type.
>> 
>> as I said, these are wire protocol definitions. They come in as bits and 
>> bytes and not as enums.
> 
> I was suggesting an *anonymous* enum. That doesn't introduce a type or
> anything, it just maps name to integers (of any size). That's all. And
> it does so on the compiler level, rather than in the pre-processor. This
> is useful because gdb knows about the thing then.

fair enough on the gdb level. It is only helping you if the enum value is 
actually used. So for example in assignments and if-clauses or similar. It does 
not help you values from the wire that you do not process.

I never really cared about gdb here. Either we have builtin debug/trace 
printing in our protocol code or we use external tools like tcpdump etc. From 
BlueZ, oFono and ConnMan past experience, the builtin debug/trace printing has 
been the most useful tool when we had to debug something that is a wire 
protocol.

Regards

Marcel

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

Re: [systemd-devel] [PATCH 01/28] dhcp: Add DHCP protocol structures and initial defines

[Lennart Poettering]

On Thu, 14.11.13 09:50, Marcel Holtmann (mar...@holtmann.org) wrote:

> > +#define BOOTREQUEST 1
> > +#define BOOTREPLY   2
> > +
> > +#define DHCP_DISCOVER   1
> > +#define DHCP_OFFER  2
> > +#define DHCP_REQUEST3
> > +#define DHCP_DECLINE4
> > +#define DHCP_ACK5
> > +#define DHCP_NAK6
> > +#define DHCP_RELEASE7
> > +
> > +#define DHCP_OVERLOAD_FILE  1
> > +#define DHCP_OVERLOAD_SNAME 2
> > +
> > +#define DHCP_OPTION_PAD 0
> > +#define DHCP_OPTION_SUBNET_MASK 1
> > +#define DHCP_OPTION_ROUTER  3
> > +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> > +#define DHCP_OPTION_HOST_NAME   12
> > +#define DHCP_OPTION_DOMAIN_NAME 15
> > +#define DHCP_OPTION_NTP_SERVER  42
> > +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> > +#define DHCP_OPTION_OVERLOAD52
> > +#define DHCP_OPTION_MESSAGE_TYPE53
> > +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> > +#define DHCP_OPTION_END 255
>  
>  For defines like these I'd really suggest using anonymous enums. It's a
>  good thing if the compiler knows these things, not just the
>  pre-processor...
> >>> 
> >>> these are wire protocol definitions. What benefit do you gain if the
> >>> compiler knows them. You always have to handle invalid cases anyway
> >>> since malicious servers are a reality.
> >> 
> >> For example, it's nicer to work with gdb if it can resolve them...
> > You'll also get an error if you use an enum value of one kind
> > in a call requiring an enum of different type.
> 
> as I said, these are wire protocol definitions. They come in as bits and 
> bytes and not as enums.

I was suggesting an *anonymous* enum. That doesn't introduce a type or
anything, it just maps name to integers (of any size). That's all. And
it does so on the compiler level, rather than in the pre-processor. This
is useful because gdb knows about the thing then.

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 01/28] dhcp: Add DHCP protocol structures and initial defines

[Marcel Holtmann]

Hi Zbyszek,

> +#define BOOTREQUEST 1
> +#define BOOTREPLY   2
> +
> +#define DHCP_DISCOVER   1
> +#define DHCP_OFFER  2
> +#define DHCP_REQUEST3
> +#define DHCP_DECLINE4
> +#define DHCP_ACK5
> +#define DHCP_NAK6
> +#define DHCP_RELEASE7
> +
> +#define DHCP_OVERLOAD_FILE  1
> +#define DHCP_OVERLOAD_SNAME 2
> +
> +#define DHCP_OPTION_PAD 0
> +#define DHCP_OPTION_SUBNET_MASK 1
> +#define DHCP_OPTION_ROUTER  3
> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> +#define DHCP_OPTION_HOST_NAME   12
> +#define DHCP_OPTION_DOMAIN_NAME 15
> +#define DHCP_OPTION_NTP_SERVER  42
> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> +#define DHCP_OPTION_OVERLOAD52
> +#define DHCP_OPTION_MESSAGE_TYPE53
> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> +#define DHCP_OPTION_END 255
 
 For defines like these I'd really suggest using anonymous enums. It's a
 good thing if the compiler knows these things, not just the
 pre-processor...
>>> 
>>> these are wire protocol definitions. What benefit do you gain if the
>>> compiler knows them. You always have to handle invalid cases anyway
>>> since malicious servers are a reality.
>> 
>> For example, it's nicer to work with gdb if it can resolve them...
> You'll also get an error if you use an enum value of one kind
> in a call requiring an enum of different type.

as I said, these are wire protocol definitions. They come in as bits and bytes 
and not as enums.

Regards

Marcel

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

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

[Colin Walters]

On Thu, 2013-11-14 at 00:47 +0100, Lennart Poettering wrote:

> I am pretty sure it makes sense to have domain-specific event loops. I
> am not convinced that it would even be possible to unify all event loop
> implementations into one. For example, GLib and and sd-event support
> priorization of events, 

Right, this is:
https://bugzilla.gnome.org/show_bug.cgi?id=156048

> I doubt moving OpenJDK or Python to sd-event or something like it ever
> would make sense. (Do either even have any event loop? Java at least
> prefers async stuff via threads as the solution for IO, not multiplexing
> via poll()-like stuff -- which they only added very very late to the
> language...).

Yes, Java added it late, but it is AFAIK fairly actively used in some
subsets of that world.  Zbigniew already linked to the Python version.

> For all event loop consuming library code I write I will make sure that
> it can be connected to any reasonable event loop, not only sd-event.

Ok, that's a reasonable short term answer.

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

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

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

>   Hi,
> 
> This patch set implements a DHCPv4 client library named libsystemd-dhcp
> to be used with systemd-network.
> 
> The code implements the DHCP protocol from the INIT state up to expiry,
> T1 and T2 timer setting, but does nothing in response to the IP address
> reacquisition T1 and T2 timers being triggered. Expiry timer is followed
> and the DHCP client stopped when it triggers. INIT-REBOOT state is also
> not yet implemented; both these missing parts are to be implemented next.
> 
> The code assumes that the caller can figure out the interface to use.
> The code expects to be handed an sd_event loop, a MAC address and an
> interface index. The outcome is a notification of an IP address being
> acquired and the callback provider is expected to set the IP address,
> netmask and default gateway. Higher level DHCP options such as DNS and
> NTP servers are requested from the server by default, but the response
> is not yet collected anywhere. I also don't know how detailed callback
> information is needed, is there for example any need to know the protocol
> state transitions or messages sent/received.

I'd keep things minimal. But the addition DHCP metadata like NTP servers
we certainly should pass back to the user of the lib. However, I'd very
much prefer doing this high-level instead of low-level, i.e. provide
calls such as sd_dhcp_client_get_ntp() to get a list of NTP servers, and
so on...

I do like the code and where this is going!

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 04/28] build: Add initial build support

[Tom Gundersen]

On Thu, Nov 14, 2013 at 1:28 AM, Tom Gundersen  wrote:
> On Wed, Nov 13, 2013 at 10:22 PM, Patrik Flykt
>  wrote:
>> The client test program is the only one to be built so far.
>> ---
>>  Makefile.am   |   15 +++
>>  configure.ac  |9 +
>>  src/dhcp/Makefile |1 +
>>  3 files changed, 25 insertions(+)
>>  create mode 12 src/dhcp/Makefile
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 789ca02..cc52f01 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -3739,6 +3739,21 @@ lib_LTLIBRARIES += \
>>  endif
>>
>>  # 
>> --
>> +if ENABLE_DHCP
>
> Maybe we just as well could put this under ENABLE_NETWORKD, to avoid
> introducing two config switches? (And I certainly don't want to ifdef
> out dhcp support in networkd)

Actually, I take that back. Just compile the dhcp unconditionally, as
it is not even shipped there is no reason not to.

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

Re: [systemd-devel] [PATCH 21/28] dhcp: Handle received DHCP Offer message

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +if (client->fd > 0)
> +close(client->fd);
> +client->fd = 0;

fd 0 is a valid fd (actually refers to stdin), I am pretty sure you want
to assign -1 here, no?

> +if (hdrlen + ntohs(offer->udp.len) > len ||
> +client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
> +return -EINVAL;

Hmm, so it might make sense to use betoh16() and friends instead of
ntohs() here, where it makes sense, and to declare the integers as
be16_t, be32_t and so on. This is defined in "sparse-endian.h" and is
useful for using the kernel sparse checker on this code, and it will
then detect whenever a non-native endian integer is accessed without
conversion.

> +lease = new0(DHCPLease, 1);
> +if (!lease)
> +return -ENOBUFS;

ENOMEM please

> +
> +error:
> +free(lease);
> +
> +return -ENOMSG;
> +}

_cleanup_free_!

> +
> +static int client_receive_raw_message(sd_event_source *s, int fd,
> +  uint32_t revents, void *userdata)
> +{
> +DHCPClient *client = userdata;
> +int err = 0;
> +int len, buflen;
> +uint8_t *buf;
> +DHCPPacket *message;
> +
> +buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
> +buf = malloc0(buflen);
> +if (!buf)
> +goto error;

Hmm, you eat up the rror here, is that intended?

> +len = read(fd, buf, buflen);
> +if (len < 0) {
> +err = -errno;
> +goto error;
> +}
> +
> +message = (DHCPPacket *)buf;
> +
> +switch (client->state) {
> +case DHCP_STATE_SELECTING:
> +
> +if (client_receive_offer(client, message, len) >= 0) {
> +
> +close(client->fd);
> +client->fd = 0;

-1 again...

> +error:
> +if (!err)
> +read(fd, buf, 1);

You forget to free buf here...

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 20/28] dhcp: Add timeout and main loop support

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

>  int index;
>  uint8_t *req_opts;
>  int req_opts_size;
>  struct in_addr *last_addr;
>  struct ether_addr mac_addr;
>  uint32_t xid;
> +uint64_t start_time;

Internally we prefer "usec_t" for times, since that indicates the unit
used here. usec_t is actually identical to uint64_t, but it's more
descriptive, hence preferred. Only in the public APIs we restraint
ourselves a bit and expose this as uint64_t, to not pollute the
namespace.

> +if (!client->start_time)
> +client->start_time = usec;
> +
> +secs = (usec - client->start_time) / USEC_PER_SEC;
> +
> +next_timeout = usec + 2 * USEC_PER_SEC + (random() &
> 0x1f);
> +
> +err = sd_event_add_monotonic(sd_event_get(s), next_timeout,
> + 10 * USEC_PER_MSEC,
> + client_timeout_resend, client,
> +
> &client->timeout_resend);

if you don't have a very good reason to specify the accuracy as 10ms,
I'd always recommend to pass 0 instead, which results in the default
accuracy of 250ms (I wouldn't be too surprised if 250ms is too
inaccurate for this usecase, so your code might be fine, just wanted to
take the opportuntine to point this out...

> -return client_send_discover(client, 0);
> +err = sd_event_add_monotonic(client->event, now(CLOCK_MONOTONIC), 0,
> + client_timeout_resend, client,
> + &client->timeout_resend);

Hmm, so this would immediately trigger since "now" is already passed, by
the time you read it... Note that "0" can be used as time here too, to
indicate that you want to be executed immediately, i.e. it indicates in
this context the ealiest sensible time.

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 01/28] dhcp: Add DHCP protocol structures and initial defines

[Zbigniew Jędrzejewski-Szmek]

On Thu, Nov 14, 2013 at 01:10:07AM +0100, Lennart Poettering wrote:
> On Thu, 14.11.13 08:56, Marcel Holtmann (mar...@holtmann.org) wrote:
> 
> > 
> > Hi Lennart,
> > 
> > >> +#define BOOTREQUEST 1
> > >> +#define BOOTREPLY   2
> > >> +
> > >> +#define DHCP_DISCOVER   1
> > >> +#define DHCP_OFFER  2
> > >> +#define DHCP_REQUEST3
> > >> +#define DHCP_DECLINE4
> > >> +#define DHCP_ACK5
> > >> +#define DHCP_NAK6
> > >> +#define DHCP_RELEASE7
> > >> +
> > >> +#define DHCP_OVERLOAD_FILE  1
> > >> +#define DHCP_OVERLOAD_SNAME 2
> > >> +
> > >> +#define DHCP_OPTION_PAD 0
> > >> +#define DHCP_OPTION_SUBNET_MASK 1
> > >> +#define DHCP_OPTION_ROUTER  3
> > >> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> > >> +#define DHCP_OPTION_HOST_NAME   12
> > >> +#define DHCP_OPTION_DOMAIN_NAME 15
> > >> +#define DHCP_OPTION_NTP_SERVER  42
> > >> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> > >> +#define DHCP_OPTION_OVERLOAD52
> > >> +#define DHCP_OPTION_MESSAGE_TYPE53
> > >> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> > >> +#define DHCP_OPTION_END 255
> > > 
> > > For defines like these I'd really suggest using anonymous enums. It's a
> > > good thing if the compiler knows these things, not just the
> > > pre-processor...
> > 
> > these are wire protocol definitions. What benefit do you gain if the
> > compiler knows them. You always have to handle invalid cases anyway
> > since malicious servers are a reality.
> 
> For example, it's nicer to work with gdb if it can resolve them...
You'll also get an error if you use an enum value of one kind
in a call requiring an enum of different type.

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

Re: [systemd-devel] [PATCH 04/28] build: Add initial build support

[Tom Gundersen]

On Wed, Nov 13, 2013 at 10:22 PM, Patrik Flykt
 wrote:
> The client test program is the only one to be built so far.
> ---
>  Makefile.am   |   15 +++
>  configure.ac  |9 +
>  src/dhcp/Makefile |1 +
>  3 files changed, 25 insertions(+)
>  create mode 12 src/dhcp/Makefile
>
> diff --git a/Makefile.am b/Makefile.am
> index 789ca02..cc52f01 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3739,6 +3739,21 @@ lib_LTLIBRARIES += \
>  endif
>
>  # 
> --
> +if ENABLE_DHCP

Maybe we just as well could put this under ENABLE_NETWORKD, to avoid
introducing two config switches? (And I certainly don't want to ifdef
out dhcp support in networkd)

> +test_dhcp_client_SOURCES = \
> +   src/dhcp/protocol.h \
> +   src/dhcp/client.h \
> +   src/dhcp/client.c \
> +   src/dhcp/test-dhcp-client.c
> +
> +test_dhcp_client_LDADD = \
> +   libsystemd-shared.la
> +
> +tests += \
> +   test-dhcp-client
> +endif
> +
> +# 
> --
>  if ENABLE_MACHINED
>  systemd_machined_SOURCES = \
> src/machine/machined.c \
> diff --git a/configure.ac b/configure.ac
> index d0bfcb8..0c28368 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -714,6 +714,14 @@ fi
>  AM_CONDITIONAL(ENABLE_RFKILL, [test "$have_rfkill" = "yes"])
>
>  # 
> --
> +have_dhcp=no
> +AC_ARG_ENABLE(dhcp, AS_HELP_STRING([--disable-dhcp], [disable dhcp]))
> +if test "x$enable_dhcp" != "xno"; then
> +have_dhcp=yes
> +fi
> +AM_CONDITIONAL(ENABLE_DHCP, [test "$have_dhcp" = "yes"])
> +
> +# 
> --
>  have_logind=no
>  AC_ARG_ENABLE(logind, AS_HELP_STRING([--disable-logind], [disable login 
> daemon]))
>  if test "x$enable_logind" != "xno"; then
> @@ -1060,6 +1068,7 @@ AC_MSG_RESULT([
>  randomseed:  ${have_randomseed}
>  backlight:   ${have_backlight}
>  rfkill:  ${have_rfkill}
> +dhcp:${have_dhcp}
>  logind:  ${have_logind}
>  machined:${have_machined}
>  hostnamed:   ${have_hostnamed}
> diff --git a/src/dhcp/Makefile b/src/dhcp/Makefile
> new file mode 12
> index 000..d0b0e8e
> --- /dev/null
> +++ b/src/dhcp/Makefile
> @@ -0,0 +1 @@
> +../Makefile
> \ No newline at end of file
> --
> 1.7.10.4
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 17/28] dhcp: Support seconds elapsed since start of DHCP negotiation

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> It was noticed by Grant Erickson in ConnMan commit
> 95e15c09350acf58d4707056ae2614570883ef66 that:
> 
>"Certain DHCP servers, such as that implemented in Mac OS X
> (< 10.7) for its "Internet Sharing" feature, refuse to issue
> a DHCP lease to clients that have not set a non-zero value
> in their DISCOVER or REQUEST packets."

I am quite sure it would be better to add this comment to the sources
too, there are easier to find there, and people will less likely drop
the assignment when they revisit the code.

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 15/28] dhcp: Add example DHCP client test program

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +static int get_mac(int index, struct ether_addr *mac)
> +{
> +struct ifreq ifr;
> +int s, err;
> +
> +if (index < 0 || !mac)
> +return -EINVAL;
> +
> +s = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
> +if (s < 0)
> +return -EIO;
> +
> +memset(&ifr, 0, sizeof(ifr));

Please initialize when declaring with = {} instead.

> +ifr.ifr_ifindex = index;
> +
> +if (ioctl(s, SIOCGIFNAME, &ifr) < 0) {
> +err = -errno;
> +close(s);
> +return err;
> +}
> +
> +if (ioctl(s, SIOCGIFHWADDR, &ifr) < 0) {
> +err = -errno;
> +close(s);
> +return err;
> +}
> +
> +memcpy(&mac->ether_addr_octet[0], &ifr.ifr_hwaddr.sa_data, ETH_ALEN);
> +
> +return 0;

You forget to close the socket on success.. But this looks like
something where you should use Tom's rtnl stuff, no? I am pretty sure we
should avoid the ioctls wherever netlink equivalents exist.

Also, _cleanup_close_ is your friend!

> +}
> +
> +static int get_index(char *ifname)

const char *ifname, no?

> +{
> +struct ifreq ifr;
> +int s;
> +
> +s = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
> +if (s < 0)
> +return -EIO;
> +
> +memset(&ifr, 0, sizeof(ifr));
> +strncpy(ifr.ifr_name, ifname, IFNAMSIZ);
> +
> +if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
> +close(s);
> +return -EIO;
> +}
> +
> +return ifr.ifr_ifindex;
> +}

You forgot to close the fd here, too. _cleanup_close_ is your friend
again... Also initialization. 

Also, rtnl sounds the better option. And if not, then this call appears
to be totally identical to glibc's ifname_to_index(), no? Better use
that then...

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 13/28] build: Add libsystemd-dhcp

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> ---
>  Makefile.am |   20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index aeca484..5c350ab 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3740,6 +3740,26 @@ endif
>  
>  # 
> --
>  if ENABLE_DHCP
> +libsystemd_dhcp_la_SOURCES = \
> + src/dhcp/protocol.h \
> + src/dhcp/internal.h \
> + src/dhcp/network.c \
> + src/dhcp/option.c \
> + src/dhcp/client.h \
> + src/dhcp/client.c

Which one of these should be public APIs? Please keep them minimal and
move them to src/systemd/ instead, and use the "sd-" prefix for them

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 12/28] dhcp: Add DHCP discover sending

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +static int client_send_discover(DHCPClient *client)
> +{
> +int err = 0;
> +DHCPPacket *discover;
> +int optlen, len;
> +uint8_t *opt;
> +
> +optlen = DHCP_CLIENT_MIN_OPTIONS_SIZE;
> +len = sizeof(DHCPPacket) + optlen;
> +
> +discover = malloc0(len);
> +if (!discover)
> +return -ENOBUFS;

Please use -ENOMEM for OOM errors

> +err = __dhcp_network_send_raw_packet(client->index, discover, len);
> +
> +error:
> +free(discover);
> +
> +return err;

_cleanup_free_ is your friend and the best thing since sliced bread. It
allows you to get rid of all gotos here!

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 11/28] dhcp: Add function for sending a raw packet

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +int __dhcp_network_send_raw_packet(int index, void *packet, int len);

Shouldn't this be "const void *"?

>  int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,

buflen should better be size_t, no?

>   uint8_t optlen, void *optval);

optval again const?

> +int __dhcp_network_send_raw_packet(int index, void *packet, int len)
> +{
> +int err, s;
> +struct sockaddr_ll link;
> +
> +err = 0;
> +
> +s = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, htons(ETH_P_IP));
> +if (s < 0)
> +return -errno;
> +
> +memset(&link, 0, sizeof(link));

Please initialize the thing when you define it:

   struct sockaddr_ll link = {}; 

It's actually quicker. Ask Zbigniew... ;-)

In cases where this kind of initialization doesn't work, use the zero()
macro for ases like this.

> +
> +link.sll_family = AF_PACKET;
> +link.sll_protocol = htons(ETH_P_IP);
> +link.sll_ifindex =  index;
> +link.sll_halen = ETH_ALEN;
> +memset(&link.sll_addr, 0xff, ETH_ALEN);
> +
> +if (bind(s, (struct sockaddr *)&link, sizeof(link)) < 0) {

Hmm, to never even have to think about aliasing issue we generally try
to use unions for cases like this. In fact in "socket-util.h" you find a
definition of "union sockaddr_union", where you could add
sockaddr_ll. If you then use the union, you can avoid the cast easily by
getting a pointer of the "sa" member of it.

> +err = -errno;
> +close(s);
> +return err;
> +}

> +
> +if (sendto(s, packet, len, 0, (struct sockaddr *)&link,
> +   sizeof(link)) < 0)
> +err = -errno;
> +
> +close(s);
> +
> +return err;
> +}

Sounds like you want to use _cleanup_close_ here!

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 01/28] dhcp: Add DHCP protocol structures and initial defines

[Lennart Poettering]

On Thu, 14.11.13 08:56, Marcel Holtmann (mar...@holtmann.org) wrote:

> 
> Hi Lennart,
> 
> >> +#define BOOTREQUEST 1
> >> +#define BOOTREPLY   2
> >> +
> >> +#define DHCP_DISCOVER   1
> >> +#define DHCP_OFFER  2
> >> +#define DHCP_REQUEST3
> >> +#define DHCP_DECLINE4
> >> +#define DHCP_ACK5
> >> +#define DHCP_NAK6
> >> +#define DHCP_RELEASE7
> >> +
> >> +#define DHCP_OVERLOAD_FILE  1
> >> +#define DHCP_OVERLOAD_SNAME 2
> >> +
> >> +#define DHCP_OPTION_PAD 0
> >> +#define DHCP_OPTION_SUBNET_MASK 1
> >> +#define DHCP_OPTION_ROUTER  3
> >> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> >> +#define DHCP_OPTION_HOST_NAME   12
> >> +#define DHCP_OPTION_DOMAIN_NAME 15
> >> +#define DHCP_OPTION_NTP_SERVER  42
> >> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> >> +#define DHCP_OPTION_OVERLOAD52
> >> +#define DHCP_OPTION_MESSAGE_TYPE53
> >> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> >> +#define DHCP_OPTION_END 255
> > 
> > For defines like these I'd really suggest using anonymous enums. It's a
> > good thing if the compiler knows these things, not just the
> > pre-processor...
> 
> these are wire protocol definitions. What benefit do you gain if the
> compiler knows them. You always have to handle invalid cases anyway
> since malicious servers are a reality.

For example, it's nicer to work with gdb if it can resolve them...

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 05/28] dhcp: Add option appending and parsing

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
> + uint8_t optlen, void *optval);
> +

The "__" prefix is actually private property by the C compiler,
according to ANSI C. Please do not define your own symbols in this
namespace. (Also why even?)

> +int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
> + uint8_t optlen, void *optval)

Shouldn't "void *optval" be const?

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 02/28] dhcp: Add DHCP client initialization

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +static uint8_t default_req_opts[] = {
> +DHCP_OPTION_SUBNET_MASK,
> +DHCP_OPTION_ROUTER,
> +DHCP_OPTION_HOST_NAME,
> +DHCP_OPTION_DOMAIN_NAME,
> +DHCP_OPTION_DOMAIN_NAME_SERVER,
> +DHCP_OPTION_NTP_SERVER,
> +};

Shouldn't this array be const?

> +
> +int dhcp_client_set_request_option(DHCPClient *client, uint8_t option)
> +{
> +int i;
> +
> +if (!client)
> +return -EINVAL;

Using assert_return() for this makes these simpler and more redable (and
also uses the _unlikely_() stuff). Use assert_return() for all those
cases where the caller made an obvious programming mistake.

Actually, to give some insight on the way how we did things so far in systemd:

- If something is internal code, then it uses assert() for checking for
  programming errors
- If something is public API code, then it uses assert_return() for
  checking for programming errors. Public APIs have the "sd_" prefix in
  their API calls.

Basically, we are more forgiving towards public users of our code than
to ourselves.

Of course, please never use assert() nor assert_return() for checking
for runtime errors as opposed to programming errors.

> +client->req_opts_size++;
> +client->req_opts = realloc(client->req_opts, client->req_opts_size);
> +if (!client->req_opts) {
> +client->req_opts_size = 0;
> +return -ENOBUFS;
> +}

GREEDY_REALLOC() is cool.

> +
> +client->req_opts[client->req_opts_size - 1] = option;
> +
> +return 0;
> +}
> +
> +int dhcp_client_set_request_address(DHCPClient *client,
> +struct in_addr *last_addr)

The parameter should be const, no?

> +{
> +if (!client)
> +return -EINVAL;
> +
> +if (client->state != DHCP_STATE_INIT)
> +return -EBUSY;
> +
> +client->last_addr = malloc(sizeof(struct in_addr));
> +if (!client->last_addr)
> +return -ENOBUFS;

Hmm, "struct in_addr" contains nothing but a uint32_t. It really sounds
overkill allocating such a structure individually on the
stack. Shouldn't this structure be directly inside DHCPClient? Or
actually, do we really want to use struct in_addr at all? It sounds so
useless, a "uint_32_t" sounds so much simpler, and easier to use...

> +
> +memcpy(&client->last_addr, last_addr, sizeof(struct
> in_addr));

memdup() and newdup() are cool...

> +struct DHCPClient;
> +typedef struct DHCPClient DHCPClient;

The struct line is implied by the typedef line, you can remove it.
> +
> +int dhcp_client_set_request_option(DHCPClient *client, uint8_t option);
> +int dhcp_client_set_request_address(DHCPClient *client,
> +struct in_addr *last_address);
> +int dhcp_client_set_index(DHCPClient *client, int interface_index);
> +
> +DHCPClient *dhcp_client_new(void);

If this is public API these calls should have a "sd_" prefix...

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 01/28] dhcp: Add DHCP protocol structures and initial defines

[Marcel Holtmann]

Hi Lennart,

>> +#define BOOTREQUEST 1
>> +#define BOOTREPLY   2
>> +
>> +#define DHCP_DISCOVER   1
>> +#define DHCP_OFFER  2
>> +#define DHCP_REQUEST3
>> +#define DHCP_DECLINE4
>> +#define DHCP_ACK5
>> +#define DHCP_NAK6
>> +#define DHCP_RELEASE7
>> +
>> +#define DHCP_OVERLOAD_FILE  1
>> +#define DHCP_OVERLOAD_SNAME 2
>> +
>> +#define DHCP_OPTION_PAD 0
>> +#define DHCP_OPTION_SUBNET_MASK 1
>> +#define DHCP_OPTION_ROUTER  3
>> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
>> +#define DHCP_OPTION_HOST_NAME   12
>> +#define DHCP_OPTION_DOMAIN_NAME 15
>> +#define DHCP_OPTION_NTP_SERVER  42
>> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
>> +#define DHCP_OPTION_OVERLOAD52
>> +#define DHCP_OPTION_MESSAGE_TYPE53
>> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
>> +#define DHCP_OPTION_END 255
> 
> For defines like these I'd really suggest using anonymous enums. It's a
> good thing if the compiler knows these things, not just the
> pre-processor...

these are wire protocol definitions. What benefit do you gain if the compiler 
knows them. You always have to handle invalid cases anyway since malicious 
servers are a reality.

Regards

Marcel

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

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

[Zbigniew Jędrzejewski-Szmek]

On Thu, Nov 14, 2013 at 12:47:24AM +0100, Lennart Poettering wrote:
> I doubt moving OpenJDK or Python to sd-event or something like it ever
> would make sense. (Do either even have any event loop?
http://www.python.org/dev/peps/pep-3156/

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

Re: [systemd-devel] [PATCH 01/28] dhcp: Add DHCP protocol structures and initial defines

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

> +struct DHCPMessage {
> +uint8_t op;
> +uint8_t htype;
> +uint8_t hlen;
> +uint8_t hops;
> +uint32_t xid;
> +uint16_t secs;
> +uint16_t flags;
> +uint32_t ciaddr;
> +uint32_t yiaddr;
> +uint32_t siaddr;
> +uint32_t giaddr;
> +uint8_t chaddr[16];
> +uint8_t sname[64];
> +uint8_t file[128];
> +} __attribute__((packed));

Use _packed_ please!

> +#define BOOTREQUEST 1
> +#define BOOTREPLY   2
> +
> +#define DHCP_DISCOVER   1
> +#define DHCP_OFFER  2
> +#define DHCP_REQUEST3
> +#define DHCP_DECLINE4
> +#define DHCP_ACK5
> +#define DHCP_NAK6
> +#define DHCP_RELEASE7
> +
> +#define DHCP_OVERLOAD_FILE  1
> +#define DHCP_OVERLOAD_SNAME 2
> +
> +#define DHCP_OPTION_PAD 0
> +#define DHCP_OPTION_SUBNET_MASK 1
> +#define DHCP_OPTION_ROUTER  3
> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> +#define DHCP_OPTION_HOST_NAME   12
> +#define DHCP_OPTION_DOMAIN_NAME 15
> +#define DHCP_OPTION_NTP_SERVER  42
> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> +#define DHCP_OPTION_OVERLOAD52
> +#define DHCP_OPTION_MESSAGE_TYPE53
> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> +#define DHCP_OPTION_END 255

For defines like these I'd really suggest using anonymous enums. It's a
good thing if the compiler knows these things, not just the
pre-processor...

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 05/28] dhcp: Add option appending and parsing

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:33PM +0200, Patrik Flykt wrote:
> Add functions to append and parse DHCP options. Not all options
> are passed to the callback function, the ones not exposed are
> pad, end, message type and overload. If indicated by the overload
> option, file and sname fields will be examined for more options.
> 
> The option functions are internal to DHCP, add a new header files
> for interal function prototypes.
> ---
>  src/dhcp/internal.h |   34 ++
>  src/dhcp/option.c   |  181 
> +++
>  2 files changed, 215 insertions(+)
>  create mode 100644 src/dhcp/internal.h
>  create mode 100644 src/dhcp/option.c
> 
> diff --git a/src/dhcp/internal.h b/src/dhcp/internal.h
> new file mode 100644
> index 000..65a3bb3
> --- /dev/null
> +++ b/src/dhcp/internal.h
> @@ -0,0 +1,34 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +#pragma once
> +
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see .
> +***/
> +
> +#include 
> +
> +#include "protocol.h"
> +
> +int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
> + uint8_t optlen, void *optval);
> +
> +typedef int (*dhcp_option_cb_t)(uint8_t code, uint8_t len, uint8_t *option,
> +void *user_data);
> +int __dhcp_option_parse(DHCPMessage *message, int32_t len,
> +dhcp_option_cb_t cb, void *user_data);
Why "__"?

> diff --git a/src/dhcp/option.c b/src/dhcp/option.c
> new file mode 100644
> index 000..f5ca6e7
> --- /dev/null
> +++ b/src/dhcp/option.c
> @@ -0,0 +1,181 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see .
> +***/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "internal.h"
> +
> +int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
> + uint8_t optlen, void *optval)
> +{
> +if (!buf || !buflen)
> +return -EINVAL;
> +
> +switch (code) {
> +
> +case DHCP_OPTION_PAD:
> +case DHCP_OPTION_END:
> +if (*buflen < 1)
> +return -ENOBUFS;
> +
> +(*buf)[0] = code;
> +*buf += 1;
> +*buflen -= 1;
> +break;
> +
> +default:
> +if (*buflen < optlen + 2)
> +return -ENOBUFS;
> +
> +if (!optval)
> +return -EINVAL;
> +
> +(*buf)[0] = code;
> +(*buf)[1] = optlen;
> +memcpy(&(*buf)[2], optval, optlen);
> +
> +*buf += optlen + 2;
> +*buflen = *buflen - optlen - 2;
> +
> +break;
> +}
> +
> +return 0;
> +}
> +
> +static int parse_options(uint8_t *buf, int32_t buflen, int *overload,
> + int *message_type, dhcp_option_cb_t cb,
> + void *user_data)
> +{
> +uint8_t *code = buf;
> +uint8_t *len;
> +
> +while (buflen > 0) {
> +switch (*code) {
> +case DHCP_OPTION_PAD:
> +buflen -= 1;
> +code++;
> +break;
> +
> +case DHCP_OPTION_END:
> +return 0;
> +
> +case DHCP_OPTION_MESSAGE_TYPE:
> +buflen -= 3;
> +if 

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

[Lennart Poettering]

On Wed, 13.11.13 17:18, Colin Walters (walt...@verbum.org) wrote:

> 
> On Thu, 2013-11-14 at 06:49 +0900, Marcel Holtmann wrote:
> 
> > that is the long term plan. Once ConnMan is switching over to use 
> > libsystemd-bus and kdbus,
> > we are switching over to using the systemd event loop instead of GLib main 
> > loop 
> 
> But I think the long term architecturally correct place for the core
> system main loop is glibc, not systemd.
> 
> For example, that would allow runtimes like OpenJDK and Python to
> unconditionally depend on it (if present).
> 
> As systemd's event loop becomes public API, it has the potential to
> create interoperability problems for GNOME - think applications like 
> https://git.gnome.org/browse/gnome-logs
> that want to both monitor the systemd journal and talk to GTK+.
> 
> At the moment it's OK because sd_journal has APIs sufficient to plug in
> external loops.
> 
> Adding a mainloop API to glibc would be a lot of work - it might even
> entail trying to get it by POSIX.  At least it'd entail describing the
> interaction with the other POSIX APIs.  But I think that'd be worth the
> effort.

I am pretty sure it makes sense to have domain-specific event loops. I
am not convinced that it would even be possible to unify all event loop
implementations into one. For example, GLib and and sd-event support
priorization of events, which means we need to flush all queued events
from the kernel before we decide which one to run first. This however is
probably not the best choice for a number of other usecases, where you
want to optimize for ridiculous numbers of fds. Hence, I believe
developers should get a lot of freedom on how much or how little event
loop they want, and which implementation they choose.

Then, GMainLoop can do some things sd-event cannot do. sd-event can do a
number of things GMainLoop cannot do.

I doubt moving OpenJDK or Python to sd-event or something like it ever
would make sense. (Do either even have any event loop? Java at least
prefers async stuff via threads as the solution for IO, not multiplexing
via poll()-like stuff -- which they only added very very late to the
language...).

For all event loop consuming library code I write I will make sure that
it can be connected to any reasonable event loop, not only sd-event.

I mean, libsystemd-bus already includes two event loop implementations,
one in sd-event, and another trivial one in sd_bus_wait() in a way...

So, please do not understand sd-event as an attempt to get everybody to
use the same event loop. It's just an attempt to provide a good event
loop by default, that is more useful than pure epoll (such as sanely
handles lots of timers), that tries to provide some of the features that
GMainLoop supported since a longer time (like timer coalescing or
priorisation), that we need for low-level stuff (like OOM safety, this
stuff should run in PID1...), and is generally just code that people can
use, if they want, but don't have to. Not an attempt for standardisation
but an attempt to just provide code that solves naked epoll()'s problems,
without going overboard and turning into libev/libevent.

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 02/28] dhcp: Add DHCP client initialization

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:30PM +0200, Patrik Flykt wrote:
> Provide functionality for initializing a DHCP client struct, setting
> interface index, last used address and additional options to request.
> On initialization the most useful options are added by default.
> ---
>  src/dhcp/client.c |  137 
> +
>  src/dhcp/client.h |   34 +
>  2 files changed, 171 insertions(+)
>  create mode 100644 src/dhcp/client.c
>  create mode 100644 src/dhcp/client.h
> 
> diff --git a/src/dhcp/client.c b/src/dhcp/client.c
> new file mode 100644
> index 000..172ddd9
> --- /dev/null
> +++ b/src/dhcp/client.c
> @@ -0,0 +1,137 @@
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see .
> +***/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "util.h"
> +#include "list.h"
> +
> +#include "protocol.h"
> +#include "client.h"
> +
> +struct DHCPClient {
> +DHCPState state;
> +int index;
> +uint8_t *req_opts;
> +int req_opts_size;
> +struct in_addr *last_addr;
> +};
> +
> +static uint8_t default_req_opts[] = {
> +DHCP_OPTION_SUBNET_MASK,
> +DHCP_OPTION_ROUTER,
> +DHCP_OPTION_HOST_NAME,
> +DHCP_OPTION_DOMAIN_NAME,
> +DHCP_OPTION_DOMAIN_NAME_SERVER,
> +DHCP_OPTION_NTP_SERVER,
> +};
> +
> +int dhcp_client_set_request_option(DHCPClient *client, uint8_t option)
> +{
> +int i;
> +
> +if (!client)
> +return -EINVAL;
> +
> +if (client->state != DHCP_STATE_INIT)
> +return -EBUSY;
> +
> +switch(option) {
> +case DHCP_OPTION_PAD:
> +case DHCP_OPTION_OVERLOAD:
> +case DHCP_OPTION_MESSAGE_TYPE:
> +case DHCP_OPTION_PARAMETER_REQUEST_LIST:
> +case DHCP_OPTION_END:
> +return -EINVAL;
> +
> +default:
> +break;
> +}
> +
> +for (i = 0; i < client->req_opts_size; i++)
> +if (client->req_opts[i] == option)
> +return -EEXIST;
> +
> +client->req_opts_size++;
> +client->req_opts = realloc(client->req_opts, client->req_opts_size);
> +if (!client->req_opts) {
> +client->req_opts_size = 0;
> +return -ENOBUFS;
> +}
Hm, you have memory leak here. And repeated realloc is slow.

Try something like this:
   if (!GREEDY_REALLOC(client->req_opts,
   client->req_opts_size,
   client->req_opts_size + 1))
   return -ENOMEM;

In general we use -ENOMEM, so for consistency it should be used here too. 

> +client->req_opts[client->req_opts_size - 1] = option;
> +
> +return 0;
> +}
> +
> +int dhcp_client_set_request_address(DHCPClient *client,
> +struct in_addr *last_addr)
> +{
> +if (!client)
> +return -EINVAL;
> +
> +if (client->state != DHCP_STATE_INIT)
> +return -EBUSY;
> +

> +client->last_addr = malloc(sizeof(struct in_addr));
> +if (!client->last_addr)
> +return -ENOBUFS;
> +
> +memcpy(&client->last_addr, last_addr, sizeof(struct in_addr));
client->last_addr = memdup(last_addr, sizeof(struct in_addr));

> +
> +return 0;
> +}
> +
> +int dhcp_client_set_index(DHCPClient *client, int interface_index)
> +{
> +if (!client || interface_index < -1)
> +return -EINVAL;
> +
> +if (client->state != DHCP_STATE_INIT)
> +return -EBUSY;
> +
> +client->index = interface_index;
> +
> +return 0;
> +}
> +
> +DHCPClient *dhcp_client_new(void)
> +{
> +DHCPClient *client;
> +int i;
> +
> +client = new0(DHCPClient, 1);
> +if (!client)
> +return NULL;
> +
> +client->state = DHCP_STATE_INIT;
> +client->index = -1;
> +
> +client->req_opts_size = sizeof(default_req_opts)
> +/ sizeof(default_req_opts[0]);
ELEMENTSOF(default_req_opts)

> +client->req_opts = malloc(client->req_opts_size);
> +for (i = 0; i < client->req_opts_size; i++)
> +client->req_opts[i]

Re: [systemd-devel] [PATCH 01/28] dhcp: Add DHCP protocol structures and initial defines

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:29PM +0200, Patrik Flykt wrote:
> Create a new directory to host DHCP components.
> ---
>  src/dhcp/protocol.h |   93 
> +++
>  1 file changed, 93 insertions(+)
>  create mode 100644 src/dhcp/protocol.h
> 
> diff --git a/src/dhcp/protocol.h b/src/dhcp/protocol.h
> new file mode 100644
> index 000..3fc4baf
> --- /dev/null
> +++ b/src/dhcp/protocol.h
> @@ -0,0 +1,93 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +#pragma once
> +
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see .
> +***/
> +
> +#include 
> +#include 
> +#include 
> +
> +struct DHCPMessage {
> +uint8_t op;
> +uint8_t htype;
> +uint8_t hlen;
> +uint8_t hops;
> +uint32_t xid;
> +uint16_t secs;
> +uint16_t flags;
> +uint32_t ciaddr;
> +uint32_t yiaddr;
> +uint32_t siaddr;
> +uint32_t giaddr;
> +uint8_t chaddr[16];
> +uint8_t sname[64];
> +uint8_t file[128];
> +} __attribute__((packed));
We have _packed_.

> +
> +typedef struct DHCPMessage DHCPMessage;
> +
> +struct DHCPPacket {
> +struct iphdr ip;
> +struct udphdr udp;
> +DHCPMessage dhcp;
> +} __attribute__((packed));
> +
> +typedef struct DHCPPacket DHCPPacket;
> +
> +enum DHCPState {
> +DHCP_STATE_INIT = 0,
> +DHCP_STATE_SELECTING= 1,
> +DHCP_STATE_INIT_REBOOT  = 2,
> +DHCP_STATE_REBOOTING= 3,
> +DHCP_STATE_REQUESTING   = 4,
> +DHCP_STATE_BOUND= 5,
> +DHCP_STATE_RENEWING = 6,
> +DHCP_STATE_REBINDING= 7,
> +};
> +
> +typedef enum DHCPState DHCPState;
> +
> +#define BOOTREQUEST 1
> +#define BOOTREPLY   2
> +
> +#define DHCP_DISCOVER   1
> +#define DHCP_OFFER  2
> +#define DHCP_REQUEST3
> +#define DHCP_DECLINE4
> +#define DHCP_ACK5
> +#define DHCP_NAK6
> +#define DHCP_RELEASE7
> +
> +#define DHCP_OVERLOAD_FILE  1
> +#define DHCP_OVERLOAD_SNAME 2
> +
> +#define DHCP_OPTION_PAD 0
> +#define DHCP_OPTION_SUBNET_MASK 1
> +#define DHCP_OPTION_ROUTER  3
> +#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
> +#define DHCP_OPTION_HOST_NAME   12
> +#define DHCP_OPTION_DOMAIN_NAME 15
> +#define DHCP_OPTION_NTP_SERVER  42
> +#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
> +#define DHCP_OPTION_OVERLOAD52
> +#define DHCP_OPTION_MESSAGE_TYPE53
> +#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
> +#define DHCP_OPTION_END 255

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

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

[Colin Walters]

On Thu, 2013-11-14 at 07:25 +0900, Marcel Holtmann wrote:

> I am a bit lost on your concerns here. Our focus for ConnMan is 
> libsystemd-bus and kdbus support. 

Yeah, sorry; I just kind of used your mail as a basis for the larger
picture of sd_event as public API.

> And as a system daemon, I only care about writing to the Journal. 
> No idea why I would need a event loop for that.

Ah...even if you're not spawning external processes, I'd imagine you at
least want g_timeout_add()/sd_event_add_monotonic() type thing where
library code manipulates the poll timeout.





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

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

[Marcel Holtmann]

Hi Colin,

>> that is the long term plan. Once ConnMan is switching over to use 
>> libsystemd-bus and kdbus,
>> we are switching over to using the systemd event loop instead of GLib main 
>> loop 
> 
> But I think the long term architecturally correct place for the core
> system main loop is glibc, not systemd.
> 
> For example, that would allow runtimes like OpenJDK and Python to
> unconditionally depend on it (if present).
> 
> As systemd's event loop becomes public API, it has the potential to
> create interoperability problems for GNOME - think applications like 
> https://git.gnome.org/browse/gnome-logs
> that want to both monitor the systemd journal and talk to GTK+.
> 
> At the moment it's OK because sd_journal has APIs sufficient to plug in
> external loops.
> 
> Adding a mainloop API to glibc would be a lot of work - it might even
> entail trying to get it by POSIX.  At least it'd entail describing the
> interaction with the other POSIX APIs.  But I think that'd be worth the
> effort.

I am a bit lost on your concerns here. Our focus for ConnMan is libsystemd-bus 
and kdbus support. Over time we will be removing our dependency on GLib 
completely. So the systemd event loop will become our only event loop.

And as a system daemon, I only care about writing to the Journal. No idea why I 
would need a event loop for that.

Regards

Marcel

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

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

[Colin Walters]

On Thu, 2013-11-14 at 06:49 +0900, Marcel Holtmann wrote:

> that is the long term plan. Once ConnMan is switching over to use 
> libsystemd-bus and kdbus,
> we are switching over to using the systemd event loop instead of GLib main 
> loop 

But I think the long term architecturally correct place for the core
system main loop is glibc, not systemd.

For example, that would allow runtimes like OpenJDK and Python to
unconditionally depend on it (if present).

As systemd's event loop becomes public API, it has the potential to
create interoperability problems for GNOME - think applications like 
https://git.gnome.org/browse/gnome-logs
that want to both monitor the systemd journal and talk to GTK+.

At the moment it's OK because sd_journal has APIs sufficient to plug in
external loops.

Adding a mainloop API to glibc would be a lot of work - it might even
entail trying to get it by POSIX.  At least it'd entail describing the
interaction with the other POSIX APIs.  But I think that'd be worth the
effort.





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

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

[Patrik Flykt]

Hi,

On Thu, 2013-11-14 at 06:31 +0900, Greg KH wrote:

> Nice stuff.  Where did this code come from, conman?  If so, for some
> reason I thought that it would be easier to make a library of the
> existing conman dhcp code and have both projects use it instead of
> forking and having two copies?

Looking more carefully at the code in ConnMan started to reveal the
relative age of the DHCP client part and showed some interesting code
that might not be entirely correct. After removing the GLib main loop
parts not too much valuable code was left behind. I have kept one eye in
the existing code to find additions for picky servers so I hope the
lessons learned have not been lost. I'm more positive with the DHCPv6
part, that implementation has more reusable parts.

I'm not too worried about having two copies for now, the one in ConnMan
would have met its fate anyway a bit later.


Cheers,

Patrik

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

Re: [systemd-devel] systemd-networkd questions

[Marcel Holtmann]

Hi Bill,

>> Without criticizing any of the existing solutions, some of the things
>> that motivated my interest in this is that I think we need: something
>> easily configured via plain configuration files by a sysadmin,
>> something that would take a limited amount of space (including its
>> dependencies) so it could be reasonably used in an initrd, something
>> which would be fast/efficient, so not e.g. calling out to external
>> tools/daemons for stuff like dhcp/ipv4ll.
> 
> On this specific point... are you *sure* you want to be rewriting a
> dhcp/dhcpv6 client?

yes, we want to do exactly that.

> (I ask, because we've gone through at least three over history here, and
> dealing with the assorted corner cases between vendors and configuration
> bits required would seem to make it one area not worth trying to reinvent.)

I wonder who this magical we is in this statement ;)

Speaking from ConnMan history, we actually went through the whole process of 
fork+exec of dhclient/udhcpc/autoipd for a period of time. But really early on, 
it was clear that this is not acceptable. Just spawning external processes and 
hoping for the best is not an approach that is sustainable. The state machine 
is way more complex and they are interdependencies between them.

For example just the simple case of DHCPv4 and IPv4-LL is something you need to 
run through a single state machine to get this working as it is suppose to be. 
Inside ConnMan we actually got this right since they guys running the Nao 
robots are using this heavily. Or have a look at DHCPv6 and prefix delegation. 
Having to spawn 3 instances of dhclient to get your local network configured is 
just crazy. None of these clients are synchronized with each other. Also just 
the simple fact of setting up 6to4 as fallback are these tiny details you want 
to account for.

Since a little over 2 years, ConnMan is running its own DHCP client and server. 
It runs in commercial devices like the Nest and other devices. It is 
significant more stable, more feature complete and faster. I presented our 
numbers at LinuxCon Vancouver a few years back. Look it up if you are 
interested.

Regards

Marcel

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

Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[NeilBrown]

On Wed, 13 Nov 2013 22:11:27 +0600 "Alexander E. Patrakov"
 wrote:

> 2013/11/13 NeilBrown :
> > On Tue, 12 Nov 2013 19:01:49 +0400 Andrey Borzenkov 
> > wrote:
> >
> >> В Tue, 12 Nov 2013 21:17:19 +1100
> >> NeilBrown  пишет:
> >>
> >> > On Tue, 12 Nov 2013 18:16:24 +0900 Greg KH  
> >> > wrote:
> >> >
> >> > > On Tue, Nov 12, 2013 at 07:54:42PM +1100, NeilBrown wrote:
> >> > > > On Tue, 12 Nov 2013 00:10:28 -0800 Greg KH 
> >> > > >  wrote:
> >> > > >
> >> > > > > On Tue, Nov 12, 2013 at 11:31:45AM +1100, NeilBrown wrote:
> >> > > > > > Alternately, is there some "all devices have been probed, 
> >> > > > > > nothing new will
> >> > > > > > appear unless it is hot-plugged" event.  That would be equally 
> >> > > > > > useful (and
> >> > > > > > probably mirrors what hardware-RAID cards do).
> >> > > > >
> >> > > > > No, there's no way to ever know this in a hotplug world, sorry.
> >> > > > > Especially with USB devices, they show up when they show up, 
> >> > > > > there's no
> >> > > > > "oh look, the bus is all scanned now and all devices currently 
> >> > > > > plugged
> >> > > > > in are found" type knowledge at all.
> >> > > > >
> >> > > > > Then there are hotplug PCI systems where people slam in PCI cards
> >> > > > > whenever they feel like it (remember, thunderbolt is PCI 
> >> > > > > express...)
> >> > > > >
> >> > > > > Sorry,
> >> > > > >
> >> > > > > greg k-h
> >> > > >
> >> > > > Surely something must be possible.
> >> > >
> >> > > For USB, nope, there isn't, sorry.
> >> > >
> >> > > > Clearly a physical hot-plug event will cause more devices to appear, 
> >> > > > but
> >> > > > there must come a point at which no more (non-virtual) devices will 
> >> > > > appear
> >> > > > unless a physical event happens?
> >> > >
> >> > > Not for USB, sorry.
> >> > >
> >> > > The USB bus just announces devices when it finds them, there is no "all
> >> > > is quiet" type signal or detection.
> >> > >
> >> > > Same for PCI hotplug, devices can show up at any point in time, you
> >> > > never know when, and you don't know when all devices are "found".
> >> > >
> >> > > sorry,
> >> > >
> >> > > greg k-h
> >> >
> >> >
> >> > Hmmm... OK.  USB doesn't bother me a lot, but PCI is important.
> >> >
> >> > I guess I'll just have to settle for a timeout much like the current
> >> > device-discovery timeout that systemd has.
> >> > Still hoping someone can tell me how to plug into that though...
> >> >
> >>
> >> If information about array name or other identification is available in
> >> udev rule (I see reference to device node only) what you can do is to
> >> start timer with "now+5second" (pick your timeout) that simply fires off
> >> mdadm -IRs for specific array. Something like
> >>
> >> mdadm-last-resort@.timer
> >>
> >> [Timer]
> >> OnCalendar=+5s
> >>
> >> mdadm-last-resort@.service
> >>
> >> [Service]
> >> Type=oneshot
> >> ExecStart=/sbin/mdadm -IRs %n
> >>
> >> udev rule
> >>
> >> ... SYSTEMD_WANTS=mdadm-last-resort@$ENV{SOMETHING_UNIQUE}.timer
> >>
> >
> > Thanks.  This certainly looks interesting and might be part of a solution.
> > However it gets the timeout test backwards.
> >
> > I don't want to set the timeout when the array starts to appear.  I want to
> > set the time out when someone wants to use the array.
> > If no-one is waiting for the array device, then there is no point forcing 
> > it.
> >
> > That's why I want to plug into the timeout that systemd already has.
> >
> > Maybe that requirement isn't really necessary though.  I'll experiment with
> > your approach.
> 
> It is useless to even try to plug into the existing systemd timeout,
> for a very simple reason. in the setups where your RAID array is not
> on the top of the storage device hierarchy, systemd does not know that
> it wants your RAID array to appear.
> 
> So the statement "If no-one is waiting for the array device, then
> there is no point forcing it" is false, because there is no way to
> know that no-one is waiting.
> 

"useless" seems a bit harsh.  "not-optimal" may be true.

If systemd was waiting for a device, then it is clear that something was
waiting for something.  In this case it might be justified to activate as
much as possible in the hope that the important things will get activated.
This is what "mdadm -IRs" does.  It activates all arrays that are still
inactive but have enough devices to become active (though degraded).  It
isn't selective.
If they are deep in some hierarchy, then udev will pull it all together and
the root device will appear.

If systemd is not waiting for a device, then there is no justification for
prematurely starting degraded arrays.


Maybe I could get emergency.service to run "mdadm -IRs" and if that actually
started anything, then to somehow restart local-fs.target.  Might that be
possible?

Thanks,
NeilBrown


signature.asc
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.

Re: [systemd-devel] systemd-networkd questions

[Tom Gundersen]

On Wed, Nov 13, 2013 at 10:41 PM, Patrik Flykt
 wrote:
>
> Hi,
>
> On Wed, 2013-11-13 at 17:39 +0100, Tom Gundersen wrote:
>
>> I'm sure I would like avoid that :) Hopefully we'll be able to reuse
>> the dhcp client from connman, but as the work of converting that into
>> a library has not yet finished I don't know yet exactly how that will
>> work out.
>
> Taking out the glib event loop from the existing implementation in
> ConnMan and ironing out the relative age of it did leave quite a lot to
> be desired. It was unfortunately (again) easier to build something anew
> bottoms up than to insert the existing code into a new context. I fished
> out the accumulated quirks in the current code, though, so all has not
> been lost here.
>
> The DHCPv6 part in ConnMan is of much newer date, looking at it makes me
> believe more can be reused there.
>
>>  Hopefully this will become clearer very soon.
>
> Patches have appeared on the mailing list...

Awesome stuff Patrik!

Cheers,

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

Re: [systemd-devel] systemd-networkd questions

[Patrik Flykt]

Hi,

On Wed, 2013-11-13 at 17:39 +0100, Tom Gundersen wrote:

> I'm sure I would like avoid that :) Hopefully we'll be able to reuse
> the dhcp client from connman, but as the work of converting that into
> a library has not yet finished I don't know yet exactly how that will
> work out.

Taking out the glib event loop from the existing implementation in
ConnMan and ironing out the relative age of it did leave quite a lot to
be desired. It was unfortunately (again) easier to build something anew
bottoms up than to insert the existing code into a new context. I fished
out the accumulated quirks in the current code, though, so all has not
been lost here.

The DHCPv6 part in ConnMan is of much newer date, looking at it makes me
believe more can be reused there.

>  Hopefully this will become clearer very soon.

Patches have appeared on the mailing list...


Cheers,

Patrik

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

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

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

[Marcel Holtmann]

Hi Greg,

>> This patch set implements a DHCPv4 client library named libsystemd-dhcp
>> to be used with systemd-network.
> 
> Nice stuff.  Where did this code come from, conman?  If so, for some
> reason I thought that it would be easier to make a library of the
> existing conman dhcp code and have both projects use it instead of
> forking and having two copies?

that is the long term plan. Once ConnMan is switching over to use 
libsystemd-bus and kdbus, we are switching over to using the systemd event loop 
instead of GLib main loop and then it will also start using libsystemd-dhcp.

Regards

Marcel

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

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

[Greg KH]

On Wed, Nov 13, 2013 at 11:22:28PM +0200, Patrik Flykt wrote:
>   Hi,
> 
> This patch set implements a DHCPv4 client library named libsystemd-dhcp
> to be used with systemd-network.

Nice stuff.  Where did this code come from, conman?  If so, for some
reason I thought that it would be easier to make a library of the
existing conman dhcp code and have both projects use it instead of
forking and having two copies?

thanks,

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

[systemd-devel] [PATCH 09/28] dhcp: Add option append tests

[Patrik Flykt]

Add checks for invalid lengths and parameters when using the option
appending function. Add also checks for adding options, see to it
that the resulting array is identical to the array of options added.
---
 src/dhcp/test-dhcp-option.c |   71 +++
 1 file changed, 71 insertions(+)

diff --git a/src/dhcp/test-dhcp-option.c b/src/dhcp/test-dhcp-option.c
index a2b79ce..51ed47a 100644
--- a/src/dhcp/test-dhcp-option.c
+++ b/src/dhcp/test-dhcp-option.c
@@ -294,6 +294,75 @@ static void test_options(struct option_desc *desc)
 free(message);
 }
 
+static uint8_t result[64] = {
+'A', 'B', 'C', 'D',
+};
+
+static uint8_t options[64] = {
+'A', 'B', 'C', 'D',
+160, 2, 0x11, 0x12,
+0,
+31, 8, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38,
+0,
+55, 3, 0x51, 0x52, 0x53,
+17, 7, 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77,
+255
+};
+
+static void test_option_set(void)
+{
+int len, pos, oldlen, i;
+uint8_t *opt;
+
+assert(__dhcp_option_append(NULL, NULL, 0, 0, NULL) == -EINVAL);
+
+len = 0;
+opt = &result[0];
+assert(__dhcp_option_append(&opt, NULL, 0, 0, NULL) == -EINVAL);
+assert(opt == &result[0] && len == 0);
+
+assert(__dhcp_option_append(&opt, &len, DHCP_OPTION_PAD,
+0, NULL) == -ENOBUFS);
+assert(opt == &result[0] && len == 0);
+
+opt = &result[4];
+len = 1;
+assert(__dhcp_option_append(&opt, &len, DHCP_OPTION_PAD,
+0, NULL) >= 0);
+assert(opt == &result[5] && len == 0);
+
+pos = 4;
+len = 60;
+while (pos < 64 && options[pos] != DHCP_OPTION_END) {
+opt = &result[pos];
+oldlen = len;
+
+assert(__dhcp_option_append(&opt, &len, options[pos],
+options[pos + 1],
+&options[pos + 2]) >= 0);
+
+if (options[pos] == DHCP_OPTION_PAD) {
+assert(opt == &result[pos + 1]);
+assert(len == oldlen - 1);
+pos++;
+} else {
+assert(opt == &result[pos + 2 + options[pos + 1]]);
+assert(len == oldlen - 2 - options[pos + 1]);
+pos += 2 + options[pos + 1];
+}
+}
+
+for (i = 0; i < pos; i++) {
+if (verbose)
+printf("%2d: 0x%02x(0x%02x)\n", i, result[i],
+   options[i]);
+assert(result[i] == options[i]);
+}
+
+if (verbose)
+printf ("\n");
+}
+
 int main(int argc, char *argv[])
 {
 unsigned int i;
@@ -306,5 +375,7 @@ int main(int argc, char *argv[])
 for (i = 0; i < sizeof(option_tests)/sizeof(struct option_desc); i++)
 test_options(&option_tests[i]);
 
+test_option_set();
+
 return 0;
 }
-- 
1.7.10.4

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

[systemd-devel] [PATCH 28/28] dhcp: Use callback in test client

[Patrik Flykt]

Print out the event received and possibly also IP related data.
---
 src/dhcp/dhcp-example-client.c |   53 
 1 file changed, 53 insertions(+)

diff --git a/src/dhcp/dhcp-example-client.c b/src/dhcp/dhcp-example-client.c
index afb7102..3635bd8 100644
--- a/src/dhcp/dhcp-example-client.c
+++ b/src/dhcp/dhcp-example-client.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #include "client.h"
 
@@ -79,6 +82,52 @@ static int get_index(char *ifname)
 return ifr.ifr_ifindex;
 }
 
+static const char *print_event(int event)
+{
+if (event < 0)
+return strerror(-event);
+
+switch (event) {
+case DHCP_EVENT_STOP:
+return "Stopped";
+case DHCP_EVENT_NAK:
+return "DHCP NAK";
+case DHCP_EVENT_IP_ACQUIRE:
+return "DHCP address acquired";
+case DHCP_EVENT_IP_CHANGE:
+return "DHCP address changed";
+case DHCP_EVENT_EXPIRED:
+return "DHCP lease expired";
+default:
+break;
+}
+
+return "unknown";
+}
+
+static void print_state(DHCPClient *client, int event, void *userdata)
+{
+sd_event *e = userdata;
+struct in_addr addr;
+
+if (event < 0) {
+printf("Error %d %s\n", event, print_event(event));
+sd_event_unref(e);
+return;
+}
+
+printf("Event %s\n", print_event(event));
+
+if (dhcp_client_get_address(client, &addr) >= 0)
+printf("Address %s\n", inet_ntoa(addr));
+
+if (dhcp_client_get_netmask(client, &addr) >= 0)
+printf("Netmask %s\n", inet_ntoa(addr));
+
+if (dhcp_client_get_router(client, &addr) >= 0)
+printf("Default router %s\n", inet_ntoa(addr));
+}
+
 int main(int argc, char **argv)
 {
 DHCPClient *client;
@@ -109,9 +158,13 @@ int main(int argc, char **argv)
 printf("Interface %s index %d\n", argv[1], index);
 dhcp_client_set_index(client, index);
 dhcp_client_set_mac(client, &mac);
+dhcp_client_set_callback(client, print_state, &event);
 
 dhcp_client_start(client);
 sd_event_loop(event);
 
+dhcp_client_free(client);
+printf("Exit\n");
+
 return 0;
 }
-- 
1.7.10.4

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

[systemd-devel] [PATCH 13/28] build: Add libsystemd-dhcp

[Patrik Flykt]

---
 Makefile.am |   20 
 1 file changed, 20 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index aeca484..5c350ab 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3740,6 +3740,26 @@ endif
 
 # 
--
 if ENABLE_DHCP
+libsystemd_dhcp_la_SOURCES = \
+   src/dhcp/protocol.h \
+   src/dhcp/internal.h \
+   src/dhcp/network.c \
+   src/dhcp/option.c \
+   src/dhcp/client.h \
+   src/dhcp/client.c
+
+noinst_LTLIBRARIES += \
+   libsystemd-dhcp.la
+
+libsystemd_dhcp_la_CFLAGS = \
+   $(AM_CFLAGS)
+
+libsystemd_dhcp_la_LDFLAGS = \
+   $(AM_LDFLAGS)
+
+libsystemd_dhcp_la_LIBADD = \
+   libsystemd-shared.la
+
 test_dhcp_option_SOURCES = \
src/dhcp/protocol.h \
src/dhcp/internal.h \
-- 
1.7.10.4

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

[systemd-devel] [PATCH 18/28] dhcp: Add function to stop the DHCP client

[Patrik Flykt]

The client is stopped and brought back to its initial state.
---
 src/dhcp/client.c |   35 +++
 src/dhcp/client.h |1 +
 2 files changed, 36 insertions(+)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index dc92880..79ac93b 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -133,6 +133,36 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
ether_addr *addr)
 return 0;
 }
 
+static int client_stop(DHCPClient *client, int error)
+{
+if (!client)
+return -EINVAL;
+
+if (client->state == DHCP_STATE_INIT ||
+client->state == DHCP_STATE_INIT_REBOOT)
+return -EALREADY;
+
+switch (client->state) {
+
+case DHCP_STATE_INIT:
+case DHCP_STATE_SELECTING:
+
+client->state = DHCP_STATE_INIT;
+break;
+
+case DHCP_STATE_INIT_REBOOT:
+case DHCP_STATE_REBOOTING:
+case DHCP_STATE_REQUESTING:
+case DHCP_STATE_BOUND:
+case DHCP_STATE_RENEWING:
+case DHCP_STATE_REBINDING:
+
+break;
+}
+
+return 0;
+}
+
 static int client_packet_init(DHCPClient *client, uint8_t type,
   DHCPMessage *message, uint16_t secs,
   uint8_t **opt, int *optlen)
@@ -278,6 +308,11 @@ int dhcp_client_start(DHCPClient *client)
 return client_send_discover(client, 0);
 }
 
+int dhcp_client_stop(DHCPClient *client)
+{
+return client_stop(client, 0);
+}
+
 DHCPClient *dhcp_client_new(void)
 {
 DHCPClient *client;
diff --git a/src/dhcp/client.h b/src/dhcp/client.h
index fb7682a..71fdbe3 100644
--- a/src/dhcp/client.h
+++ b/src/dhcp/client.h
@@ -34,4 +34,5 @@ int dhcp_client_set_index(DHCPClient *client, int 
interface_index);
 int dhcp_client_set_mac(DHCPClient *client, struct ether_addr *addr);
 
 int dhcp_client_start(DHCPClient *client);
+int dhcp_client_stop(DHCPClient *client);
 DHCPClient *dhcp_client_new(void);
-- 
1.7.10.4

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

[systemd-devel] [PATCH 26/28] dhcp: Add notification callback

[Patrik Flykt]

Define a notification callback and events for stopping and client
lease expiry. Add functions to fetch IP parameters from a lease.
---
 src/dhcp/client.c |   98 -
 src/dhcp/client.h |9 +
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 50008a9..0144c70 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -63,6 +63,8 @@ struct DHCPClient {
 sd_event_source *timeout_expire;
 sd_event_source *timeout_t1;
 sd_event_source *timeout_t2;
+dhcp_client_cb_t cb;
+void *userdata;
 DHCPLease *lease;
 };
 
@@ -75,6 +77,19 @@ static uint8_t default_req_opts[] = {
 DHCP_OPTION_NTP_SERVER,
 };
 
+
+int dhcp_client_set_callback(DHCPClient *client, dhcp_client_cb_t cb,
+ void *userdata)
+{
+if (!client)
+return -EINVAL;
+
+client->cb = cb;
+client->userdata = userdata;
+
+return 0;
+}
+
 int dhcp_client_set_request_option(DHCPClient *client, uint8_t option)
 {
 int i;
@@ -157,8 +172,83 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
ether_addr *addr)
 return 0;
 }
 
+int dhcp_client_get_address(DHCPClient *client, struct in_addr *addr)
+{
+if (!addr || !client)
+return -EINVAL;
+
+switch (client->state) {
+case DHCP_STATE_INIT:
+case DHCP_STATE_SELECTING:
+case DHCP_STATE_INIT_REBOOT:
+case DHCP_STATE_REBOOTING:
+case DHCP_STATE_REQUESTING:
+return -EADDRNOTAVAIL;
+
+case DHCP_STATE_BOUND:
+case DHCP_STATE_RENEWING:
+case DHCP_STATE_REBINDING:
+addr->s_addr = client->lease->address.s_addr;
+
+break;
+}
+
+return 0;
+}
+
+int dhcp_client_get_netmask(DHCPClient *client, struct in_addr *addr)
+{
+if (!addr || !client)
+return -EINVAL;
+
+switch (client->state) {
+case DHCP_STATE_INIT:
+case DHCP_STATE_SELECTING:
+case DHCP_STATE_INIT_REBOOT:
+case DHCP_STATE_REBOOTING:
+case DHCP_STATE_REQUESTING:
+return -EADDRNOTAVAIL;
+
+case DHCP_STATE_BOUND:
+case DHCP_STATE_RENEWING:
+case DHCP_STATE_REBINDING:
+addr->s_addr = client->lease->subnet_mask.s_addr;
+
+break;
+}
+
+return 0;
+}
+
+int dhcp_client_get_router(DHCPClient *client, struct in_addr *addr)
+{
+if (!addr || !client)
+return -EINVAL;
+
+switch (client->state) {
+case DHCP_STATE_INIT:
+case DHCP_STATE_SELECTING:
+case DHCP_STATE_INIT_REBOOT:
+case DHCP_STATE_REBOOTING:
+case DHCP_STATE_REQUESTING:
+return -EADDRNOTAVAIL;
+
+case DHCP_STATE_BOUND:
+case DHCP_STATE_RENEWING:
+case DHCP_STATE_REBINDING:
+addr->s_addr = client->lease->router.s_addr;
+
+break;
+}
+
+return 0;
+}
+
 static int client_notify(DHCPClient *client, int event)
 {
+if (client->cb)
+client->cb(client, event, client->userdata);
+
 return 0;
 }
 
@@ -195,6 +285,8 @@ static int client_stop(DHCPClient *client, int error)
 
 client->attempt = 1;
 
+client_notify(client, error);
+
 switch (client->state) {
 
 case DHCP_STATE_INIT:
@@ -487,6 +579,10 @@ error:
 static int client_timeout_expire(sd_event_source *s, uint64_t usec,
  void *userdata)
 {
+DHCPClient *client = userdata;
+
+client_stop(client, DHCP_EVENT_EXPIRED);
+
 return 0;
 }
 
@@ -895,7 +991,7 @@ error:
 
 int dhcp_client_stop(DHCPClient *client)
 {
-return client_stop(client, 0);
+return client_stop(client, DHCP_EVENT_STOP);
 }
 
 DHCPClient *dhcp_client_new(sd_event *event)
diff --git a/src/dhcp/client.h b/src/dhcp/client.h
index 5ce7fb4..adb4443 100644
--- a/src/dhcp/client.h
+++ b/src/dhcp/client.h
@@ -26,19 +26,28 @@
 
 #include "sd-event.h"
 
+#define DHCP_EVENT_STOP 0
 #define DHCP_EVENT_NAK  1
 #define DHCP_EVENT_IP_ACQUIRE   2
 #define DHCP_EVENT_IP_CHANGE3
+#define DHCP_EVENT_EXPIRED  4
 
 struct DHCPClient;
 typedef struct DHCPClient DHCPClient;
 
+typedef void (*dhcp_client_cb_t)(DHCPClient *client, int event, void 
*userdata);
+int dhcp_client_set_callback(DHCPClient *client, dhcp_client_cb_t cb,
+ void *userdata);
 int dhcp_client_set_request_option(DHCPClient *client, uint8_t option);
 int dhcp_client_set_request_address(DHCPClient *client,
 struct in_addr *last_address);
 int dhcp_client_set_index(DHCPClient *client, int interface_index);
 int dhcp_client_set_mac(DHCPClient *client, st

[systemd-devel] [PATCH 12/28] dhcp: Add DHCP discover sending

[Patrik Flykt]

On starting the client, use the supplied interface mac address and create
a transaction id. Puzzle together an IP/UDP/DHCP Discover message, compute
checksums and send it out as a raw packet.

Create an additional function that constructs default options common to
all DHCP messages.

Set the DHCP Client ID option as noticed by Grant Erickson in ConnMan
commit b18d9798b3a0ae46ed87d6d2be8d5a474bf3ab1e:

   "Some Internet gateways and Wi-Fi access points are unhappy when the
DHCPv4 client-id option (61) is missing and will refuse to issue a
DHCP lease."
---
 src/dhcp/client.c   |  163 +++
 src/dhcp/client.h   |3 +
 src/dhcp/protocol.h |4 ++
 3 files changed, 170 insertions(+)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 172ddd9..1d4d957 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -21,19 +21,25 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "util.h"
 #include "list.h"
 
 #include "protocol.h"
+#include "internal.h"
 #include "client.h"
 
+#define DHCP_CLIENT_MIN_OPTIONS_SIZE312
+
 struct DHCPClient {
 DHCPState state;
 int index;
 uint8_t *req_opts;
 int req_opts_size;
 struct in_addr *last_addr;
+struct ether_addr mac_addr;
+uint32_t xid;
 };
 
 static uint8_t default_req_opts[] = {
@@ -114,6 +120,163 @@ int dhcp_client_set_index(DHCPClient *client, int 
interface_index)
 return 0;
 }
 
+int dhcp_client_set_mac(DHCPClient *client, struct ether_addr *addr)
+{
+if (!client)
+return -EINVAL;
+
+if (client->state != DHCP_STATE_INIT)
+return -EBUSY;
+
+memcpy(&client->mac_addr, addr, ETH_ALEN);
+
+return 0;
+}
+
+static int client_packet_init(DHCPClient *client, uint8_t type,
+  DHCPMessage *message, uint8_t **opt,
+  int *optlen)
+{
+int err;
+
+*opt = (uint8_t *)(message + 1);
+
+*optlen -= 4;
+if (*optlen < 0)
+return -ENOBUFS;
+
+message->op = BOOTREQUEST;
+message->htype = 1;
+message->hlen = ETHER_ADDR_LEN;
+message->xid = htonl(client->xid);
+
+memcpy(&message->chaddr, &client->mac_addr, ETH_ALEN);
+(*opt)[0] = 0x63;
+(*opt)[1] = 0x82;
+(*opt)[2] = 0x53;
+(*opt)[3] = 0x63;
+
+*opt += 4;
+
+err = __dhcp_option_append(opt, optlen, DHCP_OPTION_MESSAGE_TYPE,
+   1, &type);
+if (err < 0)
+return err;
+
+err = __dhcp_option_append(opt, optlen,
+   DHCP_OPTION_CLIENT_IDENTIFIER,
+   ETH_ALEN, &client->mac_addr);
+if (err < 0)
+return err;
+
+ if (type == DHCP_DISCOVER || type == DHCP_REQUEST) {
+err = __dhcp_option_append(opt, optlen,
+   DHCP_OPTION_PARAMETER_REQUEST_LIST,
+   client->req_opts_size,
+   client->req_opts);
+if (err < 0)
+return err;
+}
+
+return 0;
+}
+
+static uint16_t client_checksum(void *buf, int len)
+{
+uint32_t sum;
+uint16_t *check;
+int i;
+uint8_t *odd;
+
+sum = 0;
+check = buf;
+
+for (i = 0; i < len / 2 ; i++)
+sum += check[i];
+
+if (len & 0x01) {
+odd = buf;
+sum += odd[len];
+}
+
+return ~((sum & 0x) + (sum >> 16));
+}
+
+static int client_send_discover(DHCPClient *client)
+{
+int err = 0;
+DHCPPacket *discover;
+int optlen, len;
+uint8_t *opt;
+
+optlen = DHCP_CLIENT_MIN_OPTIONS_SIZE;
+len = sizeof(DHCPPacket) + optlen;
+
+discover = malloc0(len);
+
+if (!discover)
+return -ENOBUFS;
+
+err = client_packet_init(client, DHCP_DISCOVER, &discover->dhcp,
+ &opt, &optlen);
+if (err < 0)
+goto error;
+
+if (client->last_addr) {
+err = __dhcp_option_append(&opt, &optlen,
+   DHCP_OPTION_REQUESTED_IP_ADDRESS,
+   4, &client->last_addr->s_addr);
+if (err < 0)
+goto error;
+}
+
+err = __dhcp_option_append(&opt, &optlen, DHCP_OPTION_END, 0, NULL);
+if (err < 0)
+goto error;
+
+discover->ip.version = IPVERSION;
+discover->ip.ihl = sizeof(discover->ip) >> 2;
+discover->ip.tot_len = htons(len);
+
+discover->ip.protocol = IPPROTO_UDP;
+discover->ip.saddr = INADDR_ANY;
+discover->ip.daddr = INADDR_BROADCAST;
+
+discover-

[systemd-devel] [PATCH 24/28] dhcp: Process DHCP Ack/Nak message

[Patrik Flykt]

Process a DHCP Ack/Nak in much the same way as an DHCP Offer. Factor
out header verification and process options sent. Add notification
functionality with discrete values for the outcome of the DHCP Ack/
Nak processing.
---
 src/dhcp/client.c |  145 +
 src/dhcp/client.h |4 ++
 2 files changed, 128 insertions(+), 21 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 45c62f3..669804a 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -151,6 +151,11 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
ether_addr *addr)
 return 0;
 }
 
+static int client_notify(DHCPClient *client, int event)
+{
+return 0;
+}
+
 static int client_stop(DHCPClient *client, int error)
 {
 if (!client)
@@ -178,6 +183,7 @@ static int client_stop(DHCPClient *client, int error)
 
 case DHCP_STATE_INIT:
 case DHCP_STATE_SELECTING:
+case DHCP_STATE_REQUESTING:
 
 client->start_time = 0;
 client->state = DHCP_STATE_INIT;
@@ -185,7 +191,6 @@ static int client_stop(DHCPClient *client, int error)
 
 case DHCP_STATE_INIT_REBOOT:
 case DHCP_STATE_REBOOTING:
-case DHCP_STATE_REQUESTING:
 case DHCP_STATE_BOUND:
 case DHCP_STATE_RENEWING:
 case DHCP_STATE_REBINDING:
@@ -499,41 +504,52 @@ static int client_parse_offer(uint8_t code, uint8_t len, 
uint8_t *option,
 return 0;
 }
 
-static int client_receive_offer(DHCPClient *client,
-DHCPPacket *offer, int len)
+static int client_verify_headers(DHCPClient *client,
+ DHCPPacket *message, int len)
 {
 int hdrlen;
-DHCPLease *lease;
 
 if (len < (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
 return -EINVAL;
 
-hdrlen = offer->ip.ihl * 4;
-if (hdrlen < 20 || hdrlen > len || client_checksum(&offer->ip,
+hdrlen = message->ip.ihl * 4;
+if (hdrlen < 20 || hdrlen > len || client_checksum(&message->ip,
hdrlen))
 return -EINVAL;
 
-offer->ip.check = offer->udp.len;
-offer->ip.ttl = 0;
+message->ip.check = message->udp.len;
+message->ip.ttl = 0;
 
-if (hdrlen + ntohs(offer->udp.len) > len ||
-client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
+if (hdrlen + ntohs(message->udp.len) > len ||
+client_checksum(&message->ip.ttl, ntohs(message->udp.len) + 12))
 return -EINVAL;
 
-if (ntohs(offer->udp.source) != DHCP_PORT_SERVER ||
-ntohs(offer->udp.dest) != DHCP_PORT_CLIENT)
+if (ntohs(message->udp.source) != DHCP_PORT_SERVER ||
+ntohs(message->udp.dest) != DHCP_PORT_CLIENT)
 return -EINVAL;
 
-if (offer->dhcp.op != BOOTREPLY)
+if (message->dhcp.op != BOOTREPLY)
 return -EINVAL;
 
-if (ntohl(offer->dhcp.xid) != client->xid)
+if (ntohl(message->dhcp.xid) != client->xid)
 return -EINVAL;
 
-if (memcmp(&offer->dhcp.chaddr[0], &client->mac_addr.ether_addr_octet,
+if (memcmp(&message->dhcp.chaddr[0], 
&client->mac_addr.ether_addr_octet,
 ETHER_ADDR_LEN))
 return -EINVAL;
 
+return 0;
+}
+
+static int client_receive_offer(DHCPClient *client, DHCPPacket *offer, int len)
+{
+int err;
+DHCPLease *lease;
+
+err = client_verify_headers(client, offer, len);
+if (err < 0)
+return err;
+
 lease = new0(DHCPLease, 1);
 if (!lease)
 return -ENOBUFS;
@@ -561,6 +577,64 @@ error:
 return -ENOMSG;
 }
 
+static int client_receive_ack(DHCPClient *client, DHCPPacket *offer, int len)
+{
+int err = -ENOMSG;
+DHCPLease *lease;
+int type;
+
+err = client_verify_headers(client, offer, len);
+if (err < 0)
+return err;
+
+lease = new0(DHCPLease, 1);
+if (!lease)
+return -ENOBUFS;
+
+len = len - DHCP_IP_UDP_SIZE;
+type = __dhcp_option_parse(&offer->dhcp, len, client_parse_offer,
+   lease);
+
+if (type == DHCP_NAK) {
+err = DHCP_EVENT_NAK;
+goto error;
+}
+
+if (type != DHCP_ACK)
+goto error;
+
+lease->address.s_addr = offer->dhcp.yiaddr;
+
+if (lease->address.s_addr == INADDR_ANY ||
+lease->server_address.s_addr == INADDR_ANY ||
+lease->subnet_mask.s_addr == INADDR_ANY ||
+lease->lifetime == 0) {
+err = -ENOMSG;
+goto error;
+}
+
+err = 0;
+
+if (client->lease) {
+if (client->lease->address.s_addr != lease->address.s_addr ||
+   

[systemd-devel] [PATCH 08/28] dhcp: Add tests for DHCP options, file and sname fields

[Patrik Flykt]

Add a structure describing the DHCP file, sname and trailing options
fields. Create a messge holding these fields and call the internal
option parsing function.

In the test callback function verify that only regular options are
passed and figure out which part of the DHCP message is the one that
is being processed. As the test program knows the full contents of
the test options in the test structure, skip all non-regular fields
and verify that the option provided to the callback indeed is the
one expected. Check also if non-regular option fields are to be
ignored in the end of the option field as the callback is not called
again and the final check when the whole message has been processed
needs to be successful.

Add a boolean flag for pretty-printing, anticipate there will be a
nice option to toggle it in the future.
---
 src/dhcp/test-dhcp-option.c |  258 +++
 1 file changed, 258 insertions(+)

diff --git a/src/dhcp/test-dhcp-option.c b/src/dhcp/test-dhcp-option.c
index 9302fd6..a2b79ce 100644
--- a/src/dhcp/test-dhcp-option.c
+++ b/src/dhcp/test-dhcp-option.c
@@ -10,6 +10,68 @@
 #include "protocol.h"
 #include "internal.h"
 
+struct option_desc {
+uint8_t sname[64];
+int snamelen;
+uint8_t file[128];
+int filelen;
+uint8_t options[128];
+int len;
+bool success;
+int filepos;
+int snamepos;
+int pos;
+};
+
+static bool verbose = false;
+
+static struct option_desc option_tests[] = {
+{ {}, 0, {}, 0, { 42, 5, 65, 66, 67, 68, 69 }, 7, false, },
+{ {}, 0, {}, 0, { 42, 5, 65, 66, 67, 68, 69, 0, 0,
+  DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK }, 12, true, },
+{ {}, 0, {}, 0, { 8, 255, 70, 71, 72 }, 5, false, },
+{ {}, 0, {}, 0, { 0x35, 0x01, 0x05, 0x36, 0x04, 0x01, 0x00, 0xa8,
+  0xc0, 0x33, 0x04, 0x00, 0x01, 0x51, 0x80, 0x01,
+  0x04, 0xff, 0xff, 0xff, 0x00, 0x03, 0x04, 0xc0,
+  0xa8, 0x00, 0x01, 0x06, 0x04, 0xc0, 0xa8, 0x00,
+  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, },
+  40, true, },
+{ {}, 0, {}, 0, { DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_OFFER,
+  42, 3, 0, 0, 0 }, 8, true, },
+{ {}, 0, {}, 0, { 42, 2, 1, 2, 44 }, 5, false, },
+
+{ {}, 0,
+  { 222, 3, 1, 2, 3, DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_NAK }, 8,
+  { DHCP_OPTION_OVERLOAD, 1, DHCP_OVERLOAD_FILE }, 3, true, },
+
+{ { 1, 4, 1, 2, 3, 4, DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK }, 9,
+  { 222, 3, 1, 2, 3 }, 5,
+  { DHCP_OPTION_OVERLOAD, 1,
+DHCP_OVERLOAD_FILE|DHCP_OVERLOAD_SNAME }, 3, true, },
+};
+
+static const char *dhcp_type(int type)
+{
+switch(type) {
+case DHCP_DISCOVER:
+return "DHCPDISCOVER";
+case DHCP_OFFER:
+return "DHCPOFFER";
+case DHCP_REQUEST:
+return "DHCPREQUEST";
+case DHCP_DECLINE:
+return "DHCPDECLINE";
+case DHCP_ACK:
+return "DHCPACK";
+case DHCP_NAK:
+return "DHCPNAK";
+case DHCP_RELEASE:
+return "DHCPRELEASE";
+default:
+return "unknown";
+}
+}
+
 static void test_invalid_buffer_length(void)
 {
 DHCPMessage message;
@@ -43,10 +105,206 @@ static void test_cookie(void)
 free(message);
 }
 
+static DHCPMessage *create_message(uint8_t *options, uint16_t optlen,
+uint8_t *file, uint8_t filelen,
+uint8_t *sname, uint8_t snamelen)
+{
+DHCPMessage *message;
+int len = sizeof(DHCPMessage) + 4 + optlen;
+uint8_t *opt;
+
+message = malloc0(len);
+opt = (uint8_t *)(message + 1);
+
+opt[0] = 99;
+opt[1] = 130;
+opt[2] = 83;
+opt[3] = 99;
+
+if (options && optlen)
+memcpy(&opt[4], options, optlen);
+
+if (file && filelen <= 128)
+memcpy(&message->file, file, filelen);
+
+if (sname && snamelen <= 64)
+memcpy(&message->sname, sname, snamelen);
+
+return message;
+}
+
+static void test_ignore_opts(uint8_t *descoption, int *descpos, int *desclen)
+{
+while (*descpos < *desclen) {
+switch(descoption[*descpos]) {
+case DHCP_OPTION_PAD:
+*descpos += 1;
+break;
+
+case DHCP_OPTION_MESSAGE_TYPE:
+case DHCP_OPTION_OVERLOAD:
+*descpos += 3;
+break;
+
+default:
+return;
+}
+}
+}
+
+static int test_options_cb(uint8_t code, uint8_t len, uint8_t *option,
+   void *user_data)
+{
+struct option_desc *desc = user_data;
+  

[systemd-devel] [PATCH 27/28] dhcp: Add function to free DHCP client data

[Patrik Flykt]

---
 src/dhcp/client.c |   11 +++
 src/dhcp/client.h |2 ++
 2 files changed, 13 insertions(+)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 0144c70..a274549 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -994,6 +994,17 @@ int dhcp_client_stop(DHCPClient *client)
 return client_stop(client, DHCP_EVENT_STOP);
 }
 
+void dhcp_client_free(DHCPClient *client)
+{
+if (!client)
+return;
+
+dhcp_client_stop(client);
+
+free(client->req_opts);
+free(client);
+}
+
 DHCPClient *dhcp_client_new(sd_event *event)
 {
 DHCPClient *client;
diff --git a/src/dhcp/client.h b/src/dhcp/client.h
index adb4443..55fec93 100644
--- a/src/dhcp/client.h
+++ b/src/dhcp/client.h
@@ -50,4 +50,6 @@ int dhcp_client_get_router(DHCPClient *client, struct in_addr 
*addr);
 
 int dhcp_client_start(DHCPClient *client);
 int dhcp_client_stop(DHCPClient *client);
+
+void dhcp_client_free(DHCPClient *client);
 DHCPClient *dhcp_client_new(sd_event *event);
-- 
1.7.10.4

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

[systemd-devel] [PATCH 06/28] dhcp: Add buffer length and invalid cookie tests for DHCP options

[Patrik Flykt]

Create an initial simple test program for these two cases.
---
 src/dhcp/test-dhcp-option.c |   54 +++
 1 file changed, 54 insertions(+)
 create mode 100644 src/dhcp/test-dhcp-option.c

diff --git a/src/dhcp/test-dhcp-option.c b/src/dhcp/test-dhcp-option.c
new file mode 100644
index 000..25cb17c
--- /dev/null
+++ b/src/dhcp/test-dhcp-option.c
@@ -0,0 +1,54 @@
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "protocol.h"
+#include "internal.h"
+
+static void test_invalid_buffer_length(void)
+{
+struct dhcp_message message;
+
+assert(__dhcp_option_parse(&message, INT32_MIN, NULL, NULL) ==
+-EINVAL);
+assert(__dhcp_option_parse(&message, sizeof(struct dhcp_message),
+NULL, NULL) == -EINVAL);
+}
+
+static void test_cookie(void)
+{
+struct dhcp_message *message;
+uint32_t len = sizeof(struct dhcp_message) + 4;
+uint8_t *opt;
+
+message = malloc0(len);
+
+opt = (uint8_t *) message + 1;
+opt[0] = 0xff;
+
+assert(__dhcp_option_parse(message, len, NULL, NULL) == -EINVAL);
+
+opt[0] = 99;
+opt[1] = 130;
+opt[2] = 83;
+opt[3] = 99;
+
+assert(__dhcp_option_parse(message, len, NULL, NULL) == -ENOMSG);
+
+free(message);
+}
+
+int main(int argc, char *argv[])
+{
+unsigned int i;
+
+test_invalid_buffer_length();
+test_cookie();
+
+return 0;
+}
-- 
1.7.10.4

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

[systemd-devel] [PATCH 10/28] dhcp: Add test function for computing checksum

[Patrik Flykt]

---
 src/dhcp/test-dhcp-client.c |   40 
 1 file changed, 40 insertions(+)

diff --git a/src/dhcp/test-dhcp-client.c b/src/dhcp/test-dhcp-client.c
index 26857b6..94f576a 100644
--- a/src/dhcp/test-dhcp-client.c
+++ b/src/dhcp/test-dhcp-client.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "protocol.h"
 #include "client.h"
@@ -73,9 +74,48 @@ static void test_request_basic(void)
 assert(dhcp_client_set_request_option(client, 44) == 0);
 }
 
+static uint16_t client_checksum(void *buf, int len)
+{
+uint32_t sum;
+uint16_t *check;
+int i;
+uint8_t *odd;
+
+sum = 0;
+check = buf;
+
+for (i = 0; i < len / 2 ; i++)
+sum += check[i];
+
+if (len & 0x01) {
+odd = buf;
+sum += odd[len];
+}
+
+return ~((sum & 0x) + (sum >> 16));
+}
+
+static void test_checksum(void)
+{
+uint8_t buf[20] = {
+0x45, 0x00, 0x02, 0x40, 0x00, 0x00, 0x00, 0x00,
+0x40, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0xff, 0xff, 0xff, 0xff
+};
+
+uint8_t check[2] = {
+0x78, 0xae
+};
+
+uint16_t *val = (uint16_t *)check;
+
+assert(client_checksum(&buf, 20) == *val);
+}
+
 int main(int argc, char *argv[])
 {
 test_request_basic();
+test_checksum();
 
 return 0;
 }
-- 
1.7.10.4

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

[systemd-devel] [PATCH 22/28] dhcp: Send DHCP Request to acquire an IP address

[Patrik Flykt]

Create and send a DHCP Request message reusing already existing parts
of the code. This causes factoring out IP and UDP header creation and
moving next timeout calculation to be done every time in the timer
callback function independent of DHCP state. Also add an exponential
part to the timer calculation, bail out if there are errors while
resending the DHCP message for the sixth or more times.
---
 src/dhcp/client.c |  162 +++--
 1 file changed, 120 insertions(+), 42 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 84d5c25..4b316dc 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -56,6 +56,7 @@ struct DHCPClient {
 struct ether_addr mac_addr;
 uint32_t xid;
 uint64_t start_time;
+unsigned int attempt;
 DHCPLease *lease;
 };
 
@@ -171,6 +172,8 @@ static int client_stop(DHCPClient *client, int error)
 client->timeout_resend =
 sd_event_source_unref(client->timeout_resend);
 
+client->attempt = 1;
+
 switch (client->state) {
 
 case DHCP_STATE_INIT:
@@ -268,6 +271,28 @@ static uint16_t client_checksum(void *buf, int len)
 return ~((sum & 0x) + (sum >> 16));
 }
 
+static void client_append_ip_headers(DHCPPacket *packet, uint16_t len)
+{
+packet->ip.version = IPVERSION;
+packet->ip.ihl = DHCP_IP_SIZE / 4;
+packet->ip.tot_len = htons(len);
+
+packet->ip.protocol = IPPROTO_UDP;
+packet->ip.saddr = INADDR_ANY;
+packet->ip.daddr = INADDR_BROADCAST;
+
+packet->udp.source = htons(DHCP_PORT_CLIENT);
+packet->udp.dest = htons(DHCP_PORT_SERVER);
+packet->udp.len = htons(len - DHCP_IP_SIZE);
+
+packet->ip.check = packet->udp.len;
+packet->udp.check = client_checksum(&packet->ip.ttl, len - 8);
+
+packet->ip.ttl = IPDEFTTL;
+packet->ip.check = 0;
+packet->ip.check = client_checksum(&packet->ip, DHCP_IP_SIZE);
+}
+
 static int client_send_discover(DHCPClient *client, uint16_t secs)
 {
 int err = 0;
@@ -300,32 +325,61 @@ static int client_send_discover(DHCPClient *client, 
uint16_t secs)
 if (err < 0)
 goto error;
 
-discover->ip.version = IPVERSION;
-discover->ip.ihl = sizeof(discover->ip) >> 2;
-discover->ip.tot_len = htons(len);
+client_append_ip_headers(discover, len);
 
-discover->ip.protocol = IPPROTO_UDP;
-discover->ip.saddr = INADDR_ANY;
-discover->ip.daddr = INADDR_BROADCAST;
+err = __dhcp_network_send_raw_socket(client->fd, &client->link,
+ discover, len);
+
+error:
+free(discover);
 
-discover->udp.source = htons(DHCP_PORT_CLIENT);
-discover->udp.dest = htons(DHCP_PORT_SERVER);
-discover->udp.len = htons(len - sizeof(discover->ip));
+return err;
+}
+
+static int client_send_request(DHCPClient *client, uint16_t secs)
+{
+DHCPPacket *request;
+int optlen, len, err;
+uint8_t *opt;
+
+optlen = DHCP_CLIENT_MIN_OPTIONS_SIZE;
+len = DHCP_MESSAGE_SIZE + optlen;
+
+request = malloc0(len);
+
+if (!request)
+return -ENOBUFS;
+
+err = client_packet_init(client, DHCP_REQUEST, &request->dhcp, secs,
+ &opt, &optlen);
+if (err < 0)
+goto error;
+
+if (client->state == DHCP_STATE_REQUESTING) {
+err = __dhcp_option_append(&opt, &optlen,
+   DHCP_OPTION_REQUESTED_IP_ADDRESS,
+   4, &client->lease->address.s_addr);
+if (err < 0)
+goto error;
+
+err = __dhcp_option_append(&opt, &optlen,
+   DHCP_OPTION_SERVER_IDENTIFIER,
+   4, 
&client->lease->server_address.s_addr);
+if (err < 0)
+goto error;
+}
 
-discover->ip.check = discover->udp.len;
-discover->udp.check = client_checksum(&discover->ip.ttl,
-  len - 8);
+err = __dhcp_option_append(&opt, &optlen, DHCP_OPTION_END, 0, NULL);
+if (err < 0)
+goto error;
 
-discover->ip.ttl = IPDEFTTL;
-discover->ip.check = 0;
-discover->ip.check = client_checksum(&discover->ip,
- sizeof(discover->ip));
+client_append_ip_headers(request, len);
 
 err = __dhcp_network_send_raw_socket(client->fd, &client->link,
- discover, len);
+ request, len);
 
 error:
-free(discover);
+free(request);
 
 return err;
 }
@@ -338,32 +392,50 @@

[systemd-devel] [PATCH 25/28] dhcp: Compute expire, T1 and T2 timers

[Patrik Flykt]

Compute the default T1 and T2 timer values if they were not set by
the DHCP server. Verify that the values are reasonable.
---
 src/dhcp/client.c   |  123 ++-
 src/dhcp/protocol.h |2 +
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 669804a..50008a9 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -34,6 +34,8 @@
 
 struct DHCPLease {
 uint32_t lifetime;
+uint32_t t2;
+uint32_t t1;
 struct in_addr address;
 struct in_addr server_address;
 struct in_addr subnet_mask;
@@ -57,6 +59,10 @@ struct DHCPClient {
 uint32_t xid;
 uint64_t start_time;
 unsigned int attempt;
+uint64_t request_sent;
+sd_event_source *timeout_expire;
+sd_event_source *timeout_t1;
+sd_event_source *timeout_t2;
 DHCPLease *lease;
 };
 
@@ -177,6 +183,16 @@ static int client_stop(DHCPClient *client, int error)
 client->timeout_resend =
 sd_event_source_unref(client->timeout_resend);
 
+if (client->timeout_expire)
+client->timeout_expire =
+sd_event_source_unref(client->timeout_expire);
+if (client->timeout_t2)
+client->timeout_t2 =
+sd_event_source_unref(client->timeout_t2);
+if (client->timeout_t1)
+client->timeout_t1 =
+sd_event_source_unref(client->timeout_t1);
+
 client->attempt = 1;
 
 switch (client->state) {
@@ -184,6 +200,7 @@ static int client_stop(DHCPClient *client, int error)
 case DHCP_STATE_INIT:
 case DHCP_STATE_SELECTING:
 case DHCP_STATE_REQUESTING:
+case DHCP_STATE_BOUND:
 
 client->start_time = 0;
 client->state = DHCP_STATE_INIT;
@@ -191,7 +208,6 @@ static int client_stop(DHCPClient *client, int error)
 
 case DHCP_STATE_INIT_REBOOT:
 case DHCP_STATE_REBOOTING:
-case DHCP_STATE_BOUND:
 case DHCP_STATE_RENEWING:
 case DHCP_STATE_REBINDING:
 
@@ -447,6 +463,8 @@ static int client_timeout_resend(sd_event_source *s, 
uint64_t usec,
 if (err < 0 && client->attempt >= 64)
 goto error;
 
+client->request_sent = usec;
+
 break;
 
 case DHCP_STATE_INIT_REBOOT:
@@ -466,6 +484,22 @@ error:
 return 0;
 }
 
+static int client_timeout_expire(sd_event_source *s, uint64_t usec,
+ void *userdata)
+{
+return 0;
+}
+
+static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata)
+{
+return 0;
+}
+
+static int client_timeout_t1(sd_event_source *s, uint64_t usec, void *userdata)
+{
+return 0;
+}
+
 static int client_parse_offer(uint8_t code, uint8_t len, uint8_t *option,
   void *user_data)
 {
@@ -499,6 +533,22 @@ static int client_parse_offer(uint8_t code, uint8_t len, 
uint8_t *option,
 memcpy(&lease->router.s_addr, option, 4);
 
 break;
+
+case DHCP_OPTION_RENEWAL_T1_TIME:
+if (len == 4) {
+memcpy(&val, option, 4);
+lease->t1 = ntohl(val);
+}
+
+break;
+
+case DHCP_OPTION_REBINDING_T2_TIME:
+if (len == 4) {
+memcpy(&val, option, 4);
+lease->t2 = ntohl(val);
+}
+
+break;
 }
 
 return 0;
@@ -635,6 +685,72 @@ error:
 return err;
 }
 
+static uint64_t client_compute_timeout(uint64_t request_sent,
+   uint32_t lifetime)
+{
+return request_sent + (lifetime - 3) * USEC_PER_SEC +
++ (random_u() & 0x1f);
+}
+
+static int client_set_lease_timeouts(DHCPClient *client, uint64_t usec)
+{
+int err;
+uint64_t next_timeout;
+
+if (client->lease->lifetime < 10)
+return -EINVAL;
+
+if (!client->lease->t1)
+client->lease->t1 = client->lease->lifetime / 2;
+
+next_timeout = client_compute_timeout(client->request_sent,
+  client->lease->t1);
+if (next_timeout < usec)
+return -EINVAL;
+
+err = sd_event_add_monotonic(client->event, next_timeout,
+ 10 * USEC_PER_MSEC,
+ client_timeout_t1, client,
+ &client->timeout_t1);
+if (err < 0)
+return err;
+
+if (!client->lease->t2)
+client->lease->t2 = client->lease->lifetime * 7 / 8;
+
+if (client->lease->t2 < client->lease->t1)
+return -EINVAL;
+
+  

[systemd-devel] [PATCH 17/28] dhcp: Support seconds elapsed since start of DHCP negotiation

[Patrik Flykt]

It was noticed by Grant Erickson in ConnMan commit
95e15c09350acf58d4707056ae2614570883ef66 that:

   "Certain DHCP servers, such as that implemented in Mac OS X
(< 10.7) for its "Internet Sharing" feature, refuse to issue
a DHCP lease to clients that have not set a non-zero value
in their DISCOVER or REQUEST packets."
---
 src/dhcp/client.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 1d4d957..dc92880 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -134,8 +134,8 @@ int dhcp_client_set_mac(DHCPClient *client, struct 
ether_addr *addr)
 }
 
 static int client_packet_init(DHCPClient *client, uint8_t type,
-  DHCPMessage *message, uint8_t **opt,
-  int *optlen)
+  DHCPMessage *message, uint16_t secs,
+  uint8_t **opt, int *optlen)
 {
 int err;
 
@@ -149,6 +149,7 @@ static int client_packet_init(DHCPClient *client, uint8_t 
type,
 message->htype = 1;
 message->hlen = ETHER_ADDR_LEN;
 message->xid = htonl(client->xid);
+message->secs = htons(secs);
 
 memcpy(&message->chaddr, &client->mac_addr, ETH_ALEN);
 (*opt)[0] = 0x63;
@@ -202,7 +203,7 @@ static uint16_t client_checksum(void *buf, int len)
 return ~((sum & 0x) + (sum >> 16));
 }
 
-static int client_send_discover(DHCPClient *client)
+static int client_send_discover(DHCPClient *client, uint16_t secs)
 {
 int err = 0;
 DHCPPacket *discover;
@@ -218,7 +219,7 @@ static int client_send_discover(DHCPClient *client)
 return -ENOBUFS;
 
 err = client_packet_init(client, DHCP_DISCOVER, &discover->dhcp,
- &opt, &optlen);
+ secs, &opt, &optlen);
 if (err < 0)
 goto error;
 
@@ -274,7 +275,7 @@ int dhcp_client_start(DHCPClient *client)
 
 client->xid = random_u();
 
-return client_send_discover(client);
+return client_send_discover(client, 0);
 }
 
 DHCPClient *dhcp_client_new(void)
-- 
1.7.10.4

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

[systemd-devel] [PATCH 19/28] build: Add dependency on libsystemd-bus needed for main loop

[Patrik Flykt]

---
 Makefile.am |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index f079822..433383d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3758,7 +3758,8 @@ libsystemd_dhcp_la_LDFLAGS = \
$(AM_LDFLAGS)
 
 libsystemd_dhcp_la_LIBADD = \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-bus.la
 
 dhcp_example_client_SOURCES = \
src/dhcp/dhcp-example-client.c
@@ -3785,7 +3786,8 @@ test_dhcp_client_SOURCES = \
src/dhcp/test-dhcp-client.c
 
 test_dhcp_client_LDADD = \
-   libsystemd-shared.la
+   libsystemd-shared.la \
+   libsystemd-bus.la
 
 tests += \
test-dhcp-option \
-- 
1.7.10.4

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

[systemd-devel] [PATCH 21/28] dhcp: Handle received DHCP Offer message

[Patrik Flykt]

Create a function for handling the full IP, UDP and DHCP packet
and tie it to the main loop. Verify IP and UDP headers and checksum.
Creat a new lease structure with using the values supplied in the
DHCP message. Free the lease structure when client is stopped.

Split out socket handling into a creation and a sending part. As a
result modify the test code.
---
 src/dhcp/client.c   |  207 ++-
 src/dhcp/internal.h |4 +
 src/dhcp/network.c  |   33 +++
 src/dhcp/protocol.h |6 ++
 src/dhcp/test-dhcp-client.c |   21 -
 5 files changed, 253 insertions(+), 18 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index cb3d41b..84d5c25 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -32,17 +32,31 @@
 
 #define DHCP_CLIENT_MIN_OPTIONS_SIZE312
 
+struct DHCPLease {
+uint32_t lifetime;
+struct in_addr address;
+struct in_addr server_address;
+struct in_addr subnet_mask;
+struct in_addr router;
+};
+
+typedef struct DHCPLease DHCPLease;
+
 struct DHCPClient {
 DHCPState state;
 sd_event *event;
 sd_event_source *timeout_resend;
 int index;
+int fd;
+struct sockaddr_ll link;
+sd_event_source *receive_message;
 uint8_t *req_opts;
 int req_opts_size;
 struct in_addr *last_addr;
 struct ether_addr mac_addr;
 uint32_t xid;
 uint64_t start_time;
+DHCPLease *lease;
 };
 
 static uint8_t default_req_opts[] = {
@@ -145,6 +159,14 @@ static int client_stop(DHCPClient *client, int error)
 client->state == DHCP_STATE_INIT_REBOOT)
 return -EALREADY;
 
+if (client->fd > 0)
+close(client->fd);
+client->fd = 0;
+
+if (client->receive_message)
+client->receive_message =
+sd_event_source_unref(client->receive_message);
+
 if (client->timeout_resend)
 client->timeout_resend =
 sd_event_source_unref(client->timeout_resend);
@@ -154,6 +176,7 @@ static int client_stop(DHCPClient *client, int error)
 case DHCP_STATE_INIT:
 case DHCP_STATE_SELECTING:
 
+client->start_time = 0;
 client->state = DHCP_STATE_INIT;
 break;
 
@@ -167,6 +190,11 @@ static int client_stop(DHCPClient *client, int error)
 break;
 }
 
+if (client->lease) {
+free(client->lease);
+client->lease = NULL;
+}
+
 return 0;
 }
 
@@ -293,7 +321,8 @@ static int client_send_discover(DHCPClient *client, 
uint16_t secs)
 discover->ip.check = client_checksum(&discover->ip,
  sizeof(discover->ip));
 
-err = __dhcp_network_send_raw_packet(client->index, discover, len);
+err = __dhcp_network_send_raw_socket(client->fd, &client->link,
+ discover, len);
 
 error:
 free(discover);
@@ -350,6 +379,168 @@ error:
 return 0;
 }
 
+static int client_parse_offer(uint8_t code, uint8_t len, uint8_t *option,
+  void *user_data)
+{
+DHCPLease *lease = user_data;
+uint32_t val;
+
+switch(code) {
+
+case DHCP_OPTION_IP_ADDRESS_LEASE_TIME:
+if (len == 4) {
+memcpy(&val, option, 4);
+lease->lifetime = ntohl(val);
+}
+
+break;
+
+case DHCP_OPTION_SERVER_IDENTIFIER:
+if (len >= 4)
+memcpy(&lease->server_address.s_addr, option, 4);
+
+break;
+
+case DHCP_OPTION_SUBNET_MASK:
+if (len >= 4)
+memcpy(&lease->subnet_mask.s_addr, option, 4);
+
+break;
+
+case DHCP_OPTION_ROUTER:
+if (len >= 4)
+memcpy(&lease->router.s_addr, option, 4);
+
+break;
+}
+
+return 0;
+}
+
+static int client_receive_offer(DHCPClient *client,
+DHCPPacket *offer, int len)
+{
+int hdrlen;
+DHCPLease *lease;
+
+if (len < (DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE))
+return -EINVAL;
+
+hdrlen = offer->ip.ihl * 4;
+if (hdrlen < 20 || hdrlen > len || client_checksum(&offer->ip,
+   hdrlen))
+return -EINVAL;
+
+offer->ip.check = offer->udp.len;
+offer->ip.ttl = 0;
+
+if (hdrlen + ntohs(offer->udp.len) > len ||
+client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
+return -EINVAL;
+
+if (ntohs(offer->udp.source) != DHCP_PORT_SERVER ||
+ntohs(offer->udp.dest) != DHCP_PORT_C

[systemd-devel] [PATCH 02/28] dhcp: Add DHCP client initialization

[Patrik Flykt]

Provide functionality for initializing a DHCP client struct, setting
interface index, last used address and additional options to request.
On initialization the most useful options are added by default.
---
 src/dhcp/client.c |  137 +
 src/dhcp/client.h |   34 +
 2 files changed, 171 insertions(+)
 create mode 100644 src/dhcp/client.c
 create mode 100644 src/dhcp/client.h

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
new file mode 100644
index 000..172ddd9
--- /dev/null
+++ b/src/dhcp/client.c
@@ -0,0 +1,137 @@
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "util.h"
+#include "list.h"
+
+#include "protocol.h"
+#include "client.h"
+
+struct DHCPClient {
+DHCPState state;
+int index;
+uint8_t *req_opts;
+int req_opts_size;
+struct in_addr *last_addr;
+};
+
+static uint8_t default_req_opts[] = {
+DHCP_OPTION_SUBNET_MASK,
+DHCP_OPTION_ROUTER,
+DHCP_OPTION_HOST_NAME,
+DHCP_OPTION_DOMAIN_NAME,
+DHCP_OPTION_DOMAIN_NAME_SERVER,
+DHCP_OPTION_NTP_SERVER,
+};
+
+int dhcp_client_set_request_option(DHCPClient *client, uint8_t option)
+{
+int i;
+
+if (!client)
+return -EINVAL;
+
+if (client->state != DHCP_STATE_INIT)
+return -EBUSY;
+
+switch(option) {
+case DHCP_OPTION_PAD:
+case DHCP_OPTION_OVERLOAD:
+case DHCP_OPTION_MESSAGE_TYPE:
+case DHCP_OPTION_PARAMETER_REQUEST_LIST:
+case DHCP_OPTION_END:
+return -EINVAL;
+
+default:
+break;
+}
+
+for (i = 0; i < client->req_opts_size; i++)
+if (client->req_opts[i] == option)
+return -EEXIST;
+
+client->req_opts_size++;
+client->req_opts = realloc(client->req_opts, client->req_opts_size);
+if (!client->req_opts) {
+client->req_opts_size = 0;
+return -ENOBUFS;
+}
+
+client->req_opts[client->req_opts_size - 1] = option;
+
+return 0;
+}
+
+int dhcp_client_set_request_address(DHCPClient *client,
+struct in_addr *last_addr)
+{
+if (!client)
+return -EINVAL;
+
+if (client->state != DHCP_STATE_INIT)
+return -EBUSY;
+
+client->last_addr = malloc(sizeof(struct in_addr));
+if (!client->last_addr)
+return -ENOBUFS;
+
+memcpy(&client->last_addr, last_addr, sizeof(struct in_addr));
+
+return 0;
+}
+
+int dhcp_client_set_index(DHCPClient *client, int interface_index)
+{
+if (!client || interface_index < -1)
+return -EINVAL;
+
+if (client->state != DHCP_STATE_INIT)
+return -EBUSY;
+
+client->index = interface_index;
+
+return 0;
+}
+
+DHCPClient *dhcp_client_new(void)
+{
+DHCPClient *client;
+int i;
+
+client = new0(DHCPClient, 1);
+if (!client)
+return NULL;
+
+client->state = DHCP_STATE_INIT;
+client->index = -1;
+
+client->req_opts_size = sizeof(default_req_opts)
+/ sizeof(default_req_opts[0]);
+
+client->req_opts = malloc(client->req_opts_size);
+for (i = 0; i < client->req_opts_size; i++)
+client->req_opts[i] = default_req_opts[i];
+
+return client;
+}
diff --git a/src/dhcp/client.h b/src/dhcp/client.h
new file mode 100644
index 000..447c03b
--- /dev/null
+++ b/src/dhcp/client.h
@@ -0,0 +1,34 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PA

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

[Patrik Flykt]

Hi,

This patch set implements a DHCPv4 client library named libsystemd-dhcp
to be used with systemd-network.

The code implements the DHCP protocol from the INIT state up to expiry,
T1 and T2 timer setting, but does nothing in response to the IP address
reacquisition T1 and T2 timers being triggered. Expiry timer is followed
and the DHCP client stopped when it triggers. INIT-REBOOT state is also
not yet implemented; both these missing parts are to be implemented next.

The code assumes that the caller can figure out the interface to use.
The code expects to be handed an sd_event loop, a MAC address and an
interface index. The outcome is a notification of an IP address being
acquired and the callback provider is expected to set the IP address,
netmask and default gateway. Higher level DHCP options such as DNS and
NTP servers are requested from the server by default, but the response
is not yet collected anywhere. I also don't know how detailed callback
information is needed, is there for example any need to know the protocol
state transitions or messages sent/received.

Two test programs are created, one for setting and getting options and
the other for client structure initialization and Discover message
verification. The intention is to pass further DHCP server responses and
verifying that the replies from the client make sense.

The socket part of the code is singled out into a file of its own in
order to be able to write some nice tests while having the DHCP client
code running as normally as possible.

A very small example client is also created, I used it to watch the client-
server interaction via Wireshark.


Please review and comment,


   Patrik


Patrik Flykt (28):
  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
  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: Add example DHCP client test program
  build: Add example DHCP test program
  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
  dhcp: Use callback in test client

 Makefile.am|   60 +++
 configure.ac   |9 +
 src/dhcp/Makefile  |1 +
 src/dhcp/client.c  | 1033 
 src/dhcp/client.h  |   55 +++
 src/dhcp/dhcp-example-client.c |  170 +++
 src/dhcp/internal.h|   40 ++
 src/dhcp/network.c |   66 +++
 src/dhcp/option.c  |  181 +++
 src/dhcp/protocol.h|  106 +
 src/dhcp/test-dhcp-client.c|  222 +
 src/dhcp/test-dhcp-option.c|  381 +++
 12 files changed, 2324 insertions(+)
 create mode 12 src/dhcp/Makefile
 create mode 100644 src/dhcp/client.c
 create mode 100644 src/dhcp/client.h
 create mode 100644 src/dhcp/dhcp-example-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

-- 
1.7.10.4

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

[systemd-devel] [PATCH 07/28] build: Add DHCP option test

[Patrik Flykt]

---
 Makefile.am |   10 ++
 src/dhcp/test-dhcp-option.c |   16 +++-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index cc52f01..aeca484 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3740,6 +3740,15 @@ endif
 
 # 
--
 if ENABLE_DHCP
+test_dhcp_option_SOURCES = \
+   src/dhcp/protocol.h \
+   src/dhcp/internal.h \
+   src/dhcp/option.c \
+   src/dhcp/test-dhcp-option.c
+
+test_dhcp_option_LDADD = \
+   libsystemd-shared.la
+
 test_dhcp_client_SOURCES = \
src/dhcp/protocol.h \
src/dhcp/client.h \
@@ -3750,6 +3759,7 @@ test_dhcp_client_LDADD = \
libsystemd-shared.la
 
 tests += \
+   test-dhcp-option \
test-dhcp-client
 endif
 
diff --git a/src/dhcp/test-dhcp-option.c b/src/dhcp/test-dhcp-option.c
index 25cb17c..9302fd6 100644
--- a/src/dhcp/test-dhcp-option.c
+++ b/src/dhcp/test-dhcp-option.c
@@ -12,23 +12,23 @@
 
 static void test_invalid_buffer_length(void)
 {
-struct dhcp_message message;
+DHCPMessage message;
 
 assert(__dhcp_option_parse(&message, INT32_MIN, NULL, NULL) ==
--EINVAL);
-assert(__dhcp_option_parse(&message, sizeof(struct dhcp_message),
-NULL, NULL) == -EINVAL);
+   -EINVAL);
+assert(__dhcp_option_parse(&message, sizeof(DHCPMessage),
+   NULL, NULL) == -EINVAL);
 }
 
 static void test_cookie(void)
 {
-struct dhcp_message *message;
-uint32_t len = sizeof(struct dhcp_message) + 4;
+DHCPMessage *message;
+uint32_t len = sizeof(DHCPMessage) + 4;
 uint8_t *opt;
 
 message = malloc0(len);
 
-opt = (uint8_t *) message + 1;
+opt = (uint8_t *)(message + 1);
 opt[0] = 0xff;
 
 assert(__dhcp_option_parse(message, len, NULL, NULL) == -EINVAL);
@@ -45,8 +45,6 @@ static void test_cookie(void)
 
 int main(int argc, char *argv[])
 {
-unsigned int i;
-
 test_invalid_buffer_length();
 test_cookie();
 
-- 
1.7.10.4

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

[systemd-devel] [PATCH 16/28] build: Add example DHCP test program

[Patrik Flykt]

---
 Makefile.am |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 1c73423..f079822 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3760,6 +3760,12 @@ libsystemd_dhcp_la_LDFLAGS = \
 libsystemd_dhcp_la_LIBADD = \
libsystemd-shared.la
 
+dhcp_example_client_SOURCES = \
+   src/dhcp/dhcp-example-client.c
+
+dhcp_example_client_LDADD = \
+   libsystemd-dhcp.la
+
 test_dhcp_option_SOURCES = \
src/dhcp/protocol.h \
src/dhcp/internal.h \
@@ -3784,6 +3790,10 @@ test_dhcp_client_LDADD = \
 tests += \
test-dhcp-option \
test-dhcp-client
+
+manual_tests += \
+   dhcp-example-client
+
 endif
 
 # 
--
-- 
1.7.10.4

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

[systemd-devel] [PATCH 14/28] dhcp: Add test for discover DHCP packet creation

[Patrik Flykt]

Set a fake MAC address and emulate raw packet sending. When the buffer
containing the Discover message is received, check selected IP and
UDP headers and compute IP header and UDP message checksums. Also
send the DHCP message for option parsing and expect a successful
outcome.
---
 Makefile.am |3 ++
 src/dhcp/test-dhcp-client.c |   79 +++
 2 files changed, 82 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 5c350ab..1c73423 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3771,6 +3771,9 @@ test_dhcp_option_LDADD = \
 
 test_dhcp_client_SOURCES = \
src/dhcp/protocol.h \
+   src/dhcp/internal.h \
+   src/dhcp/option.h \
+   src/dhcp/option.c \
src/dhcp/client.h \
src/dhcp/client.c \
src/dhcp/test-dhcp-client.c
diff --git a/src/dhcp/test-dhcp-client.c b/src/dhcp/test-dhcp-client.c
index 94f576a..b5dde05 100644
--- a/src/dhcp/test-dhcp-client.c
+++ b/src/dhcp/test-dhcp-client.c
@@ -23,10 +23,16 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "protocol.h"
+#include "internal.h"
 #include "client.h"
 
+static struct ether_addr mac_addr = {
+.ether_addr_octet = {'A', 'B', 'C', '1', '2', '3'}
+};
+
 static void test_request_basic(void)
 {
 DHCPClient *client;
@@ -112,10 +118,83 @@ static void test_checksum(void)
 assert(client_checksum(&buf, 20) == *val);
 }
 
+static int check_options(uint8_t code, uint8_t len, uint8_t *option,
+void *user_data)
+{
+return 0;
+}
+
+int __dhcp_network_send_raw_packet(int index, void *packet, int len)
+{
+int size;
+DHCPPacket *discover;
+uint16_t ip_check, udp_check;
+int res;
+
+assert(index == 42);
+assert(packet);
+
+size = sizeof(DHCPPacket) + 4;
+assert(len > size);
+
+discover = packet;
+
+assert(memcmp(discover->dhcp.chaddr,
+  &mac_addr.ether_addr_octet, 6) == 0);
+assert(discover->ip.ttl == IPDEFTTL);
+assert(discover->ip.protocol == IPPROTO_UDP);
+assert(discover->ip.saddr == INADDR_ANY);
+assert(discover->ip.daddr == INADDR_BROADCAST);
+assert(discover->udp.source == ntohs(DHCP_PORT_CLIENT));
+assert(discover->udp.dest == ntohs(DHCP_PORT_SERVER));
+
+ip_check = discover->ip.check;
+
+discover->ip.ttl = 0;
+discover->ip.check = discover->udp.len;
+
+udp_check = ~client_checksum(&discover->ip.ttl, len - 8);
+assert(udp_check == 0x);
+
+discover->ip.ttl = IPDEFTTL;
+discover->ip.check = ip_check;
+
+ip_check = ~client_checksum(&discover->ip, sizeof(discover->ip));
+assert(ip_check == 0x);
+
+size = len - sizeof(struct iphdr) - sizeof(struct udphdr);
+
+res = __dhcp_option_parse(&discover->dhcp, size, check_options, NULL);
+if (res < 0)
+return res;
+
+return 575;
+}
+
+static void test_discover_message(void)
+{
+DHCPClient *client;
+int res;
+
+client = dhcp_client_new();
+assert(client);
+
+assert(dhcp_client_set_index(client, 42) >= 0);
+assert(dhcp_client_set_mac(client, &mac_addr) >= 0);
+
+assert(dhcp_client_set_request_option(client, 248) >= 0);
+
+res = dhcp_client_start(client);
+
+assert(res == 575 || res == -EINPROGRESS);
+}
+
 int main(int argc, char *argv[])
 {
 test_request_basic();
 test_checksum();
 
+test_discover_message();
+
 return 0;
 }
-- 
1.7.10.4

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

[systemd-devel] [PATCH 01/28] dhcp: Add DHCP protocol structures and initial defines

[Patrik Flykt]

Create a new directory to host DHCP components.
---
 src/dhcp/protocol.h |   93 +++
 1 file changed, 93 insertions(+)
 create mode 100644 src/dhcp/protocol.h

diff --git a/src/dhcp/protocol.h b/src/dhcp/protocol.h
new file mode 100644
index 000..3fc4baf
--- /dev/null
+++ b/src/dhcp/protocol.h
@@ -0,0 +1,93 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+
+struct DHCPMessage {
+uint8_t op;
+uint8_t htype;
+uint8_t hlen;
+uint8_t hops;
+uint32_t xid;
+uint16_t secs;
+uint16_t flags;
+uint32_t ciaddr;
+uint32_t yiaddr;
+uint32_t siaddr;
+uint32_t giaddr;
+uint8_t chaddr[16];
+uint8_t sname[64];
+uint8_t file[128];
+} __attribute__((packed));
+
+typedef struct DHCPMessage DHCPMessage;
+
+struct DHCPPacket {
+struct iphdr ip;
+struct udphdr udp;
+DHCPMessage dhcp;
+} __attribute__((packed));
+
+typedef struct DHCPPacket DHCPPacket;
+
+enum DHCPState {
+DHCP_STATE_INIT = 0,
+DHCP_STATE_SELECTING= 1,
+DHCP_STATE_INIT_REBOOT  = 2,
+DHCP_STATE_REBOOTING= 3,
+DHCP_STATE_REQUESTING   = 4,
+DHCP_STATE_BOUND= 5,
+DHCP_STATE_RENEWING = 6,
+DHCP_STATE_REBINDING= 7,
+};
+
+typedef enum DHCPState DHCPState;
+
+#define BOOTREQUEST 1
+#define BOOTREPLY   2
+
+#define DHCP_DISCOVER   1
+#define DHCP_OFFER  2
+#define DHCP_REQUEST3
+#define DHCP_DECLINE4
+#define DHCP_ACK5
+#define DHCP_NAK6
+#define DHCP_RELEASE7
+
+#define DHCP_OVERLOAD_FILE  1
+#define DHCP_OVERLOAD_SNAME 2
+
+#define DHCP_OPTION_PAD 0
+#define DHCP_OPTION_SUBNET_MASK 1
+#define DHCP_OPTION_ROUTER  3
+#define DHCP_OPTION_DOMAIN_NAME_SERVER  6
+#define DHCP_OPTION_HOST_NAME   12
+#define DHCP_OPTION_DOMAIN_NAME 15
+#define DHCP_OPTION_NTP_SERVER  42
+#define DHCP_OPTION_REQUESTED_IP_ADDRESS50
+#define DHCP_OPTION_OVERLOAD52
+#define DHCP_OPTION_MESSAGE_TYPE53
+#define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
+#define DHCP_OPTION_END 255
-- 
1.7.10.4

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

[systemd-devel] [PATCH 05/28] dhcp: Add option appending and parsing

[Patrik Flykt]

Add functions to append and parse DHCP options. Not all options
are passed to the callback function, the ones not exposed are
pad, end, message type and overload. If indicated by the overload
option, file and sname fields will be examined for more options.

The option functions are internal to DHCP, add a new header files
for interal function prototypes.
---
 src/dhcp/internal.h |   34 ++
 src/dhcp/option.c   |  181 +++
 2 files changed, 215 insertions(+)
 create mode 100644 src/dhcp/internal.h
 create mode 100644 src/dhcp/option.c

diff --git a/src/dhcp/internal.h b/src/dhcp/internal.h
new file mode 100644
index 000..65a3bb3
--- /dev/null
+++ b/src/dhcp/internal.h
@@ -0,0 +1,34 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+
+#include "protocol.h"
+
+int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
+ uint8_t optlen, void *optval);
+
+typedef int (*dhcp_option_cb_t)(uint8_t code, uint8_t len, uint8_t *option,
+void *user_data);
+int __dhcp_option_parse(DHCPMessage *message, int32_t len,
+dhcp_option_cb_t cb, void *user_data);
diff --git a/src/dhcp/option.c b/src/dhcp/option.c
new file mode 100644
index 000..f5ca6e7
--- /dev/null
+++ b/src/dhcp/option.c
@@ -0,0 +1,181 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+#include 
+
+#include "internal.h"
+
+int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
+ uint8_t optlen, void *optval)
+{
+if (!buf || !buflen)
+return -EINVAL;
+
+switch (code) {
+
+case DHCP_OPTION_PAD:
+case DHCP_OPTION_END:
+if (*buflen < 1)
+return -ENOBUFS;
+
+(*buf)[0] = code;
+*buf += 1;
+*buflen -= 1;
+break;
+
+default:
+if (*buflen < optlen + 2)
+return -ENOBUFS;
+
+if (!optval)
+return -EINVAL;
+
+(*buf)[0] = code;
+(*buf)[1] = optlen;
+memcpy(&(*buf)[2], optval, optlen);
+
+*buf += optlen + 2;
+*buflen = *buflen - optlen - 2;
+
+break;
+}
+
+return 0;
+}
+
+static int parse_options(uint8_t *buf, int32_t buflen, int *overload,
+ int *message_type, dhcp_option_cb_t cb,
+ void *user_data)
+{
+uint8_t *code = buf;
+uint8_t *len;
+
+while (buflen > 0) {
+switch (*code) {
+case DHCP_OPTION_PAD:
+buflen -= 1;
+code++;
+break;
+
+case DHCP_OPTION_END:
+return 0;
+
+case DHCP_OPTION_MESSAGE_TYPE:
+buflen -= 3;
+if (buflen < 0)
+return -ENOBUFS;
+
+len = code + 1;
+if (*len != 1)
+return -EINVAL;
+
+if (message_type)
+*message_type = *(len + 1);
+
+code = code + 3;
+
+

[systemd-devel] [PATCH 04/28] build: Add initial build support

[Patrik Flykt]

The client test program is the only one to be built so far.
---
 Makefile.am   |   15 +++
 configure.ac  |9 +
 src/dhcp/Makefile |1 +
 3 files changed, 25 insertions(+)
 create mode 12 src/dhcp/Makefile

diff --git a/Makefile.am b/Makefile.am
index 789ca02..cc52f01 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3739,6 +3739,21 @@ lib_LTLIBRARIES += \
 endif
 
 # 
--
+if ENABLE_DHCP
+test_dhcp_client_SOURCES = \
+   src/dhcp/protocol.h \
+   src/dhcp/client.h \
+   src/dhcp/client.c \
+   src/dhcp/test-dhcp-client.c
+
+test_dhcp_client_LDADD = \
+   libsystemd-shared.la
+
+tests += \
+   test-dhcp-client
+endif
+
+# 
--
 if ENABLE_MACHINED
 systemd_machined_SOURCES = \
src/machine/machined.c \
diff --git a/configure.ac b/configure.ac
index d0bfcb8..0c28368 100644
--- a/configure.ac
+++ b/configure.ac
@@ -714,6 +714,14 @@ fi
 AM_CONDITIONAL(ENABLE_RFKILL, [test "$have_rfkill" = "yes"])
 
 # 
--
+have_dhcp=no
+AC_ARG_ENABLE(dhcp, AS_HELP_STRING([--disable-dhcp], [disable dhcp]))
+if test "x$enable_dhcp" != "xno"; then
+have_dhcp=yes
+fi
+AM_CONDITIONAL(ENABLE_DHCP, [test "$have_dhcp" = "yes"])
+
+# 
--
 have_logind=no
 AC_ARG_ENABLE(logind, AS_HELP_STRING([--disable-logind], [disable login 
daemon]))
 if test "x$enable_logind" != "xno"; then
@@ -1060,6 +1068,7 @@ AC_MSG_RESULT([
 randomseed:  ${have_randomseed}
 backlight:   ${have_backlight}
 rfkill:  ${have_rfkill}
+dhcp:${have_dhcp}
 logind:  ${have_logind}
 machined:${have_machined}
 hostnamed:   ${have_hostnamed}
diff --git a/src/dhcp/Makefile b/src/dhcp/Makefile
new file mode 12
index 000..d0b0e8e
--- /dev/null
+++ b/src/dhcp/Makefile
@@ -0,0 +1 @@
+../Makefile
\ No newline at end of file
-- 
1.7.10.4

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

[systemd-devel] [PATCH 15/28] dhcp: Add example DHCP client test program

[Patrik Flykt]

The test program runs the DHCP protocol using libsystemd-dhcp.
---
 src/dhcp/dhcp-example-client.c |  112 
 1 file changed, 112 insertions(+)
 create mode 100644 src/dhcp/dhcp-example-client.c

diff --git a/src/dhcp/dhcp-example-client.c b/src/dhcp/dhcp-example-client.c
new file mode 100644
index 000..b7f1a5f
--- /dev/null
+++ b/src/dhcp/dhcp-example-client.c
@@ -0,0 +1,112 @@
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "client.h"
+
+static int get_mac(int index, struct ether_addr *mac)
+{
+struct ifreq ifr;
+int s, err;
+
+if (index < 0 || !mac)
+return -EINVAL;
+
+s = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+if (s < 0)
+return -EIO;
+
+memset(&ifr, 0, sizeof(ifr));
+ifr.ifr_ifindex = index;
+
+if (ioctl(s, SIOCGIFNAME, &ifr) < 0) {
+err = -errno;
+close(s);
+return err;
+}
+
+if (ioctl(s, SIOCGIFHWADDR, &ifr) < 0) {
+err = -errno;
+close(s);
+return err;
+}
+
+memcpy(&mac->ether_addr_octet[0], &ifr.ifr_hwaddr.sa_data, ETH_ALEN);
+
+return 0;
+}
+
+static int get_index(char *ifname)
+{
+struct ifreq ifr;
+int s;
+
+s = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+if (s < 0)
+return -EIO;
+
+memset(&ifr, 0, sizeof(ifr));
+strncpy(ifr.ifr_name, ifname, IFNAMSIZ);
+
+if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
+close(s);
+return -EIO;
+}
+
+return ifr.ifr_ifindex;
+}
+
+int main(int argc, char **argv)
+{
+DHCPClient *client;
+int err, index;
+struct ether_addr mac;
+
+if (argc != 2) {
+printf("Usage: %s \n", argv[0]);
+return 1;
+}
+
+index = get_index(argv[1]);
+if (index < 0)
+return 2;
+
+err = get_mac(index, &mac);
+if (err < 0)
+return 2;
+
+client = dhcp_client_new();
+if (!client)
+return 3;
+
+printf("Interface %s index %d\n", argv[1], index);
+dhcp_client_set_index(client, index);
+dhcp_client_set_mac(client, &mac);
+
+dhcp_client_start(client);
+
+return 0;
+}
-- 
1.7.10.4

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

[systemd-devel] [PATCH 11/28] dhcp: Add function for sending a raw packet

[Patrik Flykt]

Open a packet socket, create a link level header, send packet and
close socket. Adding it to a separate file makes testing of the
DHCP sending much easier, as the test program can supply any socket
to the DHCP client code.
---
 src/dhcp/internal.h |2 ++
 src/dhcp/network.c  |   63 +++
 2 files changed, 65 insertions(+)
 create mode 100644 src/dhcp/network.c

diff --git a/src/dhcp/internal.h b/src/dhcp/internal.h
index 65a3bb3..92bd2a1 100644
--- a/src/dhcp/internal.h
+++ b/src/dhcp/internal.h
@@ -25,6 +25,8 @@
 
 #include "protocol.h"
 
+int __dhcp_network_send_raw_packet(int index, void *packet, int len);
+
 int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
  uint8_t optlen, void *optval);
 
diff --git a/src/dhcp/network.c b/src/dhcp/network.c
new file mode 100644
index 000..623251d
--- /dev/null
+++ b/src/dhcp/network.c
@@ -0,0 +1,63 @@
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "internal.h"
+
+int __dhcp_network_send_raw_packet(int index, void *packet, int len)
+{
+int err, s;
+struct sockaddr_ll link;
+
+err = 0;
+
+s = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, htons(ETH_P_IP));
+if (s < 0)
+return -errno;
+
+memset(&link, 0, sizeof(link));
+
+link.sll_family = AF_PACKET;
+link.sll_protocol = htons(ETH_P_IP);
+link.sll_ifindex =  index;
+link.sll_halen = ETH_ALEN;
+memset(&link.sll_addr, 0xff, ETH_ALEN);
+
+if (bind(s, (struct sockaddr *)&link, sizeof(link)) < 0) {
+err = -errno;
+close(s);
+return err;
+}
+
+if (sendto(s, packet, len, 0, (struct sockaddr *)&link,
+   sizeof(link)) < 0)
+err = -errno;
+
+close(s);
+
+return err;
+}
-- 
1.7.10.4

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

[systemd-devel] [PATCH 20/28] dhcp: Add timeout and main loop support

[Patrik Flykt]

Require a main loop to be set when creating a DHCP client. Set up
a timer to resend DHCP Discover messages and add a 0-2 second
delay to the timeout value. Move to state Selecting after successful
sending of a Discover message.
---
 src/dhcp/client.c  |   77 ++--
 src/dhcp/client.h  |4 ++-
 src/dhcp/dhcp-example-client.c |7 +++-
 src/dhcp/test-dhcp-client.c|   19 ++
 4 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 79ac93b..cb3d41b 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -34,12 +34,15 @@
 
 struct DHCPClient {
 DHCPState state;
+sd_event *event;
+sd_event_source *timeout_resend;
 int index;
 uint8_t *req_opts;
 int req_opts_size;
 struct in_addr *last_addr;
 struct ether_addr mac_addr;
 uint32_t xid;
+uint64_t start_time;
 };
 
 static uint8_t default_req_opts[] = {
@@ -142,6 +145,10 @@ static int client_stop(DHCPClient *client, int error)
 client->state == DHCP_STATE_INIT_REBOOT)
 return -EALREADY;
 
+if (client->timeout_resend)
+client->timeout_resend =
+sd_event_source_unref(client->timeout_resend);
+
 switch (client->state) {
 
 case DHCP_STATE_INIT:
@@ -294,8 +301,59 @@ error:
 return err;
 }
 
+static int client_timeout_resend(sd_event_source *s, uint64_t usec,
+ void *userdata)
+{
+DHCPClient *client = userdata;
+uint64_t next_timeout;
+uint16_t secs;
+int err = 0;
+
+switch (client->state) {
+case DHCP_STATE_INIT:
+case DHCP_STATE_SELECTING:
+
+if (!client->start_time)
+client->start_time = usec;
+
+secs = (usec - client->start_time) / USEC_PER_SEC;
+
+next_timeout = usec + 2 * USEC_PER_SEC + (random() & 0x1f);
+
+err = sd_event_add_monotonic(sd_event_get(s), next_timeout,
+ 10 * USEC_PER_MSEC,
+ client_timeout_resend, client,
+ &client->timeout_resend);
+if (err < 0)
+goto error;
+
+if (client_send_discover(client, secs) >= 0)
+client->state = DHCP_STATE_SELECTING;
+
+break;
+
+case DHCP_STATE_INIT_REBOOT:
+case DHCP_STATE_REBOOTING:
+case DHCP_STATE_REQUESTING:
+case DHCP_STATE_BOUND:
+case DHCP_STATE_RENEWING:
+case DHCP_STATE_REBINDING:
+
+break;
+}
+
+return 0;
+
+error:
+client_stop(client, err);
+
+return 0;
+}
+
 int dhcp_client_start(DHCPClient *client)
 {
+int err;
+
 if (!client || client->index == -1)
 return -EINVAL;
 
@@ -305,7 +363,18 @@ int dhcp_client_start(DHCPClient *client)
 
 client->xid = random_u();
 
-return client_send_discover(client, 0);
+err = sd_event_add_monotonic(client->event, now(CLOCK_MONOTONIC), 0,
+ client_timeout_resend, client,
+ &client->timeout_resend);
+if (err < 0)
+goto error;
+
+return 0;
+
+error:
+client_stop(client, err);
+
+return err;
 }
 
 int dhcp_client_stop(DHCPClient *client)
@@ -313,15 +382,19 @@ int dhcp_client_stop(DHCPClient *client)
 return client_stop(client, 0);
 }
 
-DHCPClient *dhcp_client_new(void)
+DHCPClient *dhcp_client_new(sd_event *event)
 {
 DHCPClient *client;
 int i;
 
+if (!event)
+return NULL;
+
 client = new0(DHCPClient, 1);
 if (!client)
 return NULL;
 
+client->event = event;
 client->state = DHCP_STATE_INIT;
 client->index = -1;
 
diff --git a/src/dhcp/client.h b/src/dhcp/client.h
index 71fdbe3..568e70a 100644
--- a/src/dhcp/client.h
+++ b/src/dhcp/client.h
@@ -24,6 +24,8 @@
 #include 
 #include 
 
+#include "sd-event.h"
+
 struct DHCPClient;
 typedef struct DHCPClient DHCPClient;
 
@@ -35,4 +37,4 @@ int dhcp_client_set_mac(DHCPClient *client, struct ether_addr 
*addr);
 
 int dhcp_client_start(DHCPClient *client);
 int dhcp_client_stop(DHCPClient *client);
-DHCPClient *dhcp_client_new(void);
+DHCPClient *dhcp_client_new(sd_event *event);
diff --git a/src/dhcp/dhcp-example-client.c b/src/dhcp/dhcp-example-client.c
index b7f1a5f..afb7102 100644
--- a/src/dhcp/dhcp-example-client.c
+++ b/src/dhcp/dhcp-example-client.c
@@ -84,6 +84,7 @@ int main(int argc, char **argv)
 DHCPClient *client;
 int err, index;
 struct ether_addr mac;
+sd_event *event;
 
 if (argc != 2) {
 pr

[systemd-devel] [PATCH 03/28] dhcp: Add test for DHCP client initialization and parameter setting

[Patrik Flykt]

---
 src/dhcp/test-dhcp-client.c |   81 +++
 1 file changed, 81 insertions(+)
 create mode 100644 src/dhcp/test-dhcp-client.c

diff --git a/src/dhcp/test-dhcp-client.c b/src/dhcp/test-dhcp-client.c
new file mode 100644
index 000..26857b6
--- /dev/null
+++ b/src/dhcp/test-dhcp-client.c
@@ -0,0 +1,81 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright (C) 2013 Intel Corporation. All rights reserved.
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see .
+***/
+
+#include 
+#include 
+#include 
+
+#include "protocol.h"
+#include "client.h"
+
+static void test_request_basic(void)
+{
+DHCPClient *client;
+
+client = dhcp_client_new();
+
+assert(client);
+
+assert(dhcp_client_set_request_option(NULL, 0) == -EINVAL);
+assert(dhcp_client_set_request_address(NULL, NULL) == -EINVAL);
+assert(dhcp_client_set_index(NULL, 0) == -EINVAL);
+
+assert(dhcp_client_set_index(client, 15) == 0);
+assert(dhcp_client_set_index(client, -42) == -EINVAL);
+assert(dhcp_client_set_index(client, -1) == 0);
+
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_SUBNET_MASK) == -EEXIST);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_ROUTER) == -EEXIST);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_HOST_NAME) == -EEXIST);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_DOMAIN_NAME) == -EEXIST);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_DOMAIN_NAME_SERVER)
+== -EEXIST);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_NTP_SERVER) == -EEXIST);
+
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_PAD) == -EINVAL);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_END) == -EINVAL);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_MESSAGE_TYPE) == -EINVAL);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_OVERLOAD) == -EINVAL);
+assert(dhcp_client_set_request_option(client,
+DHCP_OPTION_PARAMETER_REQUEST_LIST)
+== -EINVAL);
+
+assert(dhcp_client_set_request_option(client, 33) == 0);
+assert(dhcp_client_set_request_option(client, 33) == -EEXIST);
+assert(dhcp_client_set_request_option(client, 44) == 0);
+}
+
+int main(int argc, char *argv[])
+{
+test_request_basic();
+
+return 0;
+}
-- 
1.7.10.4

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

[systemd-devel] [PATCH 23/28] dhcp: Add maximum message size option

[Patrik Flykt]

Add maximum message size option to keep some DHCP server implementations
from sending too big messages. See ConnMan commit
0c5c862749c05193cf4c513628328c6db02b5222.
---
 src/dhcp/client.c   |   10 ++
 src/dhcp/protocol.h |1 +
 2 files changed, 11 insertions(+)

diff --git a/src/dhcp/client.c b/src/dhcp/client.c
index 4b316dc..45c62f3 100644
--- a/src/dhcp/client.c
+++ b/src/dhcp/client.c
@@ -206,6 +206,7 @@ static int client_packet_init(DHCPClient *client, uint8_t 
type,
   uint8_t **opt, int *optlen)
 {
 int err;
+uint16_t max_size;
 
 *opt = (uint8_t *)(message + 1);
 
@@ -245,6 +246,15 @@ static int client_packet_init(DHCPClient *client, uint8_t 
type,
client->req_opts);
 if (err < 0)
 return err;
+
+max_size = htons(DHCP_IP_UDP_SIZE + DHCP_MESSAGE_SIZE +
+ DHCP_CLIENT_MIN_OPTIONS_SIZE);
+
+err = __dhcp_option_append(opt, optlen,
+   DHCP_OPTION_MAXIMUM_MESSAGE_SIZE,
+   2, &max_size);
+if (err < 0)
+return err;
 }
 
 return 0;
diff --git a/src/dhcp/protocol.h b/src/dhcp/protocol.h
index 014a6f8..6503931 100644
--- a/src/dhcp/protocol.h
+++ b/src/dhcp/protocol.h
@@ -99,5 +99,6 @@ typedef enum DHCPState DHCPState;
 #define DHCP_OPTION_MESSAGE_TYPE53
 #define DHCP_OPTION_SERVER_IDENTIFIER   54
 #define DHCP_OPTION_PARAMETER_REQUEST_LIST  55
+#define DHCP_OPTION_MAXIMUM_MESSAGE_SIZE57
 #define DHCP_OPTION_CLIENT_IDENTIFIER   61
 #define DHCP_OPTION_END 255
-- 
1.7.10.4

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

Re: [systemd-devel] systemd-networkd questions

[Dan Williams]

On Tue, 2013-11-12 at 23:19 +0100, Tom Gundersen wrote:
> Hi Dan,
> 
> On Tue, Nov 12, 2013 at 10:38 PM, Dan Williams  wrote:
> > 1) what is lacking in other userspace solutions (NetworkManager,
> > ConnMan, wicked, initscripts, etc) that requires
> > yet-another-network-daemon?
> 
> Without criticizing any of the existing solutions, some of the things

To be honest, it would be helpful for you to raise your concerns at
least with NetworkManager, since many of the concerns that we've heard
about running in smaller and enterprise environments have recently been
addressed, or are longstanding misconceptions of things NM did in the
past but has not done for years.

> that motivated my interest in this is that I think we need: something
> easily configured via plain configuration files by a sysadmin,

NM uses .ini-style files in /etc for all configuration these days, and
also has plugins for reading distro-specific configuration files if the
user wants those.

> something that would take a limited amount of space (including its
> dependencies) so it could be reasonably used in an initrd, something

What are your space constraints in terms of both disk space and RSS
usage?

> which would be fast/efficient, so not e.g. calling out to external
> tools/daemons for stuff like dhcp/ipv4ll.

What are your requirements for being "fast/efficient"?  How long do you
require a DHCP exchange to take, on average?  How long do you require
IPv4LL addressing to take, on average?  How much worse than these
requirements are existing solutions?

> Essentially, I see us taking over from what initscripts (and similar
> solutions in various distributions) are currently doing, and
> peacefully coexisting with connman and networkmanager for the
> foreseeable future. Hopefully we'll manage to cooperate on overlapping
> functionality, but I think it is a bit early in the game to start
> discussing that yet.
> 
> > 2) do you expect that systemd-networkd will grow to include bridge,
> > bond, and team setup?  how about other "enterprise"/"server" interface
> > types like macvlan/vtap, tun/tap, gre, vxlan, ovs, etc?  All these are
> > enterprise things that typically won't change during a specific bootup,
> > but which static "servers" increasingly use.  If you don't expect these
> > to be supported, why not?
> 
> Most of these things we expect to support eventually. We'll have to
> discuss on a case-by-case precisely which ones.

At some point, when doing "most of these things", systemd-networkd
becomes another first-tier network management daemon like the 6 or 8
that already exist.  Is that the intention?  Is it useful to have two
different services that both do the same thing?

Dan

> > 3) Is there expected to be D-Bus interfaces for controlling or
> > monitoring the network interfaces and configuration data?  If not, why
> > not?
> 
> This is expected, yes. At first we will at least expose the state of
> the networks/links, and later also allow them to be configured. All
> this would be via dbus.


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

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Alexander E. Patrakov]

2013/11/14 Colin Guthrie :
> What kernel are you using out of interest?

This kernel is a git snapshot between 3.12.0-rc2 and -rc3. Compiled on Gentoo.

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

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Colin Guthrie]

'Twas brillig, and Alexander E. Patrakov at 13/11/13 17:55 did gyre and
gimble:
> 2013/11/13 Alexander E. Patrakov :
>> 2013/11/13 Colin Guthrie :
>>> What about if you generate it with the keyboard plugged in? It should
>>> include the usbhid module then, but does it actually work?
>>
>> Will try a bit later, but I suspect this is the wrong direction of thought.
> 
> A host-only initramfs generated with the USB keyboard plugged in works.

OK, thanks for that. Will have to investigate with my user as to whether
this is a new problem in a more recent kernel (e.g. with module splits
etc) or something else.

What kernel are you using out of interest?

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Alexander E. Patrakov]

2013/11/13 Alexander E. Patrakov :
> 2013/11/13 Colin Guthrie :
>> What about if you generate it with the keyboard plugged in? It should
>> include the usbhid module then, but does it actually work?
>
> Will try a bit later, but I suspect this is the wrong direction of thought.

A host-only initramfs generated with the USB keyboard plugged in works.

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

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Alexander E. Patrakov]

2013/11/13 Colin Guthrie :
> What about if you generate it with the keyboard plugged in? It should
> include the usbhid module then, but does it actually work?

Will try a bit later, but I suspect this is the wrong direction of thought.

> The problem we're encountering is that even with the usbhid module
> included in the host-only initrd, it still doesn't work - even after
> manually modprobing it from a ps/2 keyboard.

All modules that are needed for the user's keyboard are included in the initrd.

> As I don't have the h/w I cannot tell what component is missing to make
> it all work. I'm currently suspecting some kind of usb host controller
> interface driver or something, but have yet to hear back from the user
> so any insights or info you have would be appreciated.

I'd suspect something related to USB power saving. This is the most
frequent reason for USB keyboards and mice to misbehave - the host
just turns them off mistakenly. So
/lib/udev/rules.d/42-usb-hid-pm.rules does look relevant.

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

Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[Dax Kelson]

On Nov 13, 2013 8:03 AM, "Lennart Poettering" 
wrote:
>
> I also have the suspicion that the best strategy for handling degraded
> arrays is to timeout and not assemble them but rather put the system in
> a state where the admin has to become active. Auto-assembling degraded
> arrays has the feel of taping over issues. If the admin chooses to
> boot-up with degraded disks then that's ok, but I am pretty sure this is
> something the admin should explicitly decide.

As an experienced admin, I disagree with this. If I've gone to the effort
to setup a RAID volume obviously I value high availability. The box should
boot without manual intervention even with a degraded RAID volume. With
above proposal, the box automatically continue to will run properly with a
degraded RAID volume (like normal) but won't reboot properly?
Rebooting/booting is a normal part of operating.

Also. There are many scenarios where a box can reboot on its own (kernel
panic, power failure, etc). It should come back automatically if it can.
I'd be livid if it didn't.

Make it an option if you insist, but have it default to booting without
manual intervention if possible.

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

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Colin Guthrie]

'Twas brillig, and Alexander E. Patrakov at 13/11/13 17:18 did gyre and
gimble:
> 2013/11/13 Colin Guthrie :
>> 'Twas brillig, and Alexander E. Patrakov at 13/11/13 16:49 did gyre and
>> gimble:
>>> 2013/11/13 Alexander E. Patrakov :
 Colin Guthrie wrote:
>
> Hi,
>
> In this bug report https://bugs.mageia.org/show_bug.cgi?id=11645 a user is
> unable to enter their encrypted root password via a wireless keyboard
> connected with a Logitech, Inc. Unifying Receiver.


 Just in case, I do have the same model of wireless keyboard and will
 validate the bug (for the LUKS-password case) later today on my laptop.

 The keyboard does work on my desktop, but there's no initramfs there.
>>>
>>> I hereby confirm that the wireless Logitech keyboard works for
>>> entering the LUKS password on the laptop, if one does not create a
>>> host-only initramfs. This is with dracut-031.
>>
>>
>> thanks for that. If it's easy enough for you to test, does it also work
>> with a host-only initrd?
> 
> It doesn't, because (at least whe creating the initramfs without the
> USB keyboard connected) it does not include the usbhid module.

What about if you generate it with the keyboard plugged in? It should
include the usbhid module then, but does it actually work?

The problem we're encountering is that even with the usbhid module
included in the host-only initrd, it still doesn't work - even after
manually modprobing it from a ps/2 keyboard.

As I don't have the h/w I cannot tell what component is missing to make
it all work. I'm currently suspecting some kind of usb host controller
interface driver or something, but have yet to hear back from the user
so any insights or info you have would be appreciated.

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Alexander E. Patrakov]

2013/11/13 Colin Guthrie :
> 'Twas brillig, and Alexander E. Patrakov at 13/11/13 16:49 did gyre and
> gimble:
>> 2013/11/13 Alexander E. Patrakov :
>>> Colin Guthrie wrote:

 Hi,

 In this bug report https://bugs.mageia.org/show_bug.cgi?id=11645 a user is
 unable to enter their encrypted root password via a wireless keyboard
 connected with a Logitech, Inc. Unifying Receiver.
>>>
>>>
>>> Just in case, I do have the same model of wireless keyboard and will
>>> validate the bug (for the LUKS-password case) later today on my laptop.
>>>
>>> The keyboard does work on my desktop, but there's no initramfs there.
>>
>> I hereby confirm that the wireless Logitech keyboard works for
>> entering the LUKS password on the laptop, if one does not create a
>> host-only initramfs. This is with dracut-031.
>
>
> thanks for that. If it's easy enough for you to test, does it also work
> with a host-only initrd?

It doesn't, because (at least whe creating the initramfs without the
USB keyboard connected) it does not include the usbhid module.

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

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Colin Guthrie]

'Twas brillig, and Alexander E. Patrakov at 13/11/13 16:49 did gyre and
gimble:
> 2013/11/13 Alexander E. Patrakov :
>> Colin Guthrie wrote:
>>>
>>> Hi,
>>>
>>> In this bug report https://bugs.mageia.org/show_bug.cgi?id=11645 a user is
>>> unable to enter their encrypted root password via a wireless keyboard
>>> connected with a Logitech, Inc. Unifying Receiver.
>>
>>
>> Just in case, I do have the same model of wireless keyboard and will
>> validate the bug (for the LUKS-password case) later today on my laptop.
>>
>> The keyboard does work on my desktop, but there's no initramfs there.
> 
> I hereby confirm that the wireless Logitech keyboard works for
> entering the LUKS password on the laptop, if one does not create a
> host-only initramfs. This is with dracut-031.


thanks for that. If it's easy enough for you to test, does it also work
with a host-only initrd?

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/2] (dracut) hwdb needed in early boot for some keyboards

[Alexander E. Patrakov]

2013/11/13 Alexander E. Patrakov :
> Colin Guthrie wrote:
>>
>> Hi,
>>
>> In this bug report https://bugs.mageia.org/show_bug.cgi?id=11645 a user is
>> unable to enter their encrypted root password via a wireless keyboard
>> connected with a Logitech, Inc. Unifying Receiver.
>
>
> Just in case, I do have the same model of wireless keyboard and will
> validate the bug (for the LUKS-password case) later today on my laptop.
>
> The keyboard does work on my desktop, but there's no initramfs there.

I hereby confirm that the wireless Logitech keyboard works for
entering the LUKS password on the laptop, if one does not create a
host-only initramfs. This is with dracut-031.

Some relevant kernel config bits:

CONFIG_HID=y
CONFIG_HID_GENERIC=y
CONFIG_HID_LOGITECH=y
CONFIG_HID_LOGITECH_DJ=m   # dracut is smart enough to include it,
even though the password entry works well without it

CONFIG_USB_HID=m

CONFIG_USB_XHCI_HCD=m
CONFIG_USB_EHCI_HCD=m
CONFIG_USB_EHCI_PCI=m

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

Re: [systemd-devel] systemd-networkd questions

[Tom Gundersen]

On Wed, Nov 13, 2013 at 5:31 PM, Bill Nottingham  wrote:
> Tom Gundersen (t...@jklm.no) said:
>> Without criticizing any of the existing solutions, some of the things
>> that motivated my interest in this is that I think we need: something
>> easily configured via plain configuration files by a sysadmin,
>> something that would take a limited amount of space (including its
>> dependencies) so it could be reasonably used in an initrd, something
>> which would be fast/efficient, so not e.g. calling out to external
>> tools/daemons for stuff like dhcp/ipv4ll.
>
> On this specific point... are you *sure* you want to be rewriting a
> dhcp/dhcpv6 client?

I'm sure I would like avoid that :) Hopefully we'll be able to reuse
the dhcp client from connman, but as the work of converting that into
a library has not yet finished I don't know yet exactly how that will
work out. Hopefully this will become clearer very soon.

Cheers,

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

Re: [systemd-devel] systemd-networkd questions

[Bill Nottingham]

Tom Gundersen (t...@jklm.no) said: 
> Without criticizing any of the existing solutions, some of the things
> that motivated my interest in this is that I think we need: something
> easily configured via plain configuration files by a sysadmin,
> something that would take a limited amount of space (including its
> dependencies) so it could be reasonably used in an initrd, something
> which would be fast/efficient, so not e.g. calling out to external
> tools/daemons for stuff like dhcp/ipv4ll.

On this specific point... are you *sure* you want to be rewriting a
dhcp/dhcpv6 client?

(I ask, because we've gone through at least three over history here, and
dealing with the assorted corner cases between vendors and configuration
bits required would seem to make it one area not worth trying to reinvent.)

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

Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[Alexander E. Patrakov]

2013/11/13 NeilBrown :
> On Tue, 12 Nov 2013 19:01:49 +0400 Andrey Borzenkov 
> wrote:
>
>> В Tue, 12 Nov 2013 21:17:19 +1100
>> NeilBrown  пишет:
>>
>> > On Tue, 12 Nov 2013 18:16:24 +0900 Greg KH  
>> > wrote:
>> >
>> > > On Tue, Nov 12, 2013 at 07:54:42PM +1100, NeilBrown wrote:
>> > > > On Tue, 12 Nov 2013 00:10:28 -0800 Greg KH 
>> > > >  wrote:
>> > > >
>> > > > > On Tue, Nov 12, 2013 at 11:31:45AM +1100, NeilBrown wrote:
>> > > > > > Alternately, is there some "all devices have been probed, nothing 
>> > > > > > new will
>> > > > > > appear unless it is hot-plugged" event.  That would be equally 
>> > > > > > useful (and
>> > > > > > probably mirrors what hardware-RAID cards do).
>> > > > >
>> > > > > No, there's no way to ever know this in a hotplug world, sorry.
>> > > > > Especially with USB devices, they show up when they show up, there's 
>> > > > > no
>> > > > > "oh look, the bus is all scanned now and all devices currently 
>> > > > > plugged
>> > > > > in are found" type knowledge at all.
>> > > > >
>> > > > > Then there are hotplug PCI systems where people slam in PCI cards
>> > > > > whenever they feel like it (remember, thunderbolt is PCI express...)
>> > > > >
>> > > > > Sorry,
>> > > > >
>> > > > > greg k-h
>> > > >
>> > > > Surely something must be possible.
>> > >
>> > > For USB, nope, there isn't, sorry.
>> > >
>> > > > Clearly a physical hot-plug event will cause more devices to appear, 
>> > > > but
>> > > > there must come a point at which no more (non-virtual) devices will 
>> > > > appear
>> > > > unless a physical event happens?
>> > >
>> > > Not for USB, sorry.
>> > >
>> > > The USB bus just announces devices when it finds them, there is no "all
>> > > is quiet" type signal or detection.
>> > >
>> > > Same for PCI hotplug, devices can show up at any point in time, you
>> > > never know when, and you don't know when all devices are "found".
>> > >
>> > > sorry,
>> > >
>> > > greg k-h
>> >
>> >
>> > Hmmm... OK.  USB doesn't bother me a lot, but PCI is important.
>> >
>> > I guess I'll just have to settle for a timeout much like the current
>> > device-discovery timeout that systemd has.
>> > Still hoping someone can tell me how to plug into that though...
>> >
>>
>> If information about array name or other identification is available in
>> udev rule (I see reference to device node only) what you can do is to
>> start timer with "now+5second" (pick your timeout) that simply fires off
>> mdadm -IRs for specific array. Something like
>>
>> mdadm-last-resort@.timer
>>
>> [Timer]
>> OnCalendar=+5s
>>
>> mdadm-last-resort@.service
>>
>> [Service]
>> Type=oneshot
>> ExecStart=/sbin/mdadm -IRs %n
>>
>> udev rule
>>
>> ... SYSTEMD_WANTS=mdadm-last-resort@$ENV{SOMETHING_UNIQUE}.timer
>>
>
> Thanks.  This certainly looks interesting and might be part of a solution.
> However it gets the timeout test backwards.
>
> I don't want to set the timeout when the array starts to appear.  I want to
> set the time out when someone wants to use the array.
> If no-one is waiting for the array device, then there is no point forcing it.
>
> That's why I want to plug into the timeout that systemd already has.
>
> Maybe that requirement isn't really necessary though.  I'll experiment with
> your approach.

It is useless to even try to plug into the existing systemd timeout,
for a very simple reason. in the setups where your RAID array is not
on the top of the storage device hierarchy, systemd does not know that
it wants your RAID array to appear.

So the statement "If no-one is waiting for the array device, then
there is no point forcing it" is false, because there is no way to
know that no-one is waiting.

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

Re: [systemd-devel] Need help with a systemd/mdadm interaction.

[Lennart Poettering]

On Tue, 12.11.13 11:31, NeilBrown (ne...@suse.de) wrote:

> 
> Hi,
>  I wonder if I could get some advice
> 
> mdadm is quite good at assembling arrays incrementally.  "udev" runs
> "mdadm -I" for each new device and mdadm gathers them into arrays and
> activates the array once all the expected devices have been gathered.
> 
> This simple rule falls down if some expected device doesn't appear.
> 
> If an array is marked as degraded then mdadm doesn't expect the missing
> device and will happily start the degaded array.  However if the system shuts
> down with the md array fully functional, and a device is removed before the
> system is restarted, then mdadm does not know that the device is missing and
> will effectively wait for it forever.
> 
> mdadm can handle this, but it needs to be told.  The command:
>   mdadm -IRs
> will find any arrays which have enough devices to start degraded but haven't
> been started yet, and will start them.
> I used this quite effectively in out initrd.  There is a loop that count up
> to N waiting for the root device to appear and when we get to "N/2" I run
> "mdadm -IRs" which makes any newly-degraded arrays appear.
> 
> I'm wondering how to integrate this with systemd.  Systemd has its own
> mechanism to wait for devices to appear, but I cannot see anyway to run
> "mdadm -IRs" at the half-way mark.
> 
> It would of course be sufficient to wait for the complete timeout, then run
> "mdadm -IRs", then check if the device has appeared, but I can't see how to
> fit this into systemd either.
> 
> So: how can I fit this need for "run some command on device timeout which
> might be successful in activating the device"?
> 
> Alternately, is there some "all devices have been probed, nothing new will
> appear unless it is hot-plugged" event.  That would be equally useful (and
> probably mirrors what hardware-RAID cards do).

As pointed out by the others in the thread: there is no point in time
where all devices have shown. Whether USB, PCI Express or iSCSI or even
other stuff: assuming there was a point in time where everything has
shown up is wrong. There's no such point in time.

I also have the suspicion that the best strategy for handling degraded
arrays is to timeout and not assemble them but rather put the system in
a state where the admin has to become active. Auto-assembling degraded
arrays has the feel of taping over issues. If the admin chooses to
boot-up with degraded disks then that's ok, but I am pretty sure this is
something the admin should explicitly decide.

Note that systemd will timeout already when we look for devices to
mount. Hence, you could just decide to do nothing in mdadm: when the
stuff doesn't appear then systemd will already now put the system into
emergency mode and give the admin a shell.

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] Path unit configuration starts early (DefaultDependencies=no)

[Abdelghani Ouchabane]

  
  
On 11/11/13 18:23, Zbigniew
  Jędrzejewski-Szmek wrote:


  On Mon, Nov 11, 2013 at 05:50:51PM +0100, Abdelghani Ouchabane wrote:

  
On 11/11/13 17:35, Zbigniew Jędrzejewski-Szmek wrote:


  On Mon, Nov 11, 2013 at 05:26:28PM +0100, Abdelghani Ouchabane wrote:

  
Hallo,
  I have two configuration files: ezono-cyclades-t_upn.path &
ezono-cyclades-t_upn.service


Where:

ezono-cyclades-t_upn.path :

[Unit]
Description=ezono-cyclades-t_upn Service Spool
DefaultDependencies=no
Conflicts=shutdown.target
Before=shutdown.target

[Path]
PathExists=/tmp/cyclades/start-ezono-cyclades-gui

[Install]
WantedBy=ezono-cyclades.target


The problem is that by passing "DefaultDependencies=no"
ezono-cyclades-t_upn.service doesn't start after
/tmp/cyclades/start-ezono-cyclades-gui is created, even after all
other units run and I delete /tmp/cyclades/start-ezono-cyclades-gui
and create it again.

Any idea please.

  
  Is ezono-cyclaed.target enabled?


Yes

ezono-cyclades.target - eZono Cyclades GUI
   Loaded: loaded (/usr/lib/systemd/system/ezono-cyclades.target; enabled)
   Active: active since Mon 2013-11-11 17:48:43 CET; 23s ago

Nov 11 17:48:43 sonostation-usb12-eth.ezono.net systemd[1]: Starting
eZono Cyclades GUI.
Nov 11 17:48:43 sonostation-usb12-eth.ezono.net systemd[1]: Reached
target eZono Cyclades GUI.

  
  The condition is only checked once, when the unit is started. I'd guess
that whatever creates /tmp/cyclades/start-ezono-cyclades-gui, does it
after the ezono-cyclades-t_upn.service/start job has already been done.
The condition will not be checked again unless you restart the servcie.

Thanks Zbigniew

I confirm that, the condition is checked only after I execute :
systemctl daemon-reload



  


  

  Also, DefaultDependencies=no is very unlikely to be what you need.


I want to start ezono-cyclades-t_upn.path early as I can

  
  That's not very convicing. I have no idea what it does, but
DefaultDependencies=no is only appropriate for services which
participate in bringing up the system.

Right, I have two services which deal with hardware devices, and
they start early properly.


  

Units that you show are very very complex, but it's unlikely that this
is all needed. E.g. don't implement your own strange logging
solutions, just start systemd with --log-level=debug and you'll get
full information about what is started and when. Also output from
services is logged to the journal by default.

I am planning to remove my logging solutions and use the systemd
built-in.

Thanks
  

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

[systemd-devel] [PATCH] journal: timestamp support on console messages

[Umut Tezduyar Lindskog]

---
 src/journal/journald-console.c |   33 ++---
 1 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/journal/journald-console.c b/src/journal/journald-console.c
index be55f94..93e3241 100644
--- a/src/journal/journald-console.c
+++ b/src/journal/journald-console.c
@@ -19,13 +19,30 @@
   along with systemd; If not, see .
 ***/
 
+#include 
 #include 
 #include 
 #include 
 
+#include "fileio.h"
 #include "journald-server.h"
 #include "journald-console.h"
 
+static bool prefix_timestamp(void) {
+
+static int printk_time = -1;
+
+if (printk_time < 0) {
+_cleanup_free_ char *p = NULL;
+
+printk_time =
+
read_one_line_file("/sys/module/printk/parameters/time", &p) >= 0 &&
+parse_boolean(p) > 0;
+}
+
+return printk_time;
+}
+
 void server_forward_console(
 Server *s,
 int priority,
@@ -33,8 +50,10 @@ void server_forward_console(
 const char *message,
 struct ucred *ucred) {
 
-struct iovec iovec[4];
+struct iovec iovec[5];
 char header_pid[16];
+char tbuf[50];
+struct timespec ts;
 int n = 0, fd;
 char *ident_buf = NULL;
 const char *tty;
@@ -45,7 +64,15 @@ void server_forward_console(
 if (LOG_PRI(priority) > s->max_level_console)
 return;
 
-/* First: identifier and PID */
+/* First: timestamp */
+if (prefix_timestamp()) {
+assert_se(clock_gettime(CLOCK_MONOTONIC, &ts) == 0);
+snprintf(tbuf, sizeof(tbuf), "[%5llu.%06llu] ",
+   (unsigned long long)ts.tv_sec, (unsigned long 
long)ts.tv_nsec/1000);
+IOVEC_SET_STRING(iovec[n++], tbuf);
+}
+
+/* Second: identifier and PID */
 if (ucred) {
 if (!identifier) {
 get_process_comm(ucred->pid, &ident_buf);
@@ -64,7 +91,7 @@ void server_forward_console(
 IOVEC_SET_STRING(iovec[n++], ": ");
 }
 
-/* Third: message */
+/* Fourth: message */
 IOVEC_SET_STRING(iovec[n++], message);
 IOVEC_SET_STRING(iovec[n++], "\n");
 
-- 
1.7.2.5

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

Re: [systemd-devel] [PATCH] activate: fix crash when -s is passed

[David Timothy Strauss]

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

Re: [systemd-devel] [PATCH] activate: mention -E in the help text

[David Timothy Strauss]

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

Re: [systemd-devel] [PATCH] systemctl: honor --no-legend in 'list-jobs'

[David Timothy Strauss]

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