From: Shawn Routhier Date: Fri, 8 Nov 2013 20:00:02 +0000 (-0800) Subject: [master] Add option to suppress the use of fsync when writing lease files X-Git-Tag: v4_3_0a1~13 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cde11a4cdb79f5447f4459d156d759620949539f;p=thirdparty%2Fdhcp.git [master] Add option to suppress the use of fsync when writing lease files The option is dont-use-async and it defaults to disabled. --- diff --git a/RELNOTES b/RELNOTES index a99ec5fd1..48097249a 100644 --- a/RELNOTES +++ b/RELNOTES @@ -89,6 +89,13 @@ work on other platforms. Please report any problems and suggested fixes to key for security reasons. [ISC-Bugs 30461] +- Add a configuration option to the server to suppress using fsync(). + Enabling this option will mean that fsync() is never called. This + may provide better performance but there is also a risk that a lease + will not be properly written to the disk after it has been issued + to a client and before the server stops. Using this option is + not recommended. + Changes since 4.2.5 - Address static analysis warnings. diff --git a/includes/dhcpd.h b/includes/dhcpd.h index 9e18818b8..6a538555f 100644 --- a/includes/dhcpd.h +++ b/includes/dhcpd.h @@ -732,6 +732,7 @@ struct lease_state { #endif #endif #define SV_CACHE_THRESHOLD 78 +#define SV_DONT_USE_FSYNC 79 #if !defined (DEFAULT_PING_TIMEOUT) # define DEFAULT_PING_TIMEOUT 1 @@ -1916,6 +1917,7 @@ extern struct timeval cur_tv; #define cur_time cur_tv.tv_sec extern int ddns_update_style; +extern int dont_use_fsync; extern const char *path_dhcpd_conf; extern const char *path_dhcpd_db; diff --git a/server/db.c b/server/db.c index 94abfbde7..0b25316b5 100644 --- a/server/db.c +++ b/server/db.c @@ -1014,12 +1014,13 @@ int commit_leases () We need to do this even if we're rewriting the file below, just in case the rewrite fails. */ if (fflush (db_file) == EOF) { - log_info ("commit_leases: unable to commit, fflush(): %m"); - return 0; + log_info("commit_leases: unable to commit, fflush(): %m"); + return (0); } - if (fsync (fileno (db_file)) < 0) { + if ((dont_use_fsync == 0) && + (fsync(fileno (db_file)) < 0)) { log_info ("commit_leases: unable to commit, fsync(): %m"); - return 0; + return (0); } /* send out all deferred ACKs now */ @@ -1031,9 +1032,9 @@ int commit_leases () if (count && cur_time - write_time > LEASE_REWRITE_PERIOD) { count = 0; write_time = cur_time; - new_lease_file (); + new_lease_file(); } - return 1; + return (1); } /* diff --git a/server/dhcpd.c b/server/dhcpd.c index 29d4f527d..3588349fe 100644 --- a/server/dhcpd.c +++ b/server/dhcpd.c @@ -78,6 +78,7 @@ option server.ddns-rev-domainname = \"in-addr.arpa.\";"; #endif /* NSUPDATE */ int ddns_update_style; +int dont_use_fsync = 0; /* 0 = default, use fsync, 1 = don't use fsync */ const char *path_dhcpd_conf = _PATH_DHCPD_CONF; const char *path_dhcpd_db = _PATH_DHCPD_DB; @@ -800,7 +801,7 @@ main(int argc, char **argv) { void postconf_initialization (int quiet) { - struct option_state *options = (struct option_state *)0; + struct option_state *options = NULL; struct data_string db; struct option_cache *oc; char *s; @@ -808,39 +809,35 @@ void postconf_initialization (int quiet) int tmp; /* Now try to get the lease file name. */ - option_state_allocate (&options, MDL); + option_state_allocate(&options, MDL); execute_statements_in_scope(NULL, NULL, NULL, NULL, NULL, options, &global_scope, root_group, NULL, NULL); - memset (&db, 0, sizeof db); - oc = lookup_option (&server_universe, options, SV_LEASE_FILE_NAME); + memset(&db, 0, sizeof db); + oc = lookup_option(&server_universe, options, SV_LEASE_FILE_NAME); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { - s = dmalloc (db.len + 1, MDL); + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + s = dmalloc(db.len + 1, MDL); if (!s) - log_fatal ("no memory for lease db filename."); - memcpy (s, db.data, db.len); - s [db.len] = 0; - data_string_forget (&db, MDL); + log_fatal("no memory for lease db filename."); + memcpy(s, db.data, db.len); + s[db.len] = 0; + data_string_forget(&db, MDL); path_dhcpd_db = s; } - oc = lookup_option (&server_universe, options, SV_PID_FILE_NAME); + oc = lookup_option(&server_universe, options, SV_PID_FILE_NAME); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { - s = dmalloc (db.len + 1, MDL); + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + s = dmalloc(db.len + 1, MDL); if (!s) - log_fatal ("no memory for pid filename."); - memcpy (s, db.data, db.len); - s [db.len] = 0; - data_string_forget (&db, MDL); + log_fatal("no memory for pid filename."); + memcpy(s, db.data, db.len); + s[db.len] = 0; + data_string_forget(&db, MDL); path_dhcpd_pid = s; } @@ -853,137 +850,116 @@ void postconf_initialization (int quiet) oc = lookup_option(&server_universe, options, SV_DHCPV6_LEASE_FILE_NAME); if (oc && - evaluate_option_cache(&db, NULL, NULL, NULL, - options, NULL, &global_scope, - oc, MDL)) { - s = dmalloc (db.len + 1, MDL); + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + s = dmalloc(db.len + 1, MDL); if (!s) - log_fatal ("no memory for lease db filename."); - memcpy (s, db.data, db.len); - s [db.len] = 0; - data_string_forget (&db, MDL); + log_fatal("no memory for lease db filename."); + memcpy(s, db.data, db.len); + s[db.len] = 0; + data_string_forget(&db, MDL); path_dhcpd_db = s; } oc = lookup_option(&server_universe, options, SV_DHCPV6_PID_FILE_NAME); if (oc && - evaluate_option_cache(&db, NULL, NULL, NULL, - options, NULL, &global_scope, - oc, MDL)) { - s = dmalloc (db.len + 1, MDL); + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + s = dmalloc(db.len + 1, MDL); if (!s) - log_fatal ("no memory for pid filename."); - memcpy (s, db.data, db.len); - s [db.len] = 0; - data_string_forget (&db, MDL); + log_fatal("no memory for pid filename."); + memcpy(s, db.data, db.len); + s[db.len] = 0; + data_string_forget(&db, MDL); path_dhcpd_pid = s; } } #endif /* DHCPv6 */ omapi_port = -1; - oc = lookup_option (&server_universe, options, SV_OMAPI_PORT); + oc = lookup_option(&server_universe, options, SV_OMAPI_PORT); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 2) { - omapi_port = getUShort (db.data); + omapi_port = getUShort(db.data); } else - log_fatal ("invalid omapi port data length"); - data_string_forget (&db, MDL); + log_fatal("invalid omapi port data length"); + data_string_forget(&db, MDL); } - oc = lookup_option (&server_universe, options, SV_OMAPI_KEY); + oc = lookup_option(&server_universe, options, SV_OMAPI_KEY); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, - (struct option_state *)0, - &global_scope, oc, MDL)) { - s = dmalloc (db.len + 1, MDL); + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + s = dmalloc(db.len + 1, MDL); if (!s) - log_fatal ("no memory for OMAPI key filename."); - memcpy (s, db.data, db.len); - s [db.len] = 0; - data_string_forget (&db, MDL); - result = omapi_auth_key_lookup_name (&omapi_key, s); - dfree (s, MDL); + log_fatal("no memory for OMAPI key filename."); + memcpy(s, db.data, db.len); + s[db.len] = 0; + data_string_forget(&db, MDL); + result = omapi_auth_key_lookup_name(&omapi_key, s); + dfree(s, MDL); if (result != ISC_R_SUCCESS) - log_fatal ("OMAPI key %s: %s", - s, isc_result_totext (result)); + log_fatal("OMAPI key %s: %s", + s, isc_result_totext (result)); } - oc = lookup_option (&server_universe, options, SV_LOCAL_PORT); + oc = lookup_option(&server_universe, options, SV_LOCAL_PORT); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, - (struct option_state *)0, - &global_scope, oc, MDL)) { + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 2) { - local_port = htons (getUShort (db.data)); + local_port = htons(getUShort (db.data)); } else - log_fatal ("invalid local port data length"); - data_string_forget (&db, MDL); + log_fatal("invalid local port data length"); + data_string_forget(&db, MDL); } - oc = lookup_option (&server_universe, options, SV_REMOTE_PORT); + oc = lookup_option(&server_universe, options, SV_REMOTE_PORT); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 2) { - remote_port = htons (getUShort (db.data)); + remote_port = htons(getUShort (db.data)); } else - log_fatal ("invalid remote port data length"); - data_string_forget (&db, MDL); + log_fatal("invalid remote port data length"); + data_string_forget(&db, MDL); } - oc = lookup_option (&server_universe, options, - SV_LIMITED_BROADCAST_ADDRESS); + oc = lookup_option(&server_universe, options, + SV_LIMITED_BROADCAST_ADDRESS); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 4) { - memcpy (&limited_broadcast, db.data, 4); + memcpy(&limited_broadcast, db.data, 4); } else - log_fatal ("invalid broadcast address data length"); - data_string_forget (&db, MDL); + log_fatal("invalid broadcast address data length"); + data_string_forget(&db, MDL); } - oc = lookup_option (&server_universe, options, - SV_LOCAL_ADDRESS); + oc = lookup_option(&server_universe, options, SV_LOCAL_ADDRESS); if (oc && - evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, (struct client_state *)0, - options, (struct option_state *)0, - &global_scope, oc, MDL)) { + evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 4) { - memcpy (&local_address, db.data, 4); + memcpy(&local_address, db.data, 4); } else - log_fatal ("invalid local address data length"); - data_string_forget (&db, MDL); + log_fatal("invalid local address data length"); + data_string_forget(&db, MDL); } - oc = lookup_option (&server_universe, options, SV_DDNS_UPDATE_STYLE); + oc = lookup_option(&server_universe, options, SV_DDNS_UPDATE_STYLE); if (oc) { - if (evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, - (struct client_state *)0, - options, - (struct option_state *)0, - &global_scope, oc, MDL)) { + if (evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 1) { - ddns_update_style = db.data [0]; + ddns_update_style = db.data[0]; } else - log_fatal ("invalid dns update type"); - data_string_forget (&db, MDL); + log_fatal("invalid dns update type"); + data_string_forget(&db, MDL); } } else { ddns_update_style = DDNS_UPDATE_STYLE_NONE; @@ -1000,17 +976,13 @@ void postconf_initialization (int quiet) } #endif - oc = lookup_option (&server_universe, options, SV_LOG_FACILITY); + oc = lookup_option(&server_universe, options, SV_LOG_FACILITY); if (oc) { - if (evaluate_option_cache (&db, (struct packet *)0, - (struct lease *)0, - (struct client_state *)0, - options, - (struct option_state *)0, - &global_scope, oc, MDL)) { + if (evaluate_option_cache(&db, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { if (db.len == 1) { closelog (); - openlog ("dhcpd", LOG_NDELAY, db.data[0]); + openlog("dhcpd", LOG_NDELAY, db.data[0]); /* Log the startup banner into the new log file. */ if (!quiet) { @@ -1019,14 +991,14 @@ void postconf_initialization (int quiet) log_perror = 0; log_info("%s %s", message, PACKAGE_VERSION); - log_info (copyright); - log_info (arr); - log_info (url); + log_info(copyright); + log_info(arr); + log_info(url); log_perror = tmp; } } else - log_fatal ("invalid log facility"); - data_string_forget (&db, MDL); + log_fatal("invalid log facility"); + data_string_forget(&db, MDL); } } @@ -1058,8 +1030,16 @@ void postconf_initialization (int quiet) data_string_forget(&db, MDL); } + oc = lookup_option(&server_universe, options, SV_DONT_USE_FSYNC); + if ((oc != NULL) && + evaluate_boolean_option_cache(NULL, NULL, NULL, NULL, options, NULL, + &global_scope, oc, MDL)) { + dont_use_fsync = 1; + log_error("Not using fsync() to flush lease writes"); + } + /* Don't need the options anymore. */ - option_state_dereference (&options, MDL); + option_state_dereference(&options, MDL); } void postdb_startup (void) diff --git a/server/dhcpd.conf.5 b/server/dhcpd.conf.5 index f842f3376..8a132b8d3 100644 --- a/server/dhcpd.conf.5 +++ b/server/dhcpd.conf.5 @@ -2110,6 +2110,24 @@ will still honor the setting of the \fBclient-updates\fR flag. .RE .PP The +.I dont-use-async +statement +.RS 0.25i +.PP +.B dont-use-async \fIflag\fB;\fR +.PP +The \fIdont-use-async\fR statement instructs the DHCP server if +it should call fsync() when writing leases to the lease file. By +default and if the flag is set to false the server \fBwill\fR call +fsync(). Suppressing the call to fsync() may increase the performance +of the server but it also adds a risk that a lease will not be +properly written to the disk after it has been issued to a client +and before the server stops. This can lead to duplicate leases +being issued to different clients. Using this option is \fBnot +recommended\FR. +.RE +.PP +The .I dynamic-bootp-lease-cutoff statement .RS 0.25i diff --git a/server/stables.c b/server/stables.c index 36335c76c..37758cc15 100644 --- a/server/stables.c +++ b/server/stables.c @@ -267,6 +267,7 @@ static struct option server_options[] = { #endif /* LDAP_USE_SSL */ #endif /* LDAP_CONFIGURATION */ { "dhcp-cache-threshold", "B", &server_universe, 78, 1 }, + { "dont-use-fsync", "f", &server_universe, 79, 1 }, { NULL, NULL, NULL, 0, 0 } };