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?

> 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

Reply via email to