On Sat, May 9, 2020 at 7:23 AM William Tu <[email protected]> wrote:
>
> On Thu, Apr 30, 2020 at 07:00:37PM +0800, [email protected] wrote:
> > From: Tonghao Zhang <[email protected]>
> >
> > For now, the meter of the userspace datapath, don't include
> > the bucket burst size to buckets. This patch includes it now.
> >
> > Cc: Ilya Maximets <[email protected]>
> > Cc: William Tu <[email protected]>
> > Cc: Jarno Rajahalme <[email protected]>
> > Cc: Ben Pfaff <[email protected]>
> > Cc: Andy Zhou <[email protected]>
> > Signed-off-by: Tonghao Zhang <[email protected]>
> > ---
> > lib/dpif-netdev.c | 7 +------
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 17c0241aa2e2..59546db6a2a2 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -6092,15 +6092,10 @@ dpif_netdev_meter_set(struct dpif *dpif,
> > ofproto_meter_id meter_id,
> > for (i = 0; i < config->n_bands; ++i) {
> > uint32_t band_max_delta_t;
> >
> > - /* Set burst size to a workable value if none specified. */
> > - if (config->bands[i].burst_size == 0) {
> > - config->bands[i].burst_size = config->bands[i].rate;
> > - }
> > -
> > meter->bands[i].up = config->bands[i];
> > /* Convert burst size to the bucket units: */
> > /* pkts => 1/1000 packets, kilobits => bits. */
> > - meter->bands[i].up.burst_size *= 1000;
> > + meter->bands[i].up.burst_size += config->bands[i].rate * 1000ULL;
>
> I don't quite understand.
> Isn't this remove the setting of burst_size and always use
> 'config->bands[i].rate * 1000ULL;'?
Hi William, thanks for you reviews,
meter->bands[i].up.burst_size += config->bands[i].rate * 1000ULL;
burst_size will plus the config->bands[i].rate * 1000ULL and then
assigned to burst_size again.
so if user don't set the burst_size, burst_size is 0, and only plus
the config->bands[i].rate * 1000ULL.
Before the patch, if user don't set the burst_sze, burst_size = 0, and
will the rate *1000.
Here, burst_size is different from kernel datapath. burst_size in
netdev will be used as bucket. so buckets shoud be "burst_size" + rate
> Ex: When user set
> ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps burst stats
> bands=type=drop rate=1 burst_size=123
> does 123 get set?
burst_size(used for bucket size )should be (burst_size + rate) *1000
my patch should be: because burst_size uint kilobits
- /* Set burst size to a workable value if none specified. */
- if (config->bands[i].burst_size == 0) {
- config->bands[i].burst_size = config->bands[i].rate;
- }
-
meter->bands[i].up = config->bands[i];
/* Convert burst size to the bucket units: */
/* pkts => 1/1000 packets, kilobits => bits. */
- meter->bands[i].up.burst_size *= 1000;
+ meter->bands[i].up.burst_size += config->bands[i].rate;
+ meter->bands[i].up.burst_size *= 1000ULL;
> William
> > /* Initialize bucket to empty. */
> > meter->bands[i].bucket = 0;
> >
> > --
> > 1.8.3.1
> >
--
Best regards, Tonghao
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
