[ovs-dev] [PATCH 2/3] ovsdb-idl: Fix memleak when deleting orphan rows.

Han Zhou hzhou at ovn.org
Mon Nov 30 04:52:05 UTC 2020


On Tue, Nov 24, 2020 at 4:16 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Pure IDL orphan rows, i.e., for which no "insert" operation was seen,
> which are part of tables with change tracking enabled should also be
> freed when the table track_list is flushed.

Hi Dumitru, does this change suggest that there are situations that a
deleted row is tracked but there is no tracked_old_datum? Could you give an
example of how it could happen? If a client didn't see a row "inserted"
then how could the server send a notification about the "deletion"? If a
row is never seen by a client IDL then I think it shouldn't be included in
the update notification.

>
> Reported-by: Ilya Maximets <i.maximets at ovn.org>
> Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted
rows.")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
>  lib/ovsdb-idl.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index bbc22e4..b282ce4 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -1969,16 +1969,18 @@ ovsdb_idl_db_track_clear(struct ovsdb_idl_db *db)
>
>                  ovs_list_remove(&row->track_node);
>                  ovs_list_init(&row->track_node);
> -                if (ovsdb_idl_row_is_orphan(row) &&
row->tracked_old_datum) {
> +                if (ovsdb_idl_row_is_orphan(row)) {
>                      ovsdb_idl_row_unparse(row);
> -                    const struct ovsdb_idl_table_class *class =
> -
 row->table->class_;
> -                    for (size_t c = 0; c < class->n_columns; c++) {
> -                        ovsdb_datum_destroy(&row->tracked_old_datum[c],
> -                                            &class->columns[c].type);
> +                    if (row->tracked_old_datum) {
> +                        const struct ovsdb_idl_table_class *class =
> +                            row->table->class_;
> +                        for (size_t c = 0; c < class->n_columns; c++) {
> +
 ovsdb_datum_destroy(&row->tracked_old_datum[c],
> +                                                &class->columns[c].type);
> +                        }
> +                        free(row->tracked_old_datum);
> +                        row->tracked_old_datum = NULL;
>                      }
> -                    free(row->tracked_old_datum);
> -                    row->tracked_old_datum = NULL;
>                      free(row);
>                  }
>              }
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list