On 07/30/2018 09:00 PM, Justin Pettit wrote:
On Jul 30, 2018, at 11:40 AM, Mark Michelson <mmich...@redhat.com> wrote:
Hi Justin,
I took a look through the patch series, and this is the only one that I had
some immediate feedback on.
First, it would be nice if we could refer to Meters by name when issuing DB commands. For
instance `ovn-nbctl add Meter <meter_name> bands <band UUID>`.
Okay, I'll work on a follow-up patch to add that. I didn't think it was a
blocker for this series, though.
Second, I noticed that the algorithm for computing southbound meter bands can
result in a larger number of bands than is in the northbound table. For
instance, you could issue the following:
ovn-nbctl --id=@id create Meter_Band action=drop rate=1000 -- \
create Meter name=foo unit=kbps band=@id -- \
create Meter name=bar unit=kbps band=@id
In the northbound database, you'll have one entry in the meter_band table. In
the southbound database, you'll have two entries in the meter_band table. The
algorithm does not take into account that multiple northbound meters may refer
to the same meter_band. I'm not sure how big an issue this is (or really if it
is an issue), but it surprised me a bit when I was playing around with it.
That's interesting, but I don't think it's a problem. Let me know if you think
of an issue with it, though.
I figured more than anything I should bring this to your attention in
case you had not noticed it. Given that you provide an 'ovn-nbctl
meter-add' command, it strikes me that most people will probably add
meters this way. If they do it this way, then they won't run into any
surprises.
Thanks for checking out the series, and please let me know if you have any
other thoughts.
--Justin
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
