On Tue, Jan 12, 2021 at 8:09 PM Dumitru Ceara <[email protected]> wrote:
> On 1/12/21 3:30 PM, Numan Siddique wrote: > > > > > > On Mon, Jan 11, 2021 at 5:37 PM Dumitru Ceara <[email protected] > > <mailto:[email protected]>> wrote: > > > > A new module, 'ofctrl-seqno', is added to implement this generic > > barrier. Other modules can register their own types of seqno update > > requests. The barrier implementation ensures that the a seqno update > > request is acked (returned by ofctrl_acked_seqnos_get()) only if the > > OVS flow operations that have been requested when the seqno update > > request was queued have been processed by OVS. > > > > For now, the only user of this barrier is the main ovn-controller > > module but a future commit will use it too in order to mark > > Port_Bindings and OVS interfaces as "fully installed". > > > > This commit also adds unit tests for the new 'ofctrl-seqno' module. > > The unit test structure is inspired by Mark Michelson's patch: > > > http://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ > > > > Signed-off-by: Dumitru Ceara <[email protected] > > <mailto:[email protected]>> > > > > > > Hi Dumitru, > > > > Hi Numan, > > > Thanks for working on this feature. > > Thanks for reviewing this! > > > > > I applied this series and tested it on sandbox, and looks like there is > > some issue when I run this script > > - https://github.com/ovn-org/ovn/blob/master/tutorial/ovn-setup.sh > > > > ovn-controller is not claiming all the lports. > > Sorry about that, I forgot to add RBAC permissions to ovn-controller to > write to the Port_Binding.up column in patch 3/3. I'll take care of it > in the next revision. > > > > > I have still not reviewed the patches thoroughly. > > > > I have few comments now. > > > > 1. tests/ovn-ofctrl-seqno.at <http://ovn-ofctrl-seqno.at> requires a > > license stuff at the beginning ? > > > > I can add one but none of the non-system test suites has one. I guess > that's because they're included from tests/testsuite.at which has a > copyright notice. I'm not sure what's best to do. > > What do you think? > If there are existing non-system test files without a copyright notice, then I'm fine. I don't have any strong opinion on this. Thanks Numan > > > 2. For the ofctrl-seqno.c file, can you please re-arrange the public > > functions and > > static functions as per the coding guidelines. Since it is a new > > file being added, > > I think it will be helpful so that the code is organized as per the > > guidelines. > > > > > https://github.com/ovn-org/ovn/blob/master/Documentation/internals/contributing/coding-style.rst > > See section - Functions. > > > > What do you think ? > > > > Sure, I'll do that in v4. > > > Thanks > > Numan > > > > Thanks, > Dumitru > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
