Message ID | 20200523103320.47497-5-xiangxia.m.yue@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | expand the meter table and fix bug. | expand |
On 5/23/20 12:33 PM, xiangxia.m.yue@gmail.com wrote: > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > This reverts commit 5c41c31ebd64fda821fb733a5784a7a440a794f8. > > 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 > > Note that 42697ca7757b ("dpif-netdev: fix meter at high packet rate.") has fixed it as 5c41c31ebd64. > > 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> > Cc: Pravin Shelar <pshelar@ovn.org> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > --- Thanks! Since this is a bug fix, I applied this one patch to master and branch-2.14. Will get back to review other patches later. Bets regards, Ilya Maximets. > lib/dpif-netdev.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 776a16dab6e8..46eb1e3d97c9 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -5904,7 +5904,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]; > @@ -5935,9 +5934,6 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, > 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 > @@ -5973,7 +5969,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; > } >
On Tue, Jul 28, 2020 at 12:36 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 5/23/20 12:33 PM, xiangxia.m.yue@gmail.com wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > > > This reverts commit 5c41c31ebd64fda821fb733a5784a7a440a794f8. > > > > 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 > > > > Note that 42697ca7757b ("dpif-netdev: fix meter at high packet rate.") has fixed it as 5c41c31ebd64. > > > > 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> > > Cc: Pravin Shelar <pshelar@ovn.org> > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > --- > > Thanks! > > Since this is a bug fix, I applied this one patch to master and branch-2.14. > Will get back to review other patches later. Thanks, Ilya > Bets regards, Ilya Maximets. > > > lib/dpif-netdev.c | 5 ----- > > 1 file changed, 5 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 776a16dab6e8..46eb1e3d97c9 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -5904,7 +5904,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]; > > @@ -5935,9 +5934,6 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, > > 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 > > @@ -5973,7 +5969,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; > > } > > >
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 776a16dab6e8..46eb1e3d97c9 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -5904,7 +5904,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]; @@ -5935,9 +5934,6 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, 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 @@ -5973,7 +5969,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; }