[ovs-dev] [PATCH] OVN: initial patch of datalog engine

[Hui Kang]

"dev" <dev-bounces at openvswitch.org> wrote on 06/16/2016 10:58:58 PM:

> From: Yusheng Wang <yshwang at vmware.com>
> To: Ryan Moats/Omaha/IBM at IBMUS
> Cc: "dev at openvswitch.org" <dev at openvswitch.org>
> Date: 06/16/2016 10:59 PM
> Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
> Sent by: "dev" <dev-bounces at openvswitch.org>
>
>
> I agree with all your comments but the last one. The benefit of
> using one source file is
> that you know everything is there as far as the engine is concerned.
> We definitely need
> to separate it once it is really large.
>
> I suppose it will take some time for the code to stabilize and the
> engine will evolve over
> time. The package right now is self contained and people could try
> with it using the
> interactive test case -- writing a datalog program, presenting it
> with some input and
> seeing what comes out. The previous man page have the implementation
details
> and it is a good start point before diving into the code.
>
> As for the prefix, I was thinking about 'dl' but it might be too
> short to distinguish itself
> while 'datalog' might be too long considering all functions will
> carry it. Since test cases
> are living in another source file, quite a few internal functions
> have to be declared as
> external so a lot more functions have to carry that name.
>
> I noticed the automake conflict when I sync my repo. I am not sure
> if we have the guideline
> of not changing the last line which does not have back slash char,
> instead adding new lines
> in the middle. Hope this will not prevent people from patching and
> trying with it.

The conflict is caused by a later change to ovn/lib/automake.mk.
A quick fix is to change line 43 to line 45 in your patch.

- Hui

>
> -- Yusheng
> ________________________________________
> From: Ryan Moats <rmoats at us.ibm.com>
> Sent: Friday, June 17, 2016 12:03 AM
> To: Yusheng Wang
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
>
> This commit message is too bare for a commit this size.
> Please provide some content for later readers that aren't
> up to speed with the technology being introduced here.
>
> Because the patch is so big, I'm not including my comments
> in-line, I'm going to put them all here:
>
> I know there was a separate patch for the ovn-datalog man patch,
> I think that patch should be folded into this one for a more
> atomic approach.
>
> Note: the log_ prefix is already used in lib/util.h and while this
> patch introduces a private include file, I'd be more comfortable
> if the log_ prefix were replaced with _datalog_ to be more consistent
> with the ovs naming convention.
>
> Similarly, this patch overloads other prefixes that are used
> elsewhere - I'd consider cleaning up the prefix usage to
> avoid confusion later on.
>
> Nit: is there any reason why ENT_* values 0x0c through 0x0f
> aren't in numerical order like the entries from 0 to 0x0b
> appear to be?
>
> Nit (found in multiple places): comments should begin with a
> capital letter and end with a period
>
> Also, I'm thinking that the datalog source file should
> be broken into multiple files rather than concatenated together
> (as it is over 3K lines long)
>
> Lastly, this patch set has a collision in the automake.mk file
> with the current master and so could use a rebase.
>
> Ryan Moats
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev