[ovs-dev] Meter measures incorrect when use multi-pmd in ovs2.10
i.maximets at ovn.org
Fri Oct 25 11:49:59 UTC 2019
> I meet a problem that when send packets using netperf in multi-thread mode.
> The reproducing condition is like this:
> 1. ovs 2.10 in dpdk usermode with 2 pmd
> 2. Set Meter with rate=100,000pps, burst=20,000packet
> 3. when use single-thread mode for netperf, Meter behaves correct, and packets larger than 100,0000pps is dropped
> when use multi-thread mode for netperf, we noticed packets come from both 2 pmds, and in this time, Meter measure does not work.
> But Meter behaves correctly in ovs2.8 whether using single pmd or multi-pmd .
> Also, we have merged the patch 42697ca7757b594cc841d944e43ffc17905e3188
> (long_delta_t = now / 1000 - meter->used / 1000) but problem still exists.
> We researched the meter code, found that meter use time_usec() to compute the delta time in ovs2.10.
> but when call the function dp_netdev_run_meter(), the input parameter 'now' variable come from pmd->ctx.now
> and pmd->ctx.now may updated when receive packet in function dp_netdev_process_rxq_port()
> so let's suppose the following condition:
> pmd1 receives packet in T1
> pmd2 receives packet in T2 (T2<T1)
> first handle Meter in pmd1, meter->used changed to pmd1->ctx.now
> than handle Meter in pmd2, now = pmd2->ctx.now
> and long_delta_t = now / 1000 - meter->used / 1000 = T2/1000 - T1/1000 which will be a negative value!!!
> then delta time changed with clause:
> delta_t = (long_delta_t > (long long int)meter->max_delta_t)
> ? meter->max_delta_t : (uint32_t)long_delta_t;
> in this time, delta_t = (uint32_t)(T2/1000 - T1/1000) will be overflow???
> now, we just fix this problem by modifing the input parameter 'now' with time_usec().
> we don't know whether this code modify has any side effect(performance issue?)
Hi. Thanks for reporting the issue!
Regarding the solution, there is still a small race window between time_usec() and
acquiring the meter lock, so it's still possible to get a negative value.
I prepared a patch with alternative solution:
Please, check and reply if it fixes your issue.
Best regards, Ilya Maximets.
More information about the dev