On Wed, Jan 4, 2017 at 3:55 PM, Ben Pfaff <b...@ovn.org> wrote: > On Thu, Dec 22, 2016 at 12:12:05PM -0800, Andy Zhou wrote: > > On Thu, Dec 22, 2016 at 9:21 AM, Ben Pfaff <b...@ovn.org> wrote: > > > > > On Tue, Dec 20, 2016 at 01:47:16AM -0800, Andy Zhou wrote: > > > > From: andy zhou <az...@ovn.org> > > > > > > > > When generating conditional monitoring update request, current code > > > > failed to update idl's 'request-id'. This bug causes the reply > > > > message of the update request, regardless an ACK or a NACK, be > > > > logged as an unexpected message at the debug level and ignored by > > > > the core idl logic. > > > > > > > > In addition, the idl should not generate another conditional > > > > monitoring update request when there is an outstanding request. > > > > So that the requests and their reply are properly serialized. > > > > > > > > When the conditional monitoring is nacked by the server, drop idl > > > > into a client visible error state. > > > > > > > > Signed-off-by: Andy Zhou <az...@ovn.org> > > > > > > When the condition is changed when an update request is already in > > > flight, and the update request completes, will the code properly send > > > a new condition change at that point? > > > > > > > When an ack is received, the request_id will be freed and set to NULL, > thus > > unblocking ovsdb_idl_send_cond_changge() to send out the next condition > > change. > > > > Am I still missing something? > > I asked the question because I hadn't looked, and it seemed like an easy > mistake to make. I agree that it seems OK. However, I think that the > loop in ovsdb_idl_run() can reset request_id without calling > ovsdb_idl_send_cond_change() afterward. That'll happen on the next trip > through the main loop but at least in theory that could happen much > later, so perhaps we should call it right away, e.g.: > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 341951d..82c8584 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -458,6 +458,7 @@ ovsdb_idl_run(struct ovsdb_idl *idl) > > case IDL_S_MONITORING_COND: > /* Conditional monitor clauses were updated. */ > + ovsdb_idl_send_cond_change(idl); > break; > > case IDL_S_MONITORING: > > Probably the VLOG calls should be rate-limited. >
Thanks for the suggestion. I folded this change into v2. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
