From patchwork Mon Oct 12 14:31:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Gray X-Patchwork-Id: 1380950 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PevJfp2j; dkim-atps=neutral Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4C91NB1rnSz9sTf for ; Tue, 13 Oct 2020 01:32:10 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9C82486D0F; Mon, 12 Oct 2020 14:32:08 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3jhYB+aV61t3; Mon, 12 Oct 2020 14:32:02 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 53A1786CBD; Mon, 12 Oct 2020 14:32:02 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 12029C0893; Mon, 12 Oct 2020 14:32:02 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0BD56C0051 for ; Mon, 12 Oct 2020 14:32:00 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id E0E0E2150A for ; Mon, 12 Oct 2020 14:31:59 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hyJwUxSL8hR8 for ; Mon, 12 Oct 2020 14:31:58 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by silver.osuosl.org (Postfix) with ESMTPS id DB4A1204EB for ; Mon, 12 Oct 2020 14:31:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1602513116; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=yIyU/LNnOnEhj5JlluT3YdHBbyCYvMMFdtds8hLbG2g=; b=PevJfp2jmdD55qYIClVj3J/pUj2jFkN6u68Z9TH4TQ04NyCZDekcTtcnxT9W3f/TXabCaV /22irmqxXKNpGLTGtg0SIETdBeb29MVvs1//+DxzknvwY2z3W0ZII1Gk+KthqNNVKI6CKC cVIkCEvWCFFp9HhnpzNyxILe/Gjq1rk= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-387-18SSgs28Oqqw0LoeuwuC-A-1; Mon, 12 Oct 2020 10:31:53 -0400 X-MC-Unique: 18SSgs28Oqqw0LoeuwuC-A-1 Received: by mail-qv1-f69.google.com with SMTP id es11so3471536qvb.10 for ; Mon, 12 Oct 2020 07:31:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=yIyU/LNnOnEhj5JlluT3YdHBbyCYvMMFdtds8hLbG2g=; b=ekK1WDY5e4GcYSxjiUAqXaelITrCko+2tHEEhnGmd2p3y+XrUMtk17wW2SfM+IRaA4 Oqk6qXsuZ0GyzGaL6mn268QKASuuqTN886GciAudKuE0YNB0sR5zbXnUsUmVo0NTk6RK VD+GytqYXJ6ADOiXebdb26nQ3CiCg7B7tdnrrKZ2/lW7f74hqobdSLabiKjzxdv61L8P ItyLEQdHwdoBIEhI66DzMBuTvZFlv46pw8buHbw4bmbi91LcsaT3x/2l+ZjgCE78jD/R 2v9HhndRidMKAIjqkW50xoYAz7za4bZ88NSeAuaOYMZ0lT+XQfF3i4AiHXM5N/S76Qbi 0jGQ== X-Gm-Message-State: AOAM532jN4EBUy2uxfoR4MdkqHtzB68L5M5nqeccwt5gVn3gHHhbP/pp ABhcQlVrixRFBzGJnIm+2jmQmmSTWJyDFdWBJpTCCCAt9y9dzJCuPL5iIGdzDfj/Qw0omHcoYL1 jSrNzviaoPC5Smr0OlQ== X-Received: by 2002:aed:20a8:: with SMTP id 37mr10021958qtb.201.1602513112350; Mon, 12 Oct 2020 07:31:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy3SvCWPYbfF9CZwvauMnBLBg0aTlnNtV4dGGGTtoZlY5NFKNSzjlDy2VcPl4Rj/hQCmsmt3g== X-Received: by 2002:aed:20a8:: with SMTP id 37mr10021915qtb.201.1602513111947; Mon, 12 Oct 2020 07:31:51 -0700 (PDT) Received: from wsfd-netdev77.ntdv.lab.eng.bos.redhat.com (nat-pool-bos-t.redhat.com. [66.187.233.206]) by smtp.gmail.com with ESMTPSA id n2sm2400800qtr.6.2020.10.12.07.31.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Oct 2020 07:31:51 -0700 (PDT) From: Mark Gray To: ovs-dev@openvswitch.org Date: Mon, 12 Oct 2020 10:31:48 -0400 Message-Id: <20201012143148.2629474-1-mark.d.gray@redhat.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=mark.d.gray@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v2] ovsdb-idl: Fix *_is_new() IDL functions 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" Currently all functions of the type *_is_new() always return 'false'. This patch resolves this issue by using the 'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' instead of the 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' to determine if a row is new and by resetting the 'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' on clear. Further to this, the code is also updated to match the following behaviour: When a row is inserted, the 'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' is updated to match the new database change_seqno. The 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' is not set for inserted rows (only for updated rows). At the end of a run, ovsdb_idl_db_track_clear() should be called to clear all tracking information, this includes resetting all row 'change_seqno' to zero. This will ensure that subsequent runs will not see a previously 'new' row. add_tracked_change_for_references() is updated to only track rows that reference the current row. Update ovsdb_idl_txn_insert() to also set this flag when inserting new rows locally. Also, update unit tests in order to test the *_is_new(), *_is_delete() functions. Suggested-by: Dumitru Ceara Reported-at: https://bugzilla.redhat.com/1883562 Fixes: ca545a787ac0 ("ovsdb-idl.c: Increase seqno for change-tracking of table references.") Signed-off-by: Mark Gray --- lib/ovsdb-idl.c | 44 ++++++++++++++++++++++++++++++++------------ ovsdb/ovsdb-idlc.in | 2 +- tests/ovsdb-idl.at | 5 ++++- tests/test-ovsdb.c | 32 ++++++++++++++++++++++++-------- 4 files changed, 61 insertions(+), 22 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index d8f221ca6073..4750d7c9018c 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -1959,6 +1959,11 @@ ovsdb_idl_db_track_clear(struct ovsdb_idl_db *db) free(row->updated); row->updated = NULL; } + + row->change_seqno[OVSDB_IDL_CHANGE_INSERT] = + row->change_seqno[OVSDB_IDL_CHANGE_MODIFY] = + row->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; + ovs_list_remove(&row->track_node); ovs_list_init(&row->track_node); if (ovsdb_idl_row_is_orphan(row) && row->tracked_old_datum) { @@ -2684,24 +2689,27 @@ ovsdb_idl_process_update2(struct ovsdb_idl_table *table, return OVSDB_IDL_UPDATE_DB_CHANGED; } -/* Recursively add rows to tracked change lists for current row - * and the rows that reference this row. */ +/* Recursively add rows to tracked change lists for all rows that reference + 'row'. */ static void add_tracked_change_for_references(struct ovsdb_idl_row *row) { - if (ovs_list_is_empty(&row->track_node) && - ovsdb_idl_track_is_set(row->table)) { - ovs_list_push_back(&row->table->track_list, - &row->track_node); - row->change_seqno[OVSDB_IDL_CHANGE_MODIFY] - = row->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY] - = row->table->db->change_seqno + 1; - const struct ovsdb_idl_arc *arc; LIST_FOR_EACH (arc, dst_node, &row->dst_arcs) { - add_tracked_change_for_references(arc->src); + struct ovsdb_idl_row *ref = arc->src; + + if (ovs_list_is_empty(&ref->track_node) && + ovsdb_idl_track_is_set(ref->table)) { + ovs_list_push_back(&ref->table->track_list, + &ref->track_node); + + ref->change_seqno[OVSDB_IDL_CHANGE_MODIFY] + = ref->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY] + = ref->table->db->change_seqno + 1; + + add_tracked_change_for_references(ref); + } } - } } @@ -2767,7 +2775,14 @@ ovsdb_idl_row_change__(struct ovsdb_idl_row *row, const struct json *row_json, row->change_seqno[change] = row->table->change_seqno[change] = row->table->db->change_seqno + 1; + if (table->modes[column_idx] & OVSDB_IDL_TRACK) { + if (ovs_list_is_empty(&row->track_node) && + ovsdb_idl_track_is_set(row->table)) { + ovs_list_push_back(&row->table->track_list, + &row->track_node); + } + add_tracked_change_for_references(row); if (!row->updated) { row->updated = bitmap_allocate(class->n_columns); @@ -4843,6 +4858,11 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn, hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid)); hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid)); ovsdb_idl_add_to_indexes(row); + + row->change_seqno[OVSDB_IDL_CHANGE_INSERT] + = row->table->change_seqno[OVSDB_IDL_CHANGE_INSERT] + = row->table->db->change_seqno + 1; + return row; } diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 698fe25f3095..89c3eb68247f 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -282,7 +282,7 @@ const struct %(s)s *%(s)s_table_track_get_first(const struct %(s)s_table *); /* Returns true if 'row' was inserted since the last change tracking reset. */ static inline bool %(s)s_is_new(const struct %(s)s *row) { - return %(s)s_row_get_seqno(row, OVSDB_IDL_CHANGE_MODIFY) == 0; + return %(s)s_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT) > 0; } /* Returns true if 'row' was deleted since the last change tracking reset. */ diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 261f4f323274..023f0d99fc35 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -1162,6 +1162,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated], "where": [], "row": {"b": true}}]']], [[000: i=1 r=2 b=true s=mystring u=<0> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<1> <2>] uuid=<3> +000: inserted row: uuid=<3> 000: updated columns: b ba i ia r ra s sa u ua 001: {"error":null,"result":[{"count":2}]} 002: i=0 r=0 b=true s= u=<4> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<5> @@ -1224,6 +1225,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops], [[000: empty 001: {"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]}]} 002: i=1 r=2 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<0> +002: inserted row: uuid=<0> 002: updated columns: b ba i ia r ra s sa u ua 003: {"error":null,"result":[{"count":2}]} 004: i=0 r=0 b=true s= u=<5> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> @@ -1235,6 +1237,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops], 006: updated columns: r 007: {"error":null,"result":[{"uuid":["uuid","<6>"]}]} 008: i=-1 r=125 b=false s= u=<5> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] uuid=<6> +008: inserted row: uuid=<6> 008: updated columns: ba i ia r ra 009: {"error":null,"result":[{"count":2}]} 010: i=-1 r=125 b=false s=newstring u=<5> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] uuid=<6> @@ -1242,7 +1245,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops], 010: updated columns: s 010: updated columns: s 011: {"error":null,"result":[{"count":1}]} -012: ##deleted## uuid=<1> +012: deleted row: uuid=<1> 013: reconnect 014: i=-1 r=125 b=false s=newstring u=<5> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] uuid=<6> 014: i=1 r=123.5 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<0> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index b1a4be36bb1e..6dd476f75be0 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2030,7 +2030,7 @@ print_idl(struct ovsdb_idl *idl, int step) } static void -print_idl_track(struct ovsdb_idl *idl, int step, unsigned int seqno) +print_idl_track(struct ovsdb_idl *idl, int step) { const struct idltest_simple *s; const struct idltest_link1 *l1; @@ -2038,26 +2038,42 @@ print_idl_track(struct ovsdb_idl *idl, int step, unsigned int seqno) int n = 0; IDLTEST_SIMPLE_FOR_EACH_TRACKED (s, idl) { - if (idltest_simple_row_get_seqno(s, OVSDB_IDL_CHANGE_DELETE) >= seqno) { - printf("%03d: ##deleted## uuid="UUID_FMT"\n", step, UUID_ARGS(&s->header_.uuid)); + if (idltest_simple_is_deleted(s)) { + printf("%03d: deleted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&s->header_.uuid)); } else { print_idl_row_simple(s, step); + if (idltest_simple_is_new(s)) { + printf("%03d: inserted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&s->header_.uuid)); + } } n++; } IDLTEST_LINK1_FOR_EACH_TRACKED (l1, idl) { - if (idltest_simple_row_get_seqno(s, OVSDB_IDL_CHANGE_DELETE) >= seqno) { - printf("%03d: ##deleted## uuid="UUID_FMT"\n", step, UUID_ARGS(&s->header_.uuid)); + if (idltest_link1_is_deleted(l1)) { + printf("%03d: deleted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&l1->header_.uuid)); } else { print_idl_row_link1(l1, step); + if (idltest_link1_is_new(l1)) { + printf("%03d: inserted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&l1->header_.uuid)); + } } n++; } IDLTEST_LINK2_FOR_EACH_TRACKED (l2, idl) { - if (idltest_simple_row_get_seqno(s, OVSDB_IDL_CHANGE_DELETE) >= seqno) { - printf("%03d: ##deleted## uuid="UUID_FMT"\n", step, UUID_ARGS(&s->header_.uuid)); + if (idltest_link2_is_deleted(l2)) { + printf("%03d: deleted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&l2->header_.uuid)); } else { print_idl_row_link2(l2, step); + if (idltest_link2_is_new(l2)) { + printf("%03d: inserted row: uuid="UUID_FMT"\n", step, + UUID_ARGS(&l2->header_.uuid)); + } + } n++; } @@ -2465,7 +2481,7 @@ do_idl(struct ovs_cmdl_context *ctx) /* Print update. */ if (track) { - print_idl_track(idl, step++, ovsdb_idl_get_seqno(idl)); + print_idl_track(idl, step++); ovsdb_idl_track_clear(idl); } else { print_idl(idl, step++);