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

2013-12-12 Thread Patrik Flykt
On Tue, 2013-12-10 at 18:48 +0100, Lennart Poettering wrote:
 I agree with Zbigniew: this should probably be in
 src/libsystemd-dhcp/, so that this is similar to libsystemd-bus/,
 libsystemd-id128/ and so on...

Sent a patch for that, and in the process of creating an account on
freedesktop.org. As that will most probably take some time, someone with
proper access could bite the bullet...

Cheers,

Patrik


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


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

2013-12-12 Thread Marcel Holtmann
Hi Patrik,

 I agree with Zbigniew: this should probably be in
 src/libsystemd-dhcp/, so that this is similar to libsystemd-bus/,
 libsystemd-id128/ and so on...
 
 Sent a patch for that, and in the process of creating an account on
 freedesktop.org. As that will most probably take some time, someone with
 proper access could bite the bullet...

I will take care of getting these patches committed. Can you just send an 
updated version with everything in place in the right location.

Regards

Marcel

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


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

2013-12-10 Thread Patrik Flykt
On Tue, 2013-12-10 at 02:46 +0100, Lennart Poettering wrote:
  +uint32_t ciaddr;
  +uint32_t yiaddr;
  +uint32_t siaddr;
  +uint32_t giaddr;
 
 Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
 right? which is just synoymous to big endian...
 
 Or even, if you now use struct in_addr elsewhere anyway, why not
 just use that here too?

be32_t seems reasonable, the 'struct in_addr' was deemed to add slightly
too much cruft without being particularly useful in internal code. What
if we figure out whether this patch set is otherwise reasonable and fix
this and add the remaining parts of DHCP in a coming patch set?
Meanwhile Tom could take the code for an official test spin...


Cheers,

Patrik


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


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

2013-12-10 Thread Tom Gundersen
On 10 Dec 2013 17:45, Patrik Flykt patrik.fl...@linux.intel.com wrote:

 On Tue, 2013-12-10 at 02:46 +0100, Lennart Poettering wrote:
   +uint32_t ciaddr;
   +uint32_t yiaddr;
   +uint32_t siaddr;
   +uint32_t giaddr;
 
  Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
  right? which is just synoymous to big endian...
 
  Or even, if you now use struct in_addr elsewhere anyway, why not
  just use that here too?

 be32_t seems reasonable, the 'struct in_addr' was deemed to add slightly
 too much cruft without being particularly useful in internal code. What
 if we figure out whether this patch set is otherwise reasonable and fix
 this and add the remaining parts of DHCP in a coming patch set?
 Meanwhile Tom could take the code for an official test spin...

Sounds reasonable to me. I'm mostly afk this week, but should be able to
get it done next week. Do you have a public got repo I could easily pull
from?

Cheers,

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


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

2013-12-10 Thread Lennart Poettering
On Tue, 10.12.13 14:15, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 
 On Tue, 2013-12-10 at 02:46 +0100, Lennart Poettering wrote:
   +uint32_t ciaddr;
   +uint32_t yiaddr;
   +uint32_t siaddr;
   +uint32_t giaddr;
  
  Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
  right? which is just synoymous to big endian...
  
  Or even, if you now use struct in_addr elsewhere anyway, why not
  just use that here too?
 
 be32_t seems reasonable, the 'struct in_addr' was deemed to add slightly
 too much cruft without being particularly useful in internal code. What
 if we figure out whether this patch set is otherwise reasonable and fix
 this and add the remaining parts of DHCP in a coming patch set?
 Meanwhile Tom could take the code for an official test spin...

This code looks as if it's ready to be merged. WHatever else comes up
can be fixed post-commit. I think it would be best if you'd get commit
access so that you can maintain this.

Could you request an fdo account if you don't have one please? If you
already do, please file a bug so that you get commit access to the
systemd repo.

http://www.freedesktop.org/wiki/AccountRequests/

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

2013-12-10 Thread Marcel Holtmann
Hi Lennart,

 +uint32_t ciaddr;
 +uint32_t yiaddr;
 +uint32_t siaddr;
 +uint32_t giaddr;
 
 Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
 right? which is just synoymous to big endian...
 
 Or even, if you now use struct in_addr elsewhere anyway, why not
 just use that here too?
 
 be32_t seems reasonable, the 'struct in_addr' was deemed to add slightly
 too much cruft without being particularly useful in internal code. What
 if we figure out whether this patch set is otherwise reasonable and fix
 this and add the remaining parts of DHCP in a coming patch set?
 Meanwhile Tom could take the code for an official test spin...
 
 This code looks as if it's ready to be merged. WHatever else comes up
 can be fixed post-commit. I think it would be best if you'd get commit
 access so that you can maintain this.

are you fine with the layout of files and directories? Then I can start 
handling maintaining it and dealing with patches.

Regards

Marcel

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


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

2013-12-10 Thread Lennart Poettering
On Tue, 10.12.13 18:12, Marcel Holtmann (mar...@holtmann.org) wrote:

 Hi Lennart,
 
  +uint32_t ciaddr;
  +uint32_t yiaddr;
  +uint32_t siaddr;
  +uint32_t giaddr;
  
  Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
  right? which is just synoymous to big endian...
  
  Or even, if you now use struct in_addr elsewhere anyway, why not
  just use that here too?
  
  be32_t seems reasonable, the 'struct in_addr' was deemed to add slightly
  too much cruft without being particularly useful in internal code. What
  if we figure out whether this patch set is otherwise reasonable and fix
  this and add the remaining parts of DHCP in a coming patch set?
  Meanwhile Tom could take the code for an official test spin...
  
  This code looks as if it's ready to be merged. WHatever else comes up
  can be fixed post-commit. I think it would be best if you'd get commit
  access so that you can maintain this.
 
 are you fine with the layout of files and directories? Then I can start 
 handling maintaining it and dealing with patches.

I agree with Zbigniew: this should probably be in src/libsystemd-dhcp/,
so that this is similar to libsystemd-bus/, libsystemd-id128/ and so
on...

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

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

 +struct DHCPMessage {
 +uint8_t op;
 +uint8_t htype;
 +uint8_t hlen;
 +uint8_t hops;
 +be32_t xid;
 +be16_t secs;
 +be16_t flags;
 +uint32_t ciaddr;
 +uint32_t yiaddr;
 +uint32_t siaddr;
 +uint32_t giaddr;

Hmmm, why uin32_t? Shouldn't this be32_t? THis is network byte order,
right? which is just synoymous to big endian...

Or even, if you now use struct in_addr elsewhere anyway, why not just
use that here too?

Lennart

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