On 9/24/21 12:56, taoyunxiang wrote: > While busrt is smaller than rate/1000, the band_max_delta_t will be set to 0. > And than, it will cause the buckt in dp_netdev_run_meter being 0, eventually > it will make the packets be dropped. > This fix will set the band_max_delta_t to 1, if busrt is too small, to avoid > meter not working.
Hi. Thanks for working on this. I'm not sure how useful it is to have a burst smaller than 1 ms of traffic. It doesn't seem to be a good value for a real setup. But I agree that we can fix that case. Some comments inline. Best regards, Ilya Maximets. > > > Signed-off-by: Tao YunXiang <[email protected]> > --- > lib/dpif-netdev.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 0b846cf0d..61f55639a 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -5735,8 +5735,11 @@ dpif_netdev_meter_set(struct dpif *dpif, > ofproto_meter_id meter_id, > meter->bands[i].bucket = 0; > > /* Figure out max delta_t that is enough to fill any bucket. */ > + /* If burst_size is too small, set band_max_delta_t to 1 */ > + /* to avoid the packet being dropped in dp_netdev_run_meter. */ Please, don't close/open the comment for every line. > band_max_delta_t > - = meter->bands[i].up.burst_size / meter->bands[i].up.rate; > + = (meter->bands[i].up.burst_size >= meter->bands[i].up.rate) > + ? (meter->bands[i].up.burst_size / meter->bands[i].up.rate) : > 1; Can we just check after the division? It might be easier to read, e.g.: if (!band_max_delta_t) { /* Your comment here. */ band_max_delta_t = 1; } > if (band_max_delta_t > meter->max_delta_t) { > meter->max_delta_t = band_max_delta_t; > } > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
