[master] delayed-ack is now compiled in by default

Merges in rt42444.

RELNOTES

@@ -264,6 +264,15 @@ dhcp-users@lists.isc.org.
   raising the issue and submitting the patch.
   [ISC-Bugs #22675]
 
+- Support for delayed-ack is now compiled in by default. Prior to this
+  it had to be enabled at compile time via --enable-delayed-acks. The
+  default value for delayed-ack, however, has been changed from 28 to 0
+  (i.e. disabled).  This was done to minimize the impact on users not
+  currently using the feature.  Please note that the delayed-ack feature
+  is not currently compatible with support for DHPCv4o6 (compiled out by
+  default).  You must compile with one or the other but not both.
+  [ISC-Bugs #42446]
+
 			Changes since 4.3.6 (Bugs):
 
 - Corrected an issue where the server would return a client's previously

configure

@@ -1442,7 +1442,7 @@ Optional Features:
                           yes)
   --enable-tracing        enable support for server activity tracing (default
                           is yes)
-  --enable-delayed-ack    queues multiple DHCPACK replies (default is no)
+  --enable-delayed-ack    queues multiple DHCPACK replies (default is yes)
   --enable-dhcpv6         enable support for DHCPv6 (default is yes)
   --enable-dhcpv4o6       enable support for DHCPv4-over-DHCPv6 (default is
                           no)
@@ -5440,18 +5440,17 @@ $as_echo "#define TRACING 1" >>confdefs.h
 
 fi
 
-# Delayed-ack feature support (experimental).
+# Delayed-ack feature support.
 # Check whether --enable-delayed_ack was given.
 if test "${enable_delayed_ack+set}" = set; then :
   enableval=$enable_delayed_ack;
 fi
 
-if test "$enable_delayed_ack" = "yes"; then
+if test "$enable_delayed_ack" != "no"; then
+    enable_delayed_ack="yes"
 
 $as_echo "#define DELAYED_ACK 1" >>confdefs.h
 
-else
-    enable_delayed_ack="no"
 fi
 
 # DHCPv6 optional compile-time feature.
@@ -5487,6 +5486,9 @@ if test "$enable_dhcpv4o6" = "yes"; then
 
 $as_echo "#define DHCP4o6 1" >>confdefs.h
 
+else
+    # so we can report below
+    enable_dhcpv4o6="no"
 fi
 
 # PARANOIA is off by default (until we can test it with all features)
@@ -9015,6 +9017,7 @@ Features:
   binary-leases: $enable_binary_leases
   dhcpv6:        $enable_dhcpv6
   delayed-ack:   $enable_delayed_ack
+  dhcpv4o6:      $enable_dhcpv4o6
 
 Developer:
   ATF unittests : $atf_path

configure.ac

@@ -142,14 +142,13 @@ if test "$enable_tracing" != "no" ; then
 		  [Define to include server activity tracing support.])
 fi
 
-# Delayed-ack feature support (experimental).
+# Delayed-ack feature support.
 AC_ARG_ENABLE(delayed_ack,
-	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is no)]))
+	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is yes)]))
-if test "$enable_delayed_ack" = "yes"; then
+if test "$enable_delayed_ack" != "no"; then
+    enable_delayed_ack="yes"
 	AC_DEFINE([DELAYED_ACK], [1],
 		  [Define to queue multiple DHCPACK replies per fsync.])
-else
-    enable_delayed_ack="no"
 fi
 
 # DHCPv6 optional compile-time feature.
@@ -177,6 +176,9 @@ if test "$enable_dhcpv4o6" = "yes"; then
 	fi
 	AC_DEFINE([DHCP4o6], [1], 
 		  [Define to 1 to include DHCPv4 over DHCPv6 support.])
+else
+    # so we can report below
+    enable_dhcpv4o6="no"
 fi
 
 # PARANOIA is off by default (until we can test it with all features)
@@ -1036,6 +1038,7 @@ Features:
   binary-leases: $enable_binary_leases
   dhcpv6:        $enable_dhcpv6
   delayed-ack:   $enable_delayed_ack
+  dhcpv4o6:      $enable_dhcpv4o6
 
 Developer:
   ATF unittests : $atf_path

configure.ac+lt

@@ -143,14 +143,13 @@ if test "$enable_tracing" != "no" ; then
 		  [Define to include server activity tracing support.])
 fi
 
-# Delayed-ack feature support (experimental).
+# Delayed-ack feature support.
 AC_ARG_ENABLE(delayed_ack,
-	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is no)]))
+	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is yes)]))
-if test "$enable_delayed_ack" = "yes"; then
+if test "$enable_delayed_ack" != "no"; then
+    enable_delayed_ack="yes"
 	AC_DEFINE([DELAYED_ACK], [1],
 		  [Define to queue multiple DHCPACK replies per fsync.])
-else
-    enable_delayed_ack="no"
 fi
 
 # DHCPv6 optional compile-time feature.
@@ -178,6 +177,9 @@ if test "$enable_dhcpv4o6" = "yes"; then
 	fi
 	AC_DEFINE([DHCP4o6], [1], 
 		  [Define to 1 to include DHCPv4 over DHCPv6 support.])
+else
+    # so we can report below
+    enable_dhcpv4o6="no"
 fi
 
 # PARANOIA is off by default (until we can test it with all features)
@@ -1042,6 +1044,7 @@ Features:
   binary-leases: $enable_binary_leases
   dhcpv6:        $enable_dhcpv6
   delayed-ack:   $enable_delayed_ack
+  dhcpv4o6:      $enable_dhcpv4o6
 
 Developer:
   ATF unittests : $atf_path

configure.ac-base

@@ -148,14 +148,13 @@ if test "$enable_tracing" != "no" ; then
 		  [Define to include server activity tracing support.])
 fi
 
-# Delayed-ack feature support (experimental).
+# Delayed-ack feature support.
 AC_ARG_ENABLE(delayed_ack,
-	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is no)]))
+	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is yes)]))
-if test "$enable_delayed_ack" = "yes"; then
+if test "$enable_delayed_ack" != "no"; then
+    enable_delayed_ack="yes"
 	AC_DEFINE([DELAYED_ACK], [1],
 		  [Define to queue multiple DHCPACK replies per fsync.])
-else
-    enable_delayed_ack="no"
 fi
 
 # DHCPv6 optional compile-time feature.
@@ -183,6 +182,9 @@ if test "$enable_dhcpv4o6" = "yes"; then
 	fi
 	AC_DEFINE([DHCP4o6], [1], 
 		  [Define to 1 to include DHCPv4 over DHCPv6 support.])
+else
+    # so we can report below
+    enable_dhcpv4o6="no"
 fi
 
 # PARANOIA is off by default (until we can test it with all features)
@@ -1078,6 +1080,7 @@ Features:
   binary-leases: $enable_binary_leases
   dhcpv6:        $enable_dhcpv6
   delayed-ack:   $enable_delayed_ack
+  dhcpv4o6:      $enable_dhcpv4o6
 
 Developer:
   ATF unittests : $atf_path

configure.ac-lt

@@ -142,14 +142,13 @@ if test "$enable_tracing" != "no" ; then
 		  [Define to include server activity tracing support.])
 fi
 
-# Delayed-ack feature support (experimental).
+# Delayed-ack feature support.
 AC_ARG_ENABLE(delayed_ack,
-	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is no)]))
+	AS_HELP_STRING([--enable-delayed-ack],[queues multiple DHCPACK replies (default is yes)]))
-if test "$enable_delayed_ack" = "yes"; then
+if test "$enable_delayed_ack" != "no"; then
+    enable_delayed_ack="yes"
 	AC_DEFINE([DELAYED_ACK], [1],
 		  [Define to queue multiple DHCPACK replies per fsync.])
-else
-    enable_delayed_ack="no"
 fi
 
 # DHCPv6 optional compile-time feature.
@@ -177,6 +176,9 @@ if test "$enable_dhcpv4o6" = "yes"; then
 	fi
 	AC_DEFINE([DHCP4o6], [1], 
 		  [Define to 1 to include DHCPv4 over DHCPv6 support.])
+else
+    # so we can report below
+    enable_dhcpv4o6="no"
 fi
 
 # PARANOIA is off by default (until we can test it with all features)
@@ -1036,6 +1038,7 @@ Features:
   binary-leases: $enable_binary_leases
   dhcpv6:        $enable_dhcpv6
   delayed-ack:   $enable_delayed_ack
+  dhcpv4o6:      $enable_dhcpv4o6
 
 Developer:
   ATF unittests : $atf_path

includes/dhcpd.h

@@ -815,7 +815,7 @@ struct lease_state {
 #endif
 
 #if !defined (DEFAULT_DELAYED_ACK)
-# define DEFAULT_DELAYED_ACK 28  /* default SO_SNDBUF size / 576 bytes */
+# define DEFAULT_DELAYED_ACK 0  /* default SO_SNDBUF size / 576 bytes */
 #endif
 
 #if !defined (DEFAULT_ACK_DELAY_SECS)

server/dhcp.c

@@ -3574,7 +3574,7 @@ void ack_lease (packet, lease, offer, when, msg, ms_nulltp, hp)
 	} else {
   		lease->cltt = cur_time;
 #if defined(DELAYED_ACK) && !defined(DHCP4o6)
-		if (enqueue)
+		if (enqueue && max_outstanding_acks != 0)
 			delayed_ack_enqueue(lease);
 		else 
 #endif

server/dhcpd.conf.5

@@ -2267,26 +2267,28 @@ statements
 .B max-ack-delay \fImicroseconds\fR\fB;\fR
 .PP
 .I Count
-should be an integer value from zero to 2^16-1, and defaults to 28.  The
+should be an integer value from zero to 2^16-1 and defaults to 0, which means
-count represents how many DHCPv4 replies maximum will be queued pending
+that the feature is disabled.  Otherwise, 28 may be a sensible starting point
-transmission until after a database commit event.  If this number is
+for many configurations (SO_SNDBUF size / 576 bytes.)  The count represents how
-reached, a database commit event (commonly resulting in fsync() and
+many DHCPv4 replies maximum will be queued pending transmission until after a
-representing a performance penalty) will be made, and the reply packets
+database commit event.  If this number is reached, a database commit event
-will be transmitted in a batch afterwards.  This preserves the RFC2131
+(commonly resulting in fsync() and representing a performance penalty) will be
-direction that "stable storage" be updated prior to replying to clients.
+made, and the reply packets will be transmitted in a batch afterwards.  This
-Should the DHCPv4 sockets "go dry" (select() returns immediately with no
+preserves the RFC2131 direction that "stable storage" be updated prior to
-read sockets), the commit is made and any queued packets are transmitted.
+replying to clients.  Should the DHCPv4 sockets "go dry" (select() returns
+immediately with no read sockets), the commit is made and any queued packets
+are transmitted.
 .PP
 Similarly, \fImicroseconds\fR indicates how many microseconds are permitted
 to pass inbetween queuing a packet pending an fsync, and performing the
 fsync.  Valid values range from 0 to 2^32-1, and defaults to 250,000 (1/4 of
 a second).
 .PP
-The delayed-ack feature is not compiled in by default, but must be enabled
+The delayed-ack feature is compiled in by default, but can be disabled
-at compile time with \'./configure --enable-delayed-ack\'.  While we no
+at compile time with \'./configure --disable-delayed-ack\'.  Please note
-longer consider it experimental and we don't know of any issues with it,
+that the delayed-ack feature is not currently compatible with support for
-in order to minimize problems with existing configuration files we have
+DHPCv4o6 (compiled out by default).  You must compile with one or the
-left it disabled by default.
+other but not both.
 .RE
 .PP
 The