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 <b...@ovn.org> wrote: > > > > On Mon, Nov 09, 2020 at 09:53:16AM -0500, Flavio Fernandes wrote: > >>> On Nov 7, 2020, at 4:39 PM, Ben Pfaff <b...@ovn.org> 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev