From f776c9c9a558f893b37eb4d7142e93dd22245d69 Mon Sep 17 00:00:00 2001 From: david Date: Fri, 19 Sep 2008 16:25:10 +0000 Subject: [PATCH] Use TIMEVAL_AFTER(...) instead of TIMEVAL_SUBTRACT(...) > 0 when deciding whether a probe response counts as a drop for scan delay purposes. This avoids an integer overflow in TIMEVAL_SUBTRACT that caused all responses to probes retransmitted from the retry stack to be counted as drops. This would cause the scan to grind to a near-halt, with the scan delay at 1000 ms, if even a few hundred probes were retransmitted from the bench. Increased max_successful_tryno for 192.168.0.190 to 1 (packet drop) Increased max_successful_tryno for 192.168.0.190 to 2 (packet drop) Increasing send delay for 192.168.0.190 from 0 to 5 due to 216 out of 718 dropped probes since last increase. Increased max_successful_tryno for 192.168.0.190 to 3 (packet drop) Increasing send delay for 192.168.0.190 from 5 to 10 due to 92 out of 305 dropped probes since last increase. Increasing send delay for 192.168.0.190 from 10 to 20 due to 11 out of 11 dropped probes since last increase. Increasing send delay for 192.168.0.190 from 20 to 40 due to 11 out of 11 dropped probes since last increase. Increasing send delay for 192.168.0.190 from 40 to 80 due to 11 out of 11 dropped probes since last increase. Increasing send delay for 192.168.0.190 from 80 to 160 due to 11 out of 11 dropped probes since last increase. Increasing send delay for 192.168.0.190 from 160 to 320 due to 11 out of 11 dropped probes since last increase. ... The problem was in this bit of code: if ((!rcvdtime && TIMEVAL_SUBTRACT(probe->sent, hss->sdn.last_boost) > 0) || (probe->tryno > 0 && TIMEVAL_SUBTRACT(probe->prevSent, hss->sdn.last_boost) > 0)) { the TIMEVAL_SUBTRACT(probe->prevSent, hss->sdn.last_boost) > 0) to be specific. When a probe is retransmitted, the time it was sent is recorded in the prevSent member of the retransmit probe. prevSent is properly set in retransmitProbe, but it is not set in sendNextRetryStackProbe, which sends probes that have been moved from the bench to the retry stack. The problem is that when probes are moved to the bench they are compressed to probespecs and lose most of their auxiliary information, like the send time. When they are retransmitted as real UltraProbe objects, their prevSent message is left initialized to { 0, 0 }. That led to the integer overflow, with TIMEVAL_SUBTRACT returning a nonsense (but positive) value. I fixed it by using TIMEVAL_AFTER(...), which works like TIMEVAL_SUBTRACT(...) > 0 except that it is immune to integer overflows. Every other timeval is after { 0, 0 }, so the condition is false for probes retransmitted from the bench, as it should be. However this is not the most correct solution. Better would be to somehow store each probe's send time with it on the bench so it could be restored when it is retransmitted. The way the bench and the retry stack work makes that cumbersome though, and this is the only place prevSent is used, so I think this solution is acceptable. --- scan_engine.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scan_engine.cc b/scan_engine.cc index 3b5266a2f..0e815c7ec 100644 --- a/scan_engine.cc +++ b/scan_engine.cc @@ -2066,8 +2066,8 @@ static void ultrascan_adjust_timing(UltraScanInfo *USI, HostScanStats *hss, /* First we decide whether this packet counts as a drop for send delay calculation purposes. This statement means if (a ping since last boost failed, or the previous packet was both sent after the last boost and dropped) */ - if ((!rcvdtime && TIMEVAL_SUBTRACT(probe->sent, hss->sdn.last_boost) > 0) || - (probe->tryno > 0 && TIMEVAL_SUBTRACT(probe->prevSent, hss->sdn.last_boost) > 0)) { + if ((!rcvdtime && TIMEVAL_AFTER(probe->sent, hss->sdn.last_boost)) || + (probe->tryno > 0 && TIMEVAL_AFTER(probe->prevSent, hss->sdn.last_boost))) { hss->sdn.droppedRespSinceDelayChanged++; // printf("SDELAY: increasing drops to %d (good: %d; tryno: %d, sent: %.4fs; prevSent: %.4fs, last_boost: %.4fs\n", hss->sdn.droppedRespSinceDelayChanged, hss->sdn.goodRespSinceDelayChanged, probe->tryno, o.TimeSinceStartMS(&probe->sent) / 1000.0, o.TimeSinceStartMS(&probe->prevSent) / 1000.0, o.TimeSinceStartMS(&hss->sdn.last_boost) / 1000.0); } else if (rcvdtime) {