It seems that email that I send to Tao YunXiang bounces; at least, this
one did.  I hope that he sees my previous email via the list.

----- Forwarded message from Mail Delivery System 
<[email protected]> -----

Date: Tue, 11 May 2021 20:00:42 +0000 (UTC)
From: Mail Delivery System <[email protected]>
To: [email protected]
Subject: Undelivered Mail Returned to Sender
Message-Id: <[email protected]>

This is the mail system at host relay12.mail.gandi.net.

I'm sorry to have to inform you that your message could not
be delivered to one or more recipients. It's attached below.

For further assistance, please send mail to postmaster.

If you do so, please include this problem report. You can
delete your own text from the attached returned message.

                   The mail system

<[email protected]>: host mx.cmss.chinamobile.com[221.176.66.77]
    said: 550 2eec609ae26442b-87c46 Mail rejected (in reply to end of DATA
    command)

Reporting-MTA: dns; relay12.mail.gandi.net
X-Postfix-Queue-ID: E4E0E200006
X-Postfix-Sender: rfc822; [email protected]
Arrival-Date: Tue, 11 May 2021 20:00:34 +0000 (UTC)

Final-Recipient: rfc822; [email protected]
Original-Recipient: rfc822;[email protected]
Action: failed
Status: 5.0.0
Remote-MTA: dns; mx.cmss.chinamobile.com
Diagnostic-Code: smtp; 550 2eec609ae26442b-87c46 Mail rejected

Date: Tue, 11 May 2021 13:00:30 -0700
From: Ben Pfaff <[email protected]>
To: Tao YunXiang <[email protected]>
Cc: [email protected]
Subject: Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Improve concurrency by
 adjust flow-limit
Message-ID: <[email protected]>

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.


----- End forwarded message -----
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to