diff mbox series

[ovs-dev] 答复: [PATCH OVS 4/4] revert: dpif-netdev: includes microsecond delta in meter bucket calculation

Message ID 956f27fc5a2d4f99a553c18c82d3c3e0@jd.com
State Superseded
Headers show
Series [ovs-dev] 答复: [PATCH OVS 4/4] revert: dpif-netdev: includes microsecond delta in meter bucket calculation | expand

Commit Message

姜立东 May 8, 2020, 8:11 a.m. UTC 
Oh, this is due to patch porting from 2.10. 
Between latest and 2.10, commit 42697ca77 is introduced to fix millisecond token insertion rate as below.
     /* All packets will hit the meter at the same time. */
-    long_delta_t = (now - meter->used) / 1000; /* msec */
+    long_delta_t = now / 1000 - meter->used / 1000; /* msec */

While my patch is ported from 2.10 base line, so additional token is counted incorrectly. 
Commit 42697ca77 is good enough to fix the loss of token in delta computation.
So we are looking into if higher token insertion rate in micro second is needed in some cases.

Thanks,
Lidong
 

-----邮件原件-----
发件人: xiangxia.m.yue@gmail.com <xiangxia.m.yue@gmail.com> 
发送时间: 2020年4月30日 19:01
收件人: i.maximets@ovn.org; blp@ovn.org; azhou@ovn.org; u9012063@gmail.com; jarno@ovn.org
抄送: dev@openvswitch.org; Tonghao Zhang <xiangxia.m.yue@gmail.com>; 姜立东 <jianglidong3@jd.com>
主题: [PATCH OVS 4/4] revert: dpif-netdev: includes microsecond delta in meter bucket calculation

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Use the pktgen-dpdk to test the commit 5c41c31ebd64
("dpif-netdev: includes microsecond delta in meter bucket calculation"), it does't work as expected. And it broken the meter function (e.g. set rate 200Mbps, the rate watched was 400Mbps). To reproduce it:

$ ovs-vsctl add-br br-int -- set bridge br-int datapath_type=netdev $ ovs-ofctl -O OpenFlow13 add-meter br-int "meter=100 kbps burst stats bands=type=drop rate=200000 burst_size=200000"
$ ovs-ofctl -O OpenFlow13 add-flow br-int "in_port=dpdk0 action=meter:100,output:dpdk1"

$ pktgen -l 1,3,5,7,9,11,13,15,17,19 -n 8 --socket-mem 4096 --file-prefix pg1 \
        -w 0000:82:00.0 -w 0000:82:00.1 -- -T -P -m "[3/5/7/9/11/13/15].[0-1]" -f meter-test.pkt

| meter-test.pkt:
| set 0 count 0
| set 0 size 1500
| set 0 rate 100
| set 0 burst 64
| set 0 sport 1234
| set 0 dport 5678
| set 0 prime 1
| set 0 type ipv4
| set 0 proto udp
| set 0 dst ip 1.1.1.2
| set 0 src ip 1.1.1.1/24
| set 0 dst mac ec:0d:9a:ab:54:0a
| set 0 src mac ec:0d:9a:bf:df:bb
| set 0 vlanid 0
| start 0

Cc: Ilya Maximets <i.maximets@ovn.org>
Cc: William Tu <u9012063@gmail.com>
Cc: Jarno Rajahalme <jarno@ovn.org>
Cc: Ben Pfaff <blp@ovn.org>
Cc: Andy Zhou <azhou@ovn.org>
Cc: Jiang Lidong <jianglidong3@jd.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/dpif-netdev.c | 5 -----
 1 file changed, 5 deletions(-)

            Assuming that all racing threads received packets at the same time
            to avoid overflow. */
         long_delta_t = 0;
-        delta_in_us  = 0;
-    } else {
-        delta_in_us  = (now - meter->used) % 1000;
     }
 
     /* Make sure delta_t will not be too large, so that bucket will not @@ -5971,7 +5967,6 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
 
         /* Update band's bucket. */
         band->bucket += (uint64_t)delta_t * band->up.rate;
-        band->bucket += delta_in_us * band->up.rate / 1000;
         if (band->bucket > band->up.burst_size) {
             band->bucket = band->up.burst_size;
         }
--
1.8.3.1

Comments

William Tu May 8, 2020, 4:51 p.m. UTC | #1 
On Fri, May 8, 2020 at 1:11 AM 姜立东 <jianglidong3@jd.com> wrote:
>
> Oh, this is due to patch porting from 2.10.
> Between latest and 2.10, commit 42697ca77 is introduced to fix millisecond token insertion rate as below.
>      /* All packets will hit the meter at the same time. */
> -    long_delta_t = (now - meter->used) / 1000; /* msec */
> +    long_delta_t = now / 1000 - meter->used / 1000; /* msec */
>
> While my patch is ported from 2.10 base line, so additional token is counted incorrectly.
> Commit 42697ca77 is good enough to fix the loss of token in delta computation.
> So we are looking into if higher token insertion rate in micro second is needed in some cases.
>

Thanks, I forgot this is already fixed.
I will revert the patch if no other comments.

William
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 104347d8b251..cf10b5a62423 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5902,7 +5902,6 @@  dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
     struct dp_packet *packet;
     long long int long_delta_t; /* msec */
     uint32_t delta_t; /* msec */
-    uint32_t delta_in_us; /* usec */
     const size_t cnt = dp_packet_batch_size(packets_);
     uint32_t bytes, volume;
     int exceeded_band[NETDEV_MAX_BURST]; @@ -5933,9 +5932,6 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,