2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-29 13:27:59 +00:00

188 Commits

Author SHA1 Message Date
Dumitru Ceara
ae25f8c8ff ovsdb-idl: Avoid inconsistent IDL state with OVSDB_MONITOR_V3.
Assuming an ovsdb client connected to a database using OVSDB_MONITOR_V3
(i.e., "monitor_cond_since" method) with the initial monitor condition
MC1.

Assuming the following two transactions are executed on the
ovsdb-server:
TXN1: "insert record R1 in table T1"
TXN2: "insert record R2 in table T2"

If the client's monitor condition MC1 for table T2 matches R2 then the
client will receive the following update3 message:
method="update3", "insert record R2 in table T2", last-txn-id=TXN2

At this point, if the presence of the new record R2 in the IDL triggers
the client to update its monitor condition to MC2 and add a clause for
table T1 which matches R1, a monitor_cond_change message is sent to the
server:
method="monitor_cond_change", "clauses from MC2"

In normal operation the ovsdb-server will reply with a new update3
message of the form:
method="update3", "insert record R1 in table T1", last-txn-id=TXN2

However, if the connection drops in the meantime, this last update might
get lost.

It might happen that during the reconnect a new transaction happens
that modifies the original record R1:
TXN3: "modify record R1 in table T1"

When the client reconnects, it will try to perform a fast resync by
sending:
method="monitor_cond_since", "clauses from MC2", last-txn-id=TXN2

Because TXN2 is still in the ovsdb-server transaction history, the
server replies with the changes from the most recent transactions only,
i.e., TXN3:
result="true", last-txbb-id=TXN3, "modify record R1 in table T1"

This causes the IDL on the client in to end up in an inconsistent
state because it has never seen the update that created R1.

Such a scenario is described in:
https://bugzilla.redhat.com/show_bug.cgi?id=1808580#c22

To avoid this issue, the IDL will now maintain (up to) 3 different
types of conditions for each DB table:
- new_cond: condition that has been set by the IDL client but has
  not yet been sent to the server through monitor_cond_change.
- req_cond: condition that has been sent to the server but the reply
  acknowledging the change hasn't been received yet.
- ack_cond: condition that has been acknowledged by the server.

Whenever the IDL FSM is restarted (e.g., voluntary or involuntary
disconnect):
- if there is a known last_id txn-id the code ensures that new_cond
  will contain the most recent condition set by the IDL client
  (either req_cond if there was a request in flight, or new_cond
  if the IDL client set a condition while the IDL was disconnected)
- if there is no known last_id txn-id the code ensures that ack_cond will
  contain the most recent conditions set by the IDL client regardless
  whether they were acked by the server or not.

When monitor_cond_since/monitor_cond requests are sent they will
always include ack_cond and if new_cond is not NULL a follow up
monitor_cond_change will be generated afterwards.

On the other hand ovsdb_idl_db_set_condition() will always modify new_cond.

This ensures that updates of type "insert" that happened before the last
transaction known by the IDL but didn't match old monitor conditions are
sent upon reconnect if the monitor condition has changed to include them
in the meantime.

Fixes: 403a6a0cb003 ("ovsdb-idl: Fast resync from server when connection reset.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
2020-06-15 14:07:29 +02:00
Mark Michelson
89b522aee3 ovsdb-idl: Add function to reset min_index.
If an administrator removes all of the databases in a cluster from
disk, then ovsdb IDL clients will have a problem. The databases will all
reset their stored indexes to 0, so The IDL client's min_index will be
higher than the indexes of all databases in the cluster. This results in
the client constantly connecting to databases, detecting the data as
"stale", and then attempting to connect to another.

This function provides a way to reset the IDL to an initial state with
min_index of 0. This way, the client will not wrongly detect the
database data as stale and will recover properly.

Notice that this function is not actually used anywhere in this patch.
This will be used by OVN, though, since OVN is the primary user of
clustered OVSDB.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
2020-05-28 18:36:48 +02:00
William Tu
68bc6f88a3 ovsdb-idl: Fix NULL deref reported by Coverity.
When 'datum.values' or 'datum.keys' is NULL, some code path calling
into ovsdb_idl_txn_write__ triggers NULL deref.

An example:
ovsrec_open_vswitch_set_cur_cfg(const struct ovsrec_open_vswitch
{
    struct ovsdb_datum datum;
    union ovsdb_atom key;

    datum.n = 1;
    datum.keys = &key;

    key.integer = cur_cfg;
//  1. assign_zero: Assigning: datum.values = NULL.
    datum.values = NULL;
//  CID 1421356 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
//  2. var_deref_model: Passing &datum to ovsdb_idl_txn_write_clone,\
//     which dereferences null datum.values.
    ovsdb_idl_txn_write_clone(&row->header_, &ovsrec_open_vswitch_col
}

And with the following calls:
ovsdb_idl_txn_write_clone
  ovsdb_idl_txn_write__
    6. deref_parm_in_call: Function ovsdb_datum_destroy dereferences
       datum->values
    ovsdb_datum_destroy

Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: William Tu <u9012063@gmail.com>
2020-05-20 08:49:27 -07:00
Dumitru Ceara
2b7e536fa5 Revert "ovsdb-idl: Avoid sending redundant conditional monitoring updates"
This reverts commit 5351980b047f4dd40be7a59a1e4b910df21eca0a.

If the ovsdb-server reply to "monitor_cond_since" requests has
"found" == false then ovsdb_idl_db_parse_monitor_reply() calls
ovsdb_idl_db_clear() which iterates through all tables and
unconditionally sets table->cond_changed to false.

However, if the client had already set a new condition for some of the
tables, this new condition request will never be sent to ovsdb-server
until the condition is reset to a different value. This is due to the
check in ovsdb_idl_db_set_condition().

One way to replicate the issue is described in the bugzilla reporting
the bug, when ovn-controller is configured to use "ovn-monitor-all":
https://bugzilla.redhat.com/show_bug.cgi?id=1808125#c6

Commit 5351980b047f tried to optimize sending redundant conditional
monitoring updates but the chances that this scenario happens with the
latest code is quite low since commit 403a6a0cb003 ("ovsdb-idl: Fast
resync from server when connection reset.") changed the behavior of
ovsdb_idl_db_parse_monitor_reply() to avoid calling ovsdb_idl_db_clear()
in most cases.

Reported-by: Dan Williams <dcbw@redhat.com>
Reported-at: https://bugzilla.redhat.com/1808125
CC: Andy Zhou <azhou@ovn.org>
Fixes: 5351980b047f ("ovsdb-idl: Avoid sending redundant conditional monitoring updates")
Acked-by: Han Zhou <hzhou@ovn.org>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
2020-03-26 22:27:45 +01:00
Han Zhou
ca367fa5f8 ovsdb-idl.c: Allows retry even when using a single remote.
When clustered mode is used, the client needs to retry connecting
to new servers when certain failures happen. Today it is allowed to
retry new connection only if multiple remotes are used, which prevents
using LB VIP with clustered nodes. This patch makes sure the retry
logic works when using LB VIP: although same IP is used for retrying,
the LB can actually redirect the connection to a new node.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-08-21 11:30:03 -07:00
Dumitru Ceara
300ac0e184 ovsdb-idl: Mark row "parsed" in ovsdb_idl_txn_write__
Once a column is set in a row using ovsdb_idl_txn_write__ we also mark
the row "parsed". Otherwise the memory allocated by
sbrec_<table>_parse_<col>() will never be freed. After marking the row
"parsed", the ovsdb_idl_txn_disassemble function will properly free the
newly allocated memory for the column (ovsdb_idl_row_unparse).

The problem is present only for rows that are inserted by the
running application because rows that are loaded from the database
will always have row->parsed == true.

One way to detect the leak is to start northd with valgrind:

valgrind --tool=memcheck --leak-check=yes ./ovn-northd

Then create a logical switch and bind a logical port to it:

ovn-nbctl ls-add ls1
ovn-nbctl lsp-add ls1 ls1-vm1

The valgrind report:

==9270== Memcheck, a memory error detector
==9270== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9270== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright
info
==9270== Command: ./ovn-northd
==9270==

<snip>

==9270==
==9270== 8 bytes in 1 blocks are definitely lost in loss record 30 of
292
==9270==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==9270==    by 0x4D31EF: xmalloc (util.c:138)
==9270==    by 0x45CB8E: sbrec_multicast_group_parse_ports (ovn-sb-idl.c:18141)
==9270==    by 0x4BB12D: ovsdb_idl_txn_write__ (ovsdb-idl.c:4489)
==9270==    by 0x4BB1B5: ovsdb_idl_txn_write (ovsdb-idl.c:4527)
==9270==    by 0x45D167: sbrec_multicast_group_set_ports (ovn-sb-idl.c:18561)
==9270==    by 0x40F416: ovn_multicast_update_sbrec (ovn-northd.c:2947)
==9270==    by 0x41FC55: build_lflows (ovn-northd.c:7981)
==9270==    by 0x421830: ovnnb_db_run (ovn-northd.c:8522)
==9270==    by 0x422B2D: ovn_db_run (ovn-northd.c:9089)
==9270==    by 0x423909: main (ovn-northd.c:9409)
==9270==
==9270== 157 (32 direct, 125 indirect) bytes in 1 blocks are definitely
lost in loss record 199 of 292
==9270==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==9270==    by 0x4D31EF: xmalloc (util.c:138)
==9270==    by 0x471E3D: resize (hmap.c:100)
==9270==    by 0x4720C8: hmap_expand_at (hmap.c:175)
==9270==    by 0x4C74F1: hmap_insert_at (hmap.h:277)
==9270==    by 0x4C825A: smap_add__ (smap.c:392)
==9270==    by 0x4C7783: smap_add (smap.c:55)
==9270==    by 0x451054: sbrec_datapath_binding_parse_external_ids (ovn-sb-idl.c:7181)
==9270==    by 0x4BB12D: ovsdb_idl_txn_write__ (ovsdb-idl.c:4489)
==9270==    by 0x4BB1B5: ovsdb_idl_txn_write (ovsdb-idl.c:4527)
==9270==    by 0x451436: sbrec_datapath_binding_set_external_ids (ovn-sb-idl.c:7444)
==9270==    by 0x4090F1: ovn_datapath_update_external_ids (ovn-northd.c:817)
==9270==

<snip>

==9270==
==9270== LEAK SUMMARY:
==9270==    definitely lost: 1,322 bytes in 47 blocks
==9270==    indirectly lost: 4,653 bytes in 240 blocks
==9270==      possibly lost: 0 bytes in 0 blocks
==9270==    still reachable: 254,004 bytes in 7,780 blocks
==9270==         suppressed: 0 bytes in 0 blocks
==9270== Reachable blocks (those to which a pointer was found) are not
shown.
==9270== To see them, rerun with: --leak-check=full
--show-leak-kinds=all
==9270==
==9270== For counts of detected and suppressed errors, rerun with: -v
==9270== ERROR SUMMARY: 9 errors from 9 contexts (suppressed: 0 from 0)

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-07-18 08:51:34 -07:00
Damjan Skvarc
f42a37b08d ovsdb-idl: memory leak while destroying database
While checking unit tests with valgrind option (make check-valgrind) I have
noticed several memory leaks of the following format:

.....
==20019== 13,883 (296 direct, 13,587 indirect) bytes in 1 blocks are definitely lost in loss record 346 of 346
==20019==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20019==    by 0x530F52: xcalloc (util.c:121)
==20019==    by 0x5037A1: ovsdb_idl_row_create__ (ovsdb-idl.c:3120)
==20019==    by 0x5045A3: ovsdb_idl_row_create (ovsdb-idl.c:3133)
==20019==    by 0x507240: ovsdb_idl_process_update2 (ovsdb-idl.c:2478)
==20019==    by 0x507240: ovsdb_idl_db_parse_update__ (ovsdb-idl.c:2328)
==20019==    by 0x507240: ovsdb_idl_db_parse_update (ovsdb-idl.c:2380)
==20019==    by 0x508128: ovsdb_idl_process_response (ovsdb-idl.c:742)
==20019==    by 0x508128: ovsdb_idl_process_msg (ovsdb-idl.c:831)
==20019==    by 0x508128: ovsdb_idl_run (ovsdb-idl.c:915)
==20019==    by 0x4106D9: bridge_run (bridge.c:2977)
==20019==    by 0x40719C: main (ovs-vswitchd.c:127)
==20019==
==20019== LEAK SUMMARY:
==20019==    definitely lost: 296 bytes in 1 blocks
==20019==    indirectly lost: 13,587 bytes in 10 blocks
==20019==      possibly lost: 0 bytes in 0 blocks
==20019==    still reachable: 43,563 bytes in 440 blocks
==20019==         suppressed: 288 bytes in 1 blocks
....

The problem is that table records maintained by database which is going to
be destroyed with ovsdb_idl_db_destroy() function are not destroyed.

Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-07-01 14:47:59 -07:00
Han Zhou
72aeb243a5 ovsdb-idl: Tracking - preserve data for deleted rows.
OVSDB IDL can track changes, but for deleted rows, the data is
destroyed and only uuid is tracked. In some cases we need to
check the data of the deleted rows. This patch preserves data
for deleted rows until track clear is called.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-05-24 11:37:29 -07:00
Han Zhou
d59050555c ovsdb-idl: Support optionally not shuffling multiple remotes.
This patch allows remotes not being shuffled if desired (mostly for
testing purpose, when we need the order of remotes during retrying
be predictable). By default it still shuffles as how it behaves today.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-04-15 12:54:41 -07:00
Han Zhou
b80675c6b3 ovsdb-idl.c: Remove meaningless MAX().
In the else condition, it is already ensured that index >= idl->min_index.
So the MAX() is confusing and misleading here.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-03-26 13:44:25 -07:00
Han Zhou
ea11bb0643 ovsdb-idl: Fix memory leak of ovsdb_idl_db_clear.
ovsdb_idl_row_destroy() doesn't free the memory of row structure itself.
This is because of the ovsdb change tracking feature: the deleted row
may be accessed in the current iteration of main loop. The function
ovsdb_idl_row_destroy_postprocess() is called at the end of
ovsdb_idl_run() to free the deleted rows that are not tracked; the
function ovsdb_idl_db_track_clear() is called (indirectly) by user
at the end of each main loop iteration to free the deleted rows that
are tracked. However, in ovsdb_idl_db_clear(), which may be called when
a session is reset, or when the idl is destroyed, it didn't call
ovsdb_idl_row_destroy_postprocess(), which would result in all the
untracked rows leaked. This patch fixes that.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-03-07 16:48:53 -08:00
Han Zhou
75926610dd ovsdb-idl: Fix memory leak of idl->remote.
Reported by Address Sanitizer.

Fixes: 5e07b8f93f03 ("ovsdb-idl: New function ovsdb_idl_create_unconnected().")
Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-03-07 11:17:25 -08:00
Han Zhou
403a6a0cb0 ovsdb-idl: Fast resync from server when connection reset.
Use monitor_cond_since to request changes after last version of local
data when connection to server is reset, without clearing the local
data. It falls back to clearing and repopulating all the data when
the requested id cannot be fulfilled by the server.

Test result at ovn-scale-test environment using clustered mode:
- 1K HVs (ovsdb clients)
- 10K lports

Without the patch it took 30+ min for the SB ovsdb-server to calm down
and HVs to stablize the connectin and finish syncing data.

With the patch there were no noticible CPU spike of SB ovsdb-server,
and all HVs were in sync with SB within 1 min, which is the probe
interval set in this test (so it took at most 1 min for HVs to notice
the TCP connection reset and reconnect and resync finished immediately
after that).

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047457.html
Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-02-28 11:14:51 -08:00
Han Zhou
b49b9316a4 ovsdb-idl: Support monitor_cond_since method in C IDL.
Use monitor_cond_since in C IDL. If it is not supported by server,
fall back to old method (monitor_cond, or monitor).

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2019-02-28 11:14:49 -08:00
Ben Pfaff
e83ea0d652 ovsdb-idl: Treat "unknown database" error as reason to reconnect.
Ordinarily the IDL finds out in advance whether a particular database is
on its server, or it finds out via notifications.  But it's also a good
idea to adopt a belt-and-suspenders approach so that, if the IDL does
receive an "unknown database" error, we treat it as a "soft" error that
can be fixed by reconnecting to another server, rather than a "hard" error
that should cause an immediate abort.

Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-11-19 08:47:36 -08:00
Ben Pfaff
f50714bf9a ovsdb-idl: Avoid sending transactions when the DB is not synced up.
Until now the code here would happily try to send transactions to the
database server even if the database connection was not in the correct
state.  In some cases this could lead to strange behavior, such as sending
a database transaction for a database that the IDL had just learned did not
exist on the server.

Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-11-19 08:47:22 -08:00
Han Zhou
ca545a787a ovsdb-idl.c: Increase seqno for change-tracking of table references.
This fixes the change-tracking feature.  The seqno change is needed so that
the change-tracking helper function ..._is_new() can work properly.

Fixes: 102781cc02c6 ("ovsdb-idl: Track changes for table references.")
Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-10-08 10:29:16 -07:00
Ben Pfaff
5e07b8f93f ovsdb-idl: New function ovsdb_idl_create_unconnected().
This new function makes it possible to create an instance of the IDL
without connecting it to a remote OVSDB server.  The caller can then
connect and disconnect using ovsdb_idl_set_remote(); the ability to
disconnect is a new feature.

With this patch, the ovsdb_idl 'session' member can be null whereas
previously it was always nonnull.  The scattered changes throughout
ovsdb-idl are to cope with this new possibility.

An upcoming patch will introduce the first user of this new feature.

Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-10-04 12:55:21 -07:00
Ben Pfaff
aee7f431df ovsdb-idl: Adjust indexes during transactions.
When transactions modified tables with indexes, the indexes were not
properly updated to reflect the changes.  For deleted rows, in particular,
this could cause use-after-free errors.

This commit fixes the problem and adds some simple test cases provided by
Han Zhou that, without the fix, cause a crash.

Reported-by: Han Zhou <zhouhan@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047185.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-08-16 10:24:05 -07:00
Han Zhou
102781cc02 ovsdb-idl: Track changes for table references.
If a change of a row is tracked, make sure the rows that reference
this row are also added in tracked changes, unless change tracking
is not required for those rows.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-08-14 14:43:20 -07:00
Han Zhou
fcb7e398e0 ovsdb-idl.c: Update conditions when handling change tracking list.
There are places with the pattern:
    if (!ovs_list_is_empty(&row->track_node)) {
        ovs_list_remove(&row->track_node);
    }
    ovs_list_push_back(&row->table->track_list,
                       &row->track_node);

It seems to be trying to prevent double removing the node from a list,
but actually it doesn't, and may be misleading.

The function ovs_list_is_empty() returns true only if the node has been
initialized with ovs_list_init(). If a node is deleted but not
initialized, the check will return false and the above code will continue
deleting it again. But if a node is already initialized, calling
ovs_list_remove() is a no-op. So the check is not necessary and misleading.

In fact there is already a double removal resulted by this code: the function
ovsdb_idl_db_clear() removes the node but it then calls ovsdb_idl_row_destroy()
immediately which removes the node again. It should not result in any real
issue yet since the list was not changed so the second removal just
assigns the pointers with same value.

It is in fact not necessary to remove and then add back to the list, because
the purpose of the change tracking is just to tell if a row is changed
or not. So this patch removes the "check and remove" code before adding
the node to a list, but instead, adding it to the list only if it is
not in a list. This way it ensures the node is added to a list only once
in one idl loop. (ovsdb_idl_db_track_clear() will be called in each
iteration and the check ovs_list_is_empty() will return true at the first
check in next iteration).

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-08-14 14:25:48 -07:00
Han Zhou
c6cc8f38eb ovsdb-idl: Remove a misleading comment for change tracking.
The comment was added when the feature was introduced but what it
described is not what is implemented, probably because of revisions
after code reviews.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-08-14 14:25:09 -07:00
Han Zhou
d0bde28610 ovsdb-idl.c: Fix IDL index problem when rows are updated.
In current IDL index code it doesn't updated index when handling
"update2" messages, which is the default case. The consequence
is that when a row is updated, the index is not updated accordingly,
and even worse, it causes crash when calling ovsdb_idl_destroy().
It can be easily reproduced by the test cases added in this patch.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-08-13 15:37:34 -07:00
Lorenzo Bianconi
82b261b9ef Introduce ovs-appctl command to monitor HVs sb connection status
Add 'connection-status' command to ovs-appctl utility in order to check
if a given chassis is currently connected to SB db

Acked-by: Mark Michelson <mmichels@redhat.com>
Co-authored-by: aginwala <aginwala@ebay.com>
Signed-off-by: aginwala <aginwala@ebay.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-07-31 12:49:19 -07:00
Jakub Sitnicki
ef744a7233 ovsdb-idl: Allow monitoring columns that are already monitored.
If IDL was created with monitoring and alerts turned on by default for
all columns, then there is no harm in allowing the API users to ask
again for monitoring and alerts to be enabled for any given column.

This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
commands once the IDL has already ran once.

Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-07-23 15:48:10 -07:00
Han Zhou
8ea0969bcd ovsdb-idl: Remove unnecessary code in track clear.
In ovsdb_idl_db_track_clear(), it needs to free the deleted row.
However, it unnecessary to call ovsdb_idl_row_clear_old(), because
this has been called in ovsdb_idl_row_destroy(). It is also confusing
because it is called only if:
    if (ovsdb_idl_row_is_orphan(row))
This is contradict with the check in ovsdb_idl_row_clear_old():
    if (!ovsdb_idl_row_is_orphan(row))

(Currently the tracked row doesn't maintain any data, so there is no
leak.)

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-06-18 15:46:58 -07:00
Mark Michelson
079ace1fa4 ovsdb-idl: Correct singleton insert logic
When inserting data into a "singleton" table (one that has maxRows ==
1), there is a check that ensures that the table is currently empty
before inserting the row. The intention is to prevent races where
multiple clients might attempt to insert rows at the same time.

The problem is that this singleton check can cause legitimate
transactions to fail. Specifically, a transaction that attempts to
delete the current content of the table and insert new data will cause
the singleton check to fail since the table currently has data.

This patch corrects the issue by keeping a count of the rows being
deleted and added to singleton tables. If the total is larger than zero,
then the net operation is attempting to insert rows. If the total is
less than zero, then the net operation is attempting to remove rows. If
the total is zero, then the operation is inserting and deleting an equal
number of rows (or is just updating rows). We only add the singleton
check if the total is larger than zero.

This patch also includes a new test for singleton tables that ensures
that the maxRows constraint works as expected.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-06-15 16:26:15 -07:00
Ben Pfaff
d14e007c1f ovsdb-idl: Redesign use of indexes.
The design of the compound index feature in the C OVSDB IDL was unusual.
Indexes were generally referenced only by name rather than by pointer, and
could be obtained only from the top-level ovsdb_idl object.  To iterate or
otherwise search an index required explicitly creating a special
ovsdb_idl_cursor object, which at least seemed somewhat heavy-weight given
that it required a string lookup in a table of indexes.

This commit redesigns the compound index interface.  It discards the use of
names for indexes, instead having clients pass in a pointer to the index
object itself.  It simplifies how indexes are created, gets rid of the need
for explicit cursor objects, and updates all of the users to the new
interface.

The underlying reason for this commit is to make it easier in
ovn-controller to keep track of the dependencies for a given function, by
making the indexes explicit arguments to any function that needs to use
them.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Han Zhou <hzhou8@ebay.com>
2018-06-12 08:24:08 -07:00
Ben Pfaff
fa37affad3 Embrace anonymous unions.
Several OVS structs contain embedded named unions, like this:

struct {
    ...
    union {
        ...
    } u;
};

C11 standardized a feature that many compilers already implemented
anyway, where an embedded union may be unnamed, like this:

struct {
    ...
    union {
        ...
    };
};

This is more convenient because it allows the programmer to omit "u."
in many places.  OVS already used this feature in several places.  This
commit embraces it in several others.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
Tested-by: Alin Gabriel Serdean <aserdean@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2018-05-25 13:36:05 -07:00
Yifeng Sun
b339ef592a ovsdb-idl: properly destroy ovsdb_idl.server
This patch fixes the memory leak reported by valgrind in testing
"learning action - TCPv6 port learning"

150 (40 direct, 110 indirect) bytes in 1 blocks are definitely lost in loss record 329 of 363
at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x51D0D4: xmalloc (util.c:120)
by 0x572E17: json_create (json.c:1442)
by 0x572E17: json_array_create (json.c:217)
by 0x572E17: json_array_create_2 (json.c:238)
by 0x4F69EA: ovsdb_idl_db_init (ovsdb-idl.c:424)
by 0x4F6A58: ovsdb_idl_create (ovsdb-idl.c:454)
by 0x40FAC7: bridge_init (bridge.c:396)
by 0x406F93: main (ovs-vswitchd.c:106)

3,727 (40 direct, 3,687 indirect) bytes in 1 blocks are definitely lost in loss record 358 of 363
at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x51D0D4: xmalloc (util.c:120)
by 0x572EBA: json_create (json.c:1442)
by 0x572EBA: json_object_create (json.c:254)
by 0x573254: json_parser_push_object (json.c:1264)
by 0x573254: json_parse_value.isra.12 (json.c:1293)
by 0x57339F: json_parser_input (json.c:1398)
by 0x5742C1: json_lex_input (json.c:982)
by 0x5748EB: json_parser_feed (json.c:1140)
by 0x57597A: jsonrpc_recv.part.7 (jsonrpc.c:332)
by 0x5768A7: jsonrpc_recv (jsonrpc.c:1140)
by 0x5768A7: jsonrpc_session_recv (jsonrpc.c:1113)
by 0x4F4E5C: ovsdb_idl_run (ovsdb-idl.c:818)
by 0x4100F9: bridge_run (bridge.c:2949)
by 0x406FB4: main (ovs-vswitchd.c:121)

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-04-04 14:02:51 -07:00
Han Zhou
b97b1f0f88 ovsdb-idl: omit_alert should implicitly turn off tracking.
OVSDB_IDL_TRACK is not valid without OVSDB_IDL_ALERT, so it should
be turned off as well in ovsdb_idl_omit_alert().

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-04-04 10:37:50 -07:00
Ben Pfaff
1b1d2e6daa ovsdb: Introduce experimental support for clustered databases.
This commit adds support for OVSDB clustering via Raft.  Please read
ovsdb(7) for information on how to set up a clustered database.  It is
simple and boils down to running "ovsdb-tool create-cluster" on one server
and "ovsdb-tool join-cluster" on each of the others and then starting
ovsdb-server in the usual way on all of them.

One you have a clustered database, you configure ovn-controller and
ovn-northd to use it by pointing them to all of the servers, e.g. where
previously you might have said "tcp:1.2.3.4" was the database server,
now you say that it is "tcp:1.2.3.4,tcp:5.6.7.8,tcp:9.10.11.12".

This also adds support for database clustering to ovs-sandbox.

Acked-by: Justin Pettit <jpettit@ovn.org>
Tested-by: aginwala <aginwala@asu.edu>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-03-24 12:04:53 -07:00
Ben Pfaff
1456335dc5 ovsdb-idl: Break out database-specific stuff into new data structure.
Until now, a given ovsdb-idl instances has only monitored a single
database.  In an upcoming commit, it will grow to also monitor a second
database that represents the state of the database server itself.  Much of
the work is the same for both databases, so this commit breaks the common
code and data out into new data structures and functions.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
2018-03-24 12:04:51 -07:00
Ben Pfaff
62bba60935 ovsdb-idl: Use modern form of <monitor-requests>.
Long ago, a <monitor-requests> object in the OVSDB protocol mapped a table
name to a single <monitor-request>.  Since then, it has mapped a table name
to an *array of* <monitor-request> objects, but the OVSDB IDL has never
been updated to use the modern form.  This commit makes that change.

Reported-by: Anil Jangam <anilj.mailing@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>
2018-03-14 11:27:19 -07:00
Ben Pfaff
01928c964f ovsdb-idlc: Implement synthetic columns.
A synthetic column is one that is not present in the actual database but
instead calculated by code in the client based on columns in the row.  This
can be useful to avoid repeatedly calculating the same function of a row.

Signed-off-by: Ben Pfaff <blp@ovn.org>
2018-02-16 15:08:08 -08:00
Ben Pfaff
fe0fb88551 ovsdb-client: Add new "restore" command.
Signed-off-by: Ben Pfaff <blp@ovn.org>
2017-12-21 16:41:30 -08:00
Ben Pfaff
3865965dd9 ovsdb-error: New function ovsdb_error_to_string_free().
This allows slight code simplifications across the tree.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
2017-12-13 11:32:29 -08:00
Ben Pfaff
25540a777f ovsdb-idl: Tolerate initialization races for singleton tables.
By verifying that singleton tables (that is, tables that should have exactly
one row) are empty when they emit transactions that insert into them,
ovs-vsctl and similar tools tolerate initialization races, where more than one
client at a time tries to initialize a singleton table.

The upshot is that if you create a database and then run multiple ovs-vsctl
(etc.) commands against it in parallel (without first initializing it
serially), then without this patch sometimes you will sometimes get failures
but this patch avoids them.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2017-12-11 14:40:47 -08:00
Ben Pfaff
7a3608245c ovsdb-idl: Remove 'uuid' member of struct ovsdb_idl.
This was used to uniquely identify the monitor, but there's no need for
that.  A fixed monitor name works fine.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2017-12-11 14:40:39 -08:00
Ben Pfaff
fca1b42abe ovsdb-idl: Fix assertion failure on error path parsing server reply.
If the database server sent an error reply to a monitor_cond request, and
the error was not a JSON string, then passing the error to json_string()
caused an assertion failure.

Found by inspection.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2017-12-11 14:31:03 -08:00
Ben Pfaff
1ac62a0e09 ovsdb-idl: Fix indentation in a couple of places.
White space changes only.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2017-12-11 14:30:52 -08:00
Ben Pfaff
f5a12b82b8 ovsdb-idl: Improve comments.
This change documents the IDL state machine, adds other comments,
and fixes a spelling error in a comment.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
2017-12-11 14:30:34 -08:00
Xiao Liang
fd016ae3fb lib: Move lib/poll-loop.h to include/openvswitch
Poll-loop is the core to implement main loop. It should be available in
libopenvswitch.

Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2017-11-03 10:47:55 -07:00
Yifeng Sun
17b85cd9b3 ovsdb-idl: Fix memory leak
Valgrind testcase 2339 (ovn -- ipam connectivity) reports the leak below:
45 (32 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss record 65 of 83
    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x4A6D64: xmalloc (util.c:120)
    by 0x49C847: shash_add_nocopy__ (shash.c:109)
    by 0x49C847: shash_add_nocopy (shash.c:121)
    by 0x49CA85: shash_add (shash.c:129)
    by 0x49CA85: shash_add_once (shash.c:136)
    by 0x4914B5: ovsdb_idl_create_index (ovsdb-idl.c:2067)
    by 0x406C98: create_ovnsb_indexes (ovn-controller.c:568)
    by 0x406C98: main (ovn-controller.c:619)

The leak happens when vsdb_idl_table is freed but its indexes are not freed.

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2017-11-02 14:17:20 -07:00
Han Zhou
3cc1634fc2 ovsdb-idl: fix index row setting with references.
IDL index should be able to be used without having to be in a
transaction. However, current implementation leads to crash if
a reference type column is being set in an index row for querying
purpose when it is not in a transaction. It is because of the
uninitialized arcs and unnecessary updates of the arcs. This patch
fixes it by identifying index rows by a magic uuid, so that when
parsing index row, the arcs are not updated. A new test case is
added to cover this scenario.

Signed-off-by: Han Zhou <zhouhan@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2017-10-30 13:09:51 -07:00
Joe Stringer
4e0b4acca2 ovsdb-idl: Rename 'old' to 'old_datum'.
Now that the 'new' datum is named 'new_datum', be more consistent by
renaming 'old' to 'old_datum' to match.

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
2017-08-15 14:09:51 -07:00
Joe Stringer
fa50ab0bfb ovsdb-idl: Avoid new expression.
In C++, 'new' is a keyword. If this is used as the name for a field,
then C++ compilers can get confused about the context and fail to
compile references to such fields. Rename the field to 'new_datum' to
avoid this issue.

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
2017-08-15 14:09:49 -07:00
Joe Stringer
c6e5d064a4 ovsdb-idl: Avoid mutable type specifier.
In C++, 'mutable' is a keyword. If this is used as the name for a field,
then C++ compilers can get confused about the context and fail to
compile references to such fields. Rename the field to 'is_mutable' to
avoid this issue.

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
2017-08-15 14:09:46 -07:00
Joe Stringer
3eb1423353 ovsdb-idl: Avoid class declaration.
In C++, 'class' is a keyword. If this is used as the name for a field,
then C++ compilers can get confused about the context and fail to
compile references to such fields. Rename the field to 'class_' to
avoid this issue.

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
2017-08-15 14:09:40 -07:00
Lance Richardson
93fe026466 ovsdb-idl: idl compound indexes implementation
This patch adds support for the creation of multicolumn indexes
in the C IDL to enable for efficient search and retrieval of database
rows by key.

Signed-off-by: Esteban Rodriguez Betancourt <estebarb@hpe.com>
Co-authored-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
2017-08-03 14:48:34 -07:00