<div dir="ltr">Thanks Ben for clarification. So it means one Monitor request can request for only one Table monitoring, correct? </div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 8, 2018 at 10:36 AM, Ben Pfaff <span dir="ltr"><<a href="mailto:blp@ovn.org" target="_blank">blp@ovn.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><monitor-requests> is a JSON object. A JSON object maps from keys to<br>
values, so we only need one.<br>
<br>
Your specification for <monitor-requests> is wrong because it says that<br>
it is a JSON array. It is not.<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Mar 07, 2018 at 09:32:06PM -0800, Anil Jangam wrote:<br>
> Hello Ben,<br>
><br>
> I have one more observation. I request you to please read it carefully. If<br>
> we go by the current monitor method definition, there can be only<br>
> <monitor-requests><br>
> in one monitor RPC method. If it is expected to have only one<br>
> <monitor-requests><br>
> i.e. one table and an array of <monitor-request>, then the current<br>
> specification is good.<br>
><br>
> o "method": "monitor"<br>
><br>
> o "params": [<db-name>, <json-value>, <monitor-requests>]<br>
><br>
> o "id": <nonnull-json-value><br>
><br>
><br>
> However, if it is possible to specify multiple table monitoring in one RPC<br>
> method, IMHO the above syntax would change as below.<br>
><br>
><br>
> o "method": "monitor"<br>
><br>
> o "params": [<db-name>, <json-value>, <monitor-requests>*]<br>
><br>
> o "id": <nonnull-json-value><br>
><br>
><br>
> For completeness, I am also specifying the syntax for <monitor-requests>.<br>
><br>
> monitor-requests : [<table-name>, <monitor-request>*]<br>
><br>
><br>
> Let me know if this would be correct or if you think otherwise.<br>
><br>
> /anil.<br>
><br>
><br>
><br>
> On Wed, Mar 7, 2018 at 10:27 AM, Ben Pfaff <<a href="mailto:blp@ovn.org">blp@ovn.org</a>> wrote:<br>
><br>
> > Ah, OK, you're saying that there's a missing [] around the<br>
> > <monitor-request>. This goes back to a change that we made to the<br>
> > ovsdb-server protocol a long time ago to allow multiple<br>
> > <monitor-request> objects instead of just a single one. ovsdb-server<br>
> > still supports this form. You can see this documented in<br>
> > Documentation/ref/ovsdb-<wbr>server.7.rst:<br>
> ><br>
> > For backward compatibility, ``ovsdb-server`` currently permits a single<br>
> > <monitor-request> to be used instead of an array; it is treated as a<br>
> > single-element array. Future versions of ``ovsdb-server`` might<br>
> > remove this<br>
> > compatibility feature.<br>
> ><br>
> > I guess we should change ovsdb-idl.c to use an array now. Oops.<br>
> ><br>
> > Anyway, that's easy enough, so I sent a patch:<br>
> > <a href="https://patchwork.ozlabs.org/patch/882710/" rel="noreferrer" target="_blank">https://patchwork.ozlabs.org/<wbr>patch/882710/</a><br>
> ><br>
> > On Tue, Mar 06, 2018 at 08:38:34PM -0800, Anil Jangam wrote:<br>
> > > Hello Ben,<br>
> > ><br>
> > > The <monitor-requests> object maps the name of the table to be monitored<br>
> > ><br>
> > > to an array of <monitor-request> objects. Each <monitor-request> is an<br>
> > ><br>
> > > object with the following members:<br>
> > ><br>
> > > "columns": [<column>*] optional<br>
> > ><br>
> > > "select": <monitor-select> optional<br>
> > ><br>
> > ><br>
> > ><br>
> > > As the <monitor-requests> maps the table name to be monitored to an array<br>
> > > of <monitor-request>, my interpretation of it is as "Table Name <--><br>
> > Array<br>
> > > of <monitor-request>"<br>
> > ><br>
> > > I am giving an example message is given below.<br>
> > ><br>
> > > {<br>
> > > "id": "c5c09c07-11c1-449b-8f04-<wbr>016fefe15844",<br>
> > > "method": "monitor",<br>
> > > "params": [<br>
> > > "hardware_vtep",<br>
> > > "91c9eed4-00bb-48e3-b2d9-<wbr>51e0364bbdc2",<br>
> > > {<br>
> > > "Physical_Locator": [<br>
> > > {<br>
> > > "columns": [<br>
> > > "dst_ip",<br>
> > > "encapsulation_type",<br>
> > > "_uuid"<br>
> > > ],<br>
> > > "select": {<br>
> > > "initial": true,<br>
> > > "insert": true,<br>
> > > "delete": true,<br>
> > > "modify": true<br>
> > > }<br>
> > > },<br>
> > > {<br>
> > > "columns": [<br>
> > > "_uuid",<br>
> > > "locators"<br>
> > > ],<br>
> > > "select": {<br>
> > > "initial": true,<br>
> > > "insert": true,<br>
> > > "delete": true,<br>
> > > "modify": true<br>
> > > }<br>
> > > }<br>
> > > ]<br>
> > > }<br>
> > > ]<br>
> > > }<br>
> > ><br>
> > ><br>
> > > /anil.<br>
> > ><br>
> > ><br>
> > ><br>
> > > On Tue, Mar 6, 2018 at 11:30 AM, Ben Pfaff <<a href="mailto:blp@ovn.org">blp@ovn.org</a>> wrote:<br>
> > ><br>
> > > > On Mon, Mar 05, 2018 at 10:03:13PM -0800, Anil Jangam wrote:<br>
> > > > > Hi,<br>
> > > > ><br>
> > > > > The RFC7047 states below about the Monitor request.<br>
> > > > ><br>
> > > > > The request object has the<br>
> > > > ><br>
> > > > > following members:<br>
> > > > ><br>
> > > > > o "method": "monitor"<br>
> > > > ><br>
> > > > > o "params": [<db-name>, <json-value>, <monitor-requests>]<br>
> > > > ><br>
> > > > > o "id": <nonnull-json-value><br>
> > > > ><br>
> > > > ><br>
> > > > > The <json-value> parameter is used to match subsequent update<br>
> > > > ><br>
> > > > > notifications (see below) to this request.<br>
> > > > ><br>
> > > > ><br>
> > > > > The <monitor-requests> object maps the name of the table to be<br>
> > monitored<br>
> > > > ><br>
> > > > > to an array of <monitor-request> objects. Each <monitor-request> is<br>
> > an<br>
> > > > ><br>
> > > > > object with the following members:<br>
> > > > ><br>
> > > > > "columns": [<column>*] optional<br>
> > > > ><br>
> > > > > "select": <monitor-select> optional<br>
> > > > ><br>
> > > > > The columns, if present, define the columns within the table to be<br>
> > > > monitored.<br>
> > > > ><br>
> > > > > <monitor-select> is an object with the following members:<br>
> > > > ><br>
> > > > > "initial": <boolean> optional<br>
> > > > ><br>
> > > > > "insert": <boolean> optional<br>
> > > > ><br>
> > > > > "delete": <boolean> optional<br>
> > > > ><br>
> > > > > "modify": <boolean> optional<br>
> > > > ><br>
> > > > > The contents of this object specify how the columns or table are to<br>
> > be<br>
> > > > > monitored,<br>
> > > > ><br>
> > > > > as explained in more detail below.<br>
> > > > ><br>
> > > > ><br>
> > > > > However, when I look at some of the legitimate samples of the Monitor<br>
> > > > > requests, they are encoded as below.<br>
> > > > ><br>
> > > > > {<br>
> > > > > "id": "c5c09c07-11c1-449b-8f04-<wbr>016fefe15844",<br>
> > > > > "method": "monitor",<br>
> > > > > "params": [<br>
> > > > > "hardware_vtep",<br>
> > > > > "91c9eed4-00bb-48e3-b2d9-<wbr>51e0364bbdc2",<br>
> > > > > {<br>
> > > > > "Physical_Locator": {<br>
> > > > > "columns": [<br>
> > > > > "dst_ip",<br>
> > > > > "encapsulation_type",<br>
> > > > > "_uuid"<br>
> > > > > ],<br>
> > > > > "select": {<br>
> > > > > "initial": true,<br>
> > > > > "insert": true,<br>
> > > > > "delete": true,<br>
> > > > > "modify": true<br>
> > > > > }<br>
> > > > > },<br>
> > > > > "Physical_Locator_Set": {<br>
> > > > > "columns": [<br>
> > > > > "_uuid",<br>
> > > > > "locators"<br>
> > > > > ],<br>
> > > > > "select": {<br>
> > > > > "initial": true,<br>
> > > > > "insert": true,<br>
> > > > > "delete": true,<br>
> > > > > "modify": true<br>
> > > > > }<br>
> > > > > }<br>
> > > > > }<br>
> > > > > ]<br>
> > > > > }<br>
> > > > ><br>
> > > > ><br>
> > > > ><br>
> > > > > If we go by the RFC encoding rules, "params" contains the<br>
> > > > > <monitor-requests>, which maps the "Table name" to an array of<br>
> > > > > <Monitor-request> object. So IMHO, the table names comes only once<br>
> > in the<br>
> > > > > message. Correct?<br>
> > > ><br>
> > > > Yes. That's what I see above. The table names are Physical_Locator<br>
> > and<br>
> > > > Physical_Locator_Set, and each of them is mentioned once.<br>
> > > > In the <monitor-requests> object "Physical_Locator" is mapped to:<br>
> > > ><br>
> > > > {<br>
> > > > "columns": [<br>
> > > > "dst_ip",<br>
> > > > "encapsulation_type",<br>
> > > > "_uuid"<br>
> > > > ],<br>
> > > > "select": {<br>
> > > > "initial": true,<br>
> > > > "insert": true,<br>
> > > > "delete": true,<br>
> > > > "modify": true<br>
> > > > }<br>
> > > > }<br>
> > > ><br>
> > > > and similarly for "Physical_Locator_Set".<br>
> > > ><br>
> > > > > Also, it is explicitly mentioned that (as below) and it does NOT<br>
> > contain<br>
> > > > > the "Table name" in it.<br>
> > > > ><br>
> > > > ><br>
> > > > > Each <monitor-request> is an<br>
> > > > ><br>
> > > > > object with the following members:<br>
> > > > ><br>
> > > > > "columns": [<column>*] optional<br>
> > > > ><br>
> > > > > "select": <monitor-select> optional<br>
> > > > ><br>
> > > > ><br>
> > > > > However, in the message payload that I have, shows the tuple, which<br>
> > > > > contains "Table : Columns : Select". This list of <monitor-request><br>
> > > > constitute<br>
> > > > > the <monitor-requests> as per the RFC definition.<br>
> > > > ><br>
> > > > > I see this as the discrepancy between the RFC definition and how the<br>
> > > > > message is actually sent by the controller.<br>
> > > ><br>
> > > > I don't understand what discrepancy you see. Can you show an example,<br>
> > > > for example by providing how you think the above example should<br>
> > actually<br>
> > > > be encoded?<br>
> > > ><br>
> ><br>
</div></div></blockquote></div><br></div>