[ovs-dev] [PATCH v2 2/2] ovsdb-idl: Preserve references for deleted rows.

Dumitru Ceara dceara at redhat.com
Fri Mar 12 12:11:50 UTC 2021


On 3/10/21 9:33 AM, Han Zhou wrote:
> 
> 
> On Tue, Mar 9, 2021 at 9:03 AM Dumitru Ceara <dceara at redhat.com 
> <mailto:dceara at redhat.com>> wrote:
>  >
>  > On 3/9/21 5:56 PM, Ilya Maximets wrote:
>  > > On 3/9/21 5:54 PM, Ilya Maximets wrote:
>  > >> On 3/9/21 5:26 PM, Dumitru Ceara wrote:
>  > >>> On 3/9/21 4:07 PM, Ilya Maximets wrote:
>  > >>>> On 3/3/21 3:40 PM, Dumitru Ceara wrote:
>  > >>>>> Considering two DB rows, 'a' from table A and 'b' from table B 
> (with
>  > >>>>> column 'ref_a' a reference to table A):
>  > >>>>> a = {A._uuid=<U1>}
>  > >>>>> b = {B._uuid=<U2>, B.ref_a=<U1>}
>  > >>>>>
>  > >>>>> Assuming both records are present in the IDL client's in-memory 
> view of
>  > >>>>> the database, depending whether row 'b' is also deleted in the same
>  > >>>>> transaction or not, deletion of row 'a' should generate the 
> following
>  > >>>>> tracked changes:
>  > >>>>>
>  > >>>>> 1. only row 'a' is deleted:
>  > >>>>> - for table A:
>  > >>>>>     - deleted records: a = {A._uuid=<U1>}
>  > >>>>> - for table B:
>  > >>>>>     - updated records: b = {B._uuid=<U2>, B.ref_a=[]}
>  > >>>>>
>  > >>>>> 2. row 'a' and row 'b' are deleted in the same update:
>  > >>>>> - for table A:
>  > >>>>>     - deleted records: a = {A._uuid=<U1>}
>  > >>>>> - for table B:
>  > >>>>>     - deleted records: b = {B._uuid=<U2>, B.ref_a=<U1>}
>  > >>>>>
>  > >>>>> To ensure this, we now delay reparsing row backrefs until the 
> row has
>  > >>>>> been removed from the table's hmap and until the IDL client has
>  > >>>>> processed all tracked changes (ovsdb_idl_track_clear() was called).
>  > >>>>>
>  > >>>>> Without this change, in scenario 2 above, the tracked changes 
> for table
>  > >>>>> B would be:
>  > >>>>> - deleted records: b = {B._uuid=<U2>, B.ref_a=[]}
>  > >>>>>
>  > >>>>> In particular, for strong references, row 'a' can never be 
> deleted in
>  > >>>>> a transaction that happens strictly before row 'b' is deleted.  
> In some
>  > >>>>> cases [0] both rows are deleted in the same transaction and having
>  > >>>>> B.ref_a=[] would violate the integrity of the database from client
>  > >>>>> perspective.  This would force the client to always validate that
>  > >>>>> strong reference fields are non-NULL.  This is not really an option
>  > >>>>> because the information in the original reference is required for
>  > >>>>> incrementally processing the record deletion.
>  > >>>>>
>  > >>>>> [0] with ovn-monitor-all=true, the following command triggers a 
> crash
>  > >>>>>       in ovn-controller because a strong reference field 
> becomes NULL:
>  > >>>>>       $ ovn-nbctl --wait=hv -- lr-add r -- lrp-add r rp 
> 00:00:00:00:00:01 1.0.0.1/24 <http://1.0.0.1/24>
>  > >>>>>       $ ovn-nbctl lr-del r
>  > >>>>>
>  > >>>>> Reported-at: https://bugzilla.redhat.com/1932642 
> <https://bugzilla.redhat.com/1932642>
>  > >>>>> Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for 
> deleted rows.")
>  > >>>>> Signed-off-by: Dumitru Ceara <dceara at redhat.com 
> <mailto:dceara at redhat.com>>
>  > >>>>> ---
>  > >>>>> v2:
>  > >>>>>     - Added ovsdb-idl.at <http://ovsdb-idl.at> test for strong 
> references.
>  > >>>>> ---
>  > >>>>>    lib/ovsdb-idl.c    |    6 ++-
>  > >>>>>    tests/ovsdb-idl.at <http://ovsdb-idl.at> |  118 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  > >>>>>    tests/test-ovsdb.c |   45 ++++++++++++++++++++
>  > >>>>>    3 files changed, 167 insertions(+), 2 deletions(-)
>  > >>>>>
>  > >>>>> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
>  > >>>>> index 9e1e787..ecd4924 100644
>  > >>>>> --- a/lib/ovsdb-idl.c
>  > >>>>> +++ b/lib/ovsdb-idl.c
>  > >>>>> @@ -163,6 +163,7 @@ static void ovsdb_idl_row_unparse(struct 
> ovsdb_idl_row *);
>  > >>>>>    static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *);
>  > >>>>>    static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *);
>  > >>>>>    static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, 
> bool destroy_dsts);
>  > >>>>> +static void ovsdb_idl_row_reparse_backrefs(struct 
> ovsdb_idl_row *row);
>  > >>>>>      static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
>  > >>>>>    static bool ovsdb_idl_txn_extract_mutations(struct 
> ovsdb_idl_row *,
>  > >>>>> @@ -367,6 +368,8 @@ ovsdb_idl_clear(struct ovsdb_idl *db)
>  > >>>>>                    ovsdb_idl_row_unparse(row);
>  > >>>>>                }
>  > >>>>>                LIST_FOR_EACH_SAFE (arc, next_arc, src_node, 
> &row->src_arcs) {
>  > >>>>> +                ovs_list_remove(&arc->src_node);
>  > >>>>> +                ovs_list_remove(&arc->dst_node);
>  > >>>>>                    free(arc);
>  > >>>>>                }
>  > >>>>>                /* No need to do anything with dst_arcs: some 
> node has those arcs
>  > >>>>> @@ -1234,6 +1237,7 @@ ovsdb_idl_track_clear__(struct ovsdb_idl 
> *idl, bool flush_all)
>  > >>>>>                    ovs_list_remove(&row->track_node);
>  > >>>>>                    ovs_list_init(&row->track_node);
>  > >>>>>                    if (ovsdb_idl_row_is_orphan(row)) {
>  > >>>>> +                    ovsdb_idl_row_reparse_backrefs(row);
>  > >>>>>                        ovsdb_idl_row_unparse(row);
>  > >>>>>                        if (row->tracked_old_datum) {
>  > >>>>>                            const struct ovsdb_idl_table_class 
> *class =
>  > >>>>> @@ -2156,8 +2160,6 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row 
> *row)
>  > >>>>>        ovsdb_idl_row_clear_old(row);
>  > >>>>>        if (ovs_list_is_empty(&row->dst_arcs)) {
>  > >>>>>            ovsdb_idl_row_destroy(row);
>  > >>>>> -    } else {
>  > >>>>> -        ovsdb_idl_row_reparse_backrefs(row);
>  > >>>>>        }
>  > >>>>
>  > >>>> By removing the ovsdb_idl_row_reparse_backrefs() you're making 
> an assumption
>  > >>>> that we always have a modification event for all rows that are 
> referencing
>  > >>>> this one.  But that is not always the case.
>  > >>>>
>  > >>>> There is scenario where this re-parsing is needed.  In short, if 
> you're
>  > >>>> reducing the scope of conditional monitoring and some rows becomes
>  > >>>> orphan rows.  In this case ovsdb-server will not generate 
> 'modify' event
>  > >>>> for the row that references our newly orphan row, so it will not be
>  > >>>> re-parsed and these orphan rows will be accessible the row that 
> references
>  > >>>> them.
>  > >>>
>  > >>> You're right, thanks for spotting this and for the test case.
>  > >>>
>  > >>>>
>  > >>>> Here is a quick'n'dirty testcase, how to reproduce:
>  > >>>> diff --git a/tests/ovsdb-idl.at <http://ovsdb-idl.at> 
> b/tests/ovsdb-idl.at <http://ovsdb-idl.at>
>  > >>>> index 702502280..c6bb3f348 100644
>  > >>>> --- a/tests/ovsdb-idl.at <http://ovsdb-idl.at>
>  > >>>> +++ b/tests/ovsdb-idl.at <http://ovsdb-idl.at>
>  > >>>> @@ -1174,7 +1174,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, 
> initially populated],
>  > >>>>    dnl This test creates database with weak references and 
> checks that orphan
>  > >>>>    dnl rows created for weak references are not available for 
> iteration via
>  > >>>>    dnl list of tracked changes.
>  > >>>> -OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, 
> orphan weak references],
>  > >>>> +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, 
> orphan weak references quickndirty],
>  > >>>>      [['["idltest",
>  > >>>>          {"op": "insert",
>  > >>>>           "table": "simple",
>  > >>>> @@ -1196,7 +1196,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, 
> initially populated, orphan weak refer
>  > >>>>                                  ["named-uuid", "weak_row1"],
>  > >>>>                                  ["named-uuid", "weak_row2"]]
>  > >>>>                               ]}}]']],
>  > >>>> -  [['condition simple []' \
>  > >>>> +  [['condition simple [true]' \
>  > >>>>        'condition simple [["s","==","row1_s"]]' \
>  > >>>>        '["idltest",
>  > >>>>          {"op": "update",
>  > >>>> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
>  > >>>> index 36fb9cc08..ace87b13d 100644
>  > >>>> --- a/tests/test-ovsdb.c
>  > >>>> +++ b/tests/test-ovsdb.c
>  > >>>> @@ -2157,6 +2157,7 @@ print_idl_row_singleton(const struct 
> idltest_singleton *sng, int step)
>  > >>>>    static void
>  > >>>>    print_idl(struct ovsdb_idl *idl, int step)
>  > >>>>    {
>  > >>>> +    const struct idltest_simple6 *s6;
>  > >>>>        const struct idltest_simple *s;
>  > >>>>        const struct idltest_link1 *l1;
>  > >>>>        const struct idltest_link2 *l2;
>  > >>>> @@ -2175,6 +2176,10 @@ print_idl(struct ovsdb_idl *idl, int step)
>  > >>>>            print_idl_row_link2(l2, step);
>  > >>>>            n++;
>  > >>>>        }
>  > >>>> +    IDLTEST_SIMPLE6_FOR_EACH (s6, idl) {
>  > >>>> +        print_idl_row_simple6(s6, step);
>  > >>>> +        n++;
>  > >>>> +    }
>  > >>>>        IDLTEST_SINGLETON_FOR_EACH (sng, idl) {
>  > >>>>            print_idl_row_singleton(sng, step);
>  > >>>>            n++;
>  > >>>> @@ -2638,8 +2643,12 @@ do_idl(struct ovs_cmdl_context *ctx)
>  > >>>>                  /* Print update. */
>  > >>>>                if (track) {
>  > >>>> -                print_idl_track(idl, step++);
>  > >>>> +                print_idl_track(idl, step);
>  > >>>> +                print_and_log("%03d: print_idl[1]", step);
>  > >>>> +                print_idl(idl, step);
>  > >>>>                    ovsdb_idl_track_clear(idl);
>  > >>>> +                print_and_log("%03d: print_idl[2]", step);
>  > >>>> +                print_idl(idl, step++);
>  > >>>>                } else {
>  > >>>>                    print_idl(idl, step++);
>  > >>>>                }
>  > >>>> ---
>  > >>>>
>  > >>>> By running make -j8 check TESTSUITEFLAGS='-k quickndirty -v', 
> you will have
>  > >>>> some output for the commands in the log.
>  > >>>>
>  > >>>>
>  > >>>> The first condition for a 'simple' table is 'true', so we will 
> receive all
>  > >>>> 3 rows from this table and also row from the 'simple6'.
>  > >>>> All 3 rows are in a weak references:
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row0_s 
> <> uuid=2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row2_s 
> <> uuid=1fd88ded-06f0-4fee-a21b-4e552d8a2f50
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row1_s 
> <> uuid=124ddccb-7eb8-4def-b768-887cb8d84ada
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple6: inserted row: 
> name=first_row weak_ref=[124ddccb-7eb8-4def-b768-887cb8d84ada 
> 1fd88ded-06f0-4fee-a21b-4e552d8a2f50 2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0]
>  > >>>> test-ovsdb|test_ovsdb|001: table simple6: updated columns: name 
> weak_ref
>  > >>>>
>  > >>>>
>  > >>>> All of them are accessible for a user before the track_clear():
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|001: print_idl[1]
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row0_s 
> <> uuid=2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row2_s 
> <> uuid=1fd88ded-06f0-4fee-a21b-4e552d8a2f50
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row1_s 
> <> uuid=124ddccb-7eb8-4def-b768-887cb8d84ada
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: updated columns: s
>  > >>>> test-ovsdb|test_ovsdb|001: table simple6: inserted row: 
> name=first_row weak_ref=[124ddccb-7eb8-4def-b768-887cb8d84ada 
> 1fd88ded-06f0-4fee-a21b-4e552d8a2f50 2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0]
>  > >>>> test-ovsdb|test_ovsdb|001: table simple6: updated columns: name 
> weak_ref
>  > >>>>
>  > >>>> And after:
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|001: print_idl[2]
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row0_s 
> <> uuid=2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row2_s 
> <> uuid=1fd88ded-06f0-4fee-a21b-4e552d8a2f50
>  > >>>> test-ovsdb|test_ovsdb|001: table simple: inserted row: s=row1_s 
> <> uuid=124ddccb-7eb8-4def-b768-887cb8d84ada
>  > >>>> test-ovsdb|test_ovsdb|001: table simple6: name=first_row 
> weak_ref=[124ddccb-7eb8-4def-b768-887cb8d84ada 
> 1fd88ded-06f0-4fee-a21b-4e552d8a2f50 2309fce8-a6dd-46e5-a603-fe1ea1ab6bb0]
>  > >>>>
>  > >>>>
>  > >>>> Now condition is changed.  This makes 2 of 3 rows to become 
> orphan rows.
>  > >>>> Note that the update notification only removes orphan rows, but 
> doesn't
>  > >>>> modify the row in simple6, because rows wasn't actually removed, 
> just
>  > >>>> not needed anymore:
>  > >>>>
>  > >>>> test-ovsdb|jsonrpc|unix:socket: send request, 
> method="monitor_cond_change", 
> params=[["monid","idltest"],["monid","idltest"],{"simple":[{"where":[["s","==","row1_s"]]}]}], 
> id=5
>  > >>>> test-ovsdb|jsonrpc|unix:socket: received notification, 
> method="update3", 
> params=[["monid","idltest"],"00000000-0000-0000-0000-000000000000",{"simple":{"3aa79e4a-d752-4d53-8e16-b62887d73ad8":{"delete":null},"d1b023f3-3b4d-4bb5-b4d9-cc9791afde7a":{"delete":null}}}]
>  > >>>> test-ovsdb|jsonrpc|unix:socket: received reply, result={}, id=5
>  > >>>>
>  > >>>> Track list is empty, but 2 orphan rows should be on a track list 
> as deleted:
>  > >>>
>  > >>> This actually uncovers a new potential issue: the 2 orphan rows 
> are not on the tracked list even without my change.  That's because 
> ovsdb_idl_row_destroy() doesn't get called if the row becomes orphan so 
> it won't be added to 'row->table->track_list'.
>  > >>
>  > >> Hmm.  Thanks for sponning that.
>  > >
>  > > *spotting
>  > >
>  > >>
>  > >> On current master these 2 rows appears on a tracked list only after
>  > >> occasional unrelated modification of a row in 'simple6' and that is
>  > >> not good:
>  > >>
>  > >> test-ovsdb|test_ovsdb|002: change conditions
>  > >> test-ovsdb|jsonrpc|unix:socket: send request, 
> method="monitor_cond_change", 
> params=[["monid","idltest"],["monid","idltest"],{"simple":[{"where":[["s","==","row1_s"]]}]}], 
> id=5
>  > >> test-ovsdb|jsonrpc|unix:socket: received notification, 
> method="update3", 
> params=[["monid","idltest"],"00000000-0000-0000-0000-000000000000",{"simple":{"0f94ec57-2fc2-4a70-a0cf-b96b11fe0d78":{"delete":null},"5a1d0257-fc27-4046-9e18-25539f122052":{"delete":null}}}]
>  > >> test-ovsdb|jsonrpc|unix:socket: received reply, result={}, id=5
>  > >> test-ovsdb|test_ovsdb|003: empty
>  > >> test-ovsdb|test_ovsdb|003: print_idl[1]
>  > >> test-ovsdb|test_ovsdb|003: table simple: s=row1_s 
> uuid=77db422f-042f-4e1f-9a40-6487e5cdd3ae
>  > >> test-ovsdb|test_ovsdb|003: table simple6: name=first_row 
> weak_ref=[77db422f-042f-4e1f-9a40-6487e5cdd3ae]
>  > >> test-ovsdb|test_ovsdb|003: print_idl[2]
>  > >> test-ovsdb|test_ovsdb|003: table simple: s=row1_s 
> uuid=77db422f-042f-4e1f-9a40-6487e5cdd3ae
>  > >> test-ovsdb|test_ovsdb|003: table simple6: name=first_row 
> weak_ref=[77db422f-042f-4e1f-9a40-6487e5cdd3ae]
>  > >>
>  > >> test-ovsdb|jsonrpc|unix:socket: send request, method="transact", 
> params=["idltest",{"where":[],"row":{"name":"new_name"},"op":"update","table":"simple6"}], 
> id=6
>  > >> test-ovsdb|jsonrpc|unix:socket: received reply, 
> result=[{"count":1}], id=6
>  > >> test-ovsdb|test_ovsdb|004: {"error":null,"result":[{"count":1}]}
>  > >> test-ovsdb|jsonrpc|unix:socket: received notification, 
> method="update3", 
> params=[["monid","idltest"],"00000000-0000-0000-0000-000000000000",{"simple6":{"618cef34-e635-4841-9dc7-a14eb2b9627e":{"modify":{"name":"new_name"}}}}]
>  > >> test-ovsdb|test_ovsdb|005: table simple: deleted row: s=row2_s 
> uuid=5a1d0257-fc27-4046-9e18-25539f122052
>  > >> test-ovsdb|test_ovsdb|005: table simple: deleted row: s=row0_s 
> uuid=0f94ec57-2fc2-4a70-a0cf-b96b11fe0d78
>  > >> test-ovsdb|test_ovsdb|005: table simple6: name=new_name 
> weak_ref=[77db422f-042f-4e1f-9a40-6487e5cdd3ae]
>  > >> test-ovsdb|test_ovsdb|005: table simple6: updated columns: name
>  > >>
>  > >>
>  > >> It's probably because ovsdb_idl_row_reparse_backrefs() calls
>  > >> ovsdb_idl_row_clear_arcs() with destroy_dsts=false.  So, re-parsing
>  > >> invoked for two deleted rows doesn't actually destroy them.
>  > >>
>  > >>>
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|003: empty
>  > >>>>
>  > >>>> But all 3 rows are still accessible for a user from the row in 
> 'simple6' even
>  > >>>> though table 'simple' has only 1 row accessible:
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|003: print_idl[1]
>  > >>>> test-ovsdb|test_ovsdb|003: table simple: s=row1_s <> 
> uuid=b8315694-7855-43e7-a963-ca9a8666682a
>  > >>>> test-ovsdb|test_ovsdb|003: table simple6: name=first_row 
> weak_ref=[3aa79e4a-d752-4d53-8e16-b62887d73ad8 
> b8315694-7855-43e7-a963-ca9a8666682a d1b023f3-3b4d-4bb5-b4d9-cc9791afde7a]
>  > >>>>
>  > >>>> And they are still here after the track_clear():
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|003: print_idl[2]
>  > >>>> test-ovsdb|test_ovsdb|003: table simple: s=row1_s <> 
> uuid=b8315694-7855-43e7-a963-ca9a8666682a
>  > >>>> test-ovsdb|test_ovsdb|003: table simple6: name=first_row 
> weak_ref=[3aa79e4a-d752-4d53-8e16-b62887d73ad8 
> b8315694-7855-43e7-a963-ca9a8666682a d1b023f3-3b4d-4bb5-b4d9-cc9791afde7a]
>  > >>>>
>  > >>>>
>  > >>>> When we have unrelated update on a row in 'simple6' it finally gets
>  > >>>> re-parsed and we have tracked orphan rows as deleted and they are
>  > >>>> no longer accessible for a user from a row in 'simple6':
>  > >>>>
>  > >>>> test-ovsdb|jsonrpc|unix:socket: received notification, 
> method="update3", 
> params=[["monid","idltest"],"00000000-0000-0000-0000-000000000000",{"simple6":{"6fcb8599-4161-4314-901c-2ca4a18686c3":{"modify":{"name":"new_name"}}}}]
>  > >>>>
>  > >>>> Tracked:
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|005: table simple: deleted row: s=row0_s 
> <> uuid=d1b023f3-3b4d-4bb5-b4d9-cc9791afde7a
>  > >>>> test-ovsdb|test_ovsdb|005: table simple: deleted row: s=row2_s 
> <> uuid=3aa79e4a-d752-4d53-8e16-b62887d73ad8
>  > >>>> test-ovsdb|test_ovsdb|005: table simple6: name=new_name 
> weak_ref=[b8315694-7855-43e7-a963-ca9a8666682a]
>  > >>>> test-ovsdb|test_ovsdb|005: table simple6: updated columns: name
>  > >>>>
>  > >>>> Before track_clear():
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|005: print_idl[1]
>  > >>>> test-ovsdb|test_ovsdb|005: table simple: s=row1_s <> 
> uuid=b8315694-7855-43e7-a963-ca9a8666682a
>  > >>>> test-ovsdb|test_ovsdb|005: table simple6: name=new_name 
> weak_ref=[b8315694-7855-43e7-a963-ca9a8666682a]
>  > >>>> test-ovsdb|test_ovsdb|005: table simple6: updated columns: name
>  > >>>>
>  > >>>> After:
>  > >>>>
>  > >>>> test-ovsdb|test_ovsdb|005: print_idl[2]
>  > >>>> test-ovsdb|test_ovsdb|005: table simple: s=row1_s <> 
> uuid=b8315694-7855-43e7-a963-ca9a8666682a
>  > >>>> test-ovsdb|test_ovsdb|005: table simple6: name=new_name 
> weak_ref=[b8315694-7855-43e7-a963-ca9a8666682a]
>  > >>>>
>  > >>>
>  > >>> To cover all cases above I'm thinking of the following approach:
>  > >>>
>  > >>> 1. Delay ovsdb_idl_row_reparse_backrefs() for deleted rows until 
> ovsdb_idl_parse_update() has finished.
>  > >>>
>  > >>> 2. After ovsdb_idl_parse_update(), walk all deleted rows and 
> reparse backrefs but only for src rows that have not been deleted.
> 
> Sounds good. This step seems natural because in step 1) 
> ovsdb_idl_delete_row() will clear forward arcs for deleted rows, so in 
> this step 2) it won't reparse for deleted src rows.
> I *hope* it covers all cases. This part of code becomes really tricky. I 
> think maybe the tracking code needs a refactor, but I am also worried 
> about creating even more problems. (I haven't checked yet how ddlog 
> change tracking is done).
> 

Thanks, Han, Ilya and Numan for reviews!

I just sent a v3:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=233663

Numan, I didn't add your ack because I had to make quite a few changes 
to address the bug Ilya pointed out above.  I'd appreciate it very much 
if you could have another look at v3.

Thanks,
Dumitru



More information about the dev mailing list