libalias: Possible packet_limit underrun

[donner]

Summary

On low traffic boxes, the packet_limit can become zero (if less than
three packets per seconds are processed), which results in a division
by zero panic.

The obvious solution is to check for this case explicitly. Thankfully
@se already applied this hotfix to the code. OTOH such a check adds to
the instruction count of each packet processing, which is bad for high
traffic boxes.

So the proposed solution is to add an arbitrary fixed value to the
calculation. There is no risk in delaying the housekeeping, because
the necessary expiry checks are always done before using an entry. The
only effect is, that some old entries stay somewhat longer in the data
structure.

[se]

sys/netinet/libalias/alias_db.c

67

1774

1780

1782

I'd use a different approach, since the suggested one does not work well for very low packet rates.

u_long seconds = (u_long)now - (u_long)LibAliasTime;

if (seconds != 0) {
	/* retry three times a second */
	packet_limit = packets / (3 * seconds) + 1;
	packets = 0;
	LibAliasTime = now;
}

For the (pathological, but possible) case of a constant rate of 1 packet per second, the packet_limit would be 10, leading to 10 seconds before the modulo calculation results in a value of 0.
That would lead to packet_limit = 10 / 3 + 10 = 13 seconds until the condition becomes true again.

BEWARE: LibAliasTime is currently of type int, not time_t, and will not be able to hold the value of "now" after 2038, and thus "now != LibAliasTime" will then be always true ...

[se]

The obvious solution is to check for this case explicitly. Thankfully
@se already applied this hotfix to the code. OTOH such a check adds to
the instruction count of each packet processing, which is bad for high
traffic boxes.

The hot fix added a comparison of the modulo operand (already in a register) with a small constant and I'd be surprised if it added a measurable delay on any modern processor.
The implicit subtraction that is performed for this comparison will be performed in parallel to the modulo on modern multi-issue CPUs (with speculative execution of the modulo and no trap taken if packet_limit is 0).
But adding a small constant in a code path only taken a few times per second will obviously be very cheap, too.
Since the recalculation of packet_limit is only performed a few times per second, a slightly more complex (but more correct) calculation of that value does also cause only negligible extra CPU cycles.

[donner]

The current situation is good enough.