Re: [ovs-dev] [PATCH RESEND v2 2/3] dpif-netdev: Add the burst size to buckets.
On Fri, Feb 26, 2021 at 4:32 AM Ilya Maximets wrote:
>
> On 2/24/21 1:21 PM, xiangxia.m@gmail.com wrote:
> > From: Tonghao Zhang
> >
> > For now, the meter of the userspace datapath, don't include
> > the bucket burst size to buckets.
>
> Well, it was used before the patch 1/3 of this series. :)
> The problem seems to be that 'rate' was not used for bucket calculation.
>
> And yes, despite the fact that burst_size was in use before the
> patch 1/3 it was used in a very strange way.
> You will, probably, want to squash this particular change into the
> patch #2 that I suggested to split out from the patch 1/3.
Ok
patch 1, fix the meter overflow
patch 2: refactor the buckets calculation
patch 3: add burst_size to buckets
patch 4: don't set burst_size to rate if burst_size not specified.(if
not any comment)
and every patch will update the test case.
> Best regards, Ilya Maximets.
>
> > This patch includes it now.
> >
> > $ ovs-ofctl -O OpenFlow13 add-meter br0 \
> > 'meter=1 pktps burst stats bands=type=drop rate=1 burst_size=2000'
> >
> > Signed-off-by: Tonghao Zhang
> > ---
> > lib/dpif-netdev.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 614f6fef6b77..94632b85b375 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -6209,7 +6209,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
> > dp_packet_batch *packets_,
> >
> > band = &meter->bands[m];
> > /* Update band's bucket. */
> > -max_bucket_size = band->rate * 1000ULL;
> > +max_bucket_size = (band->burst_size + band->rate) * 1000ULL;
> > band->bucket += (uint64_t)delta_t * band->rate;
> > if (band->bucket > max_bucket_size) {
> > band->bucket = max_bucket_size;
> > @@ -6332,7 +6332,8 @@ dpif_netdev_meter_set(struct dpif *dpif,
> > ofproto_meter_id meter_id,
> > meter->bands[i].rate = config->bands[i].rate;
> > meter->bands[i].burst_size = config->bands[i].burst_size;
> > /* Start with a full bucket. */
> > -meter->bands[i].bucket = meter->bands[i].rate * 1000ULL;
> > +meter->bands[i].bucket =
> > +(meter->bands[i].burst_size + meter->bands[i].rate) * 1000ULL;
> >
> > /* Figure out max delta_t that is enough to fill any bucket. */
> > band_max_delta_t
> >
>
--
Best regards, Tonghao
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RESEND v2 2/3] dpif-netdev: Add the burst size to buckets.
On 2/24/21 1:21 PM, xiangxia.m@gmail.com wrote:
> From: Tonghao Zhang
>
> For now, the meter of the userspace datapath, don't include
> the bucket burst size to buckets.
Well, it was used before the patch 1/3 of this series. :)
The problem seems to be that 'rate' was not used for bucket calculation.
And yes, despite the fact that burst_size was in use before the
patch 1/3 it was used in a very strange way.
You will, probably, want to squash this particular change into the
patch #2 that I suggested to split out from the patch 1/3.
Best regards, Ilya Maximets.
> This patch includes it now.
>
> $ ovs-ofctl -O OpenFlow13 add-meter br0 \
> 'meter=1 pktps burst stats bands=type=drop rate=1 burst_size=2000'
>
> Signed-off-by: Tonghao Zhang
> ---
> lib/dpif-netdev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 614f6fef6b77..94632b85b375 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -6209,7 +6209,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
> dp_packet_batch *packets_,
>
> band = &meter->bands[m];
> /* Update band's bucket. */
> -max_bucket_size = band->rate * 1000ULL;
> +max_bucket_size = (band->burst_size + band->rate) * 1000ULL;
> band->bucket += (uint64_t)delta_t * band->rate;
> if (band->bucket > max_bucket_size) {
> band->bucket = max_bucket_size;
> @@ -6332,7 +6332,8 @@ dpif_netdev_meter_set(struct dpif *dpif,
> ofproto_meter_id meter_id,
> meter->bands[i].rate = config->bands[i].rate;
> meter->bands[i].burst_size = config->bands[i].burst_size;
> /* Start with a full bucket. */
> -meter->bands[i].bucket = meter->bands[i].rate * 1000ULL;
> +meter->bands[i].bucket =
> +(meter->bands[i].burst_size + meter->bands[i].rate) * 1000ULL;
>
> /* Figure out max delta_t that is enough to fill any bucket. */
> band_max_delta_t
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH RESEND v2 2/3] dpif-netdev: Add the burst size to buckets.
From: Tonghao Zhang
For now, the meter of the userspace datapath, don't include
the bucket burst size to buckets. This patch includes it now.
$ ovs-ofctl -O OpenFlow13 add-meter br0 \
'meter=1 pktps burst stats bands=type=drop rate=1 burst_size=2000'
Signed-off-by: Tonghao Zhang
---
lib/dpif-netdev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 614f6fef6b77..94632b85b375 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6209,7 +6209,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[m];
/* Update band's bucket. */
-max_bucket_size = band->rate * 1000ULL;
+max_bucket_size = (band->burst_size + band->rate) * 1000ULL;
band->bucket += (uint64_t)delta_t * band->rate;
if (band->bucket > max_bucket_size) {
band->bucket = max_bucket_size;
@@ -6332,7 +6332,8 @@ dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id
meter_id,
meter->bands[i].rate = config->bands[i].rate;
meter->bands[i].burst_size = config->bands[i].burst_size;
/* Start with a full bucket. */
-meter->bands[i].bucket = meter->bands[i].rate * 1000ULL;
+meter->bands[i].bucket =
+(meter->bands[i].burst_size + meter->bands[i].rate) * 1000ULL;
/* Figure out max delta_t that is enough to fill any bucket. */
band_max_delta_t
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
