From patchwork Mon Dec 18 02:02:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1877178 X-Patchwork-Delegate: i.maximets@samsung.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Stjm46nSqz23yc for ; Mon, 18 Dec 2023 13:03:32 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1C6B2416C2; Mon, 18 Dec 2023 02:03:31 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1C6B2416C2 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6wWqzjzvHvO1; Mon, 18 Dec 2023 02:03:29 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id CBBAE416BE; Mon, 18 Dec 2023 02:03:28 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org CBBAE416BE Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B25F5C0077; Mon, 18 Dec 2023 02:03:28 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 18F57C0037 for ; Mon, 18 Dec 2023 02:03:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id C09634051C for ; Mon, 18 Dec 2023 02:03:11 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org C09634051C X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DOo6YEORsqPk for ; Mon, 18 Dec 2023 02:03:07 +0000 (UTC) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::226]) by smtp2.osuosl.org (Postfix) with ESMTPS id B885040503 for ; Mon, 18 Dec 2023 02:03:06 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org B885040503 Received: by mail.gandi.net (Postfix) with ESMTPSA id 6F7C6C0005; Mon, 18 Dec 2023 02:03:04 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Mon, 18 Dec 2023 03:02:43 +0100 Message-ID: <20231218020249.3447973-5-i.maximets@ovn.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20231218020249.3447973-1-i.maximets@ovn.org> References: <20231218020249.3447973-1-i.maximets@ovn.org> MIME-Version: 1.0 X-GND-Sasl: i.maximets@ovn.org Cc: Ilya Maximets , Dumitru Ceara Subject: [ovs-dev] [PATCH 4/5] ovsdb: Preserve column diffs read from the storage. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Database file contains the column diff, but it is discarded once the 'new' state of a row is constructed. Keep it in the transaction row, as it can be used later by other parts of the code. Diffs do not live long, we keep them around only while transaction is alive, so should not affect memory consumption. Users for this data will be added in later commits. Signed-off-by: Ilya Maximets Acked-by: Mike Pattrick --- ovsdb/execution.c | 14 ++++++++--- ovsdb/file.c | 22 +++++++++++----- ovsdb/ovsdb-server.c | 7 ++++-- ovsdb/table.c | 6 +++-- ovsdb/transaction.c | 60 +++++++++++++++++++++++++++++++++----------- ovsdb/transaction.h | 6 +++-- tests/test-ovsdb.c | 2 +- 7 files changed, 86 insertions(+), 31 deletions(-) diff --git a/ovsdb/execution.c b/ovsdb/execution.c index 5587ef96f..8c20c3b54 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -490,9 +490,11 @@ update_row_cb(const struct ovsdb_row *row, void *ur_) ur->n_matches++; if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) { + struct ovsdb_row *rw_row; + + ovsdb_txn_row_modify(ur->txn, row, &rw_row, NULL); ovsdb_error_assert(ovsdb_row_update_columns( - ovsdb_txn_row_modify(ur->txn, row), - ur->row, ur->columns, false)); + rw_row, ur->row, ur->columns, false)); } return true; @@ -572,10 +574,14 @@ static bool mutate_row_cb(const struct ovsdb_row *row, void *mr_) { struct mutate_row_cbdata *mr = mr_; + struct ovsdb_row *rw_row; + + /* Not trying to track the row diff here, because user transactions + * may attempt to add duplicates or remove elements that do not exist. */ + ovsdb_txn_row_modify(mr->txn, row, &rw_row, NULL); mr->n_matches++; - *mr->error = ovsdb_mutation_set_execute(ovsdb_txn_row_modify(mr->txn, row), - mr->mutations); + *mr->error = ovsdb_mutation_set_execute(rw_row, mr->mutations); return *mr->error == NULL; } diff --git a/ovsdb/file.c b/ovsdb/file.c index 8bd1d4af3..77a89fd1a 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -80,8 +80,8 @@ ovsdb_file_column_diff_disable(void) } static struct ovsdb_error * -ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting, - bool row_contains_diff, +ovsdb_file_update_row_from_json(struct ovsdb_row *row, struct ovsdb_row *diff, + bool converting, bool row_contains_diff, const struct json *json) { struct ovsdb_table_schema *schema = row->table->schema; @@ -122,6 +122,12 @@ ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting, error = ovsdb_datum_apply_diff_in_place( &row->fields[column->index], &datum, &column->type); + if (!error && diff) { + ovs_assert(ovsdb_datum_is_default(&diff->fields[column->index], + &column->type)); + ovsdb_datum_swap(&diff->fields[column->index], &datum); + } + ovsdb_datum_destroy(&datum, &column->type); if (error) { return error; @@ -150,16 +156,20 @@ ovsdb_file_txn_row_from_json(struct ovsdb_txn *txn, struct ovsdb_table *table, ovsdb_txn_row_delete(txn, row); return NULL; } else if (row) { - return ovsdb_file_update_row_from_json(ovsdb_txn_row_modify(txn, row), - converting, row_contains_diff, - json); + struct ovsdb_row *new, *diff = NULL; + + ovsdb_txn_row_modify(txn, row, &new, + row_contains_diff ? &diff : NULL); + return ovsdb_file_update_row_from_json(new, diff, converting, + row_contains_diff, json); } else { struct ovsdb_error *error; struct ovsdb_row *new; new = ovsdb_row_create(table); *ovsdb_row_get_uuid_rw(new) = *row_uuid; - error = ovsdb_file_update_row_from_json(new, converting, false, json); + error = ovsdb_file_update_row_from_json(new, NULL, converting, + false, json); if (error) { ovsdb_row_destroy(new); } else { diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 4d29043f4..dbf85fe3b 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -1111,7 +1111,7 @@ update_remote_row(const struct ovsdb_row *row, struct ovsdb_txn *txn, /* Bad remote spec or incorrect schema. */ return; } - rw_row = ovsdb_txn_row_modify(txn, row); + ovsdb_txn_row_modify(txn, row, &rw_row, NULL); ovsdb_jsonrpc_server_get_remote_status(jsonrpc, target, &status); /* Update status information columns. */ @@ -1301,7 +1301,10 @@ update_server_status(struct shash *all_dbs) if (!db || !db->db) { ovsdb_txn_row_delete(txn, row); } else { - update_database_status(ovsdb_txn_row_modify(txn, row), db); + struct ovsdb_row *rw_row; + + ovsdb_txn_row_modify(txn, row, &rw_row, NULL); + update_database_status(rw_row, db); } } diff --git a/ovsdb/table.c b/ovsdb/table.c index 0792e1580..3e89ddd44 100644 --- a/ovsdb/table.c +++ b/ovsdb/table.c @@ -415,8 +415,10 @@ ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid, NULL, &columns, xor); if (!error && (xor || !ovsdb_row_equal_columns(row, update, &columns))) { - error = ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row), - update, &columns, xor); + struct ovsdb_row *rw_row; + + ovsdb_txn_row_modify(txn, row, &rw_row, NULL); + error = ovsdb_row_update_columns(rw_row, update, &columns, xor); } ovsdb_column_set_destroy(&columns); diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index a482588a0..b69d03b5a 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -72,6 +72,8 @@ struct ovsdb_txn_table { * 'new'. * * - A row modified by a transaction will have non-null 'old' and 'new'. + * It may have non-null 'diff' as well in this case, but it is not + * necessary. * * - 'old' and 'new' both null indicates that a row was added then deleted * within a single transaction. Most of the time we instead delete the @@ -83,6 +85,7 @@ struct ovsdb_txn_row { struct hmap_node hmap_node; /* In ovsdb_txn_table's txn_rows hmap. */ struct ovsdb_row *old; /* The old row. */ struct ovsdb_row *new; /* The new row. */ + struct ovsdb_row *diff; /* The difference between old and new. */ size_t n_refs; /* Number of remaining references. */ /* These members are the same as the corresponding members of 'old' or @@ -155,6 +158,7 @@ ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED, { struct ovsdb_row *old = txn_row->old; struct ovsdb_row *new = txn_row->new; + struct ovsdb_row *diff = txn_row->diff; ovsdb_txn_row_prefree(txn_row); if (!old) { @@ -184,6 +188,7 @@ ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED, } ovsdb_row_destroy(new); + ovsdb_row_destroy(diff); free(txn_row); return NULL; @@ -250,7 +255,10 @@ find_or_make_txn_row(struct ovsdb_txn *txn, const struct ovsdb_table *table, if (!txn_row) { const struct ovsdb_row *row = ovsdb_table_get_row(table, uuid); if (row) { - txn_row = ovsdb_txn_row_modify(txn, row)->txn_row; + struct ovsdb_row *rw_row; + + ovsdb_txn_row_modify(txn, row, &rw_row, NULL); + txn_row = rw_row->txn_row; } } return txn_row; @@ -433,6 +441,7 @@ delete_garbage_row(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) return NULL; } + ovsdb_row_destroy(txn_row->diff); row = txn_row->new; txn_row->new = NULL; hmap_remove(&txn_row->table->rows, &row->hmap_node); @@ -544,6 +553,7 @@ ovsdb_txn_row_commit(struct ovsdb_txn *txn OVS_UNUSED, txn_row->new->n_refs = txn_row->n_refs; } ovsdb_row_destroy(txn_row->old); + ovsdb_row_destroy(txn_row->diff); free(txn_row); return NULL; @@ -1178,6 +1188,7 @@ ovsdb_txn_destroy_cloned(struct ovsdb_txn *txn) if (r->new) { ovsdb_row_destroy(r->new); } + ovs_assert(!r->diff); hmap_remove(&t->txn_rows, &r->hmap_node); free(r); } @@ -1439,7 +1450,8 @@ ovsdb_txn_create_txn_table(struct ovsdb_txn *txn, struct ovsdb_table *table) static struct ovsdb_txn_row * ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table, - const struct ovsdb_row *old_, struct ovsdb_row *new) + const struct ovsdb_row *old_, struct ovsdb_row *new, + struct ovsdb_row *diff) { const struct ovsdb_row *row = old_ ? old_ : new; struct ovsdb_row *old = CONST_CAST(struct ovsdb_row *, old_); @@ -1453,6 +1465,7 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table, txn_row->table = row->table; txn_row->old = old; txn_row->new = new; + txn_row->diff = diff; txn_row->n_refs = old ? old->n_refs : 0; txn_row->serial = serial - 1; @@ -1465,6 +1478,9 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table, if (new) { new->txn_row = txn_row; } + if (diff) { + diff->txn_row = txn_row; + } txn_table = ovsdb_txn_create_txn_table(txn, table); hmap_insert(&txn_table->txn_rows, &txn_row->hmap_node, @@ -1473,24 +1489,38 @@ ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table, return txn_row; } -struct ovsdb_row * -ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_) +void +ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_, + struct ovsdb_row **rw_row, struct ovsdb_row **diff) { struct ovsdb_row *ro_row = CONST_CAST(struct ovsdb_row *, ro_row_); + ovs_assert(ro_row); + ovs_assert(rw_row); + if (ro_row->txn_row) { ovs_assert(ro_row == ro_row->txn_row->new); - return ro_row; + *rw_row = ro_row; + if (diff) { + *diff = ro_row->txn_row->diff; + } else { + /* Caller will modify the row without updating the diff. + * Destroy the existing diff, if any, so it will not be + * used for this row anymore. Modification will always + * return NULL from this point on. */ + ovsdb_row_destroy(ro_row->txn_row->diff); + ro_row->txn_row->diff = NULL; + } } else { struct ovsdb_table *table = ro_row->table; - struct ovsdb_row *rw_row; - rw_row = ovsdb_row_clone(ro_row); - rw_row->n_refs = ro_row->n_refs; - ovsdb_txn_row_create(txn, table, ro_row, rw_row); - hmap_replace(&table->rows, &ro_row->hmap_node, &rw_row->hmap_node); - - return rw_row; + *rw_row = ovsdb_row_clone(ro_row); + (*rw_row)->n_refs = ro_row->n_refs; + if (diff) { + *diff = ovsdb_row_create(table); + } + ovsdb_txn_row_create(txn, table, ro_row, *rw_row, diff ? *diff : NULL); + hmap_replace(&table->rows, &ro_row->hmap_node, &(*rw_row)->hmap_node); } } @@ -1502,7 +1532,7 @@ ovsdb_txn_row_insert(struct ovsdb_txn *txn, struct ovsdb_row *row) uuid_generate(ovsdb_row_get_version_rw(row)); - ovsdb_txn_row_create(txn, table, NULL, row); + ovsdb_txn_row_create(txn, table, NULL, row, NULL); hmap_insert(&table->rows, &row->hmap_node, hash); } @@ -1518,9 +1548,11 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct ovsdb_row *row_) hmap_remove(&table->rows, &row->hmap_node); if (!txn_row) { - ovsdb_txn_row_create(txn, table, row, NULL); + ovsdb_txn_row_create(txn, table, row, NULL, NULL); } else { ovs_assert(txn_row->new == row); + ovsdb_row_destroy(txn_row->diff); + txn_row->diff = NULL; if (txn_row->old) { txn_row->new = NULL; } else { diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h index 0e054eef3..f659838dc 100644 --- a/ovsdb/transaction.h +++ b/ovsdb/transaction.h @@ -21,6 +21,7 @@ struct json; struct ovsdb; +struct ovsdb_row; struct ovsdb_schema; struct ovsdb_table; struct uuid; @@ -50,8 +51,9 @@ const struct ovsdb_error *ovsdb_txn_progress_get_error( const struct ovsdb_txn_progress *); void ovsdb_txn_progress_destroy(struct ovsdb_txn_progress *); -struct ovsdb_row *ovsdb_txn_row_modify(struct ovsdb_txn *, - const struct ovsdb_row *); +void ovsdb_txn_row_modify(struct ovsdb_txn *, const struct ovsdb_row *, + struct ovsdb_row **row_new, + struct ovsdb_row **row_diff); void ovsdb_txn_row_insert(struct ovsdb_txn *, struct ovsdb_row *); void ovsdb_txn_row_delete(struct ovsdb_txn *, const struct ovsdb_row *); diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index c761822e6..0394eafd2 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -1798,7 +1798,7 @@ do_transact_modify(struct ovs_cmdl_context *ctx) struct ovsdb_row *row_rw; row_ro = do_transact_find_row(ctx->argv[1]); - row_rw = ovsdb_txn_row_modify(do_transact_txn, row_ro); + ovsdb_txn_row_modify(do_transact_txn, row_ro, &row_rw, NULL); do_transact_set_i_j(row_rw, ctx->argv[2], ctx->argv[3]); }