A simple typo in pump.c causes pump to only attempt a single renew.
If the renew fails, we intend to try again in 30 seconds, but the
use of += instead of = causes the value of renewAt to be too large
and almost always exceed leaseExpiration. Found this in code review
following the incident described below the patch.
--- patch file ---
diff -urN pump-0.8.2.orig/pump.c pump-0.8.2/pump.c
--- pump-0.8.2.orig/pump.c Tue Aug 8 06:25:43 2000
+++ pump-0.8.2/pump.c Fri Oct 20 19:55:53 2000
@@ -365,7 +365,12 @@
"failed to renew lease for device %s",
- if ((intf[closest].renewAt += pumpUptime() + 30) >
+ /* if the renewal failed, then set renewAt to
+ * try again in 30 seconds AND then if renewAt's
+ * value is after the lease expiration then
+ * try to get a fresh lease for the interface
+ if ((intf[closest].renewAt = pumpUptime() + 30) >
o = overrides;
while (*o->intf.device &&
--- further info ---
Long story short. Had a IP address conflicts on local segment.
Tracked down to rh6.2 box configured for dhcp with pump. Pump
had tried to renew the lease and failed on first try. Pump then
immediately exited (based on syslog). Source code analysis
reveals several problems in pump.c in lines 368-390.
First: this bug (in this report) causes pump to only attempt renew
once (in nearly all cases).
Second: that once a lease has expired, pump only tries once to get
a new lease and does so immediately after a renew fails. In nearly
all cases, the new lease request will fail for the same reason the
renewal fails (assuming even very short network problems). Reporting
this in a separate bug report. Am submitting a separate bug report
with untested patch for this.
Third: When a lease expires and pump gives up trying to get a new
lease, it doesn't actually call pumpDisableInterface or otherwise
take the IP address from the interface before calling exit(1). As
a result, the interface remains up and configured for an IP address
that it no longer ''owns.'' This causes IP address conflicts on
DHCP networks. I don't run RH and have only looked at the code
in passing so don't know whether or not you want to solve this
by calling pumpDisableInterface() or do something else.
Fourth: when we first calculate renewAt value at dhcp.c:1109,
we set it to 7/8 the duration of the lease. For short leases,
this doesn't give much time to renew if a problem occurs.
isc and ms both use 1/2 for this factor. This is something
of an opinion issue, so I won't submit a separate report on
Created attachment 4490 [details]
patch to correct pump only trying renew once
This bug is fixed in 7.0 already -- thanks. The 7/8 bit is specified by the dhcp
RFC. I think that's pushing it as well,but...
I misinterpreted the 7/8 bit, actually. That's the time to move to the
REBINDING state (which pump doesn't implement), not the time to begin
RENEWAL (which should be at 1/2 the existing lease), where you put it. I'm not
going to change this until I implement REBINDING properly though (which is your
second patch, plus a new internal state).
Are you sure this bug is corrected? I've reviewed the source code
included with both pump-0.8.3-2.src.rpm and pump-0.8.4.src.rpm and
in both cases, the following line is present in pump.c:
0.8.4 pump.c:377 if ((intf[closest].renewAt += pumpUptime() + 30) >
0.8.3-2 pump.c:370 if ((intf[closest].renewAt += pumpUptime() + 30) >
I'm proposing that the += above should actually be simply =
Is there a more up to date src rpm I should be reviewing instead?
I misread your patch -- your fix is correct.
Has this since been fixed?
Apologies for unresponsiveness of previous pump maintainer...
Patch wasn't applied, done.
thanks for finally closing this out and improving pump's behavior on dhcp networks