On Tue, Nov 10, 2020 at 03:43:17PM -0500, Flavio Fernandes wrote:
> [cc Dimitru, Numan, MarkM]
>
>
> > On Nov 10, 2020, at 2:15 PM, Ben Pfaff <[email protected]> wrote:
> >
> > On Mon, Nov 09, 2020 at 09:53:16AM -0500, Flavio Fernandes wrote:
> >>> On Nov 7, 2020, at 4:39 PM, Ben Pfaff <[email protected]> wrote:
> >>> On Tue, Nov 03, 2020 at 10:18:34PM +0000, Flavio Fernandes wrote:
> >>>> When multiple ACL rows use the same Meter for log rate-limiting, the
> >>>> 'noisiest' ACL matches may completely consume the Meter and shadow other
> >>>> ACL logs. This patch introduces an option in NB_Global that allows for a
> >>>> Meter to rate-limit each ACL log separately.
> >>>
> >>> I'm not sure I like the overall design here. This option isn't going to
> >>> scale very well to many of these meters, since they'd all need to be
> >>> shoved into a single comma-separated list. Another way might be to add
> >>> a column to the ACL table to mark a meter as shared or private. Or
> >>> there could be a name convention, e.g. prefix with "shared_" to make it
> >>> shared.
> > OK, let's step back here and consider G. Why do we need a new
> > southbound Meter row for each instance? That's a bit of a waste, isn't
> > it? Why can't we just add an indicator to the southbound Meter that
> > says "each reference to me is unique"? Or a new argument to the
> > southbound log() action that distinguishes references to a given meter,
> > so that identical values get the same one and different values get
> > distinct ones?
>
> Hmm.... That sounds really good, actually. We would still need 'A' as a way
> to propagate that desire from the CMS' perspective, agree?
>
> I must confess that the wasteful approach of rows in the SB comes from my
> limited knowledge on how to efficiently use the log action to do what you
> described. Also, because I was hoping to solve the whole problem within
> northd.
>
> If I understand you correctly, option 'G' you mention here would require
> changes in the SB schema as well as in ovn-controller? I will definitely
> need some pointers to make that happen. Wanna be my partner in
> crime? :) No pressure.
It would require ovn-controller changes. However, maybe it's not worth
doing them yet? It would only yield a small efficiency improvement, and
it's always possible to get that later.
> > Finally, we need to create new southbound Meter records for the "shared"
> > meters. I'm actually a bit confused about this. I would have thought
> > that "shared" meters wouldn't need the Meter records whose names are
> > un-suffixed by __<uuid>, but the C implementation appears to always
> > create them. So I left in place the existing DDlog code that just
> > copies from nb::Meter to sb::Out_Meter:
>
> I did not want to alienate any other users of the Meter name. So yes,
> I intentionally kept the creation of the unmodified meter in place to
> cover potential cases where something unrelated to acl log decided
> to use the same meter name. Maybe a bit too paranoid?!? ;)
I don't know. This kind of thing can always be changed later.
> > The above might need some explanation for those relatively new to
> > Datalog or DDlog. First, the style. This uses Datalog-style syntax
> > (output :- input) instead of the FLWOR-style syntax (for (input) {
> > output }). One reason for that is because the FLWOR-style syntax
> > currently doesn't support FlatMap, which we need in that case.
>
> It will be a while longer until I manage to acclimate to the syntax in
> ovn_northd.dl. But I can see how powerful it can be. Reminds me of
> the few months it took me to learn Erlang, which remains as one of
> my favorite languages.
It's kind of cool. Some of the things that are relatively easy in C are
somewhat difficult and obscure in DDlog (ID allocation is the best
example that occurs to me off-hand), but some things that are tricky in
C are really easy in DDlog. The syntax is the biggest barrier,
honestly.
> Lastly, it goes w/out saying but just to be clear: Given how fast you move
> and how far along ddlog patches are, my intentions are for not getting any
> of the 'fair' meters code merged before ddlog's.
Thanks a lot!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
