On Tue, Nov 10, 2020 at 8:44 PM Ben Pfaff <b...@ovn.org> wrote:
> 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.
>
>
Sounds reasonable. Thank you for taking the time to think/explain 'G'.
I circled back with Dumitru on this subject. He convinced me that 'B' is
actually a better approach, because things other than ACL logs may benefit
from 'fair' Meters later on. Case in point:
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-November/050778.html
So I will rebase the code to be off of
github.com/blp/ovs-reviews/ddlog_${latest}
and make changes with 'B' in mind. We tackle 'G' as a separate follow up.
Thanks,
-- flaviof
> > > 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
