On Tue, Nov 24, 2020 at 4:16 AM Dumitru Ceara <[email protected]> wrote:
>
> It's possible that the IDL client processes multiple jsonrpc updates
> in a single ovsdb_idl_run().
>
> Considering the following updates processed in a single IDL run:
> 1. Delete row R1 from table A while R1 is also referenced by row R2 from
> table B:
> - because row R2 still refers to row R1, this will create an orphan
> R1 but also sets row->tracked_old_datum to report to the IDL client
> that the row has been deleted.
> 2. Insert row R1 to table A.
> - because orphan R1 already existed in the IDL, it will be reused.
> - R1 still has row->tracked_old_datum set (and may also be on the
> table->track_list).
> 3. Delete row R2 from table B and row R1 from table A.
> - row->tracked_old_datum is set again but the previous
> tracked_old_datum was never freed.
>
> IDL clients don't currently use the deleted old_datum values but they
The old_datum which is set to tracked_old_datum is in fact used when the
client accesses the data of the tracked deleted row. At least it was used
in ovn-controller.
> might decide to do that in the future so when multiple delete
> operations are received for a row, always track the first one as that
> will match the contents of the row the IDL client knew about.
>
> Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted
rows.")
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> lib/ovsdb-idl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 6334061..bbc22e4 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -3219,7 +3219,7 @@ ovsdb_idl_row_clear_old(struct ovsdb_idl_row *row)
> {
> ovs_assert(row->old_datum == row->new_datum);
> if (!ovsdb_idl_row_is_orphan(row)) {
> - if (ovsdb_idl_track_is_set(row->table)) {
> + if (ovsdb_idl_track_is_set(row->table) &&
!row->tracked_old_datum) {
> row->tracked_old_datum = row->old_datum;
> } else {
> const struct ovsdb_idl_table_class *class =
row->table->class_;
>
Thanks for the patch. The change looks correct (at least not harmful), but
I'm not sure if I understand the scenario completely. In step 2, how could
someone insert a row that has already been deleted? Does the client specify
the row UUID in this case? Would it be better to add a test case which
could trigger the problem?
Thanks,
Han
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
