[ovs-discuss] [ovn] : ovn-northd not updating the south db when some new flows added in the code and restarted.

[Numan Siddique]

On 11/20/2015 07:49 PM, Russell Bryant wrote:
> On Fri, Nov 20, 2015 at 6:16 AM, Numan Siddique <nusiddiq at redhat.com> wrote:
>
>> Hi,
>>
>> I am seeing an issue with ovn-northd where in it is not updating the south
>> db when I applied the "ovn: support ARP response for known IPs"
>> patch from Han Zhou add restarted the ovn-northd.
>>
>> I didn't see the flows related to ARP getting reflected in the south db.
>> It only got reflected when I updated the north db from neutron like
>> creating a new port or restarting
>> the ovn-northd again.
>>
>> By putting some prints, I could see that
>> ovsdb_idl_txn_commit(ctx.ovnsb_txn) is returning TXN_ERROR after which it
>> is blocked in poll_block().
>> until some update happens in north db.
>>
>> The issue is not seen all the time. But only when ovsdb_idl_txn_commit
>> returns TXN_ERROR.
>>
>> The below code is fixing the issue. Wanted to get the comments from the
>> reviewers if it is the right fix or
>> the problem lies some where else. Also not sure if there are any side
>> effects because of the below fix.
>>
>> ---
>>  ovn/northd/ovn-northd.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>> index 8fe0c2c..1c3821f 100644
>> --- a/ovn/northd/ovn-northd.c
>> +++ b/ovn/northd/ovn-northd.c
>> @@ -1990,7 +1990,8 @@ main(int argc, char *argv[])
>>          }
>>
>>          if (ovnnb_seqno == ovsdb_idl_get_seqno(ovnnb_idl) &&
>> -                ovn_seqno == ovsdb_idl_get_seqno(ovnsb_idl)) {
>> +                ovn_seqno == ovsdb_idl_get_seqno(ovnsb_idl)
>> +                && !ovnnb_changes_pending) {
>>              ovsdb_idl_wait(ovnnb_idl);
>>              ovsdb_idl_wait(ovnsb_idl);
>>              if (ctx.ovnnb_txn) {
>>
> I'm not sure what the transaction error is, but this patch seems correct.
>
> There's actually some common code we could use that would replace all of
> this.  First we had ovn-northd, then ovn-controller, and then
> ovn-controller-vtep.  ovn-controller-vtep was modeled largely on
> ovn-controller and some common code was split out.  In particular, see the
> ovsdb_idl_loop_* functions.  It seems like we should refactor the main loop
> ovn-northd to use that.
>
Thanks Russel. I will look into it and refactor it accordingly.

Numan