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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
