3.0.1 eyeballs

[Brian Masney]

Hi,
  I am the author of the LDAP patch for ISC DHCP. Would it be possible to
get this integrated into 3.0.1? The patch barely touches any of the core code
inside of the DHCP server. I use it at my employer and there is quite a few
other places using it in production with no problems.

The patch can be found at http://home.ntelos.net/~masneyb/ 

Brian


On Tue, Feb 03, 2004 at 08:02:38PM +0000, David W. Hankins wrote:
> On Mon, Jan 26, 2004 at 12:42:31PM +0100, Martin Blapp wrote:
> >  	/* Now introduce some randomness to the renewal time: */
> > +	if (ds.data)
> >  	client -> new -> renewal = (((client -> new -> renewal + 3) * 3 / 4) +
> >  				    (random () % /* XXX NUMS */
> >  				     ((client -> new -> renewal + 3) / 4)));
> ds.data...this really amazed me when i looked at it.
> 
> so i looked at the freebsd ticket, and it made a little more sense
> when i saw that it was an FPE.  but i still don't like the idea of
> keying on wether or not the renewal was supplied to us by a server,
> to decide wether or not to add random jitter.
> 
> i would rather just not enter the arithmatic if the renewal time is
> invalid for the modulus (or would produce undesireable jitter, such
> as if the renewal value becomes negative), regardless of where that
> invalidity came from.
> 
> so i revived a patch someone sent me some time ago that i had earmarked
> for 3.1 (did not fix a bug (at least not one that people realistically
> were running up against except in this guy's lab)), which deals with
> servers sending an expiry time of -1 and all the related tomfoolery.
> 
> i've attached what i'm comitting.
> 
> > Would be good if you can integrate into 3.0.1 final.
> 
> this makes 2 code changes, and i don't think i can qualify this one as
> being small enough to slip through.  it changes enough to be frightening.
> i think we're going to have an rc13 in a week, and hopefully that will
> have a short lifetime as an rc.
> 
> which means if any of you have other patches or bugs on dhcp-bugs queued,
> i'll be comitting them as well.
> 
> -- 
> David W. Hankins		"If you don't do it right the first time,
> Operations Engineer			you'll just have to do it again."
> Internet Systems Consortium, Inc.		-- Jack T. Hankins
> 
> 
> -- Attached file included as plaintext by Ecartis --
> 
> ? work.freebsd
> Index: client/dhclient.c
> ===================================================================
> RCS file: /proj/cvs/prod/DHCP/client/dhclient.c,v
> retrieving revision 1.129.2.16
> diff -u -r1.129.2.16 dhclient.c
> --- client/dhclient.c	26 Apr 2003 21:51:39 -0000	1.129.2.16
> +++ client/dhclient.c	3 Feb 2004 19:39:14 -0000
> @@ -793,11 +793,15 @@
>  
>  	/* If it wasn't specified by the server, calculate it. */
>  	if (!client -> new -> renewal)
> -		client -> new -> renewal =
> -			client -> new -> expiry / 2;
> +		client -> new -> renewal = client -> new -> expiry / 2 + 1;
> +
> +	if (client -> new -> renewal <= 0)
> +		client -> new -> renewal = TIME_MAX;
>  
>  	/* Now introduce some randomness to the renewal time: */
> -	client -> new -> renewal = (((client -> new -> renewal + 3) * 3 / 4) +
> +	if (client -> new -> renewal <= TIME_MAX / 3 - 3)
> +		client -> new -> renewal =
> +				(((client -> new -> renewal + 3) * 3 / 4) +
>  				    (random () % /* XXX NUMS */
>  				     ((client -> new -> renewal + 3) / 4)));
>  
> @@ -816,14 +820,25 @@
>  	} else
>  			client -> new -> rebind = 0;
>  
> -	if (!client -> new -> rebind)
> -		client -> new -> rebind =
> -			(client -> new -> expiry * 7) / 8; /* XXX NUMS */
> +	if (client -> new -> rebind <= 0) {
> +		if (client -> new -> expiry <= TIME_MAX / 7)
> +			client -> new -> rebind =
> +					client -> new -> expiry * 7 / 8;
> +		else
> +			client -> new -> rebind =
> +					client -> new -> expiry / 8 * 7;
> +	}
>  
>  	/* Make sure our randomness didn't run the renewal time past the
>  	   rebind time. */
> -	if (client -> new -> renewal > client -> new -> rebind)
> -		client -> new -> renewal = (client -> new -> rebind * 3) / 4;
> +	if (client -> new -> renewal > client -> new -> rebind) {
> +		if (client -> new -> rebind <= TIME_MAX / 3)
> +			client -> new -> renewal =
> +					client -> new -> rebind * 3 / 4;
> +		else
> +			client -> new -> renewal =
> +					client -> new -> rebind / 4 * 3;
> +	}
>  
>  	client -> new -> expiry += cur_time;
>  	/* Lease lengths can never be negative. */
> 
> 
> 
>