Bug 192413

Summary: libdhcp : IPv6 and IPv4 DHCP client and network configuration library API
Product: [Fedora] Fedora Reporter: Jason Vas Dias <jvdias>
Component: Package ReviewAssignee: David Cantrell <dcantrell>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dcantrell, fedora-package-review, katzj, nalin, notting, pnasrat
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://people.redhat.com/~jvdias/libdhcp
Whiteboard:
Fixed In Version: libdhcp-1.2 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-06-08 14:15:05 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 186322, 188268    
Attachments:
Description Flags
log of builds in the dist-fc6-build root of x86_64 hs20-bc1-5, 2. ia64 altix2, ppc64 js20-bc1-12
none
patch against anaconda 11.1.0.21 to make loader2 use libdhcp none

Description Jason Vas Dias 2006-05-19 11:28:25 EDT
Description of problem:

anaconda and potentially other users need a library based implementation 
of DHCP clients for IPv6 and IPv4, and a library to configure the network
from a static configuration, similar to the facilities currently provided
by pump, but hopefully much improved and newly IPv6 capable (bug 186322).

At the request of the anaconda team, I have developed such a library:
libdhcp. The .spec file and srpm can be downloaded from:
    http://people.redhat.com/~jvdias/libdhcp
and the source is also checked into elvis as 'libdhcp'.

libdhcp requires only the 'libdhcp6client-' sub-package of dhcpv6, and the
'libdhcp4client' subpackage of dhcp. These are MINIMALLY modified versions
of the upstream dhcp clients, with header files. No dhcp client executable
is changed in any way. The client libraries's main() functions are changed 
to be named 'dhcpv[46]_client' and to accept a LIBDHCP_Control * argument,
all their dynamic memory is automatically freed on return from these functions,
and all static / global variables are re-initialized; they've also been 
changed to invoke the libdhcp callbacks for state changes and error logging,
and any use of the exit() syscall has been removed.

See the libdhcp source README file for details on the libdhcp API.

libdhcp is now useable, but is currently being improved to provide:
 o an API 100% compatible with pump, (pump.h and all it defines that
   is used by anaconda - should be complete today).
 o both clients running in separate threads (this currently works under
   GDB only). Only necessary if a timeout parameter of 0 is supplied, ie
   you want the client to wait indefinitely for either IPv6 or IPv4 server
   contact.
 o better memory cleanup. While valgrind reports no errors from libdhcp,
   libdhcp4client, or libdhcp6client code under operation modes tested
   so far, there certainly are memory leaks - these will be resolved
   shortly (ie. today).
 Coming shortly (within next week):
 o better Doxygen documentation
 o a libdhcp++ C++ class interface to the library
 o PERL and Python module interfaces to the library
Comment 1 Jason Vas Dias 2006-05-19 11:53:56 EDT
Please also see:
  http://people.redhat.com/~jvdias/libdhcp/README
(the readme file from the source tarball).
Comment 2 Jesse Keating 2006-05-19 12:48:51 EDT
Bad:
- rpmlint fails on the srpm:
E: libdhcp hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/pkgconfig
E: libdhcp hardcoded-library-path in /usr/lib/pkgconfig/libdhcp.pc
- Package does not build in mock:
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h:33:27: error:
isc-dhcp/list.h: No such file or directory

plus many other build failures.

NEEDSWORK
Comment 3 Jason Vas Dias 2006-05-19 13:29:21 EDT
I'd say whatever tool you're using NEEDSWORK also -

1. There is no way currently of 'softcoding' the pkgconfig dir.
 - To use
   %{_libdir}/pkgconfig or %{prefix}/%{_lib}/pkgconfig would fail 
   on multi-lib arches (as I found out with avahi) - 
   /usr/lib64/pkgconfig is NOT a pkgconfig directory. 
   The pkgconfig dir is ALWAYS /usr/lib/pkgconfig - 
   until /usr/lib/rpm/macros gives us a nice %{_pkgconfig} variable.
   
   MANY other packages use pkgconfig, and they all have to specify
   /usr/lib/pkgconfig.

   I can remove the libdhcp.pc if you wish, but that would be a shame
   as it would prevent software being able to discover the Cflags, Libs,
   and Libs.private it requires automatically, which leads on to the next
   point.

2. Your tool is using the wrong include path, which it could have obtained
   by using 'pkg-config --cflags', to obtain '-I/usr/lib/dhcp4client' 
   and '-I/usr/lib/dhcp4client/isc-dhcp', where the headers for the 
   dhcp4client reside.
 
If you could suggest a resolution, I'll implement it - what tool are you
using ? It sounds like it needs fixing.
Comment 4 Jason Vas Dias 2006-05-19 13:53:06 EDT
OK, I've checked the pkgcfgdir thing - it is %{_libdir} - patch applied:
--- libdhcp.spec.~1.5.~ 2006-05-19 09:24:17.000000000 -0400
+++ libdhcp.spec        2006-05-19 13:53:02.000000000 -0400
@@ -47,0 +48,3 @@
+
+%define pkgcfgdir %{_libdir}/pkgconfig
+
@@ -49 +52 @@
-%makeinstall pkgcfgdir=$RPM_BUILD_ROOT/usr/lib/pkgconfig
+%makeinstall pkgcfgdir=%{pkgcfgdir}

New spec file with the above change checked into CVS and is on the website.

But the second issue - those headers aren't even shipped by the package - they
are shipped by its dependency, libdhcp4client-devel - libdhcp has this tag:

BuildRequires: libdhcp4client-devel, libdhcp6client-devel, libnl-devel, pkgconfig

It gets the correct include path using pkgconfig, and builds fine when its
BuildRequires packages are installed.

How are you getting that error:
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h:33:27: error:
isc-dhcp/list.h: No such file or directory

Any system that had libdhcp4client-devel installed would not generate that
error when building libdhcp.
Comment 5 Jason Vas Dias 2006-05-19 14:02:57 EDT
Revised patch applied:

--- libdhcp.spec.~1.5.~ 2006-05-19 09:24:17.000000000 -0400
+++ libdhcp.spec        2006-05-19 14:07:43.000000000 -0400
@@ -47,0 +48,3 @@
+
+%define pkgcfgdir %{_libdir}/pkgconfig
+
@@ -49 +52 @@
-%makeinstall pkgcfgdir=$RPM_BUILD_ROOT/usr/lib/pkgconfig
+%makeinstall pkgcfgdir=${RPM_BUILD_ROOT}%{pkgcfgdir}
@@ -72 +75 @@
-/usr/lib/pkgconfig/libdhcp.pc
+%{pkgcfgdir}/libdhcp.pc
Comment 6 Jesse Keating 2006-05-19 14:35:14 EDT
The tool used is mock, which is what your package needs to be able to build in
as part of the Package Guidelines.  However as you state, it may be a problem in
the deps of the BuildReqs that are brought in, this needs to be investigated.
Comment 7 Jesse Keating 2006-05-19 15:32:50 EDT
+ CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic '
+ CC=gcc
+ make -j2
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic  -fPIC -I. -Wall -Werror   -c -o
libdhcp.o libdhcp.c
Package libdhcp4client was not found in the pkg-config search path.
Perhaps you should add the directory containing `libdhcp4client.pc'
to the PKG_CONFIG_PATH environment variable
No package 'libdhcp4client' found
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic  -fPIC -I. -Wall -Werror       -c
-o dhcp4_lease.o dhcp4_lease.c
Package libdhcp4client was not found in the pkg-config search path.
Perhaps you should add the directory containing `libdhcp4client.pc'
to the PKG_CONFIG_PATH environment variable
No package 'libdhcp4client' found
Package libdhcp4client was not found in the pkg-config search path.
Perhaps you should add the directory containing `libdhcp4client.pc'
to the PKG_CONFIG_PATH environment variable
No package 'libdhcp4client' found
dhcp4_lease.c:29:28: error: isc-dhcp/dhcpd.h: No such file or directory


Thats more output from the build log.
Comment 8 Jason Vas Dias 2006-05-19 16:29:29 EDT
Thanks - I see the problem - libdhcp4client and libdhcp6client both install 
their pkgconfig files in /usr/lib/pkgconfig :-(
I'm now rebuilding them - I guess its OK to do a move-pkgs -d on them and
then just rebuild the same version since they were both just built today
and haven't gone out to anyone - rebuilding now.
Comment 9 Jason Vas Dias 2006-05-19 17:15:19 EDT
OK, fixed now - sorry about that - all pkgconfigs are now in correct locations
on multi-arch machines:
$ rpm -qplv
/mnt/redhat/dist/fc6/dhcp{,v6}/{0.10-24,3.0.4-6}/{x86_64,ppc64,s390x}/* | grep
pkgconfig
-rw-r--r--    1 root    root              184 May 19 16:53
/usr/lib64/pkgconfig/libdhcp4client.pc
-rw-r--r--    1 root    root              184 May 19 16:55
/usr/lib64/pkgconfig/libdhcp4client.pc
-rw-r--r--    1 root    root              184 May 19 16:54
/usr/lib64/pkgconfig/libdhcp4client.pc
-rw-r--r--    1 root    root              224 May 19 17:14
/usr/lib64/pkgconfig/libdhcp6client.pc
-rw-r--r--    1 root    root              224 May 19 17:14
/usr/lib64/pkgconfig/libdhcp6client.pc
-rw-r--r--    1 root    root              224 May 19 17:14
/usr/lib64/pkgconfig/libdhcp6client.pc
Comment 10 Jesse Keating 2006-05-19 17:48:33 EDT
Hrm, so I don't get the pkg-config errors, now I just get:

+ make -j2
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic  -fPIC -I. -Wall -Werror   -c -o
libdhcp.o libdhcp.c
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic  -fPIC -I. -Wall -Werror
-I/usr/include/dhcp4client   -I/usr/include/dhcp
4client/isc-dhcp     -c -o dhcp4_lease.o dhcp4_lease.c
In file included from /usr/include/dhcp4client/isc-dhcp/minires/minires.h:30,
                 from /usr/include/dhcp4client/isc-dhcp/dhcpd.h:61,
                 from dhcp4_lease.c:29:
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h:33:27: error:
isc-dhcp/list.h: No such file or directory
In file included from /usr/include/dhcp4client/isc-dhcp/minires/minires.h:30,
                 from /usr/include/dhcp4client/isc-dhcp/dhcpd.h:61,
                 from dhcp4_lease.c:29:
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h:39: error: expected
specifier-qualifier-list before 'ISC_LINK'
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h:55: error: function
definition declared 'typedef'
cc1: warnings being treated as errors


Comment 11 Jason Vas Dias 2006-05-19 19:32:13 EDT
This is very strange. I'm not seeing anything like this when I build libdhcp in 
the dist-fc6 buildroot /tmp/jvd on the FC-6 build servers . I've tried i386,
x86_64, ia64, ppc64, and all builds OK with '-Wall -Werror', and the
RPM %install and binary RPM production succeeds with no errors (I'm appending
the logs - perhaps they'll give a clue as to the differences between your
build environment and that of the FC-6 build servers).

Please let me have access to the build environment you are using and I'll be
able to find the problem - all I know is that the package builds without any
errors of any sort on i386, ia64, x86_64, and ppc64 FC-6 - I can repeat on
the s390s if you wish.


 
Comment 12 Jesse Keating 2006-05-19 19:37:19 EDT
Sounds like you're missing a BuildRequires.  The beehive buildroots operate on
an 'everything' install, so a missing buildreq won't be noticed.  Mock however
starts with a minimal clean buildroot and then relies upon BuildRequires (and
subsequent Requires of those) to populate the buildroot for building.
Comment 13 Jason Vas Dias 2006-05-19 19:42:57 EDT
Created attachment 129663 [details]
log of builds in the dist-fc6-build root of x86_64 hs20-bc1-5,  2. ia64 altix2, ppc64 js20-bc1-12

Here's a log showing the successful builds on the build servers - the
/tmp/jvd/ directory should still contain the builds, unless they've been 
removed .
Comment 14 Jesse Keating 2006-05-19 19:53:51 EDT
Ah ha.  I found that dhcp-devel needs to be in the build root.  So now the
question is, do one of the other -devel packages need to bring in dhcp-devel to
satisfy their include statements?  Just BuildReq'ing dhcp-devel may not be the
answer here.

Since the error comes from
/usr/include/dhcp4client/isc-dhcp/minires/res_update.h, one would wager that
dhcp4client-devel (and probably dhcp6client-devel) should have a Requires
dhcp-devel.

Shall I file a bug for that, or will you take care of that so this package will
build?
Comment 15 Jesse Keating 2006-05-19 19:58:13 EDT
rpmlint found a few things w/ the binary package:

E: libdhcp explicit-lib-dependency libdhcp4client
E: libdhcp explicit-lib-dependency libdhcp6client
E: libdhcp explicit-lib-dependency libnl
W: libdhcp incoherent-version-in-changelog 1.0-1 1.1-1.beta

You should let rpmbuild figure out the Requires, instead of manually inserting
them, unless rpm is getting it wrong.  Is rpm getting it wrong?

The -devel package doesn't have a dep on the main package.  Is this ok?  Does
the -devel package need the main package?
Comment 16 Jason Vas Dias 2006-05-19 21:49:12 EDT
Firstly, the isc-dhcp issue - now dhcp-3.0.8's libdhcp4client-devel provides
<isc_dhcp/*>, not <isc-dhcp/*>, which is provided by dhcp-devel. 

Yes, libdhcp requires the dhcp-devel headers in <isc-dhcp/*> to build -
dhcp-devel is now in the build requires.

There are only two .c files in libdhcp that require the ISC DHCP headers,
dhcp4_lease.c and dhcp4_nic.c, whose purpose it could be said is to avoid any
programs which use them ever having to require the DHCP headers. 

Similarly for dhcpv6, ONLY libdhcp's dhcp6_lease.c and dhcp6_lease.nic need
to include the dhcp6 headers. 

This is shown clearly in a log of the build without the extra RPM stuff:

$ CFLAGS='-g3 -gdwarf-2' make
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror   -c -o libdhcp.o libdhcp.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror -I/usr/include/dhcp4client  
-I/usr/include/dhcp4client/isc_dhcp     -c -o dhcp4_lease.o dhcp4_lease.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror -I/usr/include/dhcp4client  
-I/usr/include/dhcp4client/isc_dhcp     -c -o dhcp4_nic.o dhcp4_nic.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror -I/usr/include/dhcp6client  
-I/usr/include/dhcp6client/dhcpv6   -c -o dhcp6_lease.o dhcp6_lease.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror   -c -o dhcp6_nic.o dhcp6_nic.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror   -c -o nic.o nic.c
cc -g3 -gdwarf-2 -fPIC -I. -Wall -Werror   -c -o dhcp_nic.o dhcp_nic.c
cc -shared -o libdhcp.so.1 -Wl,-soname,libdhcp.so.1 libdhcp.o dhcp4_lease.o
dhcp4_nic.o dhcp6_lease.o dhcp6_nic.o nic.o dhcp_nic.o -lnl   -ldhcp6client  
-ldhcp4client
/bin/ln -sf libdhcp.so.1 libdhcp.so
ar crus libdhcp.a libdhcp.o dhcp4_lease.o dhcp4_nic.o dhcp6_lease.o dhcp6_nic.o
nic.o dhcp_nic.o

About the "explicit-lib-dependency" - yes, libdhcp does require the 
libdhcp4client and libdhcp6client libraries - it cannot be used at all
without them, so I thought they should be  in its list of 'Requires:' -
rpm does not complain at all about them when I make the RPM - but I've
taken them out as you suggested.

About the devel package dep - in the current .spec file, it says:

%package devel
...
Requires:     %{name}-%{version}-%{release}

Please ensure you're looking at the latest version of the .spec file , which
is now in elvis & devel CVS .
Comment 17 Jesse Keating 2006-05-20 02:14:20 EDT
Elvis and Devel CVS aren't accessable to outside people who may be doing the
review (;  This is why they should be made downloadable in the bug report.

The explicit requires thing, rpm will use ldd to figure out what it actually
should require.  This most often works better than hand defining a requires, and
thats what rpmlint was complaining about.

I'll test new packages in the morning.
Comment 18 Michael Schwendt 2006-05-20 03:19:02 EDT
> About the devel package dep - in the current .spec file, it says:
> 
> %package devel
> ...
> Requires:     %{name}-%{version}-%{release}

Which would be clearly wrong, because you need:

Requires:     %{name} = %{version}-%{release}

> About the "explicit-lib-dependency" - yes, libdhcp does require the 
> libdhcp4client and libdhcp6client libraries - it cannot be used at all
> without them, so I thought they should be  in its list of 'Requires:'

You ought to check whether rpmbuild creates dependencies on the
library SONAMEs automatically. Query "rpm -qpR yourbinary.rpm",
and if it does, don't add explicit dependencies on package names.
Comment 19 Jesse Keating 2006-05-22 16:53:14 EDT
This package still does not build. Same error:

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic  -fPIC -I. -Wall -Werror
-I/usr/include/dhcp4client   -I/usr/include/dhcp4client/isc_dhcp     -c -o
dhcp4_lease.o dhcp4_lease.c
In file included from /usr/include/dhcp4client/isc_dhcp/minires/minires.h:30,
                 from /usr/include/dhcp4client/isc_dhcp/dhcpd.h:61,
                 from dhcp4_lease.c:29:
/usr/include/dhcp4client/isc_dhcp/minires/res_update.h:33:27: error:
isc-dhcp/list.h: No such file or directory
Comment 20 Jesse Keating 2006-05-22 17:12:13 EDT
Disregard, I was building from old spec file.  Package builds, rpmlint is mostly
quiet except for version thing.  The version listed in changelog is no longer
valid (it wasn't before I touched the file either).  I went ahead and fixed the
Requires part of the -devel package.

libdhcp4client.so.1()(64bit)
libdhcp6client.so.1()(64bit)

get picked up as requires for the package, so autorequires are working.

Approving this package.  Please continue w/ PackageListProcess
Comment 21 Jason Vas Dias 2006-05-24 15:27:38 EDT
Now that the review has been approved, please can someone add 'libdhcp' to the
FC dist collection so I can build it. Latest version is libdhcp-1.2-1.beta
and is checked in to CVS.
Comment 22 Jesse Keating 2006-05-24 15:31:34 EDT
Techlead (Bill) has to approve, will this change anything for the installer
team?  Contact jeremy katz if so, what about comps?  Will this need to be listed
on its own, or will it be a Requires, or BuildRequires of something already in
comps?
Comment 23 Bill Nottingham 2006-05-24 15:50:58 EDT
So, as I understand it, the architecture is something like:

libdhcp static libs -> linked into dhcp clients
libdhcp static libs -> linked into shared libdhcpclient{4,6}
libdhcp -> built on top of static dhcp libs and shared libdhcpclient{4,6}

Seems like an overly complex architecture, but... *shrug*.
Comment 24 Bill Nottingham 2006-05-24 15:53:29 EDT
Oh, and there's dhcdbd which provides yet another slightly different code path.
Comment 25 David Cantrell 2006-05-24 16:18:30 EDT
My two cents...

libdhcp is a step in the right direction, but what we really need for anaconda
now is a drop-in replacement for pump so that we can add IPv6 support rather
quickly.

After reading the code and some of the patches to the ISC software, I would rate
this software of alpha quality.  It's clearly under heavy development still and
it's not really something that I'd like to move anaconda to right now.

The last part that concerns me (again, regarding anaconda) is that this library
execs the actual DHCP clients.  Is this necessary?  Pump is able to do IPv4 DHCP
in a handful of C files.  Pump is great because we can link it right in to the
loader and have no other external requirements.  This is very important for
bring up on new architectures and/or painful architectures (e.g., zSeries).

I'm not sure that libdhcp is the best move for anaconda.  If the drop-in
replacement for pump existed, that would be good.  A library that doesn't
require the clients, that would be good.
Comment 26 Jason Vas Dias 2006-05-24 16:28:42 EDT
In reply to Comment #25 From David Cantrell (dcantrel@redhat.com) on 2006-05-24
16:18 EST:

libdhcp   DOES NOT REQUIRE ANY DHCP CLIENTS OR EXEC ANY DHCP CLIENT PROCESS .

Please read the README file shipped in the distribution.

libdhcp RUNS the DHCP client code in same process as the invoking program,
from libraries linked into to the invoker.

That way, we do not have to develop and maintain DHCP and DHCPv6 client
protocol implementations from scratch - we can re-use the same code
used for the FC dhclient and dhcp6c programs, but run from within
one process.

The DHCP and DHCPv6 protocols are separate protocols, with separate ports,
message types and options.

A drop-in replacement for pump is now being tested and will be
submitted today.


In reply to Comment #23 and Comment #24 From Bill Nottingham
(notting@redhat.com) on 2006-05-24 15:53:

Hi Bill -
RE: architecture question :
  Not quite .
    libdhcp provides a callback mechanism whereby DHCP client code can be
    invoked from user code in the same process, and user code can receive
    and process the lease information; it also provides a 'Network Interface
    Configuration' (NIC) library to configure network parameters from either 
    DHCP or static configurations, similar to and as an intended replacement
    for the libpump library used by anaconda, using the libnl Netlink library.
 
    Then libdhcp4client provides the ISC DHCPv4 client in a library,
    and libdhcp6client provides the DHCPv6 client in a library, which
    use the libdhcp interface to enable invocation from user programs
    and processing of the lease information by user programs.

    The libdhcp{4,6}client libraries contain all the objects that the 
    DHCP client programs are built from. They are in fact just the client 
    programs with a renamed 'main()' function, modified to free all 
    memory and reinitialize globals on return of the modified main().
    libdhcp{4,6}client libraries link to no other shared libraries except 
    libc - they contain all the objects from the dh{p,pv6}-client build.  
   
    libdhcp, libdhcp4client and libdhcp6client all build both shared and
    static versions of themselves. libdhcp requires only to be linked
    to a shared or static libdhcp{4,6}client library, depending on the
    functions used by the invoking program. For example, if only NIC 
    functions are used by a libdhcp using program, the linker will not 
    complain about omitting -ldhcp4client or -ldhcp6client from the 
    link command, and the program will run fine without linking to
    the dhcpclient libraries.
    IE. linking to the libdhcp{4,6}client libraries or including the
    dhcp client headers is entirely optional depending on how libdhcp
    is used by the program that links to it.

    libdhcp contains objects which link to libdhcp4client, libdhcp6client, 
    and libnl - depending on which objects an invoking program uses, some
    or all of these libraries may need to be linked in to the invoking program.

RE: dhcdbd issue -
    dhcdbd is intended for a totally different purpose, to maintain a
    lease database and distribute lease information to D-BUS listeners.

    libdhcp is meant to be used by programs such as anaconda's loader2 that
    require a library to do both static network configuration and to do dhcp
    in the same process, without exec-ing other programs or depending on
    external daemons running. The only library that can do this currently
    is libpump, which is not DHCPv6 capable - hence the need for libdhcp.

    However, it's quite possible a future version of dhcdbd could do away
    with having to exec the clients and parse the options from environment
    variable setting strings by using libdhcp. 
Comment 27 David Cantrell 2006-05-24 17:04:02 EDT
(In reply to comment #26)
> In reply to Comment #25 From David Cantrell (dcantrel@redhat.com) on 2006-05-24
> 16:18 EST:
> 
> libdhcp   DOES NOT REQUIRE ANY DHCP CLIENTS OR EXEC ANY DHCP CLIENT PROCESS .

It doesn't?  Perhaps I'm just drowning in a sea of code then.  At the top of
your README file, I see this:

LIBDHCP enables programs to invoke the ISC dhclient (IPv4 DHCP) and DHCPv6
client (IPv6 DHCP) libraries, libdhcp4client and libdhcp6client, within one
process, and to use the lease objects returned to configure network interface
parameters.

I read the two patches to the dhcp package before looking at libdhcp (because I
couldn't get libdhcp source before seeing the dhcp patches).  While I don't hack
on ISC dhcp, I do know that dhclient was it for a while.  There was no library.
 And looking at the code now, I see there's still no library.  So this is why I
assumed libdhcp4client and libdhcp6client just exec the appropriate client and
send info back to libdhcp.

Looking at /usr/include/dhcp4client/isc-dhcp/dhcp4client.h and then the one for
IPv6, I only see one function prototype and it looks like an entry point for
execing the client daemon to me.

If I am going to use this, I'd like the API explained better than what I see in
the README.  Is libnl required?  I don't like libnl or libnetlink because both
are pretty piss poor 80% solutions.

> Please read the README file shipped in the distribution.

I did.  Many times.  Please don't make long lines.
 
> libdhcp RUNS the DHCP client code in same process as the invoking program,
> from libraries linked into to the invoker.
> 
> That way, we do not have to develop and maintain DHCP and DHCPv6 client
> protocol implementations from scratch - we can re-use the same code
> used for the FC dhclient and dhcp6c programs, but run from within
> one process.

That makes sense to me and that's certainly the right way to go.  I'm just not
seeing that when I look at ALL of these packages and source files.  Too many
things with 'dhc' in the name.
 
> The DHCP and DHCPv6 protocols are separate protocols, with separate ports,
> message types and options.

Right.

> A drop-in replacement for pump is now being tested and will be
> submitted today.

Sounds good.
Comment 28 Jason Vas Dias 2006-05-24 17:28:15 EDT
In reply to Comment #27 From David Cantrell (dcantrel@redhat.com)
on 2006-05-24 17:04 EST:

Yes, documentation has taken back seat to getting everything working and 
tested, which it now is - I apologize for the confusion.

I've begun work on a complete Doxygen documentation set for libdhcp which
will explain things much better than my 'README' file first attempt - 
I'll have finished it and will submit it tomorrow, after I've completed 
testing & refining the pump interface today.

The pump replacement interface includes a 'pump.h' and will define every 
pump symbol currently used by anaconda,  only every use of 'struct in_addr'
in the structures will be replaced by an 'ip_addr_t' (a struct sockaddr
compatible structure big enough for an IPv6 address, defined in the 
latest libdhcp source 'ip_addr.h'). The pump functions (eg. setupInterface)
will attempt to behave exactly the way pump does in response to the pump
structure settings. There will have to be extra members at the end of the
pumpNetIntf structure to control libdhcp DHCPv6 / DHCPv4 preferences.

RE:
> Looking at /usr/include/dhcp4client/isc-dhcp/dhcp4client.h and then the one
> for IPv6, I only see one function prototype and it looks like an entry point
> for execing the client daemon to me.

Each libdhcp{4,6}client library provides only one entry point, which is the
client main() function renamed to dhcpv{4,6}_client . The main functions 
have been modifed not to go into daemon mode, fork any processes etc., or
to create any files at all, if running under libdhcp, and also to clean up
after themselves - before returning, they free all memory used by the
client and reintialize all the global variables used by the client.
I've tested running  both the clients 100 times in succession in the same
process under valgrind and valgrind reports no leaked files, memory or 
memory access errors from client code.

Please re-consider libdhcp for use in anaconda after I've submitted the pump
API and the documentation - I'll append a comment to this bug when this is
done.
Comment 29 Jason Vas Dias 2006-05-26 14:13:53 EDT
Created attachment 130041 [details]
patch against anaconda 11.1.0.21 to make loader2 use libdhcp

There is now a slot-in pump replacement header file and a pump-compatible API 
in libdhcp-1.2 . 

Please check out the 'test_pump_static.c' and 'test_pump_dhcp.c' in the
/usr/share/doc/libdhcp-1.2/examples directory from the libdhcp installation
for examples of using the pump compatibility API - just type 'make' in that
directory to build them. Running them under valgrind reports 0 errors or
memory leaks.

The first stab at the Doxygen documentation is now shipped in 
/usr/share/doc/libdhcp-*/html - this is still work in progress,
and I hope to submit updated Doxygen documentation later today.

Here is a patch against the latest anaconda-11.1.0.21 source to allow loader2
to compile and link with libdhcp .  The one missing piece in the current FC-6
distro is a static /usr/lib/libnl.a - I've mailed Chris Aillon a patch to 
make libnl build a static version of itself in libnl-devel, and I hope to get
this built later today.

If using libnl is a major stumbling block, removing its use altogether from
libdhcp would not be difficult - libdhcp only uses the lowest level libnl API
for raw message building / sending - I removed any use of the libnl cache from
libdhcp to avoid the memory leaks and valgrind errors that resulted. 
But having libnl support in libdhcp I think is a good idea since it allows 
users to do much more with libdhcp, eg. set queue disciplines / firewall rules
that are not catered for by libdhcp alone.
Comment 31 David Cantrell 2006-05-30 13:46:04 EDT
(In reply to comment #28)
> RE:
> > Looking at /usr/include/dhcp4client/isc-dhcp/dhcp4client.h and then the one
> > for IPv6, I only see one function prototype and it looks like an entry point
> > for execing the client daemon to me.
> 
> Each libdhcp{4,6}client library provides only one entry point, which is the
> client main() function renamed to dhcpv{4,6}_client . The main functions 
> have been modifed not to go into daemon mode, fork any processes etc., or
> to create any files at all, if running under libdhcp, and also to clean up
> after themselves - before returning, they free all memory used by the
> client and reintialize all the global variables used by the client.
> I've tested running  both the clients 100 times in succession in the same
> process under valgrind and valgrind reports no leaked files, memory or 
> memory access errors from client code.

OK, then this is probably why I thought it was execing the client.  It's not
doing that, but rather it is the client code (minus some daemon housekeeping).

I'm all for code reuse, but the libdhcp{4,6}client + libdhcp solution is simply
too big for loader2.  Loader2 is static and needs to fit in tight spaces.

I also have questions about these new client libraries for DHCP.  It's just
duplicating the client code in the form of the library, right?  Having two
copies of large programs, one as a client and one as a library, opens the door
to more maintenance issues...at least down the road.  Are these libraries
something you want to send upstream to ISC and then patch the clients to use
them?  This really has no bearing on its use in anaconda, but it's a concern I have.

A drop-in replacement for pump now helps, but it pulls in libdhcp{4,6}client and
libdhcp, which doesn't help loader2.

Pump doesn't need to be a client, we just need the library.  I think this is the
way to go for loader2 in anaconda.  We need a minimal IPv4 and IPv6 library that
can link in to loader2.  It only needs to be there for bringing up the network
interface so we can grab stage2 and make it go.  It's hard to define minimal,
but there are certainly things in dhclient (and the one for IPv6) that just
don't matter for the installer.
Comment 32 Jason Vas Dias 2006-05-30 14:23:08 EDT
Please define "too big" - the libraries used by libdhcp are quite small:
-rw-r--r--  1 root root  761964 May 30 12:11 /usr/lib/libdhcp4client.a
-rw-r--r--  1 root root  253272 May 26 13:19 /usr/lib/libdhcp6client.a
-rw-r--r--  1 root root  118898 May 30 12:20 /usr/lib/libdhcp.a
-rw-r--r--  1 root root  345150 May 26 20:13 /usr/lib/libnl.a
 = a total size of      1479284 bytes.

This is quite small compared to the libc.a already linked in to loader2:
 
-rw-r--r--  1 root root 2667018 May 24 04:39 /usr/lib/libc.a

How small does loader2 have to be ? Have you tried using libdhcp and found
it to be too big ? What is the actual size limitation of loader2 ?

The libdhcp* footprint would be MUCH smaller if libdhcp could simply 
execute the dhcp client binaries in separate processes, and use IPC
to obtain the DHCP parameters, but I was told that this was not an 
option and that a library implementation was required.

libdhcp could also be MUCH smaller if loader2 could use shared libs - 
why can't it ?

RE: 
> there are certainly things in dhclient (and the one for IPv6) that just
> don't matter for the installer.

Which things ?  
Everything in dhclient and dhcpv6 is required, depending on configuration 
settings.

RE: 
> Pump doesn't need to be a client, we just need the library
How would you intend to obtain DHCP parameters from a DHCP server without
implementing the DHCP client protocol ?

RE: 
> We need a minimal IPv4 and IPv6 library that can link in to loader2.
That is what libdhcp and libdhcp{4,6}client provide.

Either we use the ISC dhclient and DHCPv6 code for anaconda, giving users a
full featured and configurable DHCP client that they already know how to 
configure, and allowing us to re-use the code from the FC clients, or we 
write our own DHCPv4 and DHCPv6 clients for anaconda from scratch.

I can undertake to write DHCP and DHCPv4 clients from scratch if desired, 
but it seems to me to be the wrong way to go when we already have tried and
tested DHCP client code that users are familiar with and know how to configure.

Please at least give libdhcp a try - if we come across any obstacles, I can
resolve them.
Comment 33 David Cantrell 2006-06-05 16:52:28 EDT
(In reply to comment #32)
> How small does loader2 have to be ? Have you tried using libdhcp and found
> it to be too big ? What is the actual size limitation of loader2 ?

It's not really a hard limit, but rather a mindset.  How much memory do we want
to require during installation is a better way to think about it.

> libdhcp could also be MUCH smaller if loader2 could use shared libs - 
> why can't it ?

Because it's a PITA trying to work on a initrd.img when you have 17 thousand
things to copy in instead of just one binary.  Ooops, forgot those 30 symlinks.
 Ooops, forgot all of those things.  And so on.

Originally the requirement for a static loader was to cram everything on to a
1.44MB boot image for diskettes.  We don't make boot floppies anymore, but
having a static loader makes it WAY more easy to do development.  The installer
environment is special.

> > there are certainly things in dhclient (and the one for IPv6) that just
> > don't matter for the installer.
> 
> Which things ?  
> Everything in dhclient and dhcpv6 is required, depending on configuration 
> settings.

I'm just thinking about things supported as command line switches.  A
user-defined timeout, for example.

> > We need a minimal IPv4 and IPv6 library that can link in to loader2.
> That is what libdhcp and libdhcp{4,6}client provide.
> 
> Either we use the ISC dhclient and DHCPv6 code for anaconda, giving users a
> full featured and configurable DHCP client that they already know how to 
> configure, and allowing us to re-use the code from the FC clients, or we 
> write our own DHCPv4 and DHCPv6 clients for anaconda from scratch.
> 
> I can undertake to write DHCP and DHCPv4 clients from scratch if desired, 
> but it seems to me to be the wrong way to go when we already have tried and
> tested DHCP client code that users are familiar with and know how to configure.
> 
> Please at least give libdhcp a try - if we come across any obstacles, I can
> resolve them.

I have moved anaconda's loader to using libdhcp and friends.  I do have some
questions, concerns, *BUT* the end result is that I think this library is the
right direction to move for Fedora.  libdhcp has my vote for inclusion in FC.

Now for the questions/concerns for down-the-road things:

1) The pump compatibility layer is great right now, but down the road we want to
remove that entirely.  I know we asked for that, but just know that it's a
transition tool and we shouldn't be making other users aware of that API as
something they can rely on forever.

2) Are there plans to get the ISC client library patches integrated upstream?

3) Are there plans to merge the ISC client libraries in to one?
Comment 34 Jason Vas Dias 2006-06-05 17:49:21 EDT
Great news! I'm glad to hear loader2 will now be IPv6 capable with libdhcp usage.

I've also just heard that libdhcp has been added to the dist-fc6 brew package
list, and it is now building in brew - the packages should be in tomorrow's
rawhide.

RE: 
> pump compatibility layer ... we shouldn't be making other users aware of that
> API as something they can rely on forever

Actually, adding the pump.[ch] Doxygen documentation was something I hadn't
got around to yet ... I guess I'll continue holding off on that.
The pump API is entirely superfluous as is, and is just a wrapper around the
underlying libdhcp API, which is fully Doxygen documented.

> Are there plans to get the ISC client library patches integrated upstream?
> Are there plans to merge the ISC client libraries in to one ?
As for 'fully integrated', probably not, but ISC have already agreed to ship
dhcdbd in the contrib/ directory of their source tarball, so would probably
be amenable to shipping the libdhcp4client patch in contrib/ also. 
DHCPv6 is not an ISC product, but is a sourceforge project - I can attach
the libdhcp6client patch in sourceforge, so that should not be a problem -
I'll get going on this now.

Bear in mind that libdhcp (and dhcdbd) are actually a stopgap until ISC release
their super-duper integrated DHCP and DHCPv6 natively D-BUS based server and
client, which is rumored to be in dhcp-4.0.x, to be released within the next
year or so (I have not heard any hard dates yet) .

Thanks!
Comment 35 Jeremy Katz 2006-06-05 17:57:53 EDT
Two things:
1) Shared libraries actually don't really do anything to impact overall size and
they actually tend to end up with more space used overall as you get more than
just the referenced symbols

2) Are we actively involved in the work for the new super-duper dhcp stuff?  If
not, why? ;-)  I really don't want to be betting the farm on something that then
ends up being completely unsuitable for our needs.  And one part of that is
ensuring a sane library API.
Comment 36 Jason Vas Dias 2006-06-05 18:25:57 EDT
RE: > Are we actively involved in the work for the new super-duper dhcp stuff?

Yes, I'm in correspondence with the primary developer on the project, Ted Lemon,
and am registered to receive early releases, and while I haven't heard anything
from him in a while, he did say he'd keep us in the loop. The last I heard was
that he had a working client that required a kernel patch to allow reception
of ethernet packets on an interface without an IP address, without LPF - this
would allow DHCP over IPSEC, for instance - and was looking for ways to work
around the kernel patch requirement. I will follow up with him and try to get
access to the alpha test version.

I will ensure that whatever eventually gets shipped by ISC, the version we ship
has a 'sane library API' - though its native use of D-BUS for invocation and
communication of DHCP options should make use of a library API superfluous, so
ammend that to a 'sane API for invocation, control, and DHCP option information
retrieval by other programs'.
Comment 37 Jeremy Katz 2006-06-05 18:31:26 EDT
dbus is way too heavy-weight for some of the really early boot places where you
need to get a network.  Outside of the anaconda use case, there are similar
needs for nfsroot and iscsiroot.
Comment 38 David Cantrell 2006-06-06 09:42:10 EDT
On a slightly-related note, I see today is the day 6bone is shutting down.  No
more 3FFE addresses.
Comment 39 Jason Vas Dias 2006-06-08 14:15:05 EDT
RE: > dbus is way too heavy-weight for some of the really early boot places 
    > where you need to get a network.
Perhaps so - I will ensure that the next ISC DHCP version that we ship,
which will incorporate DHCPv6 support and D-BUS support natively, provides 
"a sane, lightweight library interface" for anaconda.

Since libdhcp is now in Fedora Core, and is apparently being used by anaconda,
this bug can now be closed as CURRENTRELEASE.