On Tue, May 11, 2021 at 03:52:00PM +0800, Tao YunXiang wrote:
> Author: Tao YunXiang <[email protected]>
You don't need an Author: tag because Git stores the author in a
separate field.
> + /* Use duration as a reference, adjust the number of flow_limit,
> + * when the duration is small, increase the flow-limit, and vice
> versa */
> + if (duration >= 1000) {
> flow_limit /= duration / 1000;
> - } else if (duration > 1300) {
> - flow_limit = flow_limit * 3 / 4;
> - } else if (duration < 1000 &&
> - flow_limit < n_flows * 1000 / duration) {
> - flow_limit += 1000;
> + } else {
> + flow_limit *= 1000 / duration;
> }
> flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));
> atomic_store_relaxed(&udpif->flow_limit, flow_limit);
The above is very abrupt. It always tries to adjust the flow limit
upward or downward. I think that this is a bad idea, especially in the
upward direction. If there are only a few flows, which only take a few
milliseconds to revalidate, then it will keep increasing the flow limit
upward until it overflows the range of unsigned int. It will happen
very quickly, in fact: if duration is 1 ms three times in a row, then we
will multiply flow_limit by 1,000,000,000 and overflow 32-bit UINT_MAX;
if it happens six times in a row, we will overflow 64-bit.
Furthermore, it doesn't work well even if we have longer durations. If
revalidation takes 501 ms, then we can adjust the flow_limit upward, but
this won't do it.
On the downward direction, this new code does nothing if the duration is
less than 2 seconds. We want to aim for 1-second revalidation times.
I don't think that this approach has been thought through very well.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
