*** From dhcp-server -- To unsubscribe, see the end of this message. ***
I recently switched over to ISC DHCPD from the bootpd provided with
HP-UX because of features I needed that the HP-UX daemon didn't have.
I ran into one memory leak and two other issues that casued problems
for me (but aren't necessarily bugs). I've patched them all, in case
anyone else is interested. (I'm not sure what the procedure for
submitting bug fixes or reporting bugs is ... but the first one below
seems to me to be a real bug.)
All of these diffs below apply to 1.0-PL2. After a cursory glance at
the latest 2.0 beta, it appears as if all of these items are relevant
there also, but I didn't look all that closely.
The diff's below will probably have to be manually applied by anyone
who's interested. Essentially, I diff's my directory and a directory
with a copy of the straight PL2 code and deleted the changes I made
that were specific to me and didn't represent bug fixes or even
enhancements that anyone else is likely to be interested in. I think
they should more-or-less work in version two also, if you find the
right place to apply them.
(1) Memory leak:
parse_options_buffer malloc()'s memory for every option it finds. This
memory is never free()'d. The result is a memory leak of some number of
bytes per request received (unless all your requests are BootP requests
with no options). The following patch at the end of do_packet() in
dispatch.c will free up the relevant memory. (The line numbers aren't
exactly right on the "-RBF" side, because I just diff'd my development
directory with a 1.0pl2 directory and I have a lot of other changes that
I'm not including in these messages.
*** dhcp-1.0pl2/dispatch.c Mon Apr 5 20:56:25 1999
--- dhcp-1.0pl2-RBF/dispatch.c Mon Apr 5 14:26:32 1999
***************
*** 462,467 ****
--- 464,470 ----
struct hardware *hfrom;
{
struct packet tp;
+ int code; /* -- RBF */
if (packet -> hlen > sizeof packet -> chaddr) {
note ("Discarding packet with invalid hlen.");
***************
*** 485,490 ****
--- 488,499 ----
dhcp (&tp);
else if (packet -> op == BOOTREQUEST)
bootp (&tp);
+
+ /* Free the memory that was allocated by parse_options and
+ never freed. -- RBF */
+ for (code=0; code<256; code++)
+ if (tp.options[code].data)
+ free(tp.options[code].data);
}
(After developing this patch, I became concerned that the code might, when
storing the uid (client identifier) in the lease structure, simply update a
pointer in the lease structure to point to the memory allocated for that
option by parse_option_buffer. This turns out to be the case, but, after
doing so (in ack_lease), the code proceeds to set the relevant pointer in
the packet structure to NULL, which prevents it from being free()'d by my
new code above.)
(2) Bazillions of static hosts
The nature of my network is such that I have about 80000 host declarations
in my dhcpd.conf. The host_decl structure is about 32 bytes or so.
80000*32=2.5MB. No big deal. But the group structure is about 1K, and the
code allocates a group structure for every static host, even if the static
host doesn't have anything different from it's parent. Since I'm defining
hosts by their hardware address, I wrote the patch below to check after a
static host is created, and if it's group structure matches it's parent's
group structure, it just updates some pointers and doesn't actually
allocate a new group structure. This saves me about 80MB of virtual
memory. (Although this patch works fine if you define all your static
hosts with client-identifiers instead of hardware addresses, the
client-identifier is stored as an option, so you won't get any memory
savings.) The one issue this causes is that the code originally
processed "use-host-decl-names" at startup by creating a hostname option.
This would eliminate any memory savings for those who enable
use-host-decl-names (which I do). To handle this, I moved
"use-host-decl-names" processing to occur when the response packet is being
built (inack_lease). (Same caveat as before. Line numbers on the "-RBF" side
aren't accurate, because I've snipped some other unrelated patches.)
diff -c dhcp-1.0pl2/bootp.c dhcp-1.0pl2-RBF/bootp.c
*** dhcp-1.0pl2/bootp.c Mon Apr 5 20:56:12 1999
--- dhcp-1.0pl2-RBF/bootp.c Tue Mar 30 13:19:21 1999
***************
*** 62,67 ****
--- 62,69 ----
struct subnet *subnet;
struct lease *lease;
struct iaddr ip_address;
+ struct tree_cache hostname_tree; /* -- RBF */
int i;
note ("BOOTREQUEST from %s via %s",
***************
*** 188,193 ****
--- 190,227 ----
for (i = 0; i < 256; i++) {
if (hp -> group -> options [i])
options [i] = hp -> group -> options [i];
+ }
+
+ /* If we don't have a hostname but we have set use_host_decl_names
+ is enabled, create a hostname. --RBF */
+ if (!options [DHO_HOST_NAME] &&
+ lease -> host &&
+ lease -> host -> group -> use_host_decl_names) {
+ options [DHO_HOST_NAME] = &hostname_tree;
+ options [DHO_HOST_NAME] -> value = lease -> host -> name;
+ options [DHO_HOST_NAME] -> buf_size =
+ options [DHO_HOST_NAME] -> len =
+ strlen (lease -> host -> name);
+ options [DHO_HOST_NAME] -> timeout = 0xFFFFFFFF;
+ options [DHO_HOST_NAME] -> tree = (struct tree *)0;
+ };
+
}
/* Pack the options into the buffer. Unlike DHCP, we can't
diff -c dhcp-1.0pl2/confpars.c dhcp-1.0pl2-RBF/confpars.c
*** dhcp-1.0pl2/confpars.c Mon Apr 5 20:56:14 1999
--- dhcp-1.0pl2-RBF/confpars.c Sun Mar 28 23:42:14 1999
***************
*** 489,494 ****
--- 512,518 ----
char *val;
int token;
struct host_decl *host;
+ struct group tempgroup; /* --RBF */
char *name = parse_host_name (cfile);
int declaration = 0;
***************
*** 501,507 ****
error ("can't allocate host decl struct %s.", name);
host -> name = name;
! host -> group = clone_group (group, "parse_host_declaration");
if (!parse_lbrace (cfile))
return;
--- 525,535 ----
error ("can't allocate host decl struct %s.", name);
host -> name = name;
! /* Put the stuff in tempgroup first, to see if it changes --RBF */
! host -> group = &tempgroup;
! memcpy(&tempgroup, group, sizeof (struct group)); /* memcpy so pad
! bytes match */
! /* host -> group = clone_group (group, "parse_host_declaration"); */
if (!parse_lbrace (cfile))
return;
***************
*** 522,527 ****
--- 550,556 ----
declaration);
} while (1);
+ /* Don't do this --RBF
if (!host -> group -> options [DHO_HOST_NAME] &&
host -> group -> use_host_decl_names) {
host -> group -> options [DHO_HOST_NAME] =
***************
*** 539,545 ****
--- 568,582 ----
host -> group -> options [DHO_HOST_NAME] -> tree =
(struct tree *)0;
}
+ */
+ /* See if the group changed. If not, use the parent's group structure.
+ otherwise, create our own --RBF*/
+ if (!memcmp(&tempgroup, group, sizeof (struct group)))
+ host->group = group;
+ else
+ host->group = clone_group(&tempgroup,"parse_host_declaration");
+
enter_host (host);
}
*** dhcp-1.0pl2/dhcp.c Mon Apr 5 20:56:17 1999
--- dhcp-1.0pl2-RBF/dhcp.c Sun Mar 28 23:41:26 1999
***************
*** 733,738 ****
--- 781,806 ----
group -> options [i]);
}
+ /* If we don't have a hostname but we have set use_host_decl_names
+ is enabled, create a hostname. --RBF */
+ if (!options [DHO_HOST_NAME] &&
+ lease -> host &&
+ lease -> host -> group -> use_host_decl_names) {
+ options [DHO_HOST_NAME] = &hostname_tree;
+ options [DHO_HOST_NAME] -> value = lease -> host -> name;
+ options [DHO_HOST_NAME] -> buf_size =
+ options [DHO_HOST_NAME] -> len =
+ strlen (lease -> host -> name);
+ options [DHO_HOST_NAME] -> timeout = 0xFFFFFFFF;
+ options [DHO_HOST_NAME] -> tree = (struct tree *)0;
+ };
+
/* If we didn't get a hostname from an option somewhere, see if
we can get one from the lease. */
if (!options [DHO_HOST_NAME] && lease -> hostname) {
(3) Subnet masks on BootP
The DHCP side will use the mask from the "subnet a.b.c.d netmask e.f.g.h"
declaration. The BootP code, though, requires "option subnet-mask". The
patch below makes BootP work the same as DHCP with respect to this.
diff -c dhcp-1.0pl2/bootp.c dhcp-1.0pl2-RBF/bootp.c
*** dhcp-1.0pl2/bootp.c Mon Apr 5 20:56:12 1999
--- dhcp-1.0pl2-RBF/bootp.c Tue Mar 30 13:19:21 1999
***************
*** 62,67 ****
--- 62,69 ----
struct subnet *subnet;
struct lease *lease;
struct iaddr ip_address;
+ struct tree_cache netmask_tree; /* -- RBF */
int i;
note ("BOOTREQUEST from %s via %s",
***************
*** 188,193 ****
--- 190,227 ----
if (hp -> group -> options [i])
options [i] = hp -> group -> options [i];
}
+ /* Use the subnet mask from the subnet declaration if no other
+ mask has been provided. */
+ if (!options [DHO_SUBNET_MASK]) {
+ options [DHO_SUBNET_MASK] = &netmask_tree;
+ netmask_tree.value = lease -> subnet -> netmask.iabuf;
+ netmask_tree.len = lease -> subnet -> netmask.len;
+ netmask_tree.buf_size = lease -> subnet -> netmask.len;
+ netmask_tree.timeout = 0xFFFFFFFF;
+ netmask_tree.tree = (struct tree *)0;
+ }
/* Pack the options into the buffer. Unlike DHCP, we can't
- Brett ([EMAIL PROTECTED])
------------------------------------------------------------------------------
... Coming soon to a | Brett Frankenberger
.sig near you ... a Humorous Quote ... | [EMAIL PROTECTED]
------------------------------------------------------------------------------
To unsubscribe from this list, please visit http://www.fugue.com/dhcp/lists
If you are without web access, or if you are having trouble with the web page,
please send mail to [EMAIL PROTECTED] Please try to use the web
page first - it will take a long time for your request to be processed by hand.
------------------------------------------------------------------------------
