Hi Ilya, Thanks for your patch! When I merge it into my project, it works to clear the error message. Looking forward for your final version in the community.
Best Regards! Gang Chen > -----Original Message----- > From: Ilya Maximets [mailto:i.maxim...@ovn.org] > Sent: Wednesday, September 8, 2021 7:16 PM > To: Chengang (Russell Lab) <chengan...@huawei.com>; Ilya Maximets > <i.maxim...@ovn.org>; Han Zhou <hz...@ovn.org> > Cc: d...@openvswitch.org; wangyunjian <wangyunj...@huawei.com>; > dingxiaoxiong <dingxiaoxi...@huawei.com>; Dumitru Ceara > <dce...@redhat.com> > Subject: Re: [ovs-dev] [PATCH] ovsdb-idl: Send "set_db_change_aware" before > "monitor_cond_since". > > On 9/8/21 5:01 AM, Chengang (Russell Lab) wrote: > > Hi Ilya, > > > >> On Tuesday, September 7, 2021 7:56 PM, Ilya Maximets > [mailto:i.maxim...@ovn.org] wrote: > >> I had a patch somewhere. I guess, I need to revise it and send. > > > > Can you info me where to get your patch, I would like to test it and reply > > you > the test result. > > I never sent it anywhere, but here it is: > > https://github.com/igsilya/ovs/commit/d37f663994f2cc3c2de6ed39d81b890fe > 0aa4f3f > > The final version of this patch will need changes, because we can't simply > switch all the clients from requests to notifications, because new clients > will > not be able to use db_change_aware feature with old servers. Need to think > how to perform this transition. > > > > > Best Regards! > > Gang Chen > > > >> -----Original Message----- > >> From: Ilya Maximets [mailto:i.maxim...@ovn.org] > >> Sent: Tuesday, September 7, 2021 7:56 PM > >> To: Chengang (Russell Lab) <chengan...@huawei.com>; Han Zhou > >> <hz...@ovn.org>; Ilya Maximets <i.maxim...@ovn.org> > >> Cc: d...@openvswitch.org; wangyunjian <wangyunj...@huawei.com>; > >> dingxiaoxiong <dingxiaoxi...@huawei.com>; Dumitru Ceara > >> <dce...@redhat.com> > >> Subject: Re: [ovs-dev] [PATCH] ovsdb-idl: Send "set_db_change_aware" > >> before "monitor_cond_since". > >> > >> On 9/6/21 3:51 PM, Chengang (Russell Lab) wrote: > >>> Hi Han and Ilya, > >>> > >>> This patch fix the issue: IDL client closes the connection after got > >>> the response to the monitor_cond_since request, without wait for the > >>> reponse to the set_db_change_aware request. Warning message will be > >>> add to ovsdb-server.log > >>> > >>> It makes user confused the issue will occurred even with the command > >> "ovs-vsctl show" or ovs-vsctl other commands. > >>> > >>> Is there any other ideas on this issue? > >> > >> The root cause of a problem is that set_db_change_aware request is > >> hacked into the IDL state machine and IDL doesn't wait for reply and > >> doesn't handle the reply in any way. Moving this hack around the IDl > >> code doesn't seem to be a good solution as it interferes with the > >> rest of the state machine which we definitely don't want to accidentally > break. > >> > >> The correct way to eliminate these benign warnings, as we discussed > >> previously, is to create a new set_db_change_aware notification (not > >> a request), so it will not require sending reply from a server. This > >> will fix the problem and will also make IDL code logically correct. > >> The downside of this is that if new clients will connect to old > >> servers, they will not be able to use the feature if notification is > >> used. So, there should be some transition period until we can enable use > of notifications by default. > >> > >> I had a patch somewhere. I guess, I need to revise it and send. > >> > >>> If this patch can fix it, can we back port this patch to 2.13.0 as > >>> it has the same > >> issue? > >> > >> The issue exists starting from 2.9, however, the correct solution (to > >> use > >> notifications) doesn't seem backportable. > >> > >> Best regards, Ilya Maximets. > >> > >>> > >>> Best Regards! > >>> Gang Chen > >>> > >>>> -----Original Message----- > >>>> From: dev [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of > >>>> Dumitru Ceara > >>>> Sent: Friday, July 10, 2020 7:33 PM > >>>> To: d...@openvswitch.org > >>>> Cc: Han Zhou <hz...@ovn.org>; Ilya Maximets <i.maxim...@ovn.org> > >>>> Subject: [ovs-dev] [PATCH] ovsdb-idl: Send "set_db_change_aware" > >>>> before "monitor_cond_since". > >>>> > >>>> For short lived IDL clients (e.g., ovn-sbctl) if the client sends > >>>> monitor_cond_since before set_db_change_aware, the client might > >>>> close the DB connection immediately after it received the reply for > >>>> monitor_cond_since and before the server has a chance to reply to > >> set_db_change_aware. > >>>> > >>>> E.g., from the logs of the ovsdb-server: > >>>> 2020-07-10T09:29:52.649Z|04479|jsonrpc|DBG|unix#72: received > >>>> request, method="monitor_cond_since", params=["OVN_Southbound", > >>>> ["monid","OVN_Southbound"],{"SB_Global":[{"columns":["options"]}]}, > >>>> "00000000-0000-0000-0000-000000000000"], id=2 > >>>> 2020-07-10T09:29:52.649Z|04480|jsonrpc|DBG|unix#72: send reply, > >>>> result=[false,"00000000-0000-0000-0000-000000000000", > >>>> {"SB_Global":{"6ad26b48-a742-4fe1-8671-3975e2146ce6":{"initial": > >>>> {"options":["map",[["mac_prefix","be:85:cb"],["svc_monitor_mac", > >>>> "52:58:b5:19:8c:40"]]]}}}}], id=2 > >>>> 2020-07-10T09:29:52.649Z|04482|jsonrpc|DBG|unix#72: received > >>>> request, method="set_db_change_aware", params=[true], id=3 > >>>> > >>>> <<< IDL client closes the connection here because it already got > >>>> the response to the monitor_cond_since request. > >>>> > >>>> 2020-07-10T09:29:59.023Z|04483|jsonrpc|DBG|unix#72: send reply, > >>>> result={}, > >>>> id=3 > >>>> 2020-07-10T09:29:59.023Z|04484|stream_fd|DBG|send: Broken pipe > >>>> 2020-07-10T09:29:59.023Z|04485|jsonrpc|WARN|unix#72: send error: > >>>> Broken pipe > >>>> > >>>> While this is not a critical issue, it can be easily mitigated by > >>>> changing the IDL client to always send "set_db_change_aware" before > >> "monitor_cond_since". > >>>> This way we ensure that a well behaving IDL client doesn't close > >>>> the connection too early, avoiding the error logs in ovsdb-server. > >>>> > >>>> This patch moves the code to send monitor_cond_since(data) from > >>>> function > >>>> ovsdb_idl_check_server_db() to ovsdb_idl_process_response() as we > >>>> can transition to IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED only > upon > >>>> receiving a reply for monitor_cond(server). > >>>> > >>>> CC: Ben Pfaff <b...@ovn.org> > >>>> CC: Han Zhou <hz...@ovn.org> > >>>> CC: Ilya Maximets <i.maxim...@ovn.org> > >>>> Reported-by: Girish Moodalbail <gmoodalb...@gmail.com> > >>>> Reported-at: > >>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-July/050343 > >>>> .h > >>>> tml > >>>> Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for > >>>> clustered > >>>> databases.") > >>>> Signed-off-by: Dumitru Ceara <dce...@redhat.com> > >>>> --- > >>>> lib/ovsdb-idl.c | 7 ++++--- > >>>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index > >>>> ef3b97b..c6427f5 > >>>> 100644 > >>>> --- a/lib/ovsdb-idl.c > >>>> +++ b/lib/ovsdb-idl.c > >>>> @@ -770,6 +770,10 @@ ovsdb_idl_process_response(struct ovsdb_idl > >>>> *idl, struct jsonrpc_msg *msg) > >>>> > >>>> OVSDB_IDL_MM_MONITOR_COND); > >>>> if (ovsdb_idl_check_server_db(idl)) { > >>>> ovsdb_idl_send_db_change_aware(idl); > >>>> + ovsdb_idl_send_monitor_request( > >>>> + idl, &idl->data, > >>>> OVSDB_IDL_MM_MONITOR_COND_SINCE); > >>>> + ovsdb_idl_transition( > >>>> + idl, > >>>> IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED); > >>>> } > >>>> } else { > >>>> ovsdb_idl_send_schema_request(idl, &idl->data); @@ > >>>> -2057,9 > >>>> +2061,6 @@ ovsdb_idl_check_server_db(struct ovsdb_idl *idl) > >>>> if (idl->state == IDL_S_SERVER_MONITOR_COND_REQUESTED) { > >>>> json_destroy(idl->data.schema); > >>>> idl->data.schema = json_from_string(database->schema); > >>>> - ovsdb_idl_send_monitor_request(idl, &idl->data, > >>>> - > >>>> OVSDB_IDL_MM_MONITOR_COND_SINCE); > >>>> - ovsdb_idl_transition(idl, > >>>> IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED); > >>>> } > >>>> return true; > >>>> } > >>>> -- > >>>> 1.8.3.1 > >>>> > >>>> _______________________________________________ > >>>> dev mailing list > >>>> d...@openvswitch.org > >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
