[ovs-dev] [ovsdb speedup 00/18] improve ovsdb connection scaling
This patch set implements two ideas improve ovsdb connection scaling: * Allow multiple jsonrpc connection to share a single ovsdb monitor, thus maintaining a single set of changes. * With in the same monitor, cache the json object generated for an update. If the same update is requested by another jsonrpc connection, the cached json object can be used, avoid regenerating it from scratch. --- I don't yet have any useful data on speed improvements from both changes. The plan is to to collect them and report back in the next few days. If we don't want to apply them without the performance data, then please consider this posting as RFC. Andy Zhou (18): ovsdb: refactoring jsonrpc-server.c ovsdb: make setting mt->select into its own functions ovsdb: refactor ovsdb_jsonrpc_parse_monitor_request ovsdb: refactor ovsdb_monitor_add_column() ovsdb: refactoring ovsdb_jsonrpc_monitor_compose_table_update() ovsdb: refactoring ovsdb_jsonrpc_monitor_needs_flush ovsdb: rename ovsdb_jsonrpc_monitor_get_initial() ovsdb: refactoring ovsdb_monitor_destroy() ovsdb: Split out monitor backend functions to ovsdb-monitor.c/h ovsdb: refactoring ovsdb_monitor_get_initial ovsdb: ovsdb-monitor stores jsonrpc-monitor in a linked-list ovsdb: add transaction ids ovsdb: rename jsonrpc_monitor_compose_table_update() ovsdb: add ovsdb_monitor_changes ovsdb: allow a list of 'ovsdb_monitor_changes' in each ovsdb monitor table ovsdb: refactor ovsdb_monitor_create() ovsdb: allow multiple jsonrpc monitors to share a single ovsdb monitor ovsdb: add json cache ovsdb/automake.mk |2 + ovsdb/jsonrpc-server.c | 556 -- ovsdb/jsonrpc-server.h |3 + ovsdb/ovsdb-monitor.c | 1024 ovsdb/ovsdb-monitor.h | 71 5 files changed, 1167 insertions(+), 489 deletions(-) create mode 100644 ovsdb/ovsdb-monitor.c create mode 100644 ovsdb/ovsdb-monitor.h -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 01/18] ovsdb: refactoring jsonrpc-server.c
jsonrpc-server.c has two main functions. One deals with handling the
jsonrpc connections, the other deals with monitoring the database.
Currently, each jsonrpc connections has its own set of DB monitors.
This can be wasteful if a number of connections shares the same
monitors.
This patch, and a few following refactoring patches attempts to
split the jsonrpc handling front end off the main monitoring
functions within jsonrpc.c.
This patch changes the monitoring functions and data structures from
'ovsdb_jsonrpc_monitor_xxx' into 'ovsdb_monitor_xxx'
Signed-off-by: Andy Zhou
--
The end goal of those refactoring patches is to move ovsdb_monitor
functions into its own file.
---
ovsdb/jsonrpc-server.c | 221 +++--
1 file changed, 124 insertions(+), 97 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 2ba5ddd..5fdf08b 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1026,9 +1026,9 @@ ovsdb_jsonrpc_trigger_complete_done(struct
ovsdb_jsonrpc_session *s)
}
}
-/* JSON-RPC database table monitors. */
+/* database table monitors. */
-enum ovsdb_jsonrpc_monitor_selection {
+enum ovsdb_monitor_selection {
OJMS_INITIAL = 1 << 0, /* All rows when monitor is created. */
OJMS_INSERT = 1 << 1, /* New rows. */
OJMS_DELETE = 1 << 2, /* Deleted rows. */
@@ -1036,13 +1036,13 @@ enum ovsdb_jsonrpc_monitor_selection {
};
/* A particular column being monitored. */
-struct ovsdb_jsonrpc_monitor_column {
+struct ovsdb_monitor_column {
const struct ovsdb_column *column;
-enum ovsdb_jsonrpc_monitor_selection select;
+enum ovsdb_monitor_selection select;
};
/* A row that has changed in a monitored table. */
-struct ovsdb_jsonrpc_monitor_row {
+struct ovsdb_monitor_row {
struct hmap_node hmap_node; /* In ovsdb_jsonrpc_monitor_table.changes. */
struct uuid uuid; /* UUID of row that changed. */
struct ovsdb_datum *old;/* Old data, NULL for an inserted row. */
@@ -1050,38 +1050,49 @@ struct ovsdb_jsonrpc_monitor_row {
};
/* A particular table being monitored. */
-struct ovsdb_jsonrpc_monitor_table {
+struct ovsdb_monitor_table {
const struct ovsdb_table *table;
/* This is the union (bitwise-OR) of the 'select' values in all of the
* members of 'columns' below. */
-enum ovsdb_jsonrpc_monitor_selection select;
+enum ovsdb_monitor_selection select;
/* Columns being monitored. */
-struct ovsdb_jsonrpc_monitor_column *columns;
+struct ovsdb_monitor_column *columns;
size_t n_columns;
-/* Contains 'struct ovsdb_jsonrpc_monitor_row's for rows that have been
+/* Contains 'struct ovsdb_monitor_row's for rows that have been
* updated but not yet flushed to the jsonrpc connection. */
struct hmap changes;
};
+struct ovsdb_jsonrpc_monitor;
+/* Backend monitor.
+ *
+ * ovsdb_monitor keep track of the ovsdb changes.
+ */
/* A collection of tables being monitored. */
-struct ovsdb_jsonrpc_monitor {
+struct ovsdb_monitor {
struct ovsdb_replica replica;
+struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
+struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
+};
+
+/* Jsonrpc front end monitor. */
+struct ovsdb_jsonrpc_monitor {
struct ovsdb_jsonrpc_session *session;
struct ovsdb *db;
struct hmap_node node; /* In ovsdb_jsonrpc_session's "monitors". */
struct json *monitor_id;
-struct shash tables; /* Holds "struct ovsdb_jsonrpc_monitor_table"s. */
+struct ovsdb_monitor *dbmon;
};
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
-static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_replica *);
+static void ovsdb_monitor_destroy(struct ovsdb_replica *);
static struct json *ovsdb_jsonrpc_monitor_get_initial(
const struct ovsdb_jsonrpc_monitor *);
@@ -1110,12 +1121,12 @@ ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session
*s,
}
static void
-ovsdb_jsonrpc_add_monitor_column(struct ovsdb_jsonrpc_monitor_table *mt,
- const struct ovsdb_column *column,
- enum ovsdb_jsonrpc_monitor_selection select,
- size_t *allocated_columns)
+ovsdb_add_monitor_column(struct ovsdb_monitor_table *mt,
+ const struct ovsdb_column *column,
+ enum ovsdb_monitor_selection select,
+ size_t *allocated_columns)
{
-struct ovsdb_jsonrpc_monitor_column *c;
+struct ovsdb_monitor_column *c;
if (mt->n_columns >= *allocated_columns) {
mt->columns = x2nrealloc(mt->columns, allocated_columns,
@@ -1128,21 +1139,21 @@ ovsdb_jsonrpc_add_monitor_column(struct
ovsdb_jsonrpc_monitor_table *mt,
}
static int
-comp
[ovs-dev] [ovsdb speedup 04/18] ovsdb: refactor ovsdb_monitor_add_column()
To hide ovsdb_monitor_table object from ovsdb_jsonrpc serve.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 1241537..973cc46 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1121,13 +1121,17 @@ ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session
*s,
}
static void
-ovsdb_add_monitor_column(struct ovsdb_monitor_table *mt,
+ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
+ const struct ovsdb_table *table,
const struct ovsdb_column *column,
enum ovsdb_monitor_selection select,
size_t *allocated_columns)
{
+struct ovsdb_monitor_table *mt;
struct ovsdb_monitor_column *c;
+mt = shash_find_data(&dbmon->tables, table->schema->name);
+
if (mt->n_columns >= *allocated_columns) {
mt->columns = x2nrealloc(mt->columns, allocated_columns,
sizeof *mt->columns);
@@ -1148,9 +1152,12 @@ compare_ovsdb_monitor_column(const void *a_, const void
*b_)
}
static void
-ovsdb_monitor_set_select(struct ovsdb_monitor_table *mt,
+ovsdb_monitor_table_set_select(struct ovsdb_monitor *dbmon,
+ const struct ovsdb_table *table,
enum ovsdb_monitor_selection select)
{
+struct ovsdb_monitor_table * mt;
+mt = shash_find_data(&dbmon->tables, table->schema->name);
mt->select = select;
}
@@ -1165,7 +1172,6 @@ ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor
*dbmon,
const struct json *columns, *select_json;
struct ovsdb_parser parser;
struct ovsdb_error *error;
-struct ovsdb_monitor_table *mt;
ovsdb_parser_init(&parser, monitor_request, "table %s", ts->name);
columns = ovsdb_parser_member(&parser, "columns", OP_ARRAY | OP_OPTIONAL);
@@ -1199,8 +1205,7 @@ ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor
*dbmon,
select = OJMS_INITIAL | OJMS_INSERT | OJMS_DELETE | OJMS_MODIFY;
}
-mt = shash_find_data(&dbmon->tables, table->schema->name);
-ovsdb_monitor_set_select(mt, select);
+ovsdb_monitor_table_set_select(dbmon, table, select);
if (columns) {
size_t i;
@@ -1219,12 +1224,13 @@ ovsdb_jsonrpc_parse_monitor_request(struct
ovsdb_monitor *dbmon,
}
s = columns->u.array.elems[i]->u.string;
-column = shash_find_data(&mt->table->schema->columns, s);
+column = shash_find_data(&table->schema->columns, s);
if (!column) {
return ovsdb_syntax_error(columns, NULL, "%s is not a valid "
"column name", s);
}
-ovsdb_add_monitor_column(mt, column, select, allocated_columns);
+ovsdb_monitor_add_column(dbmon, table, column, select,
+ allocated_columns);
}
} else {
struct shash_node *node;
@@ -1232,7 +1238,7 @@ ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor
*dbmon,
SHASH_FOR_EACH (node, &ts->columns) {
const struct ovsdb_column *column = node->data;
if (column->index != OVSDB_COL_UUID) {
-ovsdb_add_monitor_column(mt, column, select,
+ovsdb_monitor_add_column(dbmon, table, column, select,
allocated_columns);
}
}
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 03/18] ovsdb: refactor ovsdb_jsonrpc_parse_monitor_request
Change ovsdb_jsonrpc_parse_monitor_request() to make
ovsdb_monitor_table an opaque object.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 73 +-
1 file changed, 54 insertions(+), 19 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index ba0e28c..1241537 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1155,15 +1155,17 @@ ovsdb_monitor_set_select(struct ovsdb_monitor_table *mt,
}
static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
-ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor_table *mt,
+ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor *dbmon,
+const struct ovsdb_table *table,
const struct json *monitor_request,
size_t *allocated_columns)
{
-const struct ovsdb_table_schema *ts = mt->table->schema;
+const struct ovsdb_table_schema *ts = table->schema;
enum ovsdb_monitor_selection select;
const struct json *columns, *select_json;
struct ovsdb_parser parser;
struct ovsdb_error *error;
+struct ovsdb_monitor_table *mt;
ovsdb_parser_init(&parser, monitor_request, "table %s", ts->name);
columns = ovsdb_parser_member(&parser, "columns", OP_ARRAY | OP_OPTIONAL);
@@ -1197,6 +1199,7 @@ ovsdb_jsonrpc_parse_monitor_request(struct
ovsdb_monitor_table *mt,
select = OJMS_INITIAL | OJMS_INSERT | OJMS_DELETE | OJMS_MODIFY;
}
+mt = shash_find_data(&dbmon->tables, table->schema->name);
ovsdb_monitor_set_select(mt, select);
if (columns) {
size_t i;
@@ -1255,6 +1258,44 @@ ovsdb_monitor_create(struct ovsdb *db,
return m;
}
+static void
+ovsdb_monitor_add_table(struct ovsdb_monitor *m,
+const struct ovsdb_table *table)
+{
+struct ovsdb_monitor_table *mt;
+
+mt = xzalloc(sizeof *mt);
+mt->table = table;
+hmap_init(&mt->changes);
+shash_add(&m->tables, table->schema->name, mt);
+}
+
+/* Check for duplicated column names. Return the first
+ * duplicated column's name if found. Otherwise return
+ * NULL. */
+static const char * OVS_WARN_UNUSED_RESULT
+ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor *m,
+ const struct ovsdb_table *table)
+{
+struct ovsdb_monitor_table *mt;
+int i;
+
+mt = shash_find_data(&m->tables, table->schema->name);
+
+if (mt) {
+/* Check for duplicate columns. */
+qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
+ compare_ovsdb_monitor_column);
+for (i = 1; i < mt->n_columns; i++) {
+if (mt->columns[i].column == mt->columns[i - 1].column) {
+ return mt->columns[i].column->name;
+}
+}
+}
+
+return NULL;
+}
+
static struct json *
ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
struct json *params)
@@ -1291,7 +1332,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
SHASH_FOR_EACH (node, json_object(monitor_requests)) {
const struct ovsdb_table *table;
-struct ovsdb_monitor_table *mt;
+const char *column_name;
size_t allocated_columns;
const struct json *mr_value;
size_t i;
@@ -1303,10 +1344,7 @@ ovsdb_jsonrpc_monitor_create(struct
ovsdb_jsonrpc_session *s, struct ovsdb *db,
goto error;
}
-mt = xzalloc(sizeof *mt);
-mt->table = table;
-hmap_init(&mt->changes);
-shash_add(&m->dbmon->tables, table->schema->name, mt);
+ovsdb_monitor_add_table(m->dbmon, table);
/* Parse columns. */
mr_value = node->data;
@@ -1316,29 +1354,26 @@ ovsdb_jsonrpc_monitor_create(struct
ovsdb_jsonrpc_session *s, struct ovsdb *db,
for (i = 0; i < array->n; i++) {
error = ovsdb_jsonrpc_parse_monitor_request(
-mt, array->elems[i], &allocated_columns);
+m->dbmon, table, array->elems[i], &allocated_columns);
if (error) {
goto error;
}
}
} else {
error = ovsdb_jsonrpc_parse_monitor_request(
-mt, mr_value, &allocated_columns);
+m->dbmon, table, mr_value, &allocated_columns);
if (error) {
goto error;
}
}
-/* Check for duplicate columns. */
-qsort(mt->columns, mt->n_columns, sizeof *mt->columns,
- compare_ovsdb_monitor_column);
-for (i = 1; i < mt->n_columns; i++) {
-if (mt->columns[i].column == mt->columns[i - 1].column) {
-error = ovsdb_syntax_error(mr_value, NULL, "column %s "
- "mentioned more than once",
-
[ovs-dev] [ovsdb speedup 02/18] ovsdb: make setting mt->select into its own functions
To make ovsdb_monitor an opaque to ovsdb_jsonrpc server object.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 5fdf08b..ba0e28c 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1147,6 +1147,13 @@ compare_ovsdb_monitor_column(const void *a_, const void
*b_)
return a->column < b->column ? -1 : a->column > b->column;
}
+static void
+ovsdb_monitor_set_select(struct ovsdb_monitor_table *mt,
+ enum ovsdb_monitor_selection select)
+{
+mt->select = select;
+}
+
static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
ovsdb_jsonrpc_parse_monitor_request(struct ovsdb_monitor_table *mt,
const struct json *monitor_request,
@@ -1189,8 +1196,8 @@ ovsdb_jsonrpc_parse_monitor_request(struct
ovsdb_monitor_table *mt,
} else {
select = OJMS_INITIAL | OJMS_INSERT | OJMS_DELETE | OJMS_MODIFY;
}
-mt->select |= select;
+ovsdb_monitor_set_select(mt, select);
if (columns) {
size_t i;
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 05/18] ovsdb: refactoring ovsdb_jsonrpc_monitor_compose_table_update()
Now it simply calls ovsdb_monitor_compose_table_update(), which
is actually creates the json object.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 973cc46..a25a6f1 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1674,8 +1674,8 @@ ovsdb_monitor_compose_row_update(
* be used as part of the initial reply to a "monitor" request, false if it is
* going to be used as part of an "update" notification. */
static struct json *
-ovsdb_jsonrpc_monitor_compose_table_update(
-const struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+ovsdb_monitor_compose_table_update(
+const struct ovsdb_monitor *dbmon, bool initial)
{
struct shash_node *node;
unsigned long int *changed;
@@ -1683,7 +1683,7 @@ ovsdb_jsonrpc_monitor_compose_table_update(
size_t max_columns;
max_columns = 0;
-SHASH_FOR_EACH (node, &monitor->dbmon->tables) {
+SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
max_columns = MAX(max_columns, mt->n_columns);
@@ -1691,7 +1691,7 @@ ovsdb_jsonrpc_monitor_compose_table_update(
changed = xmalloc(bitmap_n_bytes(max_columns));
json = NULL;
-SHASH_FOR_EACH (node, &monitor->dbmon->tables) {
+SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
struct ovsdb_monitor_row *row, *next;
struct json *table_json = NULL;
@@ -1730,6 +1730,13 @@ ovsdb_jsonrpc_monitor_compose_table_update(
return json;
}
+static struct json *
+ovsdb_jsonrpc_monitor_compose_table_update(
+const struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+{
+return ovsdb_monitor_compose_table_update(monitor->dbmon, initial);
+}
+
static bool
ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *s)
{
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 06/18] ovsdb: refactoring ovsdb_jsonrpc_monitor_needs_flush
split out per monitoring needs_flush() into
ovsdb_monitor_needs_flush().
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index a25a6f1..0630664 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1738,19 +1738,28 @@ ovsdb_jsonrpc_monitor_compose_table_update(
}
static bool
+ovsdb_monitor_needs_flush(struct ovsdb_monitor *dbmon)
+{
+struct shash_node *node;
+
+SHASH_FOR_EACH (node, &dbmon->tables) {
+struct ovsdb_monitor_table *mt = node->data;
+
+if (!hmap_is_empty(&mt->changes)) {
+return true;
+}
+}
+return false;
+}
+
+static bool
ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *s)
{
struct ovsdb_jsonrpc_monitor *m;
HMAP_FOR_EACH (m, node, &s->monitors) {
-struct shash_node *node;
-
-SHASH_FOR_EACH (node, &m->dbmon->tables) {
-struct ovsdb_monitor_table *mt = node->data;
-
-if (!hmap_is_empty(&mt->changes)) {
-return true;
-}
+if (ovsdb_monitor_needs_flush(m->dbmon)) {
+return true;
}
}
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 07/18] ovsdb: rename ovsdb_jsonrpc_monitor_get_initial()
rename ovsdb_jsonrpc_monitor_get_initial() to
ovsdb_monitor_get_initial()
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 0630664..886062c 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1093,8 +1093,8 @@ static const struct ovsdb_replica_class
ovsdb_jsonrpc_replica_class;
struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
static void ovsdb_monitor_destroy(struct ovsdb_replica *);
-static struct json *ovsdb_jsonrpc_monitor_get_initial(
-const struct ovsdb_jsonrpc_monitor *);
+static struct json *ovsdb_monitor_get_initial(
+const struct ovsdb_monitor *);
static bool
parse_bool(struct ovsdb_parser *parser, const char *name, bool default_value)
@@ -1383,7 +1383,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
-return ovsdb_jsonrpc_monitor_get_initial(m);
+return ovsdb_monitor_get_initial(m->dbmon);
error:
if (m) {
@@ -1809,14 +1809,14 @@ ovsdb_monitor_commit(struct ovsdb_replica *replica,
}
static struct json *
-ovsdb_jsonrpc_monitor_get_initial(const struct ovsdb_jsonrpc_monitor *m)
+ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
{
struct ovsdb_monitor_aux aux;
struct shash_node *node;
struct json *json;
-ovsdb_monitor_init_aux(&aux, m->dbmon);
-SHASH_FOR_EACH (node, &m->dbmon->tables) {
+ovsdb_monitor_init_aux(&aux, dbmon);
+SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
if (mt->select & OJMS_INITIAL) {
@@ -1827,7 +1827,7 @@ ovsdb_jsonrpc_monitor_get_initial(const struct
ovsdb_jsonrpc_monitor *m)
}
}
}
-json = ovsdb_jsonrpc_monitor_compose_table_update(m, true);
+json = ovsdb_jsonrpc_monitor_compose_table_update(dbmon, true);
return json ? json : json_object_create();
}
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 08/18] ovsdb: refactoring ovsdb_monitor_destroy()
Add ovsdb_minitor_destory() function to properly cleanup ovsdb_monitor.
It is also responsible for unhook from the replica chain.
The replica destroy callback is now called
ovsdb_monitor_destroy_callback()
Minor variable renaming in ovsdb_monitor_create() to make it
more consistent.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 59 --
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 886062c..78c7554 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -43,6 +43,7 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_jsonrpc_server);
struct ovsdb_jsonrpc_remote;
struct ovsdb_jsonrpc_session;
+struct ovsdb_jsonrpc_monitor;
/* Message rate-limiting. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -80,6 +81,7 @@ static void ovsdb_jsonrpc_trigger_complete_done(
/* Monitors. */
static struct json *ovsdb_jsonrpc_monitor_create(
struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
+static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -1092,7 +1094,7 @@ static const struct ovsdb_replica_class
ovsdb_jsonrpc_replica_class;
struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
-static void ovsdb_monitor_destroy(struct ovsdb_replica *);
+static void ovsdb_monitor_destroy(struct ovsdb_monitor *);
static struct json *ovsdb_monitor_get_initial(
const struct ovsdb_monitor *);
@@ -1252,16 +1254,16 @@ ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
const struct ovsdb_replica_class *replica_class)
{
-struct ovsdb_monitor *m;
+struct ovsdb_monitor *dbmon;
-m = xzalloc(sizeof *m);
+dbmon = xzalloc(sizeof *dbmon);
-ovsdb_replica_init(&m->replica, replica_class);
-ovsdb_add_replica(db, &m->replica);
-m->jsonrpc_monitor = jsonrpc_monitor;
-shash_init(&m->tables);
+ovsdb_replica_init(&dbmon->replica, replica_class);
+ovsdb_add_replica(db, &dbmon->replica);
+dbmon->jsonrpc_monitor = jsonrpc_monitor;
+shash_init(&dbmon->tables);
-return m;
+return dbmon;
}
static void
@@ -1387,7 +1389,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
error:
if (m) {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
}
json = ovsdb_error_to_json(error);
@@ -1411,7 +1413,7 @@ ovsdb_jsonrpc_monitor_cancel(struct ovsdb_jsonrpc_session
*s,
return jsonrpc_create_error(json_string_create("unknown monitor"),
request_id);
} else {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
return jsonrpc_create_reply(json_object_create(), request_id);
}
}
@@ -1423,7 +1425,7 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
struct ovsdb_jsonrpc_monitor *m, *next;
HMAP_FOR_EACH_SAFE (m, next, node, &s->monitors) {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
}
}
@@ -1832,14 +1834,22 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor
*dbmon)
}
static void
-ovsdb_monitor_destroy(struct ovsdb_replica *replica)
+ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *m)
+{
+json_destroy(m->monitor_id);
+hmap_remove(&m->session->monitors, &m->node);
+ovsdb_monitor_destroy(m->dbmon);
+free(m);
+}
+
+static void
+ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon)
{
-struct ovsdb_monitor *m = ovsdb_monitor_cast(replica);
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor = m->jsonrpc_monitor;
struct shash_node *node;
-json_destroy(jsonrpc_monitor->monitor_id);
-SHASH_FOR_EACH (node, &m->tables) {
+list_remove(&dbmon->replica.node);
+
+SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
struct ovsdb_monitor_row *row, *next;
@@ -1852,13 +1862,20 @@ ovsdb_monitor_destroy(struct ovsdb_replica *replica)
free(mt->columns);
free(mt);
}
-shash_destroy(&m->tables);
-hmap_remove(&jsonrpc_monitor->session->monitors, &jsonrpc_monitor->node);
-free(jsonrpc_monitor);
-free(m);
+shash_destroy(&dbmon->tables);
+free(dbmon);
+}
+
+static void
+ovsdb_monitor_destroy_callback(struct ovsdb_replica *replica)
+{
+struct ovsdb_monitor *dbmon = ovsdb_monitor_cast(replica);
+struct ovsdb_jsonrpc_monitor *m = dbmon->jsonrpc_monitor;
+
+ovsdb_jsonrpc_monitor_destroy(m);
}
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_clas
[ovs-dev] [ovsdb speedup 09/18] ovsdb: Split out monitor backend functions to ovsdb-monitor.c/h
Added new files ovsdb-monitor.[ch] for monitor backend functions.
There is no functional changes.
Signed-off-by: Andy Zhou
---
ovsdb/automake.mk | 2 +
ovsdb/jsonrpc-server.c | 577 +---
ovsdb/jsonrpc-server.h | 3 +
ovsdb/ovsdb-monitor.c | 583 +
ovsdb/ovsdb-monitor.h | 58 +
5 files changed, 655 insertions(+), 568 deletions(-)
create mode 100644 ovsdb/ovsdb-monitor.c
create mode 100644 ovsdb/ovsdb-monitor.h
diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index a66974a..846932a 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -20,6 +20,8 @@ ovsdb_libovsdb_la_SOURCES = \
ovsdb/mutation.h \
ovsdb/ovsdb.c \
ovsdb/ovsdb.h \
+ ovsdb/ovsdb-monitor.c \
+ ovsdb/ovsdb-monitor.h \
ovsdb/query.c \
ovsdb/query.h \
ovsdb/row.c \
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 78c7554..8ddf6c7 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -37,13 +37,13 @@
#include "timeval.h"
#include "transaction.h"
#include "trigger.h"
+#include "ovsdb-monitor.h"
#include "openvswitch/vlog.h"
VLOG_DEFINE_THIS_MODULE(ovsdb_jsonrpc_server);
struct ovsdb_jsonrpc_remote;
struct ovsdb_jsonrpc_session;
-struct ovsdb_jsonrpc_monitor;
/* Message rate-limiting. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -81,7 +81,6 @@ static void ovsdb_jsonrpc_trigger_complete_done(
/* Monitors. */
static struct json *ovsdb_jsonrpc_monitor_create(
struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
-static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -1028,86 +1027,16 @@ ovsdb_jsonrpc_trigger_complete_done(struct
ovsdb_jsonrpc_session *s)
}
}
-/* database table monitors. */
-
-enum ovsdb_monitor_selection {
-OJMS_INITIAL = 1 << 0, /* All rows when monitor is created. */
-OJMS_INSERT = 1 << 1, /* New rows. */
-OJMS_DELETE = 1 << 2, /* Deleted rows. */
-OJMS_MODIFY = 1 << 3/* Modified rows. */
-};
-
-/* A particular column being monitored. */
-struct ovsdb_monitor_column {
-const struct ovsdb_column *column;
-enum ovsdb_monitor_selection select;
-};
-
-/* A row that has changed in a monitored table. */
-struct ovsdb_monitor_row {
-struct hmap_node hmap_node; /* In ovsdb_jsonrpc_monitor_table.changes. */
-struct uuid uuid; /* UUID of row that changed. */
-struct ovsdb_datum *old;/* Old data, NULL for an inserted row. */
-struct ovsdb_datum *new;/* New data, NULL for a deleted row. */
-};
-
-/* A particular table being monitored. */
-struct ovsdb_monitor_table {
-const struct ovsdb_table *table;
-
-/* This is the union (bitwise-OR) of the 'select' values in all of the
- * members of 'columns' below. */
-enum ovsdb_monitor_selection select;
-
-/* Columns being monitored. */
-struct ovsdb_monitor_column *columns;
-size_t n_columns;
-
-/* Contains 'struct ovsdb_monitor_row's for rows that have been
- * updated but not yet flushed to the jsonrpc connection. */
-struct hmap changes;
-};
-
-struct ovsdb_jsonrpc_monitor;
-/* Backend monitor.
- *
- * ovsdb_monitor keep track of the ovsdb changes.
- */
-/* A collection of tables being monitored. */
-struct ovsdb_monitor {
-struct ovsdb_replica replica;
-struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
-};
-
/* Jsonrpc front end monitor. */
struct ovsdb_jsonrpc_monitor {
struct ovsdb_jsonrpc_session *session;
struct ovsdb *db;
struct hmap_node node; /* In ovsdb_jsonrpc_session's "monitors". */
-
struct json *monitor_id;
struct ovsdb_monitor *dbmon;
};
-static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
-
-struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
-struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
-static void ovsdb_monitor_destroy(struct ovsdb_monitor *);
-static struct json *ovsdb_monitor_get_initial(
-const struct ovsdb_monitor *);
-
-static bool
-parse_bool(struct ovsdb_parser *parser, const char *name, bool default_value)
-{
-const struct json *json;
-
-json = ovsdb_parser_member(parser, name, OP_BOOLEAN | OP_OPTIONAL);
-return json ? json_boolean(json) : default_value;
-}
-
-struct ovsdb_jsonrpc_monitor *
+static struct ovsdb_jsonrpc_monitor *
ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session *s,
const struct json *monitor_id)
{
@@ -1122,45 +1051,13 @@ ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session
*s,
return NULL;
}
-static void
-ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
- const s
[ovs-dev] [PATCH 10/20] ovsdb: refactoring ovsdb_monitor_destroy()
Add ovsdb_minitor_destory() function to properly cleanup ovsdb_monitor.
It is also responsible for unhook from the replica chain.
The replica destroy callback is now called
ovsdb_monitor_destroy_callback()
Minor variable renaming in ovsdb_monitor_create() to make it
more consistent.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 59 --
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 886062c..78c7554 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -43,6 +43,7 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_jsonrpc_server);
struct ovsdb_jsonrpc_remote;
struct ovsdb_jsonrpc_session;
+struct ovsdb_jsonrpc_monitor;
/* Message rate-limiting. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -80,6 +81,7 @@ static void ovsdb_jsonrpc_trigger_complete_done(
/* Monitors. */
static struct json *ovsdb_jsonrpc_monitor_create(
struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
+static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -1092,7 +1094,7 @@ static const struct ovsdb_replica_class
ovsdb_jsonrpc_replica_class;
struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
-static void ovsdb_monitor_destroy(struct ovsdb_replica *);
+static void ovsdb_monitor_destroy(struct ovsdb_monitor *);
static struct json *ovsdb_monitor_get_initial(
const struct ovsdb_monitor *);
@@ -1252,16 +1254,16 @@ ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor,
const struct ovsdb_replica_class *replica_class)
{
-struct ovsdb_monitor *m;
+struct ovsdb_monitor *dbmon;
-m = xzalloc(sizeof *m);
+dbmon = xzalloc(sizeof *dbmon);
-ovsdb_replica_init(&m->replica, replica_class);
-ovsdb_add_replica(db, &m->replica);
-m->jsonrpc_monitor = jsonrpc_monitor;
-shash_init(&m->tables);
+ovsdb_replica_init(&dbmon->replica, replica_class);
+ovsdb_add_replica(db, &dbmon->replica);
+dbmon->jsonrpc_monitor = jsonrpc_monitor;
+shash_init(&dbmon->tables);
-return m;
+return dbmon;
}
static void
@@ -1387,7 +1389,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
error:
if (m) {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
}
json = ovsdb_error_to_json(error);
@@ -1411,7 +1413,7 @@ ovsdb_jsonrpc_monitor_cancel(struct ovsdb_jsonrpc_session
*s,
return jsonrpc_create_error(json_string_create("unknown monitor"),
request_id);
} else {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
return jsonrpc_create_reply(json_object_create(), request_id);
}
}
@@ -1423,7 +1425,7 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
struct ovsdb_jsonrpc_monitor *m, *next;
HMAP_FOR_EACH_SAFE (m, next, node, &s->monitors) {
-ovsdb_remove_replica(m->db, &m->dbmon->replica);
+ovsdb_jsonrpc_monitor_destroy(m);
}
}
@@ -1832,14 +1834,22 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor
*dbmon)
}
static void
-ovsdb_monitor_destroy(struct ovsdb_replica *replica)
+ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *m)
+{
+json_destroy(m->monitor_id);
+hmap_remove(&m->session->monitors, &m->node);
+ovsdb_monitor_destroy(m->dbmon);
+free(m);
+}
+
+static void
+ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon)
{
-struct ovsdb_monitor *m = ovsdb_monitor_cast(replica);
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor = m->jsonrpc_monitor;
struct shash_node *node;
-json_destroy(jsonrpc_monitor->monitor_id);
-SHASH_FOR_EACH (node, &m->tables) {
+list_remove(&dbmon->replica.node);
+
+SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
struct ovsdb_monitor_row *row, *next;
@@ -1852,13 +1862,20 @@ ovsdb_monitor_destroy(struct ovsdb_replica *replica)
free(mt->columns);
free(mt);
}
-shash_destroy(&m->tables);
-hmap_remove(&jsonrpc_monitor->session->monitors, &jsonrpc_monitor->node);
-free(jsonrpc_monitor);
-free(m);
+shash_destroy(&dbmon->tables);
+free(dbmon);
+}
+
+static void
+ovsdb_monitor_destroy_callback(struct ovsdb_replica *replica)
+{
+struct ovsdb_monitor *dbmon = ovsdb_monitor_cast(replica);
+struct ovsdb_jsonrpc_monitor *m = dbmon->jsonrpc_monitor;
+
+ovsdb_jsonrpc_monitor_destroy(m);
}
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_clas
[ovs-dev] [ovsdb speedup 10/18] ovsdb: refactoring ovsdb_monitor_get_initial
Refactoring ovsdb_monitor_get_initial() to not generate JSON object.
It only collect changes within the ovsdb_monitor().
ovsdb_jsonrpc_monitor_compose_table_update() is then used to generate
JSON object.
This change will also make future patch easier.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 7 ++-
ovsdb/ovsdb-monitor.c | 5 +
ovsdb/ovsdb-monitor.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 8ddf6c7..2437d4b 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -88,6 +88,9 @@ static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
static void ovsdb_jsonrpc_monitor_remove_all(struct ovsdb_jsonrpc_session *);
static void ovsdb_jsonrpc_monitor_flush_all(struct ovsdb_jsonrpc_session *);
static bool ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *);
+static struct json *ovsdb_jsonrpc_monitor_compose_table_update(
+const struct ovsdb_jsonrpc_monitor *monitor, bool initial);
+
/* JSON-RPC database server. */
@@ -1227,7 +1230,9 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
-return ovsdb_monitor_get_initial(m->dbmon);
+ovsdb_monitor_get_initial(m->dbmon);
+json = ovsdb_jsonrpc_monitor_compose_table_update(m, true);
+return json ? json : json_object_create();
error:
if (m) {
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 8d141b4..01a9828 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -507,12 +507,11 @@ ovsdb_monitor_change_cb(const struct ovsdb_row *old,
return true;
}
-struct json *
+void
ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
{
struct ovsdb_monitor_aux aux;
struct shash_node *node;
-struct json *json;
ovsdb_monitor_init_aux(&aux, dbmon);
SHASH_FOR_EACH (node, &dbmon->tables) {
@@ -526,8 +525,6 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
}
}
}
-json = ovsdb_monitor_compose_table_update(dbmon, true);
-return json ? json : json_object_create();
}
void
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index 4841302..6b7f730 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -52,7 +52,7 @@ void ovsdb_monitor_table_set_select(struct ovsdb_monitor
*dbmon,
bool ovsdb_monitor_needs_flush(struct ovsdb_monitor *dbmon);
-struct json *ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon);
+void ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon);
void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
#endif
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 11/18] ovsdb: ovsdb-monitor stores jsonrpc-monitor in a linked-list
Currently, each ovsdb-monitor points to a single jsonrpc_monitor object.
This means there is 1:1 relationship between them.
In case multiple jsonrpc-monitors needs to monitor the same tables and
the columns within them, then can share a single ovsdb-monitor, so the
updates only needs to be maintained once.
This patch, with a few following patches is to allow for N:1 mapping
between jsonrpc-monitor and ovsdb-monitor.
Maintain jsonrpc-monitors pointers in a linked-list is the first
step towards allowing N:1 mapping. The ovsdb-monitor life cycle
is now reference counted. An empty list means zero references.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 2 +-
ovsdb/ovsdb-monitor.c | 53 ++
ovsdb/ovsdb-monitor.h | 5 +++--
3 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 2437d4b..4eb6f24 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1322,6 +1322,6 @@ ovsdb_jsonrpc_monitor_destroy(struct
ovsdb_jsonrpc_monitor *m)
{
json_destroy(m->monitor_id);
hmap_remove(&m->session->monitors, &m->node);
-ovsdb_monitor_destroy(m->dbmon);
+ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
free(m);
}
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 01a9828..d20593e 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -49,10 +49,15 @@ static const struct ovsdb_replica_class
ovsdb_jsonrpc_replica_class;
struct ovsdb_monitor {
struct ovsdb_replica replica;
struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
+struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
struct ovsdb *db;
};
+struct jsonrpc_monitor_node {
+struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
+struct ovs_list node;
+};
+
/* A particular column being monitored. */
struct ovsdb_monitor_column {
const struct ovsdb_column *column;
@@ -84,6 +89,8 @@ struct ovsdb_monitor_table {
struct hmap changes;
};
+static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
+
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
{
@@ -201,15 +208,20 @@ ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
{
struct ovsdb_monitor *dbmon;
+struct jsonrpc_monitor_node *jm;
dbmon = xzalloc(sizeof *dbmon);
ovsdb_replica_init(&dbmon->replica, &ovsdb_jsonrpc_replica_class);
ovsdb_add_replica(db, &dbmon->replica);
-dbmon->jsonrpc_monitor = jsonrpc_monitor;
+list_init(&dbmon->jsonrpc_monitors);
dbmon->db = db;
shash_init(&dbmon->tables);
+jm = xzalloc(sizeof *jm);
+jm->jsonrpc_monitor = jsonrpc_monitor;
+list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
+
return dbmon;
}
@@ -528,6 +540,33 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor
*dbmon)
}
void
+ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+ struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+{
+struct jsonrpc_monitor_node *jm;
+
+/* Find and remove the jsonrpc monitor from the list. */
+LIST_FOR_EACH(jm, node, &dbmon->jsonrpc_monitors) {
+if (jm->jsonrpc_monitor == jsonrpc_monitor) {
+list_remove(&jm->node);
+free(jm);
+
+/* If this is the last jsonrpc monitor, also destory
+ * ovsdb monitor, since there is no other users of
+ * the monitor. */
+if (list_is_empty(&dbmon->jsonrpc_monitors)) {
+ovsdb_monitor_destroy(dbmon);
+}
+
+return;
+};
+}
+
+/* Should never reach here. jsonrpc_monitor should be on the list. */
+ovs_assert(true);
+}
+
+static void
ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon)
{
struct shash_node *node;
@@ -569,9 +608,15 @@ static void
ovsdb_monitor_destroy_callback(struct ovsdb_replica *replica)
{
struct ovsdb_monitor *dbmon = ovsdb_monitor_cast(replica);
-struct ovsdb_jsonrpc_monitor *m = dbmon->jsonrpc_monitor;
+struct jsonrpc_monitor_node *jm, *next;
+
+/* Delete all front end monitors. Removing the last front
+ * end monitor will also destroy the corresponding 'ovsdb_monitor'.
+ * ovsdb monitor will also be destroied. */
+LIST_FOR_EACH_SAFE(jm, next, node, &dbmon->jsonrpc_monitors) {
+ovsdb_jsonrpc_monitor_destroy(jm->jsonrpc_monitor);
+}
-ovsdb_jsonrpc_monitor_destroy(m);
}
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class = {
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index 6b7f730..829a135 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -30,6 +30,9 @@ enum ovsdb_monitor_selection {
struct ovsdb_monitor *ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc
[ovs-dev] [PATCH 11/20] ovsdb: Split out monitor backend functions to ovsdb-monitor.c/h
Added new files ovsdb-monitor.[ch] for monitor backend functions.
There is no functional changes.
Signed-off-by: Andy Zhou
---
ovsdb/automake.mk | 2 +
ovsdb/jsonrpc-server.c | 577 +---
ovsdb/jsonrpc-server.h | 3 +
ovsdb/ovsdb-monitor.c | 583 +
ovsdb/ovsdb-monitor.h | 58 +
5 files changed, 655 insertions(+), 568 deletions(-)
create mode 100644 ovsdb/ovsdb-monitor.c
create mode 100644 ovsdb/ovsdb-monitor.h
diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index a66974a..846932a 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -20,6 +20,8 @@ ovsdb_libovsdb_la_SOURCES = \
ovsdb/mutation.h \
ovsdb/ovsdb.c \
ovsdb/ovsdb.h \
+ ovsdb/ovsdb-monitor.c \
+ ovsdb/ovsdb-monitor.h \
ovsdb/query.c \
ovsdb/query.h \
ovsdb/row.c \
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 78c7554..8ddf6c7 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -37,13 +37,13 @@
#include "timeval.h"
#include "transaction.h"
#include "trigger.h"
+#include "ovsdb-monitor.h"
#include "openvswitch/vlog.h"
VLOG_DEFINE_THIS_MODULE(ovsdb_jsonrpc_server);
struct ovsdb_jsonrpc_remote;
struct ovsdb_jsonrpc_session;
-struct ovsdb_jsonrpc_monitor;
/* Message rate-limiting. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -81,7 +81,6 @@ static void ovsdb_jsonrpc_trigger_complete_done(
/* Monitors. */
static struct json *ovsdb_jsonrpc_monitor_create(
struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
-static void ovsdb_jsonrpc_monitor_destroy(struct ovsdb_jsonrpc_monitor *);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -1028,86 +1027,16 @@ ovsdb_jsonrpc_trigger_complete_done(struct
ovsdb_jsonrpc_session *s)
}
}
-/* database table monitors. */
-
-enum ovsdb_monitor_selection {
-OJMS_INITIAL = 1 << 0, /* All rows when monitor is created. */
-OJMS_INSERT = 1 << 1, /* New rows. */
-OJMS_DELETE = 1 << 2, /* Deleted rows. */
-OJMS_MODIFY = 1 << 3/* Modified rows. */
-};
-
-/* A particular column being monitored. */
-struct ovsdb_monitor_column {
-const struct ovsdb_column *column;
-enum ovsdb_monitor_selection select;
-};
-
-/* A row that has changed in a monitored table. */
-struct ovsdb_monitor_row {
-struct hmap_node hmap_node; /* In ovsdb_jsonrpc_monitor_table.changes. */
-struct uuid uuid; /* UUID of row that changed. */
-struct ovsdb_datum *old;/* Old data, NULL for an inserted row. */
-struct ovsdb_datum *new;/* New data, NULL for a deleted row. */
-};
-
-/* A particular table being monitored. */
-struct ovsdb_monitor_table {
-const struct ovsdb_table *table;
-
-/* This is the union (bitwise-OR) of the 'select' values in all of the
- * members of 'columns' below. */
-enum ovsdb_monitor_selection select;
-
-/* Columns being monitored. */
-struct ovsdb_monitor_column *columns;
-size_t n_columns;
-
-/* Contains 'struct ovsdb_monitor_row's for rows that have been
- * updated but not yet flushed to the jsonrpc connection. */
-struct hmap changes;
-};
-
-struct ovsdb_jsonrpc_monitor;
-/* Backend monitor.
- *
- * ovsdb_monitor keep track of the ovsdb changes.
- */
-/* A collection of tables being monitored. */
-struct ovsdb_monitor {
-struct ovsdb_replica replica;
-struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
-};
-
/* Jsonrpc front end monitor. */
struct ovsdb_jsonrpc_monitor {
struct ovsdb_jsonrpc_session *session;
struct ovsdb *db;
struct hmap_node node; /* In ovsdb_jsonrpc_session's "monitors". */
-
struct json *monitor_id;
struct ovsdb_monitor *dbmon;
};
-static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
-
-struct ovsdb_jsonrpc_monitor *ovsdb_jsonrpc_monitor_find(
-struct ovsdb_jsonrpc_session *, const struct json *monitor_id);
-static void ovsdb_monitor_destroy(struct ovsdb_monitor *);
-static struct json *ovsdb_monitor_get_initial(
-const struct ovsdb_monitor *);
-
-static bool
-parse_bool(struct ovsdb_parser *parser, const char *name, bool default_value)
-{
-const struct json *json;
-
-json = ovsdb_parser_member(parser, name, OP_BOOLEAN | OP_OPTIONAL);
-return json ? json_boolean(json) : default_value;
-}
-
-struct ovsdb_jsonrpc_monitor *
+static struct ovsdb_jsonrpc_monitor *
ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session *s,
const struct json *monitor_id)
{
@@ -1122,45 +1051,13 @@ ovsdb_jsonrpc_monitor_find(struct ovsdb_jsonrpc_session
*s,
return NULL;
}
-static void
-ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
- const s
[ovs-dev] [PATCH 12/20] ovsdb: refactoring ovsdb_monitor_get_initial
Refactoring ovsdb_monitor_get_initial() to not generate JSON object.
It only collect changes within the ovsdb_monitor().
ovsdb_jsonrpc_monitor_compose_table_update() is then used to generate
JSON object.
This change will also make future patch easier.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 7 ++-
ovsdb/ovsdb-monitor.c | 5 +
ovsdb/ovsdb-monitor.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 8ddf6c7..2437d4b 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -88,6 +88,9 @@ static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
static void ovsdb_jsonrpc_monitor_remove_all(struct ovsdb_jsonrpc_session *);
static void ovsdb_jsonrpc_monitor_flush_all(struct ovsdb_jsonrpc_session *);
static bool ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *);
+static struct json *ovsdb_jsonrpc_monitor_compose_table_update(
+const struct ovsdb_jsonrpc_monitor *monitor, bool initial);
+
/* JSON-RPC database server. */
@@ -1227,7 +1230,9 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
-return ovsdb_monitor_get_initial(m->dbmon);
+ovsdb_monitor_get_initial(m->dbmon);
+json = ovsdb_jsonrpc_monitor_compose_table_update(m, true);
+return json ? json : json_object_create();
error:
if (m) {
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 8d141b4..01a9828 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -507,12 +507,11 @@ ovsdb_monitor_change_cb(const struct ovsdb_row *old,
return true;
}
-struct json *
+void
ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
{
struct ovsdb_monitor_aux aux;
struct shash_node *node;
-struct json *json;
ovsdb_monitor_init_aux(&aux, dbmon);
SHASH_FOR_EACH (node, &dbmon->tables) {
@@ -526,8 +525,6 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon)
}
}
}
-json = ovsdb_monitor_compose_table_update(dbmon, true);
-return json ? json : json_object_create();
}
void
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index 4841302..6b7f730 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -52,7 +52,7 @@ void ovsdb_monitor_table_set_select(struct ovsdb_monitor
*dbmon,
bool ovsdb_monitor_needs_flush(struct ovsdb_monitor *dbmon);
-struct json *ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon);
+void ovsdb_monitor_get_initial(const struct ovsdb_monitor *dbmon);
void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
#endif
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [ovsdb speedup 12/18] ovsdb: add transaction ids
With N:1 mappings, multiple jsonrpc server may be servicing the rpc
connection at a different pace. ovsdb-monitor thus needs to maintain
different change sets, depends on connection speed of each rpc
connections. Connections servicing at the same speed can share the
same change set.
Transaction ID is an concept added to describe the change set. One
possible view of the database state is a sequence of changes, more
precisely, commits be applied to it in order, starting from an
initial state, with commit 0. The logic can also be applied to the
jsonrpc monitor; each change it pushes corresponds to commits between
two transaction IDs.
This patch introduces transaction IDs. For ovsdb-monitor, it maintains
n_transactions, starting from 0. Each commit add 1 to the number.
Jsonrpc maintains and 'unflushed' transaction number, corresponding to
the next commit the remote has not seen. jsonrpc's job is simply to
notice there are changes in the ovsdb-monitor that it is interested in,
i.e. 'n_transactions' >= 'unflushed', get the changes in json format,
and push them to the remote site.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 12
ovsdb/ovsdb-monitor.c | 24 +++-
ovsdb/ovsdb-monitor.h | 7 ---
3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 4eb6f24..bb19da0 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -89,7 +89,7 @@ static void ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *);
static void ovsdb_jsonrpc_monitor_flush_all(struct ovsdb_jsonrpc_session *);
static bool ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *);
static struct json *ovsdb_jsonrpc_monitor_compose_table_update(
-const struct ovsdb_jsonrpc_monitor *monitor, bool initial);
+struct ovsdb_jsonrpc_monitor *monitor, bool initial);
/* JSON-RPC database server. */
@@ -1037,6 +1037,8 @@ struct ovsdb_jsonrpc_monitor {
struct hmap_node node; /* In ovsdb_jsonrpc_session's "monitors". */
struct json *monitor_id;
struct ovsdb_monitor *dbmon;
+uint64_t unflushed; /* The first transaction that has not been
+ flushed to the jsonrpc remote client. */
};
static struct ovsdb_jsonrpc_monitor *
@@ -1180,6 +1182,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
m->session = s;
m->db = db;
m->dbmon = ovsdb_monitor_create(db, m);
+m->unflushed = 0;
hmap_insert(&s->monitors, &m->node, json_hash(monitor_id, 0));
m->monitor_id = json_clone(monitor_id);
@@ -1278,9 +1281,10 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
static struct json *
ovsdb_jsonrpc_monitor_compose_table_update(
-const struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+struct ovsdb_jsonrpc_monitor *monitor, bool initial)
{
-return ovsdb_monitor_compose_table_update(monitor->dbmon, initial);
+return ovsdb_monitor_compose_table_update(monitor->dbmon, initial,
+ &monitor->unflushed);
}
static bool
@@ -1289,7 +1293,7 @@ ovsdb_jsonrpc_monitor_needs_flush(struct
ovsdb_jsonrpc_session *s)
struct ovsdb_jsonrpc_monitor *m;
HMAP_FOR_EACH (m, node, &s->monitors) {
-if (ovsdb_monitor_needs_flush(m->dbmon)) {
+if (ovsdb_monitor_needs_flush(m->dbmon, m->unflushed)) {
return true;
}
}
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index d20593e..d336b12 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -51,6 +51,7 @@ struct ovsdb_monitor {
struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
struct ovsdb *db;
+uint64_t n_transactions; /* Count number of commited transactions. */
};
struct jsonrpc_monitor_node {
@@ -216,6 +217,7 @@ ovsdb_monitor_create(struct ovsdb *db,
ovsdb_add_replica(db, &dbmon->replica);
list_init(&dbmon->jsonrpc_monitors);
dbmon->db = db;
+dbmon->n_transactions = 0;
shash_init(&dbmon->tables);
jm = xzalloc(sizeof *jm);
@@ -378,14 +380,16 @@ ovsdb_monitor_compose_row_update(
* be used as part of the initial reply to a "monitor" request, false if it is
* going to be used as part of an "update" notification. */
struct json *
-ovsdb_monitor_compose_table_update(
-const struct ovsdb_monitor *dbmon, bool initial)
+ovsdb_monitor_compose_table_update(const struct ovsdb_monitor *dbmon,
+ bool initial, uint64_t *unflushed)
{
struct shash_node *node;
unsigned long int *changed;
struct json *json;
size_t max_columns;
+*unflushed = dbmon->n_transactions + 1;
+
max_columns = 0;
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
@@ -434,18 +4
[ovs-dev] [PATCH 13/20] ovsdb: ovsdb-monitor stores jsonrpc-monitor in a linked-list
Currently, each ovsdb-monitor points to a single jsonrpc_monitor object.
This means there is 1:1 relationship between them.
In case multiple jsonrpc-monitors needs to monitor the same tables and
the columns within them, then can share a single ovsdb-monitor, so the
updates only needs to be maintained once.
This patch, with a few following patches is to allow for N:1 mapping
between jsonrpc-monitor and ovsdb-monitor.
Maintain jsonrpc-monitors pointers in a linked-list is the first
step towards allowing N:1 mapping. The ovsdb-monitor life cycle
is now reference counted. An empty list means zero references.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 2 +-
ovsdb/ovsdb-monitor.c | 53 ++
ovsdb/ovsdb-monitor.h | 5 +++--
3 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 2437d4b..4eb6f24 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1322,6 +1322,6 @@ ovsdb_jsonrpc_monitor_destroy(struct
ovsdb_jsonrpc_monitor *m)
{
json_destroy(m->monitor_id);
hmap_remove(&m->session->monitors, &m->node);
-ovsdb_monitor_destroy(m->dbmon);
+ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
free(m);
}
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 01a9828..d20593e 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -49,10 +49,15 @@ static const struct ovsdb_replica_class
ovsdb_jsonrpc_replica_class;
struct ovsdb_monitor {
struct ovsdb_replica replica;
struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
-struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
+struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
struct ovsdb *db;
};
+struct jsonrpc_monitor_node {
+struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
+struct ovs_list node;
+};
+
/* A particular column being monitored. */
struct ovsdb_monitor_column {
const struct ovsdb_column *column;
@@ -84,6 +89,8 @@ struct ovsdb_monitor_table {
struct hmap changes;
};
+static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
+
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
{
@@ -201,15 +208,20 @@ ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
{
struct ovsdb_monitor *dbmon;
+struct jsonrpc_monitor_node *jm;
dbmon = xzalloc(sizeof *dbmon);
ovsdb_replica_init(&dbmon->replica, &ovsdb_jsonrpc_replica_class);
ovsdb_add_replica(db, &dbmon->replica);
-dbmon->jsonrpc_monitor = jsonrpc_monitor;
+list_init(&dbmon->jsonrpc_monitors);
dbmon->db = db;
shash_init(&dbmon->tables);
+jm = xzalloc(sizeof *jm);
+jm->jsonrpc_monitor = jsonrpc_monitor;
+list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
+
return dbmon;
}
@@ -528,6 +540,33 @@ ovsdb_monitor_get_initial(const struct ovsdb_monitor
*dbmon)
}
void
+ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+ struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+{
+struct jsonrpc_monitor_node *jm;
+
+/* Find and remove the jsonrpc monitor from the list. */
+LIST_FOR_EACH(jm, node, &dbmon->jsonrpc_monitors) {
+if (jm->jsonrpc_monitor == jsonrpc_monitor) {
+list_remove(&jm->node);
+free(jm);
+
+/* If this is the last jsonrpc monitor, also destory
+ * ovsdb monitor, since there is no other users of
+ * the monitor. */
+if (list_is_empty(&dbmon->jsonrpc_monitors)) {
+ovsdb_monitor_destroy(dbmon);
+}
+
+return;
+};
+}
+
+/* Should never reach here. jsonrpc_monitor should be on the list. */
+ovs_assert(true);
+}
+
+static void
ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon)
{
struct shash_node *node;
@@ -569,9 +608,15 @@ static void
ovsdb_monitor_destroy_callback(struct ovsdb_replica *replica)
{
struct ovsdb_monitor *dbmon = ovsdb_monitor_cast(replica);
-struct ovsdb_jsonrpc_monitor *m = dbmon->jsonrpc_monitor;
+struct jsonrpc_monitor_node *jm, *next;
+
+/* Delete all front end monitors. Removing the last front
+ * end monitor will also destroy the corresponding 'ovsdb_monitor'.
+ * ovsdb monitor will also be destroied. */
+LIST_FOR_EACH_SAFE(jm, next, node, &dbmon->jsonrpc_monitors) {
+ovsdb_jsonrpc_monitor_destroy(jm->jsonrpc_monitor);
+}
-ovsdb_jsonrpc_monitor_destroy(m);
}
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class = {
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index 6b7f730..829a135 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -30,6 +30,9 @@ enum ovsdb_monitor_selection {
struct ovsdb_monitor *ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc
[ovs-dev] [ovsdb speedup 14/18] ovsdb: add ovsdb_monitor_changes
Currently, each monitor table contains a single hmap 'changes' to
track updates. This patch introduces a new data structure
'ovsdb_monitor_changes' that stores the updates 'rows' tagged by
its first commit transaction id. Each 'ovsdb_monitor_changes' is
refenece counted allowing multiple jsonrpc_monitors to share them.
The next patch will allow each ovsdb monitor table to store a list
of 'ovsdb_monitor_changes'. This patch stores only one, same as
before.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 16 --
ovsdb/ovsdb-monitor.c | 133 +++--
ovsdb/ovsdb-monitor.h | 3 ++
3 files changed, 133 insertions(+), 19 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index ddcacbb..c9d84a5 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1280,11 +1280,19 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
}
static struct json *
-ovsdb_jsonrpc_monitor_compose_update(
-struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+ovsdb_jsonrpc_monitor_compose_update(struct ovsdb_jsonrpc_monitor *m,
+ bool initial)
{
-return ovsdb_monitor_compose_update(monitor->dbmon, initial,
-&monitor->unflushed);
+uint64_t unflushed;
+struct json *json;
+
+unflushed = initial ? 0 : m->unflushed;
+
+json = ovsdb_monitor_compose_update(m->dbmon, initial,
+&m->unflushed);
+ovsdb_monitor_renew_tracking_changes(m->dbmon, unflushed, m->unflushed);
+
+return json;
}
static bool
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 397aa2a..a1efa66 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -73,6 +73,22 @@ struct ovsdb_monitor_row {
struct ovsdb_datum *new;/* New data, NULL for a deleted row. */
};
+/* Contains 'struct ovsdb_monitor_row's for rows that have been
+ * updated but not yet flushed to all the jsonrpc connection.
+ *
+ * 'n_refs' represent the number of jsonrpc connections that have
+ * not received updates. Generate the update for the last jsonprc
+ * connection will also remove rows contained in 'changes'.
+ *
+ * 'transaction' stores the first update's transaction id.
+ * */
+struct ovsdb_monitor_changes {
+struct ovsdb_monitor_table *mt;
+struct hmap rows;
+int n_refs;
+uint64_t transaction;
+};
+
/* A particular table being monitored. */
struct ovsdb_monitor_table {
const struct ovsdb_table *table;
@@ -87,10 +103,16 @@ struct ovsdb_monitor_table {
/* Contains 'struct ovsdb_monitor_row's for rows that have been
* updated but not yet flushed to the jsonrpc connection. */
-struct hmap changes;
+struct ovsdb_monitor_changes *changes;
};
static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
+static void ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
+uint64_t next_txn);
+static void ovsdb_monitor_changes_destroy_rows(
+ struct ovsdb_monitor_changes *changes);
+static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
+ uint64_t transaction);
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
@@ -108,7 +130,7 @@ ovsdb_monitor_cast(struct ovsdb_replica *replica)
return CONTAINER_OF(replica, struct ovsdb_monitor, replica);
}
-/* Finds and returns the ovsdb_monitor_row in 'mt->changes' for the
+/* Finds and returns the ovsdb_monitor_row in 'mt->changes->rows' for the
* given 'uuid', or NULL if there is no such row. */
static struct ovsdb_monitor_row *
ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
@@ -116,7 +138,8 @@ ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_row *row;
-HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid), &mt->changes) {
+HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid),
+ &mt->changes->rows) {
if (uuid_equals(uuid, &row->uuid)) {
return row;
}
@@ -235,7 +258,7 @@ ovsdb_monitor_add_table(struct ovsdb_monitor *m,
mt = xzalloc(sizeof *mt);
mt->table = table;
-hmap_init(&mt->changes);
+mt->changes = NULL;
shash_add(&m->tables, table->schema->name, mt);
}
@@ -288,6 +311,70 @@ ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor
*m,
return NULL;
}
+static void
+ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
+uint64_t next_txn)
+{
+struct ovsdb_monitor_changes *changes;
+
+changes = xzalloc(sizeof *changes);
+
+changes->transaction = next_txn;
+changes->mt = mt;
+changes->n_refs = 1;
+hmap_init(&changes->rows);
+mt->changes = changes;
+};
+
+/* Stop currently tracking changes to table 'mt' since 'transaction'.
[ovs-dev] [ovsdb speedup 18/18] ovsdb: add json cache
Although multiple jsonrpc monitor can share the same ovsdb monitor, each
change still needs to transalted into json object from scratch. This
can be wastful is mutiple jsonrpc monitors are interested in the
same changes.
Json chche impoves this by keeping an copy of json object generated
for transaction X to current transaction. When jsonrpc is interested
in a change, the cache is lookup first, if an json object is found,
a copy of it is handed back, skipping the regeneration process.
Any commit to the monitor will empty the cache. since none of them
will be useful anymore.
Signed-off-by: Andy Zhou
---
ovsdb/ovsdb-monitor.c | 100 ++
1 file changed, 100 insertions(+)
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 210b33c..2fda283 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -41,6 +41,7 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_monitor);
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
static struct hmap ovsdb_monitors;
+static struct hmap json_cache__;
/* Backend monitor.
*
@@ -58,6 +59,15 @@ struct ovsdb_monitor {
struct hmap_node hmap_node; /* Elements within ovsdb_monitors. */
};
+/* A json object of updates between 'from_txn' and 'dbmon->n_transactions'
+ * inclusive. */
+struct ovsdb_monitor_json_cache_node {
+struct hmap_node hmap_node; /* Elements in json cache. */
+const struct ovsdb_monitor *dbmon;
+uint64_t from_txn;
+struct json *json;/* Null, or a cloned of json */
+};
+
struct jsonrpc_monitor_node {
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
struct ovs_list node;
@@ -122,6 +132,84 @@ static void ovsdb_monitor_changes_destroy_rows(
static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
uint64_t unflushed);
+static struct hmap *
+get_json_cache(void)
+{
+static bool init__ = false;
+
+if (!init__) {
+hmap_init(&json_cache__);
+ init__ = true;
+}
+return &json_cache__;
+}
+
+static uint32_t
+ovsdb_monitor_json_cache_hash(const struct ovsdb_monitor *dbmon,
+ uint64_t from_txn)
+{
+uint32_t hash;
+
+hash = hash_uint64(from_txn);
+hash = hash_pointer(dbmon, hash);
+return hash;
+}
+
+static struct ovsdb_monitor_json_cache_node *
+ovsdb_monitor_json_cache_search(const struct ovsdb_monitor *dbmon,
+uint64_t from_txn)
+{
+struct ovsdb_monitor_json_cache_node *node;
+struct hmap *json_cache = get_json_cache();
+uint32_t hash;
+
+hash = ovsdb_monitor_json_cache_hash(dbmon, from_txn);
+
+HMAP_FOR_EACH_WITH_HASH(node, hmap_node, hash, json_cache) {
+if ((node->from_txn == from_txn) && (node->dbmon == dbmon)) {
+return node;
+}
+}
+
+return NULL;
+}
+
+static void
+ovsdb_monitor_json_cache_insert(const struct ovsdb_monitor *dbmon,
+uint64_t from_txn, struct json *json)
+{
+struct ovsdb_monitor_json_cache_node *node;
+struct hmap *json_cache = get_json_cache();
+uint32_t hash;
+
+hash = ovsdb_monitor_json_cache_hash(dbmon, from_txn);
+
+node = xmalloc(sizeof *node);
+
+node->from_txn = from_txn;
+node->json = json ? json_clone(json) : NULL;
+node->dbmon = dbmon;
+
+hmap_insert(json_cache, &node->hmap_node, hash);
+}
+
+static void
+ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon)
+{
+struct ovsdb_monitor_json_cache_node *node, *next;
+struct hmap *json_cache = get_json_cache();
+
+HMAP_FOR_EACH_SAFE(node, next, hmap_node, json_cache) {
+if (node->dbmon == dbmon) {
+hmap_remove(json_cache, &node->hmap_node);
+if (node->json) {
+json_destroy(node->json);
+}
+free(node);
+}
+}
+}
+
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
{
@@ -499,6 +587,7 @@ struct json *
ovsdb_monitor_compose_update(const struct ovsdb_monitor *dbmon,
bool initial, uint64_t *unflushed)
{
+struct ovsdb_monitor_json_cache_node *cache_node;
struct shash_node *node;
unsigned long int *changed;
struct json *json;
@@ -508,6 +597,12 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
from_txn = initial ? 0 : *unflushed;
*unflushed = dbmon->n_transactions + 1;
+/* Return cached json if one has been created already */
+cache_node = ovsdb_monitor_json_cache_search(dbmon, from_txn);
+if (cache_node) {
+return cache_node->json ? json_clone(cache_node->json) : NULL;
+}
+
max_columns = 0;
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
@@ -555,6 +650,8 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
}
free(changed);
+
+ovsdb_monitor_json_cache_insert(dbmon, from_txn, j
[ovs-dev] [ovsdb speedup 15/18] ovsdb: allow a list of 'ovsdb_monitor_changes' in each ovsdb monitor table
Signed-off-by: Andy Zhou
---
ovsdb/ovsdb-monitor.c | 124 ++
1 file changed, 85 insertions(+), 39 deletions(-)
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index a1efa66..3259f3c 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -87,6 +87,7 @@ struct ovsdb_monitor_changes {
struct hmap rows;
int n_refs;
uint64_t transaction;
+struct ovs_list node; /* Element in ovsdb_monitor_tables' changes list */
};
/* A particular table being monitored. */
@@ -101,18 +102,21 @@ struct ovsdb_monitor_table {
struct ovsdb_monitor_column *columns;
size_t n_columns;
-/* Contains 'struct ovsdb_monitor_row's for rows that have been
- * updated but not yet flushed to the jsonrpc connection. */
-struct ovsdb_monitor_changes *changes;
+/* Contains unsorted list of 'ovsdb_monitor_changes'. Each 'changes'
+ * tracks updates of rows starting from a different
+ * 'unflushed_transaction'. */
+struct ovs_list changes_list;
};
static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
static void ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
uint64_t next_txn);
+static struct ovsdb_monitor_changes *ovsdb_monitor_table_find_changes(
+struct ovsdb_monitor_table *mt, uint64_t unflushed);
static void ovsdb_monitor_changes_destroy_rows(
struct ovsdb_monitor_changes *changes);
static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
- uint64_t transaction);
+ uint64_t unflushed);
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
@@ -133,13 +137,13 @@ ovsdb_monitor_cast(struct ovsdb_replica *replica)
/* Finds and returns the ovsdb_monitor_row in 'mt->changes->rows' for the
* given 'uuid', or NULL if there is no such row. */
static struct ovsdb_monitor_row *
-ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
- const struct uuid *uuid)
+ovsdb_monitor_changes_row_find(const struct ovsdb_monitor_changes *changes,
+ const struct uuid *uuid)
{
struct ovsdb_monitor_row *row;
HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid),
- &mt->changes->rows) {
+ &changes->rows) {
if (uuid_equals(uuid, &row->uuid)) {
return row;
}
@@ -258,8 +262,8 @@ ovsdb_monitor_add_table(struct ovsdb_monitor *m,
mt = xzalloc(sizeof *mt);
mt->table = table;
-mt->changes = NULL;
shash_add(&m->tables, table->schema->name, mt);
+list_init(&mt->changes_list);
}
void
@@ -323,9 +327,25 @@ ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table
*mt,
changes->mt = mt;
changes->n_refs = 1;
hmap_init(&changes->rows);
-mt->changes = changes;
+list_push_back(&mt->changes_list, &changes->node);
};
+
+static struct ovsdb_monitor_changes *
+ovsdb_monitor_table_find_changes(struct ovsdb_monitor_table *mt,
+ uint64_t transaction)
+{
+struct ovsdb_monitor_changes *changes;
+
+LIST_FOR_EACH(changes, node, &mt->changes_list) {
+if (changes->transaction == transaction) {
+return changes;
+}
+}
+
+return NULL;
+}
+
/* Stop currently tracking changes to table 'mt' since 'transaction'.
*
* Return 'true' if the 'transaction' is being tracked. 'false' otherwise. */
@@ -335,14 +355,14 @@ ovsdb_monitor_table_untrack_changes(struct
ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_changes *changes;
-changes = mt->changes;
+changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
ovs_assert(changes->transaction == transaction);
if (--changes->n_refs == 0) {
ovsdb_monitor_changes_destroy_rows(changes);
+list_remove(&changes->node);
free(changes);
-mt->changes = NULL;
}
}
}
@@ -355,7 +375,7 @@ ovsdb_monitor_table_track_changes(struct
ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_changes *changes;
-changes = mt->changes;
+changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
ovs_assert(false);
} else {
@@ -474,6 +494,9 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
unsigned long int *changed;
struct json *json;
size_t max_columns;
+uint64_t from_txn;
+
+from_txn = initial ? 0 : *unflushed;
*unflushed = dbmon->n_transactions + 1;
@@ -489,13 +512,15 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
struct ovsdb_monitor_row *row, *next;
+struct ovsdb_monitor_changes *changes;
[ovs-dev] [PATCH 15/20] ovsdb: rename jsonrpc_monitor_compose_table_update()
jsonrpc_monitor_compse_update() seems fit better than
jsonrpc_monitor_compose_table_update(), since it composes changes
from all tables.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 12 ++--
ovsdb/ovsdb-monitor.c | 4 ++--
ovsdb/ovsdb-monitor.h | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index bb19da0..ddcacbb 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -88,7 +88,7 @@ static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
static void ovsdb_jsonrpc_monitor_remove_all(struct ovsdb_jsonrpc_session *);
static void ovsdb_jsonrpc_monitor_flush_all(struct ovsdb_jsonrpc_session *);
static bool ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *);
-static struct json *ovsdb_jsonrpc_monitor_compose_table_update(
+static struct json *ovsdb_jsonrpc_monitor_compose_update(
struct ovsdb_jsonrpc_monitor *monitor, bool initial);
@@ -1234,7 +1234,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
ovsdb_monitor_get_initial(m->dbmon);
-json = ovsdb_jsonrpc_monitor_compose_table_update(m, true);
+json = ovsdb_jsonrpc_monitor_compose_update(m, true);
return json ? json : json_object_create();
error:
@@ -1280,11 +1280,11 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
}
static struct json *
-ovsdb_jsonrpc_monitor_compose_table_update(
+ovsdb_jsonrpc_monitor_compose_update(
struct ovsdb_jsonrpc_monitor *monitor, bool initial)
{
-return ovsdb_monitor_compose_table_update(monitor->dbmon, initial,
- &monitor->unflushed);
+return ovsdb_monitor_compose_update(monitor->dbmon, initial,
+&monitor->unflushed);
}
static bool
@@ -1309,7 +1309,7 @@ ovsdb_jsonrpc_monitor_flush_all(struct
ovsdb_jsonrpc_session *s)
HMAP_FOR_EACH (m, node, &s->monitors) {
struct json *json;
-json = ovsdb_jsonrpc_monitor_compose_table_update(m, false);
+json = ovsdb_jsonrpc_monitor_compose_update(m, false);
if (json) {
struct jsonrpc_msg *msg;
struct json *params;
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index d336b12..397aa2a 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -380,8 +380,8 @@ ovsdb_monitor_compose_row_update(
* be used as part of the initial reply to a "monitor" request, false if it is
* going to be used as part of an "update" notification. */
struct json *
-ovsdb_monitor_compose_table_update(const struct ovsdb_monitor *dbmon,
- bool initial, uint64_t *unflushed)
+ovsdb_monitor_compose_update(const struct ovsdb_monitor *dbmon,
+ bool initial, uint64_t *unflushed)
{
struct shash_node *node;
unsigned long int *changed;
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index 5cb55dc..ea2a7aa 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -46,8 +46,8 @@ const char * OVS_WARN_UNUSED_RESULT
ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor *,
const struct ovsdb_table *);
-struct json *ovsdb_monitor_compose_table_update(const struct ovsdb_monitor
*dbmon,
- bool initial, uint64_t *unflushed_transaction);
+struct json *ovsdb_monitor_compose_update(const struct ovsdb_monitor *dbmon,
+bool initial, uint64_t *unflushed_transaction);
void ovsdb_monitor_table_set_select(struct ovsdb_monitor *dbmon,
const struct ovsdb_table *table,
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 16/20] ovsdb: add ovsdb_monitor_changes
Currently, each monitor table contains a single hmap 'changes' to
track updates. This patch introduces a new data structure
'ovsdb_monitor_changes' that stores the updates 'rows' tagged by
its first commit transaction id. Each 'ovsdb_monitor_changes' is
refenece counted allowing multiple jsonrpc_monitors to share them.
The next patch will allow each ovsdb monitor table to store a list
of 'ovsdb_monitor_changes'. This patch stores only one, same as
before.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 16 --
ovsdb/ovsdb-monitor.c | 133 +++--
ovsdb/ovsdb-monitor.h | 3 ++
3 files changed, 133 insertions(+), 19 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index ddcacbb..c9d84a5 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1280,11 +1280,19 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
}
static struct json *
-ovsdb_jsonrpc_monitor_compose_update(
-struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+ovsdb_jsonrpc_monitor_compose_update(struct ovsdb_jsonrpc_monitor *m,
+ bool initial)
{
-return ovsdb_monitor_compose_update(monitor->dbmon, initial,
-&monitor->unflushed);
+uint64_t unflushed;
+struct json *json;
+
+unflushed = initial ? 0 : m->unflushed;
+
+json = ovsdb_monitor_compose_update(m->dbmon, initial,
+&m->unflushed);
+ovsdb_monitor_renew_tracking_changes(m->dbmon, unflushed, m->unflushed);
+
+return json;
}
static bool
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 397aa2a..a1efa66 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -73,6 +73,22 @@ struct ovsdb_monitor_row {
struct ovsdb_datum *new;/* New data, NULL for a deleted row. */
};
+/* Contains 'struct ovsdb_monitor_row's for rows that have been
+ * updated but not yet flushed to all the jsonrpc connection.
+ *
+ * 'n_refs' represent the number of jsonrpc connections that have
+ * not received updates. Generate the update for the last jsonprc
+ * connection will also remove rows contained in 'changes'.
+ *
+ * 'transaction' stores the first update's transaction id.
+ * */
+struct ovsdb_monitor_changes {
+struct ovsdb_monitor_table *mt;
+struct hmap rows;
+int n_refs;
+uint64_t transaction;
+};
+
/* A particular table being monitored. */
struct ovsdb_monitor_table {
const struct ovsdb_table *table;
@@ -87,10 +103,16 @@ struct ovsdb_monitor_table {
/* Contains 'struct ovsdb_monitor_row's for rows that have been
* updated but not yet flushed to the jsonrpc connection. */
-struct hmap changes;
+struct ovsdb_monitor_changes *changes;
};
static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
+static void ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
+uint64_t next_txn);
+static void ovsdb_monitor_changes_destroy_rows(
+ struct ovsdb_monitor_changes *changes);
+static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
+ uint64_t transaction);
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
@@ -108,7 +130,7 @@ ovsdb_monitor_cast(struct ovsdb_replica *replica)
return CONTAINER_OF(replica, struct ovsdb_monitor, replica);
}
-/* Finds and returns the ovsdb_monitor_row in 'mt->changes' for the
+/* Finds and returns the ovsdb_monitor_row in 'mt->changes->rows' for the
* given 'uuid', or NULL if there is no such row. */
static struct ovsdb_monitor_row *
ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
@@ -116,7 +138,8 @@ ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_row *row;
-HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid), &mt->changes) {
+HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid),
+ &mt->changes->rows) {
if (uuid_equals(uuid, &row->uuid)) {
return row;
}
@@ -235,7 +258,7 @@ ovsdb_monitor_add_table(struct ovsdb_monitor *m,
mt = xzalloc(sizeof *mt);
mt->table = table;
-hmap_init(&mt->changes);
+mt->changes = NULL;
shash_add(&m->tables, table->schema->name, mt);
}
@@ -288,6 +311,70 @@ ovsdb_monitor_table_check_duplicates(struct ovsdb_monitor
*m,
return NULL;
}
+static void
+ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
+uint64_t next_txn)
+{
+struct ovsdb_monitor_changes *changes;
+
+changes = xzalloc(sizeof *changes);
+
+changes->transaction = next_txn;
+changes->mt = mt;
+changes->n_refs = 1;
+hmap_init(&changes->rows);
+mt->changes = changes;
+};
+
+/* Stop currently tracking changes to table 'mt' since 'transaction'.
[ovs-dev] [ovsdb speedup 16/18] ovsdb: refactor ovsdb_monitor_create()
Add ovsdb_monitor_add_jsonrpc_monitor(). This change will allow
ovsdb_monitor to be reference counted.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 2 ++
ovsdb/ovsdb-monitor.c | 16 +++-
ovsdb/ovsdb-monitor.h | 3 +++
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index c9d84a5..ba6df9d 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1233,6 +1233,8 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
+ovsdb_monitor_add_jsonrpc_monitor(m->dbmon, m);
+
ovsdb_monitor_get_initial(m->dbmon);
json = ovsdb_jsonrpc_monitor_compose_update(m, true);
return json ? json : json_object_create();
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 3259f3c..70a8e26 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -231,12 +231,21 @@ ovsdb_monitor_row_destroy(const struct
ovsdb_monitor_table *mt,
}
}
+void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+ struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+{
+struct jsonrpc_monitor_node *jm;
+
+jm = xzalloc(sizeof *jm);
+jm->jsonrpc_monitor = jsonrpc_monitor;
+list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
+}
+
struct ovsdb_monitor *
ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
{
struct ovsdb_monitor *dbmon;
-struct jsonrpc_monitor_node *jm;
dbmon = xzalloc(sizeof *dbmon);
@@ -247,10 +256,7 @@ ovsdb_monitor_create(struct ovsdb *db,
dbmon->n_transactions = 0;
shash_init(&dbmon->tables);
-jm = xzalloc(sizeof *jm);
-jm->jsonrpc_monitor = jsonrpc_monitor;
-list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
-
+ovsdb_monitor_add_jsonrpc_monitor(dbmon, jsonrpc_monitor);
return dbmon;
}
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index c003184..fb7bed5 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -30,6 +30,9 @@ enum ovsdb_monitor_selection {
struct ovsdb_monitor *ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+
void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 17/20] ovsdb: allow a list of 'ovsdb_monitor_changes' in each ovsdb monitor table
Signed-off-by: Andy Zhou
---
ovsdb/ovsdb-monitor.c | 124 ++
1 file changed, 85 insertions(+), 39 deletions(-)
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index a1efa66..3259f3c 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -87,6 +87,7 @@ struct ovsdb_monitor_changes {
struct hmap rows;
int n_refs;
uint64_t transaction;
+struct ovs_list node; /* Element in ovsdb_monitor_tables' changes list */
};
/* A particular table being monitored. */
@@ -101,18 +102,21 @@ struct ovsdb_monitor_table {
struct ovsdb_monitor_column *columns;
size_t n_columns;
-/* Contains 'struct ovsdb_monitor_row's for rows that have been
- * updated but not yet flushed to the jsonrpc connection. */
-struct ovsdb_monitor_changes *changes;
+/* Contains unsorted list of 'ovsdb_monitor_changes'. Each 'changes'
+ * tracks updates of rows starting from a different
+ * 'unflushed_transaction'. */
+struct ovs_list changes_list;
};
static void ovsdb_monitor_destroy(struct ovsdb_monitor *dbmon);
static void ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table *mt,
uint64_t next_txn);
+static struct ovsdb_monitor_changes *ovsdb_monitor_table_find_changes(
+struct ovsdb_monitor_table *mt, uint64_t unflushed);
static void ovsdb_monitor_changes_destroy_rows(
struct ovsdb_monitor_changes *changes);
static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
- uint64_t transaction);
+ uint64_t unflushed);
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
@@ -133,13 +137,13 @@ ovsdb_monitor_cast(struct ovsdb_replica *replica)
/* Finds and returns the ovsdb_monitor_row in 'mt->changes->rows' for the
* given 'uuid', or NULL if there is no such row. */
static struct ovsdb_monitor_row *
-ovsdb_monitor_row_find(const struct ovsdb_monitor_table *mt,
- const struct uuid *uuid)
+ovsdb_monitor_changes_row_find(const struct ovsdb_monitor_changes *changes,
+ const struct uuid *uuid)
{
struct ovsdb_monitor_row *row;
HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid),
- &mt->changes->rows) {
+ &changes->rows) {
if (uuid_equals(uuid, &row->uuid)) {
return row;
}
@@ -258,8 +262,8 @@ ovsdb_monitor_add_table(struct ovsdb_monitor *m,
mt = xzalloc(sizeof *mt);
mt->table = table;
-mt->changes = NULL;
shash_add(&m->tables, table->schema->name, mt);
+list_init(&mt->changes_list);
}
void
@@ -323,9 +327,25 @@ ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table
*mt,
changes->mt = mt;
changes->n_refs = 1;
hmap_init(&changes->rows);
-mt->changes = changes;
+list_push_back(&mt->changes_list, &changes->node);
};
+
+static struct ovsdb_monitor_changes *
+ovsdb_monitor_table_find_changes(struct ovsdb_monitor_table *mt,
+ uint64_t transaction)
+{
+struct ovsdb_monitor_changes *changes;
+
+LIST_FOR_EACH(changes, node, &mt->changes_list) {
+if (changes->transaction == transaction) {
+return changes;
+}
+}
+
+return NULL;
+}
+
/* Stop currently tracking changes to table 'mt' since 'transaction'.
*
* Return 'true' if the 'transaction' is being tracked. 'false' otherwise. */
@@ -335,14 +355,14 @@ ovsdb_monitor_table_untrack_changes(struct
ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_changes *changes;
-changes = mt->changes;
+changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
ovs_assert(changes->transaction == transaction);
if (--changes->n_refs == 0) {
ovsdb_monitor_changes_destroy_rows(changes);
+list_remove(&changes->node);
free(changes);
-mt->changes = NULL;
}
}
}
@@ -355,7 +375,7 @@ ovsdb_monitor_table_track_changes(struct
ovsdb_monitor_table *mt,
{
struct ovsdb_monitor_changes *changes;
-changes = mt->changes;
+changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
ovs_assert(false);
} else {
@@ -474,6 +494,9 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
unsigned long int *changed;
struct json *json;
size_t max_columns;
+uint64_t from_txn;
+
+from_txn = initial ? 0 : *unflushed;
*unflushed = dbmon->n_transactions + 1;
@@ -489,13 +512,15 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
struct ovsdb_monitor_row *row, *next;
+struct ovsdb_monitor_changes *changes;
[ovs-dev] [PATCH 18/20] ovsdb: refactor ovsdb_monitor_create()
Add ovsdb_monitor_add_jsonrpc_monitor(). This change will allow
ovsdb_monitor to be reference counted.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 2 ++
ovsdb/ovsdb-monitor.c | 16 +++-
ovsdb/ovsdb-monitor.h | 3 +++
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index c9d84a5..ba6df9d 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1233,6 +1233,8 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
+ovsdb_monitor_add_jsonrpc_monitor(m->dbmon, m);
+
ovsdb_monitor_get_initial(m->dbmon);
json = ovsdb_jsonrpc_monitor_compose_update(m, true);
return json ? json : json_object_create();
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 3259f3c..70a8e26 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -231,12 +231,21 @@ ovsdb_monitor_row_destroy(const struct
ovsdb_monitor_table *mt,
}
}
+void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+ struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
+{
+struct jsonrpc_monitor_node *jm;
+
+jm = xzalloc(sizeof *jm);
+jm->jsonrpc_monitor = jsonrpc_monitor;
+list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
+}
+
struct ovsdb_monitor *
ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor)
{
struct ovsdb_monitor *dbmon;
-struct jsonrpc_monitor_node *jm;
dbmon = xzalloc(sizeof *dbmon);
@@ -247,10 +256,7 @@ ovsdb_monitor_create(struct ovsdb *db,
dbmon->n_transactions = 0;
shash_init(&dbmon->tables);
-jm = xzalloc(sizeof *jm);
-jm->jsonrpc_monitor = jsonrpc_monitor;
-list_push_back(&dbmon->jsonrpc_monitors, &jm->node);
-
+ovsdb_monitor_add_jsonrpc_monitor(dbmon, jsonrpc_monitor);
return dbmon;
}
diff --git a/ovsdb/ovsdb-monitor.h b/ovsdb/ovsdb-monitor.h
index c003184..fb7bed5 100644
--- a/ovsdb/ovsdb-monitor.h
+++ b/ovsdb/ovsdb-monitor.h
@@ -30,6 +30,9 @@ enum ovsdb_monitor_selection {
struct ovsdb_monitor *ovsdb_monitor_create(struct ovsdb *db,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+void ovsdb_monitor_add_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
+struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
+
void ovsdb_monitor_remove_jsonrpc_monitor(struct ovsdb_monitor *dbmon,
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor);
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 19/20] ovsdb: allow multiple jsonrpc monitors to share a single ovsdb monitor
Store ovsdb monitor in global hmap. A newly created ovsdb monitor
object will first search the global hmap for a possible match. If
one is found, the existing ovsdb monitor is used instead.
With this patch, jsonrpc monitor and ovsdb monitor now have N:1 mapping.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 14 -
ovsdb/ovsdb-monitor.c | 166 ++---
ovsdb/ovsdb-monitor.h | 9 ++-
3 files changed, 175 insertions(+), 14 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index ba6df9d..5696393 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1156,6 +1156,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
struct json *params)
{
struct ovsdb_jsonrpc_monitor *m = NULL;
+struct ovsdb_monitor *dbmon = NULL;
struct json *monitor_id, *monitor_requests;
struct ovsdb_error *error = NULL;
struct shash_node *node;
@@ -1233,7 +1234,13 @@ ovsdb_jsonrpc_monitor_create(struct
ovsdb_jsonrpc_session *s, struct ovsdb *db,
}
}
-ovsdb_monitor_add_jsonrpc_monitor(m->dbmon, m);
+dbmon = ovsdb_monitor_add(m->dbmon);
+if (dbmon != m->dbmon) {
+/* Found an exsiting dbmon, reuse the current one. */
+ovsdb_monitor_remove_jsonrpc_monitor(m->dbmon, m);
+ovsdb_monitor_add_jsonrpc_monitor(dbmon, m);
+m->dbmon =dbmon;
+}
ovsdb_monitor_get_initial(m->dbmon);
json = ovsdb_jsonrpc_monitor_compose_update(m, true);
@@ -1292,7 +1299,10 @@ ovsdb_jsonrpc_monitor_compose_update(struct
ovsdb_jsonrpc_monitor *m,
json = ovsdb_monitor_compose_update(m->dbmon, initial,
&m->unflushed);
-ovsdb_monitor_renew_tracking_changes(m->dbmon, unflushed, m->unflushed);
+if (unflushed != m->unflushed) {
+ovsdb_monitor_renew_tracking_changes(m->dbmon, unflushed,
+ m->unflushed);
+}
return json;
}
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 70a8e26..210b33c 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -29,6 +29,7 @@
#include "row.h"
#include "simap.h"
#include "table.h"
+#include "hash.h"
#include "timeval.h"
#include "transaction.h"
#include "jsonrpc-server.h"
@@ -39,6 +40,7 @@
VLOG_DEFINE_THIS_MODULE(ovsdb_monitor);
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
+static struct hmap ovsdb_monitors;
/* Backend monitor.
*
@@ -52,6 +54,8 @@ struct ovsdb_monitor {
struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
struct ovsdb *db;
uint64_t n_transactions; /* Count number of commited transactions. */
+
+struct hmap_node hmap_node; /* Elements within ovsdb_monitors. */
};
struct jsonrpc_monitor_node {
@@ -255,6 +259,7 @@ ovsdb_monitor_create(struct ovsdb *db,
dbmon->db = db;
dbmon->n_transactions = 0;
shash_init(&dbmon->tables);
+hmap_node_nullify(&dbmon->hmap_node);
ovsdb_monitor_add_jsonrpc_monitor(dbmon, jsonrpc_monitor);
return dbmon;
@@ -336,7 +341,6 @@ ovsdb_monitor_table_add_changes(struct ovsdb_monitor_table
*mt,
list_push_back(&mt->changes_list, &changes->node);
};
-
static struct ovsdb_monitor_changes *
ovsdb_monitor_table_find_changes(struct ovsdb_monitor_table *mt,
uint64_t transaction)
@@ -364,7 +368,6 @@ ovsdb_monitor_table_untrack_changes(struct
ovsdb_monitor_table *mt,
changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
-ovs_assert(changes->transaction == transaction);
if (--changes->n_refs == 0) {
ovsdb_monitor_changes_destroy_rows(changes);
list_remove(&changes->node);
@@ -383,7 +386,7 @@ ovsdb_monitor_table_track_changes(struct
ovsdb_monitor_table *mt,
changes = ovsdb_monitor_table_find_changes(mt, transaction);
if (changes) {
-ovs_assert(false);
+changes->n_refs++;
} else {
ovsdb_monitor_table_add_changes(mt, transaction);
}
@@ -503,7 +506,6 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
uint64_t from_txn;
from_txn = initial ? 0 : *unflushed;
-
*unflushed = dbmon->n_transactions + 1;
max_columns = 0;
@@ -549,9 +551,6 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
snprintf(uuid, sizeof uuid, UUID_FMT, UUID_ARGS(&row->uuid));
json_object_put(table_json, uuid, row_json);
}
-
-hmap_remove(&changes->rows, &row->hmap_node);
-ovsdb_monitor_row_destroy(mt, row);
}
}
@@ -624,6 +623,35 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old,
}
static bool
+ovsdb_monitor_initial_cb(const struct ovsdb_row *old,
+ const struct ovsdb_row *new,
+
[ovs-dev] [PATCH 20/20] ovsdb: add json cache
Although multiple jsonrpc monitor can share the same ovsdb monitor, each
change still needs to transalted into json object from scratch. This
can be wastful is mutiple jsonrpc monitors are interested in the
same changes.
Json chche impoves this by keeping an copy of json object generated
for transaction X to current transaction. When jsonrpc is interested
in a change, the cache is lookup first, if an json object is found,
a copy of it is handed back, skipping the regeneration process.
Any commit to the monitor will empty the cache. since none of them
will be useful anymore.
Signed-off-by: Andy Zhou
---
ovsdb/ovsdb-monitor.c | 100 ++
1 file changed, 100 insertions(+)
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index 210b33c..2fda283 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -41,6 +41,7 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_monitor);
static const struct ovsdb_replica_class ovsdb_jsonrpc_replica_class;
static struct hmap ovsdb_monitors;
+static struct hmap json_cache__;
/* Backend monitor.
*
@@ -58,6 +59,15 @@ struct ovsdb_monitor {
struct hmap_node hmap_node; /* Elements within ovsdb_monitors. */
};
+/* A json object of updates between 'from_txn' and 'dbmon->n_transactions'
+ * inclusive. */
+struct ovsdb_monitor_json_cache_node {
+struct hmap_node hmap_node; /* Elements in json cache. */
+const struct ovsdb_monitor *dbmon;
+uint64_t from_txn;
+struct json *json;/* Null, or a cloned of json */
+};
+
struct jsonrpc_monitor_node {
struct ovsdb_jsonrpc_monitor *jsonrpc_monitor;
struct ovs_list node;
@@ -122,6 +132,84 @@ static void ovsdb_monitor_changes_destroy_rows(
static void ovsdb_monitor_table_track_changes(struct ovsdb_monitor_table *mt,
uint64_t unflushed);
+static struct hmap *
+get_json_cache(void)
+{
+static bool init__ = false;
+
+if (!init__) {
+hmap_init(&json_cache__);
+ init__ = true;
+}
+return &json_cache__;
+}
+
+static uint32_t
+ovsdb_monitor_json_cache_hash(const struct ovsdb_monitor *dbmon,
+ uint64_t from_txn)
+{
+uint32_t hash;
+
+hash = hash_uint64(from_txn);
+hash = hash_pointer(dbmon, hash);
+return hash;
+}
+
+static struct ovsdb_monitor_json_cache_node *
+ovsdb_monitor_json_cache_search(const struct ovsdb_monitor *dbmon,
+uint64_t from_txn)
+{
+struct ovsdb_monitor_json_cache_node *node;
+struct hmap *json_cache = get_json_cache();
+uint32_t hash;
+
+hash = ovsdb_monitor_json_cache_hash(dbmon, from_txn);
+
+HMAP_FOR_EACH_WITH_HASH(node, hmap_node, hash, json_cache) {
+if ((node->from_txn == from_txn) && (node->dbmon == dbmon)) {
+return node;
+}
+}
+
+return NULL;
+}
+
+static void
+ovsdb_monitor_json_cache_insert(const struct ovsdb_monitor *dbmon,
+uint64_t from_txn, struct json *json)
+{
+struct ovsdb_monitor_json_cache_node *node;
+struct hmap *json_cache = get_json_cache();
+uint32_t hash;
+
+hash = ovsdb_monitor_json_cache_hash(dbmon, from_txn);
+
+node = xmalloc(sizeof *node);
+
+node->from_txn = from_txn;
+node->json = json ? json_clone(json) : NULL;
+node->dbmon = dbmon;
+
+hmap_insert(json_cache, &node->hmap_node, hash);
+}
+
+static void
+ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon)
+{
+struct ovsdb_monitor_json_cache_node *node, *next;
+struct hmap *json_cache = get_json_cache();
+
+HMAP_FOR_EACH_SAFE(node, next, hmap_node, json_cache) {
+if (node->dbmon == dbmon) {
+hmap_remove(json_cache, &node->hmap_node);
+if (node->json) {
+json_destroy(node->json);
+}
+free(node);
+}
+}
+}
+
static int
compare_ovsdb_monitor_column(const void *a_, const void *b_)
{
@@ -499,6 +587,7 @@ struct json *
ovsdb_monitor_compose_update(const struct ovsdb_monitor *dbmon,
bool initial, uint64_t *unflushed)
{
+struct ovsdb_monitor_json_cache_node *cache_node;
struct shash_node *node;
unsigned long int *changed;
struct json *json;
@@ -508,6 +597,12 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
from_txn = initial ? 0 : *unflushed;
*unflushed = dbmon->n_transactions + 1;
+/* Return cached json if one has been created already */
+cache_node = ovsdb_monitor_json_cache_search(dbmon, from_txn);
+if (cache_node) {
+return cache_node->json ? json_clone(cache_node->json) : NULL;
+}
+
max_columns = 0;
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
@@ -555,6 +650,8 @@ ovsdb_monitor_compose_update(const struct ovsdb_monitor
*dbmon,
}
free(changed);
+
+ovsdb_monitor_json_cache_insert(dbmon, from_txn, j
[ovs-dev] [PATCH 14/20] ovsdb: add transaction ids
With N:1 mappings, multiple jsonrpc server may be servicing the rpc
connection at a different pace. ovsdb-monitor thus needs to maintain
different change sets, depends on connection speed of each rpc
connections. Connections servicing at the same speed can share the
same change set.
Transaction ID is an concept added to describe the change set. One
possible view of the database state is a sequence of changes, more
precisely, commits be applied to it in order, starting from an
initial state, with commit 0. The logic can also be applied to the
jsonrpc monitor; each change it pushes corresponds to commits between
two transaction IDs.
This patch introduces transaction IDs. For ovsdb-monitor, it maintains
n_transactions, starting from 0. Each commit add 1 to the number.
Jsonrpc maintains and 'unflushed' transaction number, corresponding to
the next commit the remote has not seen. jsonrpc's job is simply to
notice there are changes in the ovsdb-monitor that it is interested in,
i.e. 'n_transactions' >= 'unflushed', get the changes in json format,
and push them to the remote site.
Signed-off-by: Andy Zhou
---
ovsdb/jsonrpc-server.c | 12
ovsdb/ovsdb-monitor.c | 24 +++-
ovsdb/ovsdb-monitor.h | 7 ---
3 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 4eb6f24..bb19da0 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -89,7 +89,7 @@ static void ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *);
static void ovsdb_jsonrpc_monitor_flush_all(struct ovsdb_jsonrpc_session *);
static bool ovsdb_jsonrpc_monitor_needs_flush(struct ovsdb_jsonrpc_session *);
static struct json *ovsdb_jsonrpc_monitor_compose_table_update(
-const struct ovsdb_jsonrpc_monitor *monitor, bool initial);
+struct ovsdb_jsonrpc_monitor *monitor, bool initial);
/* JSON-RPC database server. */
@@ -1037,6 +1037,8 @@ struct ovsdb_jsonrpc_monitor {
struct hmap_node node; /* In ovsdb_jsonrpc_session's "monitors". */
struct json *monitor_id;
struct ovsdb_monitor *dbmon;
+uint64_t unflushed; /* The first transaction that has not been
+ flushed to the jsonrpc remote client. */
};
static struct ovsdb_jsonrpc_monitor *
@@ -1180,6 +1182,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
m->session = s;
m->db = db;
m->dbmon = ovsdb_monitor_create(db, m);
+m->unflushed = 0;
hmap_insert(&s->monitors, &m->node, json_hash(monitor_id, 0));
m->monitor_id = json_clone(monitor_id);
@@ -1278,9 +1281,10 @@ ovsdb_jsonrpc_monitor_remove_all(struct
ovsdb_jsonrpc_session *s)
static struct json *
ovsdb_jsonrpc_monitor_compose_table_update(
-const struct ovsdb_jsonrpc_monitor *monitor, bool initial)
+struct ovsdb_jsonrpc_monitor *monitor, bool initial)
{
-return ovsdb_monitor_compose_table_update(monitor->dbmon, initial);
+return ovsdb_monitor_compose_table_update(monitor->dbmon, initial,
+ &monitor->unflushed);
}
static bool
@@ -1289,7 +1293,7 @@ ovsdb_jsonrpc_monitor_needs_flush(struct
ovsdb_jsonrpc_session *s)
struct ovsdb_jsonrpc_monitor *m;
HMAP_FOR_EACH (m, node, &s->monitors) {
-if (ovsdb_monitor_needs_flush(m->dbmon)) {
+if (ovsdb_monitor_needs_flush(m->dbmon, m->unflushed)) {
return true;
}
}
diff --git a/ovsdb/ovsdb-monitor.c b/ovsdb/ovsdb-monitor.c
index d20593e..d336b12 100644
--- a/ovsdb/ovsdb-monitor.c
+++ b/ovsdb/ovsdb-monitor.c
@@ -51,6 +51,7 @@ struct ovsdb_monitor {
struct shash tables; /* Holds "struct ovsdb_monitor_table"s. */
struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
struct ovsdb *db;
+uint64_t n_transactions; /* Count number of commited transactions. */
};
struct jsonrpc_monitor_node {
@@ -216,6 +217,7 @@ ovsdb_monitor_create(struct ovsdb *db,
ovsdb_add_replica(db, &dbmon->replica);
list_init(&dbmon->jsonrpc_monitors);
dbmon->db = db;
+dbmon->n_transactions = 0;
shash_init(&dbmon->tables);
jm = xzalloc(sizeof *jm);
@@ -378,14 +380,16 @@ ovsdb_monitor_compose_row_update(
* be used as part of the initial reply to a "monitor" request, false if it is
* going to be used as part of an "update" notification. */
struct json *
-ovsdb_monitor_compose_table_update(
-const struct ovsdb_monitor *dbmon, bool initial)
+ovsdb_monitor_compose_table_update(const struct ovsdb_monitor *dbmon,
+ bool initial, uint64_t *unflushed)
{
struct shash_node *node;
unsigned long int *changed;
struct json *json;
size_t max_columns;
+*unflushed = dbmon->n_transactions + 1;
+
max_columns = 0;
SHASH_FOR_EACH (node, &dbmon->tables) {
struct ovsdb_monitor_table *mt = node->data;
@@ -434,18 +4
Re: [ovs-dev] [ovsdb speedup 00/18] improve ovsdb connection scaling
It appears that I sent out some old patch files in error. Sorry. The set of 18 is the set for review. The set of 20 is an older set, please disregard them. On Thu, Mar 19, 2015 at 12:08 AM, Andy Zhou wrote: > This patch set implements two ideas improve ovsdb connection scaling: > > * Allow multiple jsonrpc connection to share a single ovsdb monitor, thus > maintaining a single set of changes. > > * With in the same monitor, cache the json object generated for an update. > If the same update is requested by another jsonrpc connection, the > cached json object can be used, avoid regenerating it from scratch. > > --- > I don't yet have any useful data on speed improvements from both changes. > The plan is to to collect them and report back in the next few days. > If we don't want to apply them without the performance data, then > please consider this posting as RFC. > > Andy Zhou (18): > ovsdb: refactoring jsonrpc-server.c > ovsdb: make setting mt->select into its own functions > ovsdb: refactor ovsdb_jsonrpc_parse_monitor_request > ovsdb: refactor ovsdb_monitor_add_column() > ovsdb: refactoring ovsdb_jsonrpc_monitor_compose_table_update() > ovsdb: refactoring ovsdb_jsonrpc_monitor_needs_flush > ovsdb: rename ovsdb_jsonrpc_monitor_get_initial() > ovsdb: refactoring ovsdb_monitor_destroy() > ovsdb: Split out monitor backend functions to ovsdb-monitor.c/h > ovsdb: refactoring ovsdb_monitor_get_initial > ovsdb: ovsdb-monitor stores jsonrpc-monitor in a linked-list > ovsdb: add transaction ids > ovsdb: rename jsonrpc_monitor_compose_table_update() > ovsdb: add ovsdb_monitor_changes > ovsdb: allow a list of 'ovsdb_monitor_changes' in each ovsdb monitor > table > ovsdb: refactor ovsdb_monitor_create() > ovsdb: allow multiple jsonrpc monitors to share a single ovsdb monitor > ovsdb: add json cache > > ovsdb/automake.mk |2 + > ovsdb/jsonrpc-server.c | 556 -- > ovsdb/jsonrpc-server.h |3 + > ovsdb/ovsdb-monitor.c | 1024 > > ovsdb/ovsdb-monitor.h | 71 > 5 files changed, 1167 insertions(+), 489 deletions(-) > create mode 100644 ovsdb/ovsdb-monitor.c > create mode 100644 ovsdb/ovsdb-monitor.h > > -- > 1.9.1 > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Returned mail: see transcript for details
The original message was received at Thu, 19 Mar 2015 16:59:42 +0800 from openvswitch.org [142.149.211.165] - The following addresses had permanent fatal errors - dev@openvswitch.org - Transcript of session follows - ... while talking to 18.174.192.14: 554 5.0.0 Service unavailable; [165.14.95.68] blocked using relays.osirusoft.com, reason: Blocked Session aborted, reason: lost connection ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Costumoized actions
Hello again,
At lib/dpif.c in function "dpif_operate" there is a line that is being
called.
dpif->dpif_class->operate(dpif, ops, chunk);
This is possibly the actions are performed, but I have no clue how to know
what function this line leads to.
In lib/dpif-provider.h I can find the definition for these structs, but I
can only see:
/* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order
* in which they are specified, placing each operation's results in the
* "output" members documented in comments.
*
* This function is optional. It is only worthwhile to implement it if
* 'dpif' can perform operations in batch faster than individually. */
void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
Kind regards,
Raúl Suárez
2015-03-18 21:26 GMT+01:00 Raul Suarez Marin :
> Hello Ben,
>
> Thank you for your answer. Let me correct myself when I said I adapated
> parts of "handle_upcalls", I realized I just added debugging lines only
> (my bad).
> I am trying to do the ovs-vswitchd ofproto/trace command but I am getting
> failed connection attempts because "address family is not supported by
> protocol". I will sort it out.
>
> My thoughts right now are:
> 1) I have done in "do_xlate_actions" the part of reading the field I want
> to apply the action to. (new case OFPACT_MY_FIELD)
> 2) I adapted "commit_odp_actions" which is called
> by "compose_output_action" which is called by "xlate_output_action" which
> is called by "do_xlate_actions" (case OFPACT_OUTPUT) and it is located in
> "lib/ofp-util.c" This function adds the actions to the ofpbuf at
> ctx->xout->odp_actions
> with enum ovs_key = OVS_KEY_ATTR_MY_FIELD. I think there is no way this
> fails (because it's just appending data).
> 3) Added OVS_KEY_ATTR_MY_FIELD where needed. Specially in function
> "odp_execute_set_action" at lib/ofp-execute.c, where looks like that the
> packet is actually modified. There, I do this:
>
> const struct ovs_key_STRUCTURE_MODEL my_key = nl_attr_get_unspec(a,
> sizeof(struct ovs_key_STRUCTURE_MODEL))
> struct MY_STRUCTURE *packet_to_do_action = ofpbuf_l3(packet);
> packet_to_do_action ->my_field = my_key ->my_field;
>
> This should make it.
>
> BUT, HERE is the problem!
>
> I placed this code at "odp_execute_set_action" [function in 3)]
>
> FILE *f;
> f = fopen("/home/raul/match_event2.txt", "ab+");
> fprintf(f, "->odp_execute_set_action(%d)\n", type);
> fclose(f);
>
> This is what I use for debugging. It is a very ugly way of doing it but it
> works for me. But, this line is not printed when debugging, so the code
> never arrives at this point (even if my action is not implemented).
> However, when my action is not implemented the action execution works, but
> still the said line is not printed... so maybe it executes that code but it
> is not able to print, I do not know really.
>
> I will be posting here updates so anyone trying to do this same thing can
> have a "clear" idea of what to do.
>
> Thanks again Ben for your help, hints and guidelines.
>
> Kind regards,
> Raúl Suárez
>
>
> 2015-03-12 16:06 GMT+01:00 Ben Pfaff :
>
>> On Thu, Mar 12, 2015 at 09:11:11AM +0100, Raul Suarez Marin wrote:
>> > I am trying to develop new actions for openvswitch, I have done all my
>> work
>> > but looks like it does not work.
>> >
>> > What I have done:
>> > -> Enable ovs to read and parse new actions, so they appear on ovs-ofctl
>> > dump-flows command.
>> > -> Adapted required parts of "handle_upcalls" function
>> > at ofproto\ofproto-dpif-upcall.c (and inside functions)
>> > -> Adapted "do_xlate_actions" function at ofproto\ofproto-dpif-xlate.c
>> >
>> > The problem is:
>> > If the new action is not programmed, everything goes as expected.
>> > If the new action is programmed, no packet forwarding is done.
>> >
>> > The last sentence means that I have done all my work, but part of it is
>> > wrong, or that I am still missing something?
>>
>> I think you've hit the right parts of the code. Try ofproto/trace (see
>> ovs-vswitchd(8)) for some hints.
>>
>> A new action wouldn't normally need any changes in handle_upcalls().
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message.
Hi Ben, This was originally authored by Mark as part of an earlier patch set - http://openvswitch.org/pipermail/dev/2015-March/052123.html I am handling the rework of same hence the dual sign-off. Is there a preferred way to handled situations like this? Cheers, Billy -Original Message- From: Ben Pfaff [mailto:b...@nicira.com] Sent: Wednesday, March 18, 2015 7:29 PM To: O Mahony, Billy Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message. On Wed, Mar 18, 2015 at 05:09:06PM +, Billy O'Mahony wrote: > Signed-off-by: Mark D. Gray > Signed-off-by: Billy O'Mahony Thanks for the patch. It looks good but I'm confused about authorship. Who is the author--you or Mark? Thanks, Ben. -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message.
To indicate that the author is different from the submitter, the first line in the body of the email should be: From: Author Name "git send-email" does that automatically if the author and submitter are different. On Thu, Mar 19, 2015 at 11:41:19AM +, O Mahony, Billy wrote: > Hi Ben, > > This was originally authored by Mark as part of an earlier patch set - > http://openvswitch.org/pipermail/dev/2015-March/052123.html > > I am handling the rework of same hence the dual sign-off. > > Is there a preferred way to handled situations like this? > > Cheers, > Billy > > -Original Message- > From: Ben Pfaff [mailto:b...@nicira.com] > Sent: Wednesday, March 18, 2015 7:29 PM > To: O Mahony, Billy > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error > message. > > On Wed, Mar 18, 2015 at 05:09:06PM +, Billy O'Mahony wrote: > > Signed-off-by: Mark D. Gray > > Signed-off-by: Billy O'Mahony > > Thanks for the patch. It looks good but I'm confused about authorship. Who > is the author--you or Mark? > > Thanks, > > Ben. > -- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > Registered Number: 308263 > Business address: Dromore House, East Park, Shannon, Co. Clare > > This e-mail and any attachments may contain confidential material for the > sole use of the intended recipient(s). Any review or distribution by others > is strictly prohibited. If you are not the intended recipient, please contact > the sender and delete all copies. > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message.
I change the author to Mark and applied this to master. Thank you! On Thu, Mar 19, 2015 at 06:34:09AM -0700, Ben Pfaff wrote: > To indicate that the author is different from the submitter, the first > line in the body of the email should be: > From: Author Name > "git send-email" does that automatically if the author and submitter are > different. > > On Thu, Mar 19, 2015 at 11:41:19AM +, O Mahony, Billy wrote: > > Hi Ben, > > > > This was originally authored by Mark as part of an earlier patch set - > > http://openvswitch.org/pipermail/dev/2015-March/052123.html > > > > I am handling the rework of same hence the dual sign-off. > > > > Is there a preferred way to handled situations like this? > > > > Cheers, > > Billy > > > > -Original Message- > > From: Ben Pfaff [mailto:b...@nicira.com] > > Sent: Wednesday, March 18, 2015 7:29 PM > > To: O Mahony, Billy > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum > > error message. > > > > On Wed, Mar 18, 2015 at 05:09:06PM +, Billy O'Mahony wrote: > > > Signed-off-by: Mark D. Gray > > > Signed-off-by: Billy O'Mahony > > > > Thanks for the patch. It looks good but I'm confused about authorship. > > Who is the author--you or Mark? > > > > Thanks, > > > > Ben. > > -- > > Intel Shannon Limited > > Registered in Ireland > > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > > Registered Number: 308263 > > Business address: Dromore House, East Park, Shannon, Co. Clare > > > > This e-mail and any attachments may contain confidential material for the > > sole use of the intended recipient(s). Any review or distribution by others > > is strictly prohibited. If you are not the intended recipient, please > > contact the sender and delete all copies. > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovsdb speedup 00/18] improve ovsdb connection scaling
Thanks for the hint! I was starting to get confused. On Thu, Mar 19, 2015 at 01:50:31AM -0700, Andy Zhou wrote: > It appears that I sent out some old patch files in error. Sorry. > > The set of 18 is the set for review. The set of 20 is an older set, > please disregard them. > > > > On Thu, Mar 19, 2015 at 12:08 AM, Andy Zhou wrote: > > This patch set implements two ideas improve ovsdb connection scaling: > > > > * Allow multiple jsonrpc connection to share a single ovsdb monitor, thus > > maintaining a single set of changes. > > > > * With in the same monitor, cache the json object generated for an update. > > If the same update is requested by another jsonrpc connection, the > > cached json object can be used, avoid regenerating it from scratch. > > > > --- > > I don't yet have any useful data on speed improvements from both changes. > > The plan is to to collect them and report back in the next few days. > > If we don't want to apply them without the performance data, then > > please consider this posting as RFC. > > > > Andy Zhou (18): > > ovsdb: refactoring jsonrpc-server.c > > ovsdb: make setting mt->select into its own functions > > ovsdb: refactor ovsdb_jsonrpc_parse_monitor_request > > ovsdb: refactor ovsdb_monitor_add_column() > > ovsdb: refactoring ovsdb_jsonrpc_monitor_compose_table_update() > > ovsdb: refactoring ovsdb_jsonrpc_monitor_needs_flush > > ovsdb: rename ovsdb_jsonrpc_monitor_get_initial() > > ovsdb: refactoring ovsdb_monitor_destroy() > > ovsdb: Split out monitor backend functions to ovsdb-monitor.c/h > > ovsdb: refactoring ovsdb_monitor_get_initial > > ovsdb: ovsdb-monitor stores jsonrpc-monitor in a linked-list > > ovsdb: add transaction ids > > ovsdb: rename jsonrpc_monitor_compose_table_update() > > ovsdb: add ovsdb_monitor_changes > > ovsdb: allow a list of 'ovsdb_monitor_changes' in each ovsdb monitor > > table > > ovsdb: refactor ovsdb_monitor_create() > > ovsdb: allow multiple jsonrpc monitors to share a single ovsdb monitor > > ovsdb: add json cache > > > > ovsdb/automake.mk |2 + > > ovsdb/jsonrpc-server.c | 556 -- > > ovsdb/jsonrpc-server.h |3 + > > ovsdb/ovsdb-monitor.c | 1024 > > > > ovsdb/ovsdb-monitor.h | 71 > > 5 files changed, 1167 insertions(+), 489 deletions(-) > > create mode 100644 ovsdb/ovsdb-monitor.c > > create mode 100644 ovsdb/ovsdb-monitor.h > > > > -- > > 1.9.1 > > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message.
Thanks, Ben. I'll follow the method you outlined for subsequent patches. Billy. -Original Message- From: Ben Pfaff [mailto:b...@nicira.com] Sent: Thursday, March 19, 2015 1:39 PM To: O Mahony, Billy Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum error message. I change the author to Mark and applied this to master. Thank you! On Thu, Mar 19, 2015 at 06:34:09AM -0700, Ben Pfaff wrote: > To indicate that the author is different from the submitter, the first > line in the body of the email should be: > From: Author Name "git send-email" does > that automatically if the author and submitter are different. > > On Thu, Mar 19, 2015 at 11:41:19AM +, O Mahony, Billy wrote: > > Hi Ben, > > > > This was originally authored by Mark as part of an earlier patch set > > - http://openvswitch.org/pipermail/dev/2015-March/052123.html > > > > I am handling the rework of same hence the dual sign-off. > > > > Is there a preferred way to handled situations like this? > > > > Cheers, > > Billy > > > > -Original Message- > > From: Ben Pfaff [mailto:b...@nicira.com] > > Sent: Wednesday, March 18, 2015 7:29 PM > > To: O Mahony, Billy > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH v2] automake.mk: Improve schema checksum > > error message. > > > > On Wed, Mar 18, 2015 at 05:09:06PM +, Billy O'Mahony wrote: > > > Signed-off-by: Mark D. Gray > > > Signed-off-by: Billy O'Mahony > > > > Thanks for the patch. It looks good but I'm confused about authorship. > > Who is the author--you or Mark? > > > > Thanks, > > > > Ben. > > -- > > Intel Shannon Limited > > Registered in Ireland > > Registered Office: Collinstown Industrial Park, Leixlip, County > > Kildare Registered Number: 308263 Business address: Dromore House, > > East Park, Shannon, Co. Clare > > > > This e-mail and any attachments may contain confidential material for the > > sole use of the intended recipient(s). Any review or distribution by others > > is strictly prohibited. If you are not the intended recipient, please > > contact the sender and delete all copies. > > > > -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] (no subject)
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] IBSP Project Attached Letter
View IBSP/UNESCO Attached Letter ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
The design was come up after inputs and discussions with multiple people, including (in alphabetical order) Aaron Rosen, Ben Pfaff, Ganesan Chandrashekhar, Justin Pettit, Russell Bryant and Somik Behera. Signed-off-by: Gurucharan Shetty --- ovn/CONTAINERS.OpenStack.md | 114 ++ ovn/automake.mk |4 +- ovn/ovn-architecture.7.xml | 186 ++- ovn/ovn-nb.ovsschema|6 ++ ovn/ovn-nb.xml | 49 ++-- ovn/ovn.ovsschema |6 ++ ovn/ovn.xml | 58 ++ 7 files changed, 380 insertions(+), 43 deletions(-) create mode 100644 ovn/CONTAINERS.OpenStack.md diff --git a/ovn/CONTAINERS.OpenStack.md b/ovn/CONTAINERS.OpenStack.md new file mode 100644 index 000..58b5588 --- /dev/null +++ b/ovn/CONTAINERS.OpenStack.md @@ -0,0 +1,114 @@ +Integration of Containers with OVN and OpenStack + + +In a multi-tenant environment, creating containers directly on hypervisors +has many risks. A container application can break out and make changes to +the Open vSwitch flows and thus impact other tenants. This document +describes creation of containers inside VMs and how they can be made part +of the logical networks securely. The created logical network can include VMs, +containers and physical machines as endpoints. To better understand the +proposed integration of containers with OVN and OpenStack, this document +describes the end to end workflow with an example. + +* A OpenStack tenant creates a VM (say VM-A) with a single network interface +that belongs to a management logical network. The VM is meant to host +containers. OpenStack Nova chooses the hypervisor on which VM-A is created. + +* A Neutron port may have been created in advance and passed in to Nova +with the request to create a new VM. If not, Nova will issue a request +to Neutron to create a new port. The ID of the logical port from +Neutron will also be used as the vif-id for the virtual network +interface (VIF) of VM-A. + +* When VM-A is created on a hypervisor, its VIF gets added to the +Open vSwitch integration bridge. This creates a row in the Interface table +of the Open_vSwitch database. As explained in the [IntegrationGuide.md], +the vif-id associated with the VM network interface gets added in the +external_ids:iface-id column of the newly created row in the Interface table. + +* Since VM-A belongs to a logical network, it gets an IP address. This IP +address is used to spawn containers (either manually or through container +orchestration systems) inside that VM and to monitor the health of the +created containers. + +* The vif-id associated with the VM's network interface can be obtained by +making a call to Neutron using tenant credentials. + +* This flow assumes a component called a "container network plugin". +If you take Docker as an example for containers, you could envision +the plugin to be either a wrapper around Docker or a feature of Docker itself +that understands how to perform part of this workflow to get a container +connected to a logical network managed by Neutron. The rest of the flow +refers to this logical component that does not yet exist as the +"container network plugin". + +* All the calls to Neutron will need tenant credentials. These calls can +either be made from inside the tenant VM as part of a container network plugin +or from outside the tenant VM (if the tenant is not comfortable using temporary +Keystone tokens from inside the tenant VMs). For simplicity, this document +explains the work flow using the former method. + +* The container hosting VM will need Open vSwitch installed in it. The only +work for Open vSwitch inside the VM is to tag network traffic coming from +containers. + +* When a container needs to be created inside the VM with a container network +interface that is expected to be attached to a particular logical switch, the +network plugin in that VM chooses any unused VLAN (This VLAN tag only needs to +be unique inside that VM. This limits the number of container interfaces to +4096 inside a single VM). This VLAN tag is stripped out in the hypervisor +by OVN and is only useful as a context (or metadata) for OVN. + +* The container network plugin then makes a call to Neutron to create a +logical port. In addition to all the inputs that a call to create a port in +Neutron that are currently needed, it sends the vif-id and the VLAN tag as +inputs. + +* Neutron in turn will verify that the vif-id belongs to the tenant in question +and then uses the OVN specific plugin to create a new row in the Logical_Port +table of the OVN Northbound Database. Neutron responds back with an +IP address and MAC address for that network interface. So Neutron becomes +the IPAM system and provides unique IP and MAC addresses across VMs and +containers in the same logical network. + +* When a container is eventually del
[ovs-dev] [PATCH] ovn-architecture: Provide the correct daemon name.
It is ovn-nbd and not ovs-nbd. Signed-off-by: Gurucharan Shetty --- ovn/ovn-architecture.7.xml |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml index a5b0807..035527f 100644 --- a/ovn/ovn-architecture.7.xml +++ b/ovn/ovn-architecture.7.xml @@ -290,7 +290,7 @@ - ovs-nbd receives the OVN Northbound database update. In + ovn-nbd receives the OVN Northbound database update. In turn, it makes the corresponding updates to the OVN database, by adding rows to the OVN database Pipeline table to reflect the new port, e.g. add a flow to recognize that packets destined to the new @@ -303,7 +303,7 @@ On every hypervisor, ovn-controller receives the - Pipeline table updates that ovs-nbd made in the + Pipeline table updates that ovn-nbd made in the previous step. As long as the VM that owns the VIF is powered off, ovn-controller cannot do much; it cannot, for example, arrange to send packets to or receive packets from the VIF, because the @@ -385,7 +385,7 @@ - ovs-nbd receives the OVN Northbound update and in turn + ovn-nbd receives the OVN Northbound update and in turn updates the OVN database accordingly, by removing or updating the rows from the OVN database Pipeline table and Bindings table that were related to the now-destroyed VIF. @@ -393,7 +393,7 @@ On every hypervisor, ovn-controller receives the - Pipeline table updates that ovs-nbd made in the + Pipeline table updates that ovn-nbd made in the previous step. ovn-controller updates OpenFlow tables to reflect the update, although there may not be much to do, since the VIF had already become unreachable when it was removed from the -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-architecture: Provide the correct daemon name.
Looks good. It looks like there's another one in "ovn-nb.xml". Acked-by: Justin Pettit --Justin > On Mar 19, 2015, at 7:32 AM, Gurucharan Shetty wrote: > > It is ovn-nbd and not ovs-nbd. > > Signed-off-by: Gurucharan Shetty > --- > ovn/ovn-architecture.7.xml |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml > index a5b0807..035527f 100644 > --- a/ovn/ovn-architecture.7.xml > +++ b/ovn/ovn-architecture.7.xml > @@ -290,7 +290,7 @@ > > > > - ovs-nbd receives the OVN Northbound database update. In > + ovn-nbd receives the OVN Northbound database update. In > turn, it makes the corresponding updates to the OVN database, by adding > rows to the OVN database Pipeline table to reflect the new > port, e.g. add a flow to recognize that packets destined to the new > @@ -303,7 +303,7 @@ > > > On every hypervisor, ovn-controller receives the > - Pipeline table updates that ovs-nbd made in > the > + Pipeline table updates that ovn-nbd made in > the > previous step. As long as the VM that owns the VIF is powered off, > ovn-controller cannot do much; it cannot, for example, > arrange to send packets to or receive packets from the VIF, because the > @@ -385,7 +385,7 @@ > > > > - ovs-nbd receives the OVN Northbound update and in turn > + ovn-nbd receives the OVN Northbound update and in turn > updates the OVN database accordingly, by removing or updating the > rows from the OVN database Pipeline table and > Bindings table that were related to the now-destroyed VIF. > @@ -393,7 +393,7 @@ > > > On every hypervisor, ovn-controller receives the > - Pipeline table updates that ovs-nbd made in > the > + Pipeline table updates that ovn-nbd made in > the > previous step. ovn-controller updates OpenFlow tables to > reflect the update, although there may not be much to do, since the VIF > had already become unreachable when it was removed from the > -- > 1.7.9.5 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-architecture: Provide the correct daemon name.
On Thu, Mar 19, 2015 at 07:32:28AM -0700, Gurucharan Shetty wrote: > It is ovn-nbd and not ovs-nbd. > > Signed-off-by: Gurucharan Shetty Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] dpif-netdev: Wait for threads to quiesce before freeing port.
port_unref() shouldn't immediately free the port and close the netdev, because otherwise threads that still have a pointer to the port would crash. This commit fixes the problem by introducing an ovsrcu_synchronize() call in port_unref(). I chose not to use ovsrcu_postpone(), because postponing the removal of the 'netdev' would cause other issues, such as not being possible to reuse the same name for another netdev. Found the crash while developing new code. Signed-off-by: Daniele Di Proietto --- lib/dpif-netdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index f01fecb..2546e5e 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -994,6 +994,8 @@ port_unref(struct dp_netdev_port *port) int n_rxq = netdev_n_rxq(port->netdev); int i; +ovsrcu_synchronize(); + netdev_close(port->netdev); netdev_restore_flags(port->sf); -- 2.1.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] (no subject)
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On 03/19/2015 10:31 AM, Gurucharan Shetty wrote: > The design was come up after inputs and discussions with multiple > people, including (in alphabetical order) Aaron Rosen, Ben Pfaff, > Ganesan Chandrashekhar, Justin Pettit, Russell Bryant and Somik Behera. > > Signed-off-by: Gurucharan Shetty > --- > ovn/CONTAINERS.OpenStack.md | 114 ++ > ovn/automake.mk |4 +- > ovn/ovn-architecture.7.xml | 186 > ++- > ovn/ovn-nb.ovsschema|6 ++ > ovn/ovn-nb.xml | 49 ++-- > ovn/ovn.ovsschema |6 ++ > ovn/ovn.xml | 58 ++ > 7 files changed, 380 insertions(+), 43 deletions(-) > create mode 100644 ovn/CONTAINERS.OpenStack.md Could you summarize the difference from v2? I don't recall any real discussion about v2 on the list other than our brief exchange about the 4096 CIFs limitation. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
> > Could you summarize the difference from v2? I don't recall any real > discussion about v2 on the list other than our brief exchange about the > 4096 CIFs limitation. Sorry about that. There are only 2 differences between the [RFC v2] and this more official patch. 1. Expanded introduction to CIF in ovn-architecture in the section [Life Cycle of a container interface inside a VM]. 2. The life-cycle of Bindings table changes with this patch. A record in Bindings table is now exclusively created/destroyed by ovn-nbd instead of ovn-controller (for both VMs and Containers). ovn-controller is only responsbile to populate the 'chassis' column in the Bindings table. > > -- > Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On Thu, Mar 19, 2015 at 10:07:40AM -0700, Gurucharan Shetty wrote: > > > > Could you summarize the difference from v2? I don't recall any real > > discussion about v2 on the list other than our brief exchange about the > > 4096 CIFs limitation. > Sorry about that. There are only 2 differences between the [RFC v2] > and this more official patch. > 1. Expanded introduction to CIF in ovn-architecture in the section > [Life Cycle of a container interface inside a VM]. > 2. The life-cycle of Bindings table changes with this patch. A record > in Bindings table is now exclusively created/destroyed by ovn-nbd > instead of ovn-controller (for both VMs and Containers). > ovn-controller is only responsbile to populate the 'chassis' column in > the Bindings table. It might be worth giving the back-story on rationale for change #2. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/6] netdev-dpdk: create smaller mempools in case of failure
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di > Proietto > Sent: Thursday, March 12, 2015 6:05 PM > > If rte_mempool_create() fails with ENOMEM, try asking for a smaller > mempools. This patch enables OVS DPDK to run on systems without 1GB > hugepages > > Signed-off-by: Daniele Di Proietto Pages smaller than 1GB will mean the dpdkr ports won't work as the ring structures and the mempools get spread over multiple smaller pages. In qemu we only share up one 1G huge page. This can be fixed but it is quite complex as you need to use the dpdk ivshmem target. Perhaps you should update the documentation to reflect this restriction. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration
> -Original Message- > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Traynor, > Kevin > Sent: Monday, March 16, 2015 6:46 PM [snip] > > > > > > Hi, I've reviewed this patchset - few comments/questions on it… > > > > Hi Kevin, thanks > > > > > I haven't tested yet - but I'm wondering what is the impact to the > > > dpdk -c parameter. Is it no longer used for OVS? > > > > > > > Yes, that’s correct. It should have no impact on OVS. I think we > > should also provide a default, i.e. generate the parameters passed to > > dpdk_eal_init at some point. +1 to removing the -c parameter as it seems to be the same as 'other_config:n-pmd-cores' and is a little confusing as to how they interact. Maybe 'other_config:n-pmd-cores' could be a mandatory option? > > > > > At present the NON_PMD_CORE_ID define overrides the db settings > > > (which is clearly documented). Is it needed now that a db key is > > > available? Perhaps it would make things simpler that the define is > > > overridden when a key is specified? > > > > > > > We check NON_PMD_CORE_ID often in the fast path. I would prefer > > leaving it as it is right now and removing this limitation later, > > making sure that it has no impact on performance. > > Sounds fine to me. > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration
> -Original Message- > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di > Proietto > Sent: Thursday, March 12, 2015 6:05 PM > This series improves OVS configuration with DPDK in three ways: > > * netdev-dpdk is patched to work on smaller systems (without 1GB > hugepages) > or with smaller NICs (without lots of transmission queues). > * the 'other_config:nonpmd-cpu-mask' key is introduced: it can be used to > limit OVS non PMD threads to a particular set of cores. > * the 'other_config:n-pmd-cores' key is introduced: it allows setting the > number of PMD threads without specifing a CPU mask. > > Daniele Di Proietto (6): > netdev-dpdk: create smaller mempools in case of failure > netdev-dpdk: Adapt the requested number of tx and rx queues. > ovs-thread: Keep a list of all threads. > dpif-netdev: Allow controlling non PMD threads' affinity > dpif-netdev: Allow configuring number of PMD threads. > INSTALL.DPDK.md: Update documentation on multiple threads > Great to see this! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] 3Com Users
Hi, Would you be interested in Acquiring 3Com Users contacts list of 2015? Information Fields - Name, Title, Email, Phone, Company Name, Physical Address, City, State, Zip Code, Country, Web Address, Employee Size, Revenue Size and Industry. We also have other technology users like: * Cisco * Avaya * IBM * Juniper Networks * Fortinet * Netgear And many more... Please review and let me know your thoughts and I will get back to you with more information for the same. Thanks, Kina To Opt Out, please reply with Leave Out in the Subject Line. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/6] netdev-dpdk: create smaller mempools in case of failure
> On 19 Mar 2015, at 18:05, Gray, Mark D wrote: > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di >> Proietto >> Sent: Thursday, March 12, 2015 6:05 PM > >> >> If rte_mempool_create() fails with ENOMEM, try asking for a smaller >> mempools. This patch enables OVS DPDK to run on systems without 1GB >> hugepages >> >> Signed-off-by: Daniele Di Proietto > > Pages smaller than 1GB will mean the dpdkr ports won't work as the ring > structures > and the mempools get spread over multiple smaller pages. In qemu we only share > up one 1G huge page. This can be fixed but it is quite complex as you need to > use the dpdk ivshmem target. Perhaps you should update the documentation to > reflect > this restriction. I wasn’t aware of this restriction. It’s (very) briefly stated in INSTALL.DPDK.md under ‘Restrictions:’, though. Do you maybe want to send a patch to clarify this? You clearly have a better understanding of the issue :-) Thanks, Daniele ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gray, Mark > D > Sent: Thursday, March 19, 2015 6:19 PM > To: Traynor, Kevin; Daniele Di Proietto > Subject: Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration > > [snip] > > > > > > > > Hi, I've reviewed this patchset - few comments/questions on it… > > > > > > Hi Kevin, thanks > > > > > > > I haven't tested yet - but I'm wondering what is the impact to the > > > > dpdk -c parameter. Is it no longer used for OVS? > > > > > > > > > > Yes, that’s correct. It should have no impact on OVS. I think we > > > should also provide a default, i.e. generate the parameters passed > > > to dpdk_eal_init at some point. > > +1 to removing the -c parameter as it seems to be the same as > 'other_config:n-pmd-cores' and is a little confusing as to how they interact. > Maybe 'other_config:n-pmd-cores' could be a mandatory option? I used the wrong config option in above. Let me rephrase it. +1 to removing the -c parameter as it seems to be the same as 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask' and is a little confusing as to how they interact. Maybe 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask' could be a mandatory option? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
> It might be worth giving the back-story on rationale for change #2. Here is the backstory: Before the introduction to Container (spawned inside VMs) networking in OVN, OVN's architecture was designed with VMs in mind. In this case, ovn-controller could create a record in Bindings table as soon as it could see a VIF created as a port in Open vSwitch integration bridge (in the hypervisor). But when a container is created inside a VM, ovn-controller does not have any information about it and it cannot create a record in the Bindings table. So the RFC proposed that just in case of containers, this record would be created by ovn-nbd. This made the workflow a little ugly, wherein 2 components were involved in creation and deletion of the records in Bindings table. So in this change, we bring consistency in the Bindings table creation. Now, Bindings table is exclusively created by ovn-nbd and ovn-controller is only responsible in changing the chassis column inside it. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH RFC 0/1] netdev-dpdk.c: add dpdk vhost-user ports
This RFC patch makes use of the vhost-user implementation that will be available in DPDK 2.0. Submitting as RFC as this implementation depends on a stable version of DPDK 2.0 and following that, OVS support for DPDK 2.0. As such, this patch can be considered experimental. This patch builds on top of the vhost-cuse patch (v8) found here: http://openvswitch.org/pipermail/dev/2015-March/052061.html Apply this first. Since DPDK 2.0 support is necessary for vhost-user, please use this patch to enable compilation with 2.0: http://openvswitch.org/pipermail/dev/2015-March/052022.html Use DPDK commit 31ff0c6a45cfc729fd7003d887fcd2612c3aca6d and QEMU v2.2.0. INSTALL.DPDK.md | 115 acinclude.m4| 13 ++ configure.ac| 1 + lib/netdev-dpdk.c | 52 -- lib/netdev.c| 4 ++ vswitchd/ovs-vswitchd.c | 2 + 6 files changed, 155 insertions(+), 32 deletions(-) -- 1.9.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH RFC 1/1] netdev-dpdk: add dpdk vhost-user ports
This patch adds support for a new port type to the userspace datapath called dpdkvhostuser. It adds to the existing infrastructure of vhost-cuse, however disables vhost-cuse ports in favour of vhost-user ports. A new dpdkvhostuser port will create a unix domain socket which when provided to QEMU is used to facilitate communication between the virtio-net device on the VM and the OVS port. Signed-off-by: Ciara Loftus --- INSTALL.DPDK.md | 115 acinclude.m4| 13 ++ configure.ac| 1 + lib/netdev-dpdk.c | 52 -- lib/netdev.c| 4 ++ vswitchd/ovs-vswitchd.c | 2 + 6 files changed, 155 insertions(+), 32 deletions(-) diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 4deeb87..bc2a5a2 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -294,47 +294,106 @@ the vswitchd. DPDK vhost: --- -vhost-cuse is only supported at present i.e. not using the standard QEMU -vhost-user interface. It is intended that vhost-user support will be added -in future releases when supported in DPDK and that vhost-cuse will eventually -be deprecated. See [DPDK Docs] for more info on vhost. +DPDK 2.0 supports two types of vhost: +1. vhost-user +2. vhost-cuse + +By default, vhost-user is enabled in DPDK and following this, the same +applies for OVS. + +Should you wish to use vhost-cuse instead of vhost-user, you must do the +following: +1. Enable vhost-cuse in DPDK and re-build. At the moment this can be +achieved by modifying the `$RTE_SDK/lib/librte_vhost/Makefile` file +and commenting out vhost-user SRCS and uncommenting vhost-cuse SRCS: + + `SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c + # SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c vhost_user/virtio-net-user.c vhost_user/fd_man.c` + +2. Enable vhost-cuse in OVS and re-build. This can be achieved by using +the `--with-vhostcuse` flag in the `./configure` step like so: + + `./configure --with-dpdk=$DPDK_BUILD --with-vhostcuse` Prerequisites: -1. DPDK 1.8 with vhost support enabled and recompile OVS as above. +1. DPDK 2.0 with vhost support enabled and recompile OVS as above. Update `config/common_linuxapp` so that DPDK is built with vhost libraries: `CONFIG_RTE_LIBRTE_VHOST=y` -2. Insert the Cuse module: +2. (Optional)If using vhost-cuse: + +2.1 Insert the Cuse module: - `modprobe cuse` + `modprobe cuse` -3. Build and insert the `eventfd_link` module: +2.2 Build and insert the `eventfd_link` module: - `cd $DPDK_DIR/lib/librte_vhost/eventfd_link/` - `make` - `insmod $DPDK_DIR/lib/librte_vhost/eventfd_link.ko` + `cd $DPDK_DIR/lib/librte_vhost/eventfd_link/` + `make` + `insmod $DPDK_DIR/lib/librte_vhost/eventfd_link.ko` Following the steps above to create a bridge, you can now add DPDK vhost -as a port to the vswitch. +as a port to the vswitch. Unlike DPDK ring ports, DPDK vhost ports can have +arbitrary names. + +When adding vhost ports to the switch, take care depending on which +type of vhost you are using. -`ovs-vsctl add-port br0 dpdkvhost0 -- set Interface dpdkvhost0 type=dpdkvhost` + - For vhost-user (default), the name of the port type is `dpdkvhostuser` -Unlike DPDK ring ports, DPDK vhost ports can have arbitrary names: + `ovs-ofctl add-port br0 vhost-user1 -- set Interface vhost-user1 type=dpdkvhostuser` -`ovs-vsctl add-port br0 port123ABC -- set Interface port123ABC type=dpdkvhost` + This action creates a socket located at `/tmp/vhost-user1`, which you + must provide to your VM on the QEMU command line. More instructions + on this can be found in the next section "DPDK vhost-user VM + configuration" -However, please note that when attaching userspace devices to QEMU, the -name provided during the add-port operation must match the ifname parameter -on the QEMU command line. + - For vhost-cuse, the name of the port type is `dpdkvhost` + `ovs-ofctl add-port br0 vhost-cuse1 -- set Interface vhost-cuse1 type=dpdkvhost` -DPDK vhost VM configuration: - + When attaching vhost-cuse ports to QEMU, the name provided during the + add-port operation must match the ifname parameter on the QEMU command + line. More instructions on this can be found in the section "DPDK + vhost-cuse VM configuration" + +DPDK vhost-user VM configuration: +- +DPDK vhost-user works with QEMU v2.2.0. Follow the steps below to attach +vhost-user port(s) to a VM. + +1. Configure sockets. + Pass the following parameters to QEMU to attach a vhost-user device. + + ``` + -chardev socket,id=char0,path=/tmp/vhost-user-1 + -netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce + -device virtio-net-pci,mac=00:00:00:
Re: [ovs-dev] [PATCH 1/6] netdev-dpdk: create smaller mempools in case of failure
> > I wasn’t aware of this restriction. It’s (very) briefly stated in > INSTALL.DPDK.md under ‘Restrictions:’, though. > > Do you maybe want to send a patch to clarify this? You clearly have a better > understanding of the issue :-) > Sure thing, I will do this. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Costumoized actions
Raúl,
The dip class functions are implemented by the different dip classes. Linux and
Windows data paths are interfaced by lib/dpif-netlink.c and the userspace
datapath is implemented in lib/dpif-netlink.c. You’ll find a class struct with
the local function pointers in those files, just search for “operate” in those
files.
Hope this helps,
Jarno
> On Mar 19, 2015, at 2:06 AM, Raul Suarez Marin
> wrote:
>
> Hello again,
>
> At lib/dpif.c in function "dpif_operate" there is a line that is being
> called.
> dpif->dpif_class->operate(dpif, ops, chunk);
>
> This is possibly the actions are performed, but I have no clue how to know
> what function this line leads to.
>
> In lib/dpif-provider.h I can find the definition for these structs, but I
> can only see:
>
> /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order
> * in which they are specified, placing each operation's results in the
> * "output" members documented in comments.
> *
> * This function is optional. It is only worthwhile to implement it if
> * 'dpif' can perform operations in batch faster than individually. */
> void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
>
> Kind regards,
> Raúl Suárez
>
> 2015-03-18 21:26 GMT+01:00 Raul Suarez Marin :
>
>> Hello Ben,
>>
>> Thank you for your answer. Let me correct myself when I said I adapated
>> parts of "handle_upcalls", I realized I just added debugging lines only
>> (my bad).
>> I am trying to do the ovs-vswitchd ofproto/trace command but I am getting
>> failed connection attempts because "address family is not supported by
>> protocol". I will sort it out.
>>
>> My thoughts right now are:
>> 1) I have done in "do_xlate_actions" the part of reading the field I want
>> to apply the action to. (new case OFPACT_MY_FIELD)
>> 2) I adapted "commit_odp_actions" which is called
>> by "compose_output_action" which is called by "xlate_output_action" which
>> is called by "do_xlate_actions" (case OFPACT_OUTPUT) and it is located in
>> "lib/ofp-util.c" This function adds the actions to the ofpbuf at
>> ctx->xout->odp_actions
>> with enum ovs_key = OVS_KEY_ATTR_MY_FIELD. I think there is no way this
>> fails (because it's just appending data).
>> 3) Added OVS_KEY_ATTR_MY_FIELD where needed. Specially in function
>> "odp_execute_set_action" at lib/ofp-execute.c, where looks like that the
>> packet is actually modified. There, I do this:
>>
>> const struct ovs_key_STRUCTURE_MODEL my_key = nl_attr_get_unspec(a,
>> sizeof(struct ovs_key_STRUCTURE_MODEL))
>> struct MY_STRUCTURE *packet_to_do_action = ofpbuf_l3(packet);
>> packet_to_do_action ->my_field = my_key ->my_field;
>>
>> This should make it.
>>
>> BUT, HERE is the problem!
>>
>> I placed this code at "odp_execute_set_action" [function in 3)]
>>
>> FILE *f;
>> f = fopen("/home/raul/match_event2.txt", "ab+");
>> fprintf(f, "->odp_execute_set_action(%d)\n", type);
>> fclose(f);
>>
>> This is what I use for debugging. It is a very ugly way of doing it but it
>> works for me. But, this line is not printed when debugging, so the code
>> never arrives at this point (even if my action is not implemented).
>> However, when my action is not implemented the action execution works, but
>> still the said line is not printed... so maybe it executes that code but it
>> is not able to print, I do not know really.
>>
>> I will be posting here updates so anyone trying to do this same thing can
>> have a "clear" idea of what to do.
>>
>> Thanks again Ben for your help, hints and guidelines.
>>
>> Kind regards,
>> Raúl Suárez
>>
>>
>> 2015-03-12 16:06 GMT+01:00 Ben Pfaff :
>>
>>> On Thu, Mar 12, 2015 at 09:11:11AM +0100, Raul Suarez Marin wrote:
I am trying to develop new actions for openvswitch, I have done all my
>>> work
but looks like it does not work.
What I have done:
-> Enable ovs to read and parse new actions, so they appear on ovs-ofctl
dump-flows command.
-> Adapted required parts of "handle_upcalls" function
at ofproto\ofproto-dpif-upcall.c (and inside functions)
-> Adapted "do_xlate_actions" function at ofproto\ofproto-dpif-xlate.c
The problem is:
If the new action is not programmed, everything goes as expected.
If the new action is programmed, no packet forwarding is done.
The last sentence means that I have done all my work, but part of it is
wrong, or that I am still missing something?
>>>
>>> I think you've hit the right parts of the code. Try ofproto/trace (see
>>> ovs-vswitchd(8)) for some hints.
>>>
>>> A new action wouldn't normally need any changes in handle_upcalls().
>>>
>>
>>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn: add start of ovn-nbctl
ovn-nbctl is intended to be a utility for manually interacting with the OVN northbound db. A real consumer of OVN, such as OpenStack, should be using ovsdb directly. However, a utility like this is useful during development and testing of both ovn itself as well as the OpenStack Neutron plugin. It could also be used when debugging the state of an ovn deployment. So far, the commands related to logical switches have been implemented. You can add, delete, list, and get/set external:ids. Signed-off-by: Russell Bryant --- ovn/.gitignore | 2 + ovn/automake.mk | 8 +- ovn/ovn-nbctl.8.xml | 22 ovn/ovn-nbctl.c | 327 4 files changed, 357 insertions(+), 2 deletions(-) diff --git a/ovn/.gitignore b/ovn/.gitignore index 011bb1c..f320a6b 100644 --- a/ovn/.gitignore +++ b/ovn/.gitignore @@ -12,3 +12,5 @@ /ovn-nb-idl.c /ovn-nb-idl.h /ovn-nb-idl.ovsidl +/ovn-nbctl +/ovn-nbctl.8 diff --git a/ovn/automake.mk b/ovn/automake.mk index cc1f8ae..37b0ca6 100644 --- a/ovn/automake.mk +++ b/ovn/automake.mk @@ -66,8 +66,8 @@ ovn/ovn-nb.5: \ $(srcdir)/ovn/ovn-nb.xml > $@.tmp && \ mv $@.tmp $@ -man_MANS += ovn/ovn-controller.8 ovn/ovn-architecture.7 -EXTRA_DIST += ovn/ovn-controller.8.in ovn/ovn-architecture.7.xml +man_MANS += ovn/ovn-controller.8 ovn/ovn-architecture.7 ovn/ovn-nbctl.8 +EXTRA_DIST += ovn/ovn-controller.8.in ovn/ovn-architecture.7.xml ovn/ovn-nbctl.8.xml SUFFIXES += .xml %: %.xml @@ -115,3 +115,7 @@ ovn_libovn_la_SOURCES = \ ovn/ovn-idl.h \ ovn/ovn-nb-idl.c \ ovn/ovn-nb-idl.h + +bin_PROGRAMS += ovn/ovn-nbctl +ovn_ovn_nbctl_SOURCES = ovn/ovn-nbctl.c +ovn_ovn_nbctl_LDADD = ovn/libovn.la ovsdb/libovsdb.la lib/libopenvswitch.la diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml new file mode 100644 index 000..beccb79 --- /dev/null +++ b/ovn/ovn-nbctl.8.xml @@ -0,0 +1,22 @@ + + +Name +ovn-nbctl -- Open Virtual Network northbound db management utility + +Synopsys +ovn-nbctl [OPTIONS] COMMAND [ARG...] + +Description +This utility can be used to manage the OVN northbound database. + +Commands +lswitch-add [name] +lswitch-del+lswitch-list +lswitch-set-external-id [value] +lswitch-get-external-id [key] + +Options +-h | --help +-V | --version + diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c new file mode 100644 index 000..0a9c039 --- /dev/null +++ b/ovn/ovn-nbctl.c @@ -0,0 +1,327 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include + +#include "command-line.h" +#include "dirs.h" +#include "fatal-signal.h" +#include "ovn/ovn-nb-idl.h" +#include "poll-loop.h" +#include "util.h" +#include "openvswitch/vlog.h" + +struct nbctl_context { +struct ovsdb_idl *idl; +struct ovsdb_idl_txn *txn; +const char *db; +}; + +static const struct ovs_cmdl_command *get_all_commands(void); + +static void +usage(void) +{ +printf("\ +%s: OVN northbound DB management utility\n\ +usage: %s [OPTIONS] COMMAND [ARG...]\n\ +\n\ +Commands:\n\ + lswitch-add [name]Create a logical switch\n\ + lswitch-del Delete a logical switch\n\ + lswitch-list List configured logical switches\n\ + lswitch-set-external-id [value]\n\ +Set or delete an external:id on a logical switch\n\ + lswitch-get-external-id [key]\n\ +List one or all external:ids set on a switch\n\ +Options:\n\ + -h, --helpdisplay this help message\n\ + -o, --options list available options\n\ + -V, --version display version information\n\ +", program_name, program_name); +} + +static const char * +default_db(void) +{ +static char *db; +if (!db) { +db = xasprintf("unix:%s/db.sock", ovs_rundir()); +} +return db; +} + +static const struct nbrec_logical_switch * +lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const char *id) +{ +const struct nbrec_logical_switch *lswitch; +int is_uuid = 0; +struct uuid lswitch_uuid; + +if (uuid_from_string(&lswitch_uuid, id)) { +is_uuid = 1; +lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, &lswitch_uuid); +} else { +NBREC_LOGICAL_SWITCH_FOR_EACH(lswitch, nb_ctx->idl) { +if (!strcmp(lswitch->name, id)) { +break;
Re: [ovs-dev] [PATCH 1/6] netdev-dpdk: create smaller mempools in case of failure
I suppose my only question with this patch is why we can't just
allocate MIN_NB_MBUF sized pools in the first place.
Acked-by: Ethan Jackson
On Thu, Mar 12, 2015 at 11:04 AM, Daniele Di Proietto
wrote:
> If rte_mempool_create() fails with ENOMEM, try asking for a smaller
> mempools. This patch enables OVS DPDK to run on systems without 1GB
> hugepages
>
> Signed-off-by: Daniele Di Proietto
> ---
> lib/netdev-dpdk.c | 46 +-
> 1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 1ba8310..54bc318 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -67,9 +67,23 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 20);
> #define MBUF_SIZE(mtu) (MTU_TO_MAX_LEN(mtu) + (512) + \
> sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
>
> -/* XXX: mempool size should be based on system resources. */
> -#define NB_MBUF (4096 * 64)
> -#define MP_CACHE_SZ (256 * 2)
> +/* Max and min number of packets in the mempool. OVS tries to allocate a
> + * mempool with MAX_NB_MBUF: if this fails (because the system doesn't have
> + * enough hugepages) we keep halving the number until the allocation succeeds
> + * or we reach MIN_NB_MBUF */
> +
> +#define MAX_NB_MBUF (4096 * 64)
> +#define MIN_NB_MBUF (4096 * 4)
> +#define MP_CACHE_SZ RTE_MEMPOOL_CACHE_MAX_SIZE
> +
> +/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
> +BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF) ==
> 0);
> +
> +/* The smallest possible NB_MBUF that we're going to try should be a multiple
> + * of MP_CACHE_SZ. This is advised by DPDK documentation. */
> +BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> + % MP_CACHE_SZ == 0);
> +
> #define SOCKET0 0
>
> #define NIC_PORT_RX_Q_SIZE 2048 /* Size of Physical NIC RX Queue, Max
> (n+32<=4096)*/
> @@ -293,6 +307,7 @@ dpdk_mp_get(int socket_id, int mtu)
> OVS_REQUIRES(dpdk_mutex)
> {
> struct dpdk_mp *dmp = NULL;
> char mp_name[RTE_MEMPOOL_NAMESIZE];
> +unsigned mp_size;
>
> LIST_FOR_EACH (dmp, list_node, &dpdk_mp_list) {
> if (dmp->socket_id == socket_id && dmp->mtu == mtu) {
> @@ -306,20 +321,25 @@ dpdk_mp_get(int socket_id, int mtu)
> OVS_REQUIRES(dpdk_mutex)
> dmp->mtu = mtu;
> dmp->refcount = 1;
>
> -if (snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_mp_%d_%d", dmp->mtu,
> - dmp->socket_id) < 0) {
> -return NULL;
> -}
> +mp_size = MAX_NB_MBUF;
> +do {
> +if (snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_mp_%d_%d_%u",
> + dmp->mtu, dmp->socket_id, mp_size) < 0) {
> +return NULL;
> +}
>
> -dmp->mp = rte_mempool_create(mp_name, NB_MBUF, MBUF_SIZE(mtu),
> - MP_CACHE_SZ,
> - sizeof(struct rte_pktmbuf_pool_private),
> - rte_pktmbuf_pool_init, NULL,
> - ovs_rte_pktmbuf_init, NULL,
> - socket_id, 0);
> +dmp->mp = rte_mempool_create(mp_name, mp_size, MBUF_SIZE(mtu),
> + MP_CACHE_SZ,
> + sizeof(struct rte_pktmbuf_pool_private),
> + rte_pktmbuf_pool_init, NULL,
> + ovs_rte_pktmbuf_init, NULL,
> + socket_id, 0);
> +} while (!dmp->mp && rte_errno == ENOMEM && (mp_size /= 2) >=
> MIN_NB_MBUF);
>
> if (dmp->mp == NULL) {
> return NULL;
> +} else {
> +VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name, mp_size
> );
> }
>
> list_push_back(&dpdk_mp_list, &dmp->list_node);
> --
> 2.1.4
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] dpif-netdev: Wait for threads to quiesce before freeing port.
Acked-by: Jarno Rajahalme Ben may also want to have a look, as commits 98de6bebb (dpif-netdev: Fix another use-after-free in port_unref().) and 87400a3d4cc4a (dpif-netdev: Fix use-after-free in port_unref().) by him have fixed earlier problems related to this. I did not check the changes in these earlier patches, but it is possible that ovsrcu_synchronize() is a more general solution to this problem, as this makes the port lookups essentially RCU-protected. Maybe we should document this fact in some comments? Jarno > On Mar 19, 2015, at 9:44 AM, Daniele Di Proietto > wrote: > > port_unref() shouldn't immediately free the port and close the netdev, > because otherwise threads that still have a pointer to the port would > crash. > > This commit fixes the problem by introducing an ovsrcu_synchronize() > call in port_unref(). > > I chose not to use ovsrcu_postpone(), because postponing the removal of > the 'netdev' would cause other issues, such as not being possible to > reuse the same name for another netdev. > > Found the crash while developing new code. > > Signed-off-by: Daniele Di Proietto > --- > lib/dpif-netdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index f01fecb..2546e5e 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -994,6 +994,8 @@ port_unref(struct dp_netdev_port *port) > int n_rxq = netdev_n_rxq(port->netdev); > int i; > > +ovsrcu_synchronize(); > + > netdev_close(port->netdev); > netdev_restore_flags(port->sf); > > -- > 2.1.4 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Costumoized actions
Thanks Jarno for the hints!
I found this function looking for ", struct dpif_op " term in every file.
static void dpif_linux_operate(struct dpif *dpif_, struct dpif_op **ops,
size_t n_ops)
The main lines in that function of my interest are:
dpif_linux_encode_execute(dpif->dp_ifindex, execute, &aux->request);
nl_transact_multiple(NETLINK_GENERIC, txnsp, n_ops);
As I understand from this function, and following the path for op->type
= DPIF_OP_EXECUTE, a request is sent at
"nl_transact_multiple(NETLINK_GENERIC, txnsp, n_ops);" through
NETLINK_GENERIC protocol (to the kernel?, any hint to where are they
sent?). Inside this function, I can find these important functions:
lib/netlink-socket.c:
nl_sock_transact_multiple__(struct nl_sock *sock,
struct nl_transaction **transactions, size_t n,
size_t *done) {
/* some lines */
do {
error = sendmsg(sock->fd, &msg, 0) < 0 ? errno : 0;
} while (error == EINTR);
/* some lines */
error = nl_sock_recv__(sock, buf_txn->reply, false);
/* some lines */
The thing is that when my action is performed, the error value is 0 (BAD,
packet is not sent); and when it is not performed, error = 11 = EAGAIN (OK,
actions are performed and packet is sent). I guess this means that whoever
receives the request, still does not understand the meaning of the custom
action I am creating.
I will be posting here any updates I have.
Kind regards,
Raúl Suárez
2015-03-19 20:50 GMT+01:00 Jarno Rajahalme :
> Raúl,
>
> The dip class functions are implemented by the different dip classes.
> Linux and Windows data paths are interfaced by lib/dpif-netlink.c and the
> userspace datapath is implemented in lib/dpif-netlink.c. You’ll find a
> class struct with the local function pointers in those files, just search
> for “operate” in those files.
>
> Hope this helps,
>
> Jarno
>
> > On Mar 19, 2015, at 2:06 AM, Raul Suarez Marin <
> raul.suarez.ma...@gmail.com> wrote:
> >
> > Hello again,
> >
> > At lib/dpif.c in function "dpif_operate" there is a line that is being
> > called.
> > dpif->dpif_class->operate(dpif, ops, chunk);
> >
> > This is possibly the actions are performed, but I have no clue how to
> know
> > what function this line leads to.
> >
> > In lib/dpif-provider.h I can find the definition for these structs, but I
> > can only see:
> >
> > /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the
> order
> > * in which they are specified, placing each operation's results in the
> > * "output" members documented in comments.
> > *
> > * This function is optional. It is only worthwhile to implement it if
> > * 'dpif' can perform operations in batch faster than individually. */
> > void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
> >
> > Kind regards,
> > Raúl Suárez
> >
> > 2015-03-18 21:26 GMT+01:00 Raul Suarez Marin <
> raul.suarez.ma...@gmail.com>:
> >
> >> Hello Ben,
> >>
> >> Thank you for your answer. Let me correct myself when I said I adapated
> >> parts of "handle_upcalls", I realized I just added debugging lines only
> >> (my bad).
> >> I am trying to do the ovs-vswitchd ofproto/trace command but I am
> getting
> >> failed connection attempts because "address family is not supported by
> >> protocol". I will sort it out.
> >>
> >> My thoughts right now are:
> >> 1) I have done in "do_xlate_actions" the part of reading the field I
> want
> >> to apply the action to. (new case OFPACT_MY_FIELD)
> >> 2) I adapted "commit_odp_actions" which is called
> >> by "compose_output_action" which is called by "xlate_output_action"
> which
> >> is called by "do_xlate_actions" (case OFPACT_OUTPUT) and it is located
> in
> >> "lib/ofp-util.c" This function adds the actions to the ofpbuf at
> ctx->xout->odp_actions
> >> with enum ovs_key = OVS_KEY_ATTR_MY_FIELD. I think there is no way this
> >> fails (because it's just appending data).
> >> 3) Added OVS_KEY_ATTR_MY_FIELD where needed. Specially in function
> >> "odp_execute_set_action" at lib/ofp-execute.c, where looks like that the
> >> packet is actually modified. There, I do this:
> >>
> >> const struct ovs_key_STRUCTURE_MODEL my_key = nl_attr_get_unspec(a,
> >> sizeof(struct ovs_key_STRUCTURE_MODEL))
> >> struct MY_STRUCTURE *packet_to_do_action = ofpbuf_l3(packet);
> >> packet_to_do_action ->my_field = my_key ->my_field;
> >>
> >> This should make it.
> >>
> >> BUT, HERE is the problem!
> >>
> >> I placed this code at "odp_execute_set_action" [function in 3)]
> >>
> >> FILE *f;
> >> f = fopen("/home/raul/match_event2.txt", "ab+");
> >> fprintf(f, "->odp_execute_set_action(%d)\n", type);
> >> fclose(f);
> >>
> >> This is what I use for debugging. It is a very ugly way of doing it but
> it
> >> works for me. But, this line is not printed when debugging, so the code
> >> never arrives at this point (even if my action is not implemented).
> >> However, when my action is not implemented the action execution works,
> but
> >> still the said lin
Re: [ovs-dev] Costumoized actions
Any custom actions need to be supported by the datapaths, otherwise they fail
flow setup validation. Linux datapath (kernel module) is part of the linux
kernel sources, but if you need to try out something you can test your feature
with the userspace datapath first, then maybe with the ova-tree linux kernel
module in the datapath directory.
However, if you are adding support for a new packet header field (as hinted by
MY_FIELD), you should not need to implement any new actions, as you should be
able to use the set_field action.
Jarno
> On Mar 19, 2015, at 2:38 PM, Raul Suarez Marin
> wrote:
>
> Thanks Jarno for the hints!
>
> I found this function looking for ", struct dpif_op " term in every file.
>
> static void dpif_linux_operate(struct dpif *dpif_, struct dpif_op **ops,
> size_t n_ops)
>
> The main lines in that function of my interest are:
> dpif_linux_encode_execute(dpif->dp_ifindex, execute, &aux->request);
> nl_transact_multiple(NETLINK_GENERIC, txnsp, n_ops);
>
> As I understand from this function, and following the path for op->type =
> DPIF_OP_EXECUTE, a request is sent at "nl_transact_multiple(NETLINK_GENERIC,
> txnsp, n_ops);" through NETLINK_GENERIC protocol (to the kernel?, any hint to
> where are they sent?). Inside this function, I can find these important
> functions:
>
> lib/netlink-socket.c:
> nl_sock_transact_multiple__(struct nl_sock *sock,
> struct nl_transaction **transactions, size_t n,
> size_t *done) {
> /* some lines */
> do {
> error = sendmsg(sock->fd, &msg, 0) < 0 ? errno : 0;
> } while (error == EINTR);
>
> /* some lines */
>
> error = nl_sock_recv__(sock, buf_txn->reply, false);
>
> /* some lines */
>
> The thing is that when my action is performed, the error value is 0 (BAD,
> packet is not sent); and when it is not performed, error = 11 = EAGAIN (OK,
> actions are performed and packet is sent). I guess this means that whoever
> receives the request, still does not understand the meaning of the custom
> action I am creating.
>
> I will be posting here any updates I have.
>
> Kind regards,
> Raúl Suárez
>
>
> 2015-03-19 20:50 GMT+01:00 Jarno Rajahalme >:
> Raúl,
>
> The dip class functions are implemented by the different dip classes. Linux
> and Windows data paths are interfaced by lib/dpif-netlink.c and the userspace
> datapath is implemented in lib/dpif-netlink.c. You’ll find a class struct
> with the local function pointers in those files, just search for “operate” in
> those files.
>
> Hope this helps,
>
> Jarno
>
> > On Mar 19, 2015, at 2:06 AM, Raul Suarez Marin > > wrote:
> >
> > Hello again,
> >
> > At lib/dpif.c in function "dpif_operate" there is a line that is being
> > called.
> > dpif->dpif_class->operate(dpif, ops, chunk);
> >
> > This is possibly the actions are performed, but I have no clue how to know
> > what function this line leads to.
> >
> > In lib/dpif-provider.h I can find the definition for these structs, but I
> > can only see:
> >
> > /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order
> > * in which they are specified, placing each operation's results in the
> > * "output" members documented in comments.
> > *
> > * This function is optional. It is only worthwhile to implement it if
> > * 'dpif' can perform operations in batch faster than individually. */
> > void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
> >
> > Kind regards,
> > Raúl Suárez
> >
> > 2015-03-18 21:26 GMT+01:00 Raul Suarez Marin > >:
> >
> >> Hello Ben,
> >>
> >> Thank you for your answer. Let me correct myself when I said I adapated
> >> parts of "handle_upcalls", I realized I just added debugging lines only
> >> (my bad).
> >> I am trying to do the ovs-vswitchd ofproto/trace command but I am getting
> >> failed connection attempts because "address family is not supported by
> >> protocol". I will sort it out.
> >>
> >> My thoughts right now are:
> >> 1) I have done in "do_xlate_actions" the part of reading the field I want
> >> to apply the action to. (new case OFPACT_MY_FIELD)
> >> 2) I adapted "commit_odp_actions" which is called
> >> by "compose_output_action" which is called by "xlate_output_action" which
> >> is called by "do_xlate_actions" (case OFPACT_OUTPUT) and it is located in
> >> "lib/ofp-util.c" This function adds the actions to the ofpbuf at
> >> ctx->xout->odp_actions
> >> with enum ovs_key = OVS_KEY_ATTR_MY_FIELD. I think there is no way this
> >> fails (because it's just appending data).
> >> 3) Added OVS_KEY_ATTR_MY_FIELD where needed. Specially in function
> >> "odp_execute_set_action" at lib/ofp-execute.c, where looks like that the
> >> packet is actually modified. There, I do this:
> >>
> >> const struct ovs_key_STRUCTURE_MODEL my_key = nl_attr_get_unspec(a,
> >> sizeof(struct ovs_key_STRUCTURE_
Re: [ovs-dev] [PATCH] ovn: add start of ovn-nbctl
> On Mar 19, 2015, at 4:02 PM, Russell Bryant wrote: > > ovn-nbctl is intended to be a utility for manually interacting with > the OVN northbound db. A real consumer of OVN, such as OpenStack, > should be using ovsdb directly. However, a utility like this is > useful during development and testing of both ovn itself as well as > the OpenStack Neutron plugin. It could also be used when debugging > the state of an ovn deployment. > > So far, the commands related to logical switches have been > implemented. You can add, delete, list, and get/set external:ids. > > Signed-off-by: Russell Bryant > --- > ovn/.gitignore | 2 + > ovn/automake.mk | 8 +- > ovn/ovn-nbctl.8.xml | 22 > ovn/ovn-nbctl.c | 327 > 4 files changed, 357 insertions(+), 2 deletions(-) > > I just realized I forgot to actually implement the handling of duplicate names that we discussed. Oops. Consider that on the list to fix in the next rev. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] (no subject)
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Hi
i_дH ]Ï\¡r©¢ÞUEü#3wmeAgëòdä¢Ù&µÑs,Pæ²mÏSëbJ5÷ÆgÕ îù¤óaó¾z0O&#MÁälrôNi]öctÖÇ]8ì¬êÆEþ UBk¦vøÚðx®VàüÐEÃ6Vw1ølQ#pónKwt¬ m Lk'øÅ_ äýÍ°ÑÚûËg,;;Ó#!1f KgÇV \NÁÔô¨§U)wêöHù(ÎS¦ùs5¾w¬.8N(ë?éÏì×ÏkÙ· ÏYêþeÛÊ¥?EÅd¬ºB|ZÔ¾O· J3ËîHwó8dÊhÆÚ.ùQú]M6j¦`Y¿PG®&v'gn6·àÈVêå7æýÁbçôéÜJÓ!CVyñ £O®LÅ¿z%êqkVpíòwÏ&¡ÙXÄqÌNHá*\9}¾pê»$Oh¶gñYF8´7m½ jQ ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] bashcomp: Install and package completion scripts.
On Wed, 18 Mar 2015 14:47:25 -0700: Alex Wang wrote: > This commit makes the bash completion scripts be installed to > /etc/bash_completion.d through 'make install' or package installation. > This will make the scripts available for each bash session. > An alternative is to put scripts to /usr/share/bash_completion/ directory. > However, this is not supported by earlier version of bash completion. > Signed-off-by: Alex Wang These changes look good to me overall, but I just realized that utilities/ovs-command-bashcomp.INSTALL.md still talks about how to manually install the scripts into /etc/bash_completion.d. Should we also change it to make it clear that the scripts will be installed by default, with an explanation of how to manually use them in other cases? Thanks, Peter Amidon ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] bashcomp: Install and package completion scripts.
On Thu, Mar 19, 2015 at 5:08 PM, Peter Amidon wrote: > On Wed, 18 Mar 2015 14:47:25 -0700: Alex Wang wrote: > > > > This commit makes the bash completion scripts be installed to > > /etc/bash_completion.d through 'make install' or package installation. > > This will make the scripts available for each bash session. > > > An alternative is to put scripts to /usr/share/bash_completion/ > directory. > > However, this is not supported by earlier version of bash completion. > > > Signed-off-by: Alex Wang > > These changes look good to me overall, but I just realized that > utilities/ovs-command-bashcomp.INSTALL.md still talks about how to > manually install the scripts into /etc/bash_completion.d. Should we > also change it to make it clear that the scripts will be installed by > default, with an explanation of how to manually use them in other cases? > > Yeah, thx for pointing it out! I'll do that, Are you okay with the patch 1/2? > Thanks, > > Peter Amidon > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 1/5] Sanitize remaining recirculation parameters.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-xlate.c | 66 +-
ofproto/ofproto-dpif-xlate.h |6
2 files changed, 39 insertions(+), 33 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0e28c77..ece9670 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -208,14 +208,13 @@ struct xlate_ctx {
uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
bool exit; /* No further actions should be processed. */
-bool use_recirc;/* Should generate recirc? */
-struct xlate_recirc recirc; /* Information used for generating
- * recirculation actions */
-
/* True if a packet was but is no longer MPLS (due to an MPLS pop action).
* This is a trigger for recirculation in cases where translating an action
* or looking up a flow requires access to the fields of the packet after
- * the MPLS label stack that was originally present. */
+ * the MPLS label stack that was originally present.
+ *
+ * XXX: output to a table and patch port do not currently recirculate even
+ * if this is true. */
bool was_mpls;
/* OpenFlow 1.1+ action set.
@@ -353,7 +352,16 @@ static bool input_vid_is_valid(uint16_t vid, struct
xbundle *, bool warn);
static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
static void output_normal(struct xlate_ctx *, const struct xbundle *,
uint16_t vlan);
-static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port);
+
+/* Optional bond recirculation parameter to compose_output_action(). */
+struct xlate_bond_recirc {
+uint32_t recirc_id; /* !0 Use recirculation instead of output. */
+uint8_t hash_alg; /* !0 Compute hash for recirc before. */
+uint32_t hash_basis; /* Compute hash for recirc before. */
+};
+
+static void compose_output_action(struct xlate_ctx *, ofp_port_t ofp_port,
+ const struct xlate_bond_recirc *xr);
static struct xbridge *xbridge_lookup(struct xlate_cfg *,
const struct ofproto_dpif *);
@@ -1670,28 +1678,28 @@ output_normal(struct xlate_ctx *ctx, const struct
xbundle *out_xbundle,
uint16_t vid;
ovs_be16 tci, old_tci;
struct xport *xport;
+struct xlate_bond_recirc xr;
+bool use_recirc = false;
vid = output_vlan_to_vid(out_xbundle, vlan);
if (list_is_empty(&out_xbundle->xports)) {
/* Partially configured bundle with no slaves. Drop the packet. */
return;
} else if (!out_xbundle->bond) {
-ctx->use_recirc = false;
xport = CONTAINER_OF(list_front(&out_xbundle->xports), struct xport,
bundle_node);
} else {
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
struct flow_wildcards *wc = &ctx->xout->wc;
-struct xlate_recirc *xr = &ctx->recirc;
struct ofport_dpif *ofport;
if (ctx->xbridge->enable_recirc) {
-ctx->use_recirc = bond_may_recirc(
-out_xbundle->bond, &xr->recirc_id, &xr->hash_basis);
+use_recirc = bond_may_recirc(
+out_xbundle->bond, &xr.recirc_id, &xr.hash_basis);
-if (ctx->use_recirc) {
+if (use_recirc) {
/* Only TCP mode uses recirculation. */
-xr->hash_alg = OVS_HASH_ALG_L4;
+xr.hash_alg = OVS_HASH_ALG_L4;
bond_update_post_recirc_rules(out_xbundle->bond, false);
/* Recirculation does not require unmasking hash fields. */
@@ -1708,9 +1716,9 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle
*out_xbundle,
return;
}
-/* If ctx->xout->use_recirc is set, the main thread will handle stats
+/* If use_recirc is set, the main thread will handle stats
* accounting for this bond. */
-if (!ctx->use_recirc) {
+if (!use_recirc) {
if (ctx->xin->resubmit_stats) {
bond_account(out_xbundle->bond, &ctx->xin->flow, vid,
ctx->xin->resubmit_stats->n_bytes);
@@ -1738,7 +1746,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle
*out_xbundle,
}
*flow_tci = tci;
-compose_output_action(ctx, xport->ofp_port);
+compose_output_action(ctx, xport->ofp_port, use_recirc ? &xr : NULL);
*flow_tci = old_tci;
}
@@ -2673,7 +2681,7 @@ build_tunnel_send(const struct xlate_ctx *ctx, const
struct xport *xport,
static void
compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
-bool check_stp)
+const struct xlate_bond_recirc *xr, bool check_stp)
{
const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
struct flow_wildcards
[ovs-dev] [PATCH v4 3/5] ofproto-dpif-xlate: More robust wildcarding for select group.
The flow key should be the same regardless of whether a live bucket is
found or not, as it would be confusing that the flow key would be
different (different mask bits) after the last group bucket goes dead.
In general, the megaflow algorithm expects the mask bits be set as
soon as we read the header bits, regardless of what happens
afterwards.
Also, use flow_mask_hash_fields() instead of individually setting mask
fields. This immediately brings in IPv6 support, and helps keeping
masks in sync with potential algorithm changes to flow hashing
functions.
Found by inspection.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-xlate.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8ca35a7..c1aedd2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3133,18 +3133,9 @@ xlate_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
uint32_t basis;
basis = flow_hash_symmetric_l4(&ctx->xin->flow, 0);
+flow_mask_hash_fields(&ctx->xin->flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4);
bucket = group_best_live_bucket(ctx, group, basis);
if (bucket) {
-memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
-memset(&wc->masks.dl_src, 0xff, sizeof wc->masks.dl_src);
-memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
-memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
-memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
-memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
-memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
-memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
-memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
-
xlate_group_bucket(ctx, bucket);
xlate_group_stats(ctx, group, bucket);
}
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 2/5] ofproto-dpif-xlate: Roll back group bucket actions after every bucket.
We used to roll back group bucket changes only for 'all' and
'indirect' group types, but the expected semantics of all group types
is that any changes by the group bucket are not visible after the
group has been executed.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-xlate.c | 37 +
tests/ofproto-dpif.at| 12
2 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ece9670..8ca35a7 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3057,6 +3057,8 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct
ofputil_bucket *bucket)
{
uint64_t action_list_stub[1024 / 8];
struct ofpbuf action_list, action_set;
+struct flow old_flow = ctx->xin->flow;
+bool old_was_mpls = ctx->was_mpls;
ofpbuf_use_const(&action_set, bucket->ofpacts, bucket->ofpacts_len);
ofpbuf_use_stub(&action_list, action_list_stub, sizeof action_list_stub);
@@ -3068,6 +3070,33 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct
ofputil_bucket *bucket)
ofpbuf_uninit(&action_set);
ofpbuf_uninit(&action_list);
+
+/* Roll back flow to previous state.
+ * This is equivalent to cloning the packet for each bucket.
+ *
+ * As a side effect any subsequently applied actions will
+ * also effectively be applied to a clone of the packet taken
+ * just before applying the all or indirect group.
+ *
+ * Note that group buckets are action sets, hence they cannot modify the
+ * main action set. Also any stack actions are ignored when executing an
+ * action set, so group buckets cannot change the stack either.
+ * However, we do allow resubmit actions in group buckets, which could
+ * break the above assumptions. It is upto the controller to not mess up
+ * with the action_set and stack in the tables resubmitted to from
+ * group buckets. */
+ctx->xin->flow = old_flow;
+
+/* The group bucket popping MPLS should have no effect after bucket
+ * execution. */
+ctx->was_mpls = old_was_mpls;
+
+/* The fact that the group bucket exits (for any reason) does not mean that
+ * the translation after the group action should exit. Specifically, if
+ * the group bucket recirculates (which typically modifies the packet), the
+ * actions after the group action must continue processing with the
+ * original, not the recirculated packet! */
+ctx->exit = false;
}
static void
@@ -3075,19 +3104,11 @@ xlate_all_group(struct xlate_ctx *ctx, struct
group_dpif *group)
{
struct ofputil_bucket *bucket;
const struct ovs_list *buckets;
-struct flow old_flow = ctx->xin->flow;
group_dpif_get_buckets(group, &buckets);
LIST_FOR_EACH (bucket, list_node, buckets) {
xlate_group_bucket(ctx, bucket);
-/* Roll back flow to previous state.
- * This is equivalent to cloning the packet for each bucket.
- *
- * As a side effect any subsequently applied actions will
- * also effectively be applied to a clone of the packet taken
- * just before applying the all or indirect group. */
-ctx->xin->flow = old_flow;
}
xlate_group_stats(ctx, group, NULL);
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 897df4e..26550c4 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -332,6 +332,18 @@ AT_CHECK([tail -1 stdout], [0],
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - group actions have no effect afterwards])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [10])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-group br0
'group_id=1234,type=select,bucket=set_field:192.168.3.90->ip_src,output:10'])
+AT_CHECK([ovs-ofctl -O OpenFlow12 add-flow br0 'ip
actions=group:1234,output:10'])
+AT_CHECK([ovs-appctl ofproto/trace br0
'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'],
[0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+ [Datapath actions:
set(ipv4(src=192.168.3.90,dst=192.168.0.2)),10,set(ipv4(src=192.168.0.1,dst=192.168.0.2)),10
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - all group in action set])
OVS_VSWITCHD_START
ADD_OF_PORTS([br0], [1], [10], [11])
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 5/5] ofproto-dpif-xlate: Fix MPLS recirculation.
Prior to this patch MPLS recirculation was not performed on a table
lookup following an MPLS_POP action. This patch refactors MPLS
recirculation triggering so that a table action can be re-done after
recirculation if that table action follows an MPLS_POP action.
Recirculation for a patch port traversal (which also does a table
lookup) after an MPLS_POP action does not need to store the output
action, as recirculation without any post-recirculation actions causes
the table lookup to happen anyway.
Furthermore, the stack actions now have the same post-MPLS_POP
optimization as the SET_FIELD and MOVE actions had already:
recirculation is triggered only if the register in the action is L3 or
higher.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-xlate.c | 165 +++---
1 file changed, 76 insertions(+), 89 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2ab958c..40a20ae 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -293,10 +293,7 @@ struct xlate_ctx {
/* True if a packet was but is no longer MPLS (due to an MPLS pop action).
* This is a trigger for recirculation in cases where translating an action
* or looking up a flow requires access to the fields of the packet after
- * the MPLS label stack that was originally present.
- *
- * XXX: output to a table and patch port do not currently recirculate even
- * if this is true. */
+ * the MPLS label stack that was originally present. */
bool was_mpls;
/* OpenFlow 1.1+ action set.
@@ -312,6 +309,19 @@ struct xlate_ctx {
static void xlate_action_set(struct xlate_ctx *ctx);
+static void
+ctx_trigger_recirculation(struct xlate_ctx *ctx)
+{
+ctx->exit = true;
+ctx->recirc_action_offset = ctx->action_set.size;
+}
+
+static bool
+ctx_first_recirculation_action(const struct xlate_ctx *ctx)
+{
+return ctx->recirc_action_offset == ctx->action_set.size;
+}
+
static inline bool
exit_recirculates(const struct xlate_ctx *ctx)
{
@@ -3059,6 +3069,11 @@ static void
xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
bool may_packet_in, bool honor_table_miss)
{
+/* Check if we need to recirculate before matching in a table. */
+if (ctx->was_mpls) {
+ctx_trigger_recirculation(ctx);
+return;
+}
if (xlate_resubmit_resource_check(ctx)) {
struct flow_wildcards *wc;
uint8_t old_table_id = ctx->table_id;
@@ -3881,85 +3896,6 @@ xlate_action_set(struct xlate_ctx *ctx)
ofpbuf_uninit(&action_list);
}
-static bool
-ofpact_needs_recirculation_after_mpls(const struct ofpact *a, struct xlate_ctx
*ctx)
-{
-struct flow_wildcards *wc = &ctx->xout->wc;
-struct flow *flow = &ctx->xin->flow;
-
-if (!ctx->was_mpls) {
-return false;
-}
-
-switch (a->type) {
-case OFPACT_OUTPUT:
-case OFPACT_GROUP:
-case OFPACT_CONTROLLER:
-case OFPACT_STRIP_VLAN:
-case OFPACT_SET_VLAN_PCP:
-case OFPACT_SET_VLAN_VID:
-case OFPACT_ENQUEUE:
-case OFPACT_PUSH_VLAN:
-case OFPACT_SET_ETH_SRC:
-case OFPACT_SET_ETH_DST:
-case OFPACT_SET_TUNNEL:
-case OFPACT_SET_QUEUE:
-case OFPACT_POP_QUEUE:
-case OFPACT_CONJUNCTION:
-case OFPACT_NOTE:
-case OFPACT_UNROLL_XLATE:
-case OFPACT_OUTPUT_REG:
-case OFPACT_EXIT:
-case OFPACT_METER:
-case OFPACT_WRITE_METADATA:
-case OFPACT_WRITE_ACTIONS:
-case OFPACT_CLEAR_ACTIONS:
-case OFPACT_SAMPLE:
-return false;
-
-case OFPACT_POP_MPLS:
-case OFPACT_DEC_MPLS_TTL:
-case OFPACT_SET_MPLS_TTL:
-case OFPACT_SET_MPLS_TC:
-case OFPACT_SET_MPLS_LABEL:
-case OFPACT_SET_IPV4_SRC:
-case OFPACT_SET_IPV4_DST:
-case OFPACT_SET_IP_DSCP:
-case OFPACT_SET_IP_ECN:
-case OFPACT_SET_IP_TTL:
-case OFPACT_SET_L4_SRC_PORT:
-case OFPACT_SET_L4_DST_PORT:
-case OFPACT_RESUBMIT:
-case OFPACT_STACK_PUSH:
-case OFPACT_STACK_POP:
-case OFPACT_DEC_TTL:
-case OFPACT_MULTIPATH:
-case OFPACT_BUNDLE:
-case OFPACT_LEARN:
-case OFPACT_FIN_TIMEOUT:
-case OFPACT_GOTO_TABLE:
-return true;
-
-case OFPACT_REG_MOVE:
-return (mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->dst.field) ||
-mf_is_l3_or_higher(ofpact_get_REG_MOVE(a)->src.field));
-
-case OFPACT_SET_FIELD:
-return mf_is_l3_or_higher(ofpact_get_SET_FIELD(a)->field);
-
-case OFPACT_PUSH_MPLS:
-/* Recirculate if it is an IP packet with a zero ttl. This may
- * indicate that the packet was previously MPLS and an MPLS pop action
- * converted it to IP. In this case recirculating should reveal the IP
- * TTL which is used as the basis for a new MPLS LSE. */
-return (!flow_count_mpls_labels(flow, wc)
-&& flow->nw_ttl == 0
-&& is_ip_any(flow));
-}
-
-OVS_
Re: [ovs-dev] [PATCH v8] netdev-dpdk: add dpdk vhost-cuse ports
On Thu, Mar 5, 2015 at 1:42 PM, Kevin Traynor wrote: > This patch adds support for a new port type to userspace datapath > called dpdkvhost. This allows KVM (QEMU) to offload the servicing > of virtio-net devices to its associated dpdkvhost port. Instructions > for use are in INSTALL.DPDK. > > This has been tested on Intel multi-core platforms and with clients > that have virtio-net interfaces. > > ver 8: >- rebased with master > ver 7: >- rebased with master >- reworked INSTALL.DPDK and performed extra testing based on > review comments > ver 6: >- rebased with master >- modified to use DPDK v1.8.0 vhost library >- reworked for review comments > ver 5: >- rebased against latest master > ver 4: >- added eventfd_link.h and eventfd_link.c to EXTRA_DIST in > utilities/automake.mk >- rebased with master to work with DPDK 1.7 ver 3: >- rebased with master > ver 2: >- rebased with master > > Signed-off-by: Ciara Loftus > Signed-off-by: Kevin Traynor > Signed-off-by: Maryam Tahhan I cleanup patch, removed unnecessary locking changes, fixed few RCU issues and applied the patch. Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] ovs-vsctl-bashcomp: Avoid setting the COMP_WORDBREAKS.
On Wed, 18 Mar 2015 14:47:24 -0700: Alex Wang wrote: > Modifying $COMP_WORDBREAKS in completion script is not the recommended > as it is a global variable and the modification could affect the behavior > of other completion scripts. As a workaround, this commit uses the > _get_comp_words_by_ref which allows user to exclude characters out of > $COMP_WORDBREAKS and reassemble input command line. However, as a side > effect, the bash completion module cannot handle characters defined in > $COMP_WORDBREAKS (e.g. ':' and '=') correctly in the resulting completions. > Thusly, we need to trim the colon-word and equal-word prefixes from reply. > Signed-off-by: Alex Wang This looks good to me now. Thanks, Peter Amidon ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] in_port action
Hello everyone, why are we using a in_port action, when we have output action ? xlate_output_action has a default case where we are checking the port should not be same as input port . Why this check is required ? Thanks and regards , Deepak ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] in_port action
On Fri, Mar 20, 2015 at 09:25:37AM +0530, deepaksingh wrote: > why are we using a in_port action, when we have output action ? > xlate_output_action has a default case where we are checking the port > should not be same as input port . > Why this check is required ? Because OpenFlow says so. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] in_port action
Please don't drop the mailing list. You will break OpenFlow compatibility. What are you trying to accomplish? On Fri, Mar 20, 2015 at 09:30:26AM +0530, deepaksingh wrote: > Thanks Ben . > > If i will remove this check, will it create any problem ? > > On Fri, Mar 20, 2015 at 9:28 AM, Ben Pfaff wrote: > > > On Fri, Mar 20, 2015 at 09:25:37AM +0530, deepaksingh wrote: > > > why are we using a in_port action, when we have output action ? > > > xlate_output_action has a default case where we are checking the port > > > should not be same as input port . > > > Why this check is required ? > > > > Because OpenFlow says so. > > > > > > -- > Thanks & Regards, > Deepak Singh > dmx.dee...@gmail.com > +91 9739743261 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] in_port action
Please don't drop the list!
On Fri, Mar 20, 2015 at 09:40:23AM +0530, deepaksingh wrote:
> Hi Ben ,
>
> the L3 flows for wireless-client communicate with wireless-client in
> another VLAN will look like below.
>
> In_port=ats,vlan=10,src_mac=client1's
> MAC,dst_mac=vRouter'sMAC,eth_type=0x800,dstip=
> 20.1.1.2/32,action=set_src_mac=vRouter's MAC, set_dst_mac=client2's MAC,
> set_vlan=20,output=ats
>
> In_port=ats,vlan=20,src_mac=client2's
> MAC,dst_mac=vRouter'sMAC,eth_type=0x800,dstip=
> 10.1.1.2/32,action=set_src_mac=vRouter's MAC, set_dst_mac=client1's MAC,
> set_vlan=10,output=ats
>
>
>
> Apparently, above flow will not forward traffic traffic as in_port and
> output is the same port "ats". As you mentioned controller have to use
> "IN_PORT". So instead of using "IN_PORT" can ats (Physical port) be used in
> this case?
Please read the FAQ.
### Q: I added a flow to send packets out the ingress port, like this:
ovs-ofctl add-flow br0 in_port=2,actions=2
but OVS drops the packets instead.
A: Yes, OpenFlow requires a switch to ignore attempts to send a packet
out its ingress port. The rationale is that dropping these packets
makes it harder to loop the network. Sometimes this behavior can
even be convenient, e.g. it is often the desired behavior in a flow
that forwards a packet to several ports ("floods" the packet).
Sometimes one really needs to send a packet out its ingress port
("hairpin"). In this case, output to OFPP_IN_PORT, which in
ovs-ofctl syntax is expressed as just "in_port", e.g.:
ovs-ofctl add-flow br0 in_port=2,actions=in_port
This also works in some circumstances where the flow doesn't match
on the input port. For example, if you know that your switch has
five ports numbered 2 through 6, then the following will send every
received packet out every port, even its ingress port:
ovs-ofctl add-flow br0 actions=2,3,4,5,6,in_port
or, equivalently:
ovs-ofctl add-flow br0 actions=all,in_port
Sometimes, in complicated flow tables with multiple levels of
"resubmit" actions, a flow needs to output to a particular port
that may or may not be the ingress port. It's difficult to take
advantage of OFPP_IN_PORT in this situation. To help, Open vSwitch
provides, as an OpenFlow extension, the ability to modify the
in_port field. Whatever value is currently in the in_port field is
the port to which outputs will be dropped, as well as the
destination for OFPP_IN_PORT. This means that the following will
reliably output to port 2 or to ports 2 through 6, respectively:
ovs-ofctl add-flow br0 in_port=2,actions=load:0->NXM_OF_IN_PORT[],2
ovs-ofctl add-flow br0 actions=load:0->NXM_OF_IN_PORT[],2,3,4,5,6
If the input port is important, then one may save and restore it on
the stack:
ovs-ofctl add-flow br0 actions=push:NXM_OF_IN_PORT[],\
load:0->NXM_OF_IN_PORT[],\
2,3,4,5,6,\
pop:NXM_OF_IN_PORT[]
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] (no subject)
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] ovs-vsctl-bashcomp: Avoid setting the COMP_WORDBREAKS.
Hey Ben, Do you have more comments? This series should be ready for commit~ Thanks, Alex Wang, On Thu, Mar 19, 2015 at 8:44 PM, Peter Amidon wrote: > On Wed, 18 Mar 2015 14:47:24 -0700: Alex Wang wrote: > > > > Modifying $COMP_WORDBREAKS in completion script is not the recommended > > as it is a global variable and the modification could affect the > behavior > > of other completion scripts. As a workaround, this commit uses the > > _get_comp_words_by_ref which allows user to exclude characters out of > > $COMP_WORDBREAKS and reassemble input command line. However, as a side > > effect, the bash completion module cannot handle characters defined in > > $COMP_WORDBREAKS (e.g. ':' and '=') correctly in the resulting > completions. > > Thusly, we need to trim the colon-word and equal-word prefixes from > reply. > > > Signed-off-by: Alex Wang > > This looks good to me now. > > Thanks, > > Peter Amidon > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 0/7] Group Select: Selection Method Extension
Hi, this patch set implements the group select selection method extension that I circulated some months ago. The implementation makes use of a group experimenter property and thus depends on Open Flow 1.5 groups (ONF EXT-250). The last patch of the series adds an implementation of a hash selection method to illustrate what a selection method might look like. It may be thought of as a less intelligent but more flexible than the default selection method which I characterise as making hash of L2 and/or L3 fields depending on which fields are present in the flow. Key differences between v3 and v4: * Use nx_pull_entry__() with NULL mask in oxm_pull_field_array() * Use mf_parse_value() instead of mf_parse() in parse_select_group_field() * Include props field of type struct ofputil_group_props in struct ofgroup * oxm_format_field_array() can never fail so use void as its return type * Enhanced hash selection method - Use all bits of selection_method_param - Only hash fields if their pre-requisites are present and if so also hash the pre-requisites Key differences between v2 and v3: As suggested by Ben Pfaff: * Use NTR instead of NMX as Netronome extension prefix * Use array of TLVs rather than OF1.1 match for fields field of NTR selection method property * Add check to only permit known selection methods * I have also removed the RFC designation on the series Key differences between v1 and v2: Thanks Ben Pfaff and other's for their review. * Use an array of TLVs rather than an of_match structure for the fields of the group experimenter property - This resulted in an extensive rework of the code * Do not enforce pre-requisites of selection method fields * Add documentation to Documentation directory To aid review I have made this series available on github: https://github.com/horms/openvswitch.git devel/ext-350+selection_method-v4 Simon Horman (7): Documentation: Add documentation of group selection method property Add types for NTR selection method Support decoding of NTR selection method Support encoding of NTR selection method Support translation of NTR selection method Support NTR selection method in ovs-ofctl group commands Implement hash fields select group Documentation/automake.mk | 2 + Documentation/group-selection-method-property.txt | 153 Makefile.am | 1 + include/openflow/automake.mk | 1 + include/openflow/netronome-ext.h | 66 ++ lib/meta-flow.c | 44 lib/meta-flow.h | 12 + lib/nx-match.c| 124 ++ lib/nx-match.h| 5 + lib/ofp-parse.c | 94 lib/ofp-print.c | 32 ++- lib/ofp-util.c| 277 +- lib/ofp-util.h| 15 ++ ofproto/ofproto-dpif-xlate.c | 82 ++- ofproto/ofproto-dpif.c| 18 ++ ofproto/ofproto-dpif.h| 3 + ofproto/ofproto-provider.h| 2 + ofproto/ofproto.c | 5 + tests/ofp-print.at| 16 +- tests/ofproto-dpif.at | 33 +++ tests/ofproto.at | 4 +- utilities/ovs-ofctl.8.in | 34 +++ 22 files changed, 1002 insertions(+), 21 deletions(-) create mode 100644 Documentation/automake.mk create mode 100644 Documentation/group-selection-method-property.txt create mode 100644 include/openflow/netronome-ext.h -- 2.1.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 2/7] Add types for NTR selection method
NTR selection method
Signed-off-by: Simon Horman
---
This patch does not define any selection method names, however,
a subsequent patch proposes a "hash" selection method.
v4
* No change
v3
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use array of TLVs rather than OF1.1 match for fields field of
NTR selection method property
---
include/openflow/automake.mk | 1 +
include/openflow/netronome-ext.h | 66
2 files changed, 67 insertions(+)
create mode 100644 include/openflow/netronome-ext.h
diff --git a/include/openflow/automake.mk b/include/openflow/automake.mk
index 50933d5..d7dac91 100644
--- a/include/openflow/automake.mk
+++ b/include/openflow/automake.mk
@@ -1,5 +1,6 @@
openflowincludedir = $(includedir)/openflow
openflowinclude_HEADERS = \
+ include/openflow/netronome-ext.h \
include/openflow/nicira-ext.h \
include/openflow/openflow-1.0.h \
include/openflow/openflow-1.1.h \
diff --git a/include/openflow/netronome-ext.h b/include/openflow/netronome-ext.h
new file mode 100644
index 000..8db7b79
--- /dev/null
+++ b/include/openflow/netronome-ext.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2014 Netronome.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef OPENFLOW_NETRONOME_EXT_H
+#define OPENFLOW_NETRONOME_EXT_H 1
+
+#include "openflow/openflow.h"
+#include "openvswitch/types.h"
+
+/* The following vendor extension, proposed by Netronome, is not yet
+ * standardized, so they are not included in openflow.h. It may
+ * be suitable for standardization */
+
+
+/* Netronome enhanced select group */
+
+enum ntr_group_mod_subtype {
+NTRT_SELECTION_METHOD = 1,
+};
+
+#define NTR_MAX_SELECTION_METHOD_LEN 16
+
+struct ntr_group_prop_selection_method {
+ovs_be16 type; /* OFPGPT15_EXPERIMENTER. */
+ovs_be16 length;/* Length in bytes of this property
+ * excluding trailing padding. */
+ovs_be32 experimenter; /* NTR_VENDOR_ID. */
+ovs_be32 exp_type; /* NTRT_SELECTION_METHOD. */
+ovs_be32 pad;
+char selection_method[NTR_MAX_SELECTION_METHOD_LEN];
+/* Null-terminated */
+ovs_be64 selection_method_param; /* Non-Field parameter for
+ * bucket selection. */
+
+/* Followed by:
+ * - Exactly (length - 40) (possibly 0) bytes containing OXM TLVs, then
+ * - Exactly ((length + 7)/8*8 - length) (between 0 and 7) bytes of
+ * all-zero bytes
+ * In summary, ntr_group_prop_selection_method is padded as needed,
+ * to make its overall size a multiple of 8, to preserve alignment
+ * in structures using it.
+ */
+/* uint8_t field_array[0]; */ /* Zero or more fields encoded as
+ * OXM TLVs where the has_mask bit must
+ * be zero and the value it specifies is
+ * a mask to apply to packet fields and
+ * then input them to the selection
+ * method of a select group. */
+/* uint8_t pad2[0]; */
+};
+OFP_ASSERT(sizeof(struct ntr_group_prop_selection_method) == 40);
+
+#endif /* openflow/netronome-ext.h */
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 1/7] Documentation: Add documentation of group selection method property
NTR selection method
Signed-off-by: Simon Horman
Acked-by: Ben Pfaff
---
v4
* Included Ben Pfaff's Ack
v3
* Use NTR instead of NMX as Netronome extension prefix
v2
* Initial post
---
Documentation/automake.mk | 2 +
Documentation/group-selection-method-property.txt | 153 ++
Makefile.am | 1 +
3 files changed, 156 insertions(+)
create mode 100644 Documentation/automake.mk
create mode 100644 Documentation/group-selection-method-property.txt
diff --git a/Documentation/automake.mk b/Documentation/automake.mk
new file mode 100644
index 000..e504801
--- /dev/null
+++ b/Documentation/automake.mk
@@ -0,0 +1,2 @@
+EXTRA_DIST += \
+ Documentation/group-selection-method-property.txt
diff --git a/Documentation/group-selection-method-property.txt
b/Documentation/group-selection-method-property.txt
new file mode 100644
index 000..36337c6
--- /dev/null
+++ b/Documentation/group-selection-method-property.txt
@@ -0,0 +1,153 @@
+Proposal for Group Selection Method Property
+Version: 0.0.3
+
+Author: Simon Horman , et al.
+Initial Public Revision: September 2014
+
+
+Contents
+
+
+1. Introduction
+2. How it Works
+3. Experimenter Id
+4. Experimenter Messages
+5. History
+
+
+1. Introduction
+===
+
+This text describes a Netronome Extension to (draft) OpenFlow 1.5 that allows a
+controller to provide more information on the selection method for select
+groups. This proposal is in the form of an enhanced select group type.
+
+This may subsequently be proposed as an extension or update to
+the OpenFlow specification.
+
+
+2. How it works
+===
+
+A new Netronome group experimenter property is defined which provides
+compatibility with the group mod message defined in draft Open Flow 1.5
+(also known as ONF EXT-350) and allows parameters for the selection
+method of select groups to be passed by the controller. In particular it
+allows controllers to:
+
+* Specify the fields used for bucket selection by the select group.
+
+* Designate the selection method used.
+
+* Provide a non-field parameter to the selection method.
+
+
+3. Experimenter ID
+==
+
+The Experimenter ID of this extension is:
+
+NTR_VENDOR_ID = 0x1540
+
+
+4. Group Experimenter Property
+==
+
+The following group property experimenter type defined by this extension.
+
+enum ntr_group_mod_subtype {
+ NTRT_SELECTION_METHOD = 1,
+};
+
+
+Modifications to the group table from the controller may be done with a
+OFPT_GROUP_MOD message described (draft) Open Flow 1.5. Group Entry
+Message. Of relevance here is that (draft) Open Flow 1.5 group messages
+have properties.
+
+This proposal is defined in terms of an implementation of struct
+ofp_group_prop_experimenter which is described in (draft) Open Flow 1.5.
+The implementation is:
+
+struct ntr_group_prop_selection_method {
+ovs_be16 type; /* OFPGPT_EXPERIMENTER. */
+ovs_be16 length;/* Length in bytes of this property. */
+ovs_be32 experimenter; /* NTR_VENDOR_ID. */
+ovs_be32 exp_type; /* NTRT_SELECTION_METHOD. */
+ovs_be32 pad;
+char selection_method[NTR_MAX_SELECTION_METHOD_LEN];
+/* Null-terminated */
+ovs_be64 selection_method_param; /* Non-Field parameter for
+ * bucket selection. */
+
+/* Followed by:
+ * - Exactly (length - 40) (possibly 0) bytes containing OXM TLVs, then
+ * - Exactly ((length + 7)/8*8 - length) (between 0 and 7) bytes of
+ * all-zero bytes
+ * In summary, ntr_group_prop_selection_method is padded as needed,
+ * to make its overall size a multiple of 8, to preserve alignment
+ * in structures using it.
+ */
+/* uint8_t field_array[0]; */ /* Zero or more fields encoded as
+ * OXM TLVs where the has_mask bit must
+ * be zero and the value it specifies is
+ * a mask to apply to packet fields and
+ * then input them to the selection
+ * method of a select group. */
+/* uint8_t pad2[0]; */
+};
+OFP_ASSERT(sizeof(struct ntr_group_mod) == 40);
+
+
+This property may only be used with group mod messages whose:
+* command is OFPGC_ADD or OFPGC_MODIFY; and
+* type is OFPGT_SELECT
+
+
+The type field is the OFPGPT_EXPERIMENTER which is
+defined in EXT-350 as 0x.
+
+
+The experimenter field is the Experimenter ID (see 3).
+
+
+The exp_type field is NTRT_SELECTION_METHOD.
+
+
+The group selection_method is a null-terminated string which if non-zero
+length specifies a selection method known to an underlying layer of the
+switch. The value of NTR_MAX_SELECTION_METHOD_LEN is 16.
+
+The group selection_method may be zero-length t
[ovs-dev] [PATCH v4 3/7] Support decoding of NTR selection method
This is in preparation for supporting group mod and desc reply
messages with an NTR selection method group experimenter property.
Currently decoding always fails as it only allows properties for known
selection methods and no selection methods are known yet. A subsequent
patch will propose a hash selection method.
NTR selection method
Signed-off-by: Simon Horman
---
v4
* Call nx_pull_entry__() rather than nx_pull_match_entry() in
oxm_pull_field_array() to bypass checking that 'value' is a valid value
as it wants 'value' to be a valid mask.
* Pass a NULL 'mask' parameter to nx_pull_entry__() to allow it to
check that no mask is present.
* Include props field of type struct ofputil_group_props in
struct ofgroup rather than adding each field of the former to the latter.
v3
* Add check to only permit known selection methods: currently none
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use list of struct field_array of TLVs rather than OF1.1 match
for fields field of NTR selection method property
---
lib/meta-flow.c| 9 ++
lib/meta-flow.h| 10 ++
lib/nx-match.c | 46 +
lib/nx-match.h | 2 +
lib/ofp-util.c | 249 +++--
lib/ofp-util.h | 15 +++
ofproto/ofproto-provider.h | 2 +
ofproto/ofproto.c | 3 +
8 files changed, 328 insertions(+), 8 deletions(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 8058e07..54e7f58 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -2278,3 +2278,12 @@ mf_format_subvalue(const union mf_subvalue *subvalue,
struct ds *s)
}
ds_put_char(s, '0');
}
+
+void
+field_array_set(enum mf_field_id id, const union mf_value *value,
+struct field_array *fa)
+{
+ovs_assert(id < MFF_N_IDS);
+bitmap_set1(fa->used.bm, id);
+fa->value[id] = *value;
+}
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 0e09036..ba87aff 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1574,6 +1574,12 @@ union mf_subvalue {
};
BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue));
+/* An array of fields with values */
+struct field_array {
+struct mf_bitmap used;
+union mf_value value[MFF_N_IDS];
+};
+
/* Finding mf_fields. */
const struct mf_field *mf_from_name(const char *name);
@@ -1652,4 +1658,8 @@ void mf_format(const struct mf_field *,
struct ds *);
void mf_format_subvalue(const union mf_subvalue *subvalue, struct ds *s);
+/* Field Arrays. */
+void field_array_set(enum mf_field_id id, const union mf_value *,
+ struct field_array *);
+
#endif /* meta-flow.h */
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 721fce5..e27e50f 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -589,6 +589,52 @@ oxm_pull_match_loose(struct ofpbuf *b, struct match *match)
{
return oxm_pull_match__(b, false, match);
}
+
+/* Verify an array of OXM TLVs treating value of each TLV as a mask,
+ * disallowing masks in each TLV and ignoring pre-requisites. */
+enum ofperr
+oxm_pull_field_array(const void *fields_data, size_t fields_len,
+ struct field_array *fa)
+{
+struct ofpbuf b;
+
+ofpbuf_use_const(&b, fields_data, fields_len);
+while (b.size) {
+const uint8_t *pos = b.data;
+const struct mf_field *field;
+union mf_value value;
+enum ofperr error;
+uint64_t header;
+
+error = nx_pull_entry__(&b, false, &header, &field, &value, NULL);
+if (error) {
+VLOG_DBG_RL(&rl, "error pulling field array field");
+return error;
+} else if (!field) {
+VLOG_DBG_RL(&rl, "unknown field array field");
+error = OFPERR_OFPBMC_BAD_FIELD;
+} else if (bitmap_is_set(fa->used.bm, field->id)) {
+VLOG_DBG_RL(&rl, "duplicate field array field '%s'", field->name);
+error = OFPERR_OFPBMC_DUP_FIELD;
+} else if (!mf_is_mask_valid(field, &value)) {
+VLOG_DBG_RL(&rl, "bad mask in field array field '%s'",
field->name);
+return OFPERR_OFPBMC_BAD_MASK;
+} else {
+field_array_set(field->id, &value, fa);
+}
+
+if (error) {
+const uint8_t *start = fields_data;
+
+VLOG_DBG_RL(&rl, "error parsing OXM at offset %"PRIdPTR" "
+"within field array (%s)", pos - start,
+ofperr_to_string(error));
+return error;
+}
+}
+
+return 0;
+}
/* nx_put_match() and helpers.
*
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 9cb6461..0992536 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -55,6 +55,8 @@ enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int
match_len,
ovs_be64 *cookie_mask);
enum ofperr oxm_pull_match(struct ofpbuf *,
[ovs-dev] [PATCH v4 4/7] Support encoding of NTR selection method
Include NTR selection method experimenter group property in
in group mod request and group desc reply.
NTR selection method
Signed-off-by: Simon Horman
---
v4
* Simplify append_group_desc() by copying properties from struct ofpgroup
which now has a properties field.
* Do not add fields parameter to ofputil_append_group_desc_reply(),
it is not needed as since v3 fields is now an array which
can be accessed via the gp parameter.
* oxm_format_field_array() can never fail so use void as its return type.
* Do not make spurious changes to ofp_print_group()
* Omit ofp-print tests. They require a selection method to be recognised
but none are implemented yet. The tests are thus moved to a subsequent
patch that adds a hash selection method.
v3
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use list of struct field_array of TLVs rather than OF1.1 match
for fields field of NTR selection method property
---
lib/nx-match.c| 78 +++
lib/nx-match.h| 3 +++
lib/ofp-print.c | 32 ++-
lib/ofp-util.c| 31 ++
ofproto/ofproto.c | 2 ++
5 files changed, 140 insertions(+), 6 deletions(-)
diff --git a/lib/nx-match.c b/lib/nx-match.c
index e27e50f..4b72460 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1067,6 +1067,84 @@ oxm_put_match(struct ofpbuf *b, const struct match
*match,
return match_len;
}
+/* Appends to 'b' the nx_match format that expresses the tlv corresponding
+ * to 'id'. If mask is not all-ones then it is also formated as the value
+ * of the tlv. */
+static void
+nx_format_mask_tlv(struct ds *ds, enum mf_field_id id,
+ const union mf_value *mask)
+{
+const struct mf_field *mf = mf_from_id(id);
+
+ds_put_format(ds, "%s", mf->name);
+
+if (!is_all_ones(mask, mf->n_bytes)) {
+ds_put_char(ds, '=');
+mf_format(mf, mask, NULL, ds);
+}
+
+ds_put_char(ds, ',');
+}
+
+/* Appends a string representation of 'fa_' to 'ds'.
+ * The TLVS value of 'fa_' is treated as a mask and
+ * only the name of fields is formated if it is all ones. */
+void
+oxm_format_field_array(struct ds *ds, const struct field_array *fa)
+{
+size_t start_len = ds->length;
+int i;
+
+for (i = 0; i < MFF_N_IDS; i++) {
+if (bitmap_is_set(fa->used.bm, i)) {
+nx_format_mask_tlv(ds, i, &fa->value[i]);
+}
+}
+
+if (ds->length > start_len) {
+ds_chomp(ds, ',');
+}
+}
+
+/* Appends to 'b' a series of OXM TLVs corresponding to the series
+ * of enum mf_field_id and value tuples in 'fa_'.
+ *
+ * OXM differs slightly among versions of OpenFlow. Specify the OpenFlow
+ * version in use as 'version'.
+ *
+ * This function can cause 'b''s data to be reallocated.
+ *
+ * Returns the number of bytes appended to 'b'. May return zero. */
+int
+oxm_put_field_array(struct ofpbuf *b, const struct field_array *fa,
+enum ofp_version version)
+{
+size_t start_len = b->size;
+int i;
+
+/* Field arrays are only used with the group selection method
+ * property and group properties are only available in OpenFlow * 1.5+.
+ * So the following assertion should never fail.
+ *
+ * If support for older OpenFlow versions is desired then some care
+ * will need to be taken of different TLVs that handle the same
+ * flow fields. In particular:
+ * - VLAN_TCI, VLAN_VID and MFF_VLAN_PCP
+ * - IP_DSCP_MASK and DSCP_SHIFTED
+ * - REGS and XREGS
+ */
+ovs_assert(version >= OFP15_VERSION);
+
+for (i = 0; i < MFF_N_IDS; i++) {
+if (bitmap_is_set(fa->used.bm, i)) {
+nxm_put_unmasked(b, i, version, &fa->value[i],
+ mf_from_id(i)->n_bytes);
+}
+}
+
+return b->size - start_len;
+}
+
static void
nx_put_header__(struct ofpbuf *b, uint64_t header, bool masked)
{
diff --git a/lib/nx-match.h b/lib/nx-match.h
index 0992536..fe0b68c 100644
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -60,6 +60,9 @@ enum ofperr oxm_pull_field_array(const void *, size_t
fields_len,
int nx_put_match(struct ofpbuf *, const struct match *,
ovs_be64 cookie, ovs_be64 cookie_mask);
int oxm_put_match(struct ofpbuf *, const struct match *, enum ofp_version);
+void oxm_format_field_array(struct ds *, const struct field_array *);
+int oxm_put_field_array(struct ofpbuf *, const struct field_array *,
+enum ofp_version version);
/* Decoding and encoding OXM/NXM headers (just a field ID) or entries (a field
* ID followed by a value and possibly a mask). */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index b7c9a26..cec074f 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2156,8 +2156,8 @@ ofp_print_bucket_id(struct ds *s, const char *label,
uint32_t bucket_id,
static void
ofp_print_group(struct ds *s,
[ovs-dev] [PATCH v4 5/7] Support translation of NTR selection method
Only the default existing behaviour is translated.
All other methods are rejected for now.
NTR selection method
Signed-off-by: Simon Horman
---
v4
* Update group_dpif_get_selection_method() to reflect
that struct ofgroup now has a props field.
* Do not needlessly include openflow/netronome-ext.h in ofproto-provider.h
* Consider an unknown selection method a bug as parsing of
groups should guard against that.
v3
* No change
v2
* Use array of TLVs rather than OF1.1 match for fields field of
NTR selection method property
---
ofproto/ofproto-dpif-xlate.c | 15 ++-
ofproto/ofproto-dpif.c | 6 ++
ofproto/ofproto-dpif.h | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0e28c77..75d3eed 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3092,7 +3092,7 @@ xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif
*group)
}
static void
-xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+xlate_default_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
struct flow_wildcards *wc = &ctx->xout->wc;
struct ofputil_bucket *bucket;
@@ -3117,6 +3117,19 @@ xlate_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
}
static void
+xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+const char *selection_method = group_dpif_get_selection_method(group);
+
+if (selection_method[0] == '\0') {
+xlate_default_select_group(ctx, group);
+} else {
+/* Parsing of groups should ensure this never happens */
+OVS_NOT_REACHED();
+}
+}
+
+static void
xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
{
ctx->in_group = true;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 38ad6e2..43a21c2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4241,6 +4241,12 @@ group_dpif_get_type(const struct group_dpif *group)
{
return group->up.type;
}
+
+const char *
+group_dpif_get_selection_method(const struct group_dpif *group)
+{
+return group->up.props.selection_method;
+}
/* Sends 'packet' out 'ofport'.
* May modify 'packet'.
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index e2359cd..fd099a2 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -137,6 +137,7 @@ bool group_dpif_lookup(struct ofproto_dpif *ofproto,
uint32_t group_id,
void group_dpif_get_buckets(const struct group_dpif *group,
const struct ovs_list **buckets);
enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
+const char *group_dpif_get_selection_method(const struct group_dpif *group);
bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 6/7] Support NTR selection method in ovs-ofctl group commands
NTR selection method
Signed-off-by: Simon Horman
---
v4
* Use mf_parse_value() instead of mf_parse() in parse_select_group_field()
as there is no mask to parse.
* Document that new features are only supported in OVS2.4+ with *OF1.5+
* '\0' is the null character. '0' is not!
* Use bitmap of field array rather than a local bitmap to
track duplicate fields in parse_select_group_field()
v3
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use list of struct field_array of TLVs rather than OF1.1 match
for fields field of NTR selection method property
---
lib/ofp-parse.c | 94
tests/ofproto.at | 4 +--
utilities/ovs-ofctl.8.in | 34 ++
3 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 8d05681..8fce546 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1158,6 +1158,67 @@ parse_bucket_str(struct ofputil_bucket *bucket, char
*str_,
}
static char * OVS_WARN_UNUSED_RESULT
+parse_select_group_field(char *s, struct field_array *fa,
+ enum ofputil_protocol *usable_protocols)
+{
+char *save_ptr = NULL;
+char *name;
+
+for (name = strtok_r(s, "=, \t\r\n", &save_ptr); name;
+ name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
+const struct mf_field *mf = mf_from_name(name);
+
+if (mf) {
+char *error;
+const char *value_str;
+union mf_value value;
+
+if (bitmap_is_set(fa->used.bm, mf->id)) {
+return xasprintf("%s: duplicate field", name);
+}
+
+value_str = strtok_r(NULL, ", \t\r\n", &save_ptr);
+if (value_str) {
+error = mf_parse_value(mf, value_str, &value);
+if (error) {
+return error;
+}
+
+/* The mask cannot be all-zeros */
+if (is_all_zeros(&value, mf->n_bytes)) {
+return xasprintf("%s: values are wildcards here "
+ "and must not be all-zeros", s);
+}
+
+/* The values parsed are masks for fields used
+ * by the selection method */
+if (!mf_is_mask_valid(mf, &value)) {
+return xasprintf("%s: invalid mask for field %s",
+ value_str, mf->name);
+}
+} else {
+memset(&value, 0xff, mf->n_bytes);
+}
+
+field_array_set(mf->id, &value, fa);
+
+if (is_all_ones(&value, mf->n_bytes)) {
+*usable_protocols &= mf->usable_protocols_exact;
+} else if (mf->usable_protocols_bitwise ==
mf->usable_protocols_cidr
+ || ip_is_cidr(value.be32)) {
+*usable_protocols &= mf->usable_protocols_cidr;
+} else {
+*usable_protocols &= mf->usable_protocols_bitwise;
+}
+} else {
+return xasprintf("%s: unknown field %s", s, name);
+}
+}
+
+return NULL;
+}
+
+static char * OVS_WARN_UNUSED_RESULT
parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, uint16_t command,
char *string,
enum ofputil_protocol *usable_protocols)
@@ -1327,6 +1388,39 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm,
uint16_t command,
} else if (!strcmp(name, "bucket")) {
error = xstrdup("bucket is not needed");
goto out;
+} else if (!strcmp(name, "selection_method")) {
+if (!(fields & F_GROUP_TYPE)) {
+error = xstrdup("selection method is not needed");
+goto out;
+}
+if (strlen(value) >= NTR_MAX_SELECTION_METHOD_LEN) {
+error = xasprintf("selection method is longer than %u"
+ " bytes long",
+ NTR_MAX_SELECTION_METHOD_LEN - 1);
+goto out;
+}
+memset(gm->props.selection_method, '\0',
+ NTR_MAX_SELECTION_METHOD_LEN);
+strcpy(gm->props.selection_method, value);
+*usable_protocols &= OFPUTIL_P_OF15_UP;
+} else if (!strcmp(name, "selection_method_param")) {
+if (!(fields & F_GROUP_TYPE)) {
+error = xstrdup("selection method param is not needed");
+goto out;
+}
+error = str_to_u64(value, &gm->props.selection_method_param);
+*usable_protocols &= OFPUTIL_P_OF15_UP;
+} else if (!strcmp(name, "fields")) {
+if (!(fields & F_GROUP_TYPE)) {
+error = xstrdup("fields are not needed");
+goto out;
+}
+error = parse_select_group_f
[ovs-dev] [PATCH v4 7/7] Implement hash fields select group
This is intended as a usable demonstration of how
the NTR selection method extension might may be used.
NTR selection method
Signed-off-by: Simon Horman
---
v4
* Update group_dpif_get_fields and group_dpif_get_selection_method_param to
reflect that struct ofgroup now has a props field.
* Use all of selection_method_param rather than only lower 32 bits.
* Use hash_bytes() instead of jhash_bytes().
* Hash flow fields as needed rather than entire flow
* Add ofp-print tests now that a selection method is defined:
they were previously part of an earlier patch
* Only hash fields which are present with their pre-requisites in the flow
* Hash pre-requisites of fields
* Expand ofproto-dpif test to check that flows are not distributed
if they only vary over a field that is not hashed
v3
* Update new check in parse_group_prop_nmx_selection_method() to
allow decoding hash selection method
* Use fixed array for fields_array rather than constructing a list
* Use NTR instead of NMX as Netronome extension prefix
v2
* Use list of struct field_array of TLVs rather than OF1.1 match
for fields field of NTR selection method property
---
lib/meta-flow.c | 35 +++
lib/meta-flow.h | 2 ++
lib/ofp-util.c | 13 -
ofproto/ofproto-dpif-xlate.c | 67
ofproto/ofproto-dpif.c | 12
ofproto/ofproto-dpif.h | 2 ++
tests/ofp-print.at | 16 ---
tests/ofproto-dpif.at| 33 ++
8 files changed, 168 insertions(+), 12 deletions(-)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 54e7f58..124b525 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -352,6 +352,41 @@ mf_mask_field_and_prereqs(const struct mf_field *mf,
struct flow *mask)
}
}
+/* Set bits of 'bm' corresponding to the field 'mf' and it's prerequisities. */
+void
+mf_bitmap_set_field_and_prereqs(const struct mf_field *mf, struct mf_bitmap
*bm)
+{
+bitmap_set1(bm->bm, mf->id);
+
+switch (mf->prereqs) {
+case MFP_ND:
+case MFP_ND_SOLICIT:
+case MFP_ND_ADVERT:
+bitmap_set1(bm->bm, MFF_TCP_SRC);
+bitmap_set1(bm->bm, MFF_TCP_DST);
+/* Fall through. */
+case MFP_TCP:
+case MFP_UDP:
+case MFP_SCTP:
+case MFP_ICMPV4:
+case MFP_ICMPV6:
+/* nw_frag always unwildcarded. */
+bitmap_set1(bm->bm, MFF_IP_PROTO);
+/* Fall through. */
+case MFP_ARP:
+case MFP_IPV4:
+case MFP_IPV6:
+case MFP_MPLS:
+case MFP_IP_ANY:
+bitmap_set1(bm->bm, MFF_ETH_TYPE);
+break;
+case MFP_VLAN_VID:
+bitmap_set1(bm->bm, MFF_VLAN_TCI);
+break;
+case MFP_NONE:
+break;
+}
+}
/* Returns true if 'value' may be a valid value *as part of a masked match*,
* false otherwise.
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index ba87aff..265f066 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1601,6 +1601,8 @@ void mf_get_mask(const struct mf_field *, const struct
flow_wildcards *,
/* Prerequisites. */
bool mf_are_prereqs_ok(const struct mf_field *, const struct flow *);
void mf_mask_field_and_prereqs(const struct mf_field *, struct flow *mask);
+void mf_bitmap_set_field_and_prereqs(const struct mf_field *mf, struct
+ mf_bitmap *bm);
static inline bool
mf_is_l3_or_higher(const struct mf_field *mf)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7b68564..a0da289 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7819,14 +7819,11 @@ parse_group_prop_ntr_selection_method(struct ofpbuf
*payload,
return OFPERR_OFPBPC_BAD_VALUE;
}
-/* Only allow selection method property if the selection_method field
- * matches a suported method. As no methods are currently supported
- * this check is a no-op that always fails. As selection methods are
- * added they should be checked against the selection_method field
- * here. */
-log_property(false, "ntr selection method '%s' is not supported",
- prop->selection_method);
-return OFPERR_OFPBPC_BAD_VALUE;
+if (strcmp("hash", prop->selection_method)) {
+log_property(false, "ntr selection method '%s' is not supported",
+ prop->selection_method);
+return OFPERR_OFPBPC_BAD_VALUE;
+}
strcpy(gp->selection_method, prop->selection_method);
gp->selection_method_param = ntohll(prop->selection_method_param);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 75d3eed..a7cfe06 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3117,12 +3117,79 @@ xlate_default_select_group(struct xlate_ctx *ctx,
struct group_dpif *group)
}
static void
+xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+struct mf_bitmap hash_fields = MF_BITMAP_INITIALIZER;
+struct flow_wildcards *wc = &ctx->xou
[ovs-dev] [PATCH 1/4] ovs-vsctl: Create AutoAttach records on demand, for backward compatibility.
The AutoAttach table is new in OVS 2.3.90, but ovs-vsctl was creating a
record in the table unconditionally whenever it created a new bridge.
This caused a gratuitous incompatibility with older databases, which can
be a problem during upgrade. This commit switches to creating the
AutoAttaach record for a bridge lazily, only when the first mapping is
created for the bridge.
VMware-BZ: #1413562
CC: Dennis Flynn
Reported-by: Alex Wang
Signed-off-by: Ben Pfaff
---
utilities/ovs-vsctl.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 1abefb4..fa73b00 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1807,7 +1807,6 @@ cmd_add_br(struct vsctl_context *ctx)
if (!parent_name) {
struct ovsrec_port *port;
-struct ovsrec_autoattach *aa;
struct ovsrec_bridge *br;
iface = ovsrec_interface_insert(ctx->txn);
@@ -1818,12 +1817,9 @@ cmd_add_br(struct vsctl_context *ctx)
ovsrec_port_set_name(port, br_name);
ovsrec_port_set_interfaces(port, &iface, 1);
-aa = ovsrec_autoattach_insert(ctx->txn);
-
br = ovsrec_bridge_insert(ctx->txn);
ovsrec_bridge_set_name(br, br_name);
ovsrec_bridge_set_ports(br, &port, 1);
-ovsrec_bridge_set_auto_attach(br, aa);
ovs_insert_bridge(ctx->ovs, br);
} else {
@@ -2732,6 +2728,10 @@ cmd_add_aa_mapping(struct vsctl_context *ctx)
}
if (br && br->br_cfg) {
+if (!br->br_cfg->auto_attach) {
+struct ovsrec_autoattach *aa = ovsrec_autoattach_insert(ctx->txn);
+ovsrec_bridge_set_auto_attach(br->br_cfg, aa);
+}
autoattach_insert_mapping(br->br_cfg->auto_attach, isid, vlan);
}
}
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 0/4] Fix upgrade problems related to AutoAttach
The first two commits fix fairly serious upgrade problems specifically related to the new auto-attach feature. The third commit is less critical; it fixes a similar upgrade bug in ovs-vswitchd specifically. The fourth fixes a long-standing bug in ovsdb-server that I've known about for a few weeks. It's not important but I'd like to get it fixed anyway. Ben Pfaff (4): ovs-vsctl: Create AutoAttach records on demand, for backward compatibility. ovs-vsctl: Only monitor AutoAttach columns when useful. ovsdb-idl: Tolerate missing tables and columns. ovsdb-server: Correct malformed error replies to certain JSON-RPC requests. lib/ovsdb-idl.c | 207 +-- lib/ovsdb-parser.c | 16 ++-- lib/ovsdb-parser.h | 5 +- ovsdb/jsonrpc-server.c | 26 +++--- ovsdb/ovsdb-server.1.in | 29 ++- tests/automake.mk| 2 +- tests/idltest2.ovsschema | 85 +++ tests/ovsdb-idl.at | 75 + tests/ovsdb-server.at| 2 +- utilities/ovs-vsctl.c| 13 ++- 10 files changed, 407 insertions(+), 53 deletions(-) create mode 100644 tests/idltest2.ovsschema -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/4] ovs-vsctl: Only monitor AutoAttach columns when useful.
Otherwise this creates a gratuitous incompatibility with older databases,
which can be a problem in upgrade situations.
VMware-BZ: #1413562
Reported-by: Alex Wang
Signed-off-by: Ben Pfaff
---
utilities/ovs-vsctl.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index fa73b00..009a8ca 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1185,7 +1185,6 @@ pre_get_info(struct vsctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &ovsrec_bridge_col_controller);
ovsdb_idl_add_column(ctx->idl, &ovsrec_bridge_col_fail_mode);
ovsdb_idl_add_column(ctx->idl, &ovsrec_bridge_col_ports);
-ovsdb_idl_add_column(ctx->idl, &ovsrec_bridge_col_auto_attach);
ovsdb_idl_add_column(ctx->idl, &ovsrec_port_col_name);
ovsdb_idl_add_column(ctx->idl, &ovsrec_port_col_fake_bridge);
@@ -1194,7 +1193,6 @@ pre_get_info(struct vsctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &ovsrec_interface_col_name);
-ovsdb_idl_add_column(ctx->idl, &ovsrec_autoattach_col_mappings);
ovsdb_idl_add_column(ctx->idl, &ovsrec_interface_col_ofport);
}
@@ -2806,6 +2804,7 @@ pre_aa_mapping(struct vsctl_context *ctx)
{
pre_get_info(ctx);
+ovsdb_idl_add_column(ctx->idl, &ovsrec_bridge_col_auto_attach);
ovsdb_idl_add_column(ctx->idl, &ovsrec_autoattach_col_mappings);
}
@@ -4678,7 +4677,7 @@ static const struct vsctl_command_syntax all_commands[] =
{
cmd_set_ssl, NULL, "--bootstrap", RW},
/* Auto Attach commands. */
-{"add-aa-mapping", 3, 3, "BRIDGE ARG ARG", pre_get_info,
cmd_add_aa_mapping,
+{"add-aa-mapping", 3, 3, "BRIDGE ARG ARG", pre_aa_mapping,
cmd_add_aa_mapping,
NULL, "", RW},
{"del-aa-mapping", 3, 3, "BRIDGE ARG ARG", pre_aa_mapping,
cmd_del_aa_mapping,
NULL, "", RW},
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/4] ovsdb-idl: Tolerate missing tables and columns.
Until now, if ovs-vsctl (or another client of the C ovsdb-idl library) was
compiled against a schema that had a column or table that was not in the
database actually being used (e.g. during an upgrade), and the column or
table was selected for monitoring, then ovsdb-idl would fail to get any
data at all because ovsdb-server would report an error due to a request
about a column or a table it didn't know about.
This commit fixes the problem by making ovsdb-idl retrieve the database
schema from the database server and omit any tables or columns that don't
exist from its monitoring request. This works OK for the kinds of upgrades
that OVSDB otherwise supports gracefully because it will simply make the
missing columns or tables appear empty, which clients of the ovsdb-idl
library already have to tolerate.
VMware-BZ: #1413562
Reported-by: Alex Wang
Signed-off-by: Ben Pfaff
---
lib/ovsdb-idl.c | 207 +--
lib/ovsdb-parser.c | 16 ++--
lib/ovsdb-parser.h | 5 +-
tests/automake.mk| 2 +-
tests/idltest2.ovsschema | 85 +++
tests/ovsdb-idl.at | 75 +
6 files changed, 358 insertions(+), 32 deletions(-)
create mode 100644 tests/idltest2.ovsschema
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9c25dbc..b35e854 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -28,11 +28,15 @@
#include "fatal-signal.h"
#include "json.h"
#include "jsonrpc.h"
+#include "ovsdb/ovsdb.h"
+#include "ovsdb/table.h"
#include "ovsdb-data.h"
#include "ovsdb-error.h"
#include "ovsdb-idl-provider.h"
+#include "ovsdb-parser.h"
#include "poll-loop.h"
#include "shash.h"
+#include "sset.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -71,16 +75,25 @@ struct ovsdb_idl_arc {
struct ovsdb_idl_row *dst; /* Destination row. */
};
+enum ovsdb_idl_state {
+IDL_S_SCHEMA_REQUESTED,
+IDL_S_MONITOR_REQUESTED,
+IDL_S_MONITORING
+};
+
struct ovsdb_idl {
const struct ovsdb_idl_class *class;
struct jsonrpc_session *session;
struct shash table_by_name;
struct ovsdb_idl_table *tables; /* Contains "struct ovsdb_idl_table *"s.*/
-struct json *monitor_request_id;
-unsigned int last_monitor_request_seqno;
unsigned int change_seqno;
bool verify_write_only;
+/* Session state. */
+unsigned int state_seqno;
+enum ovsdb_idl_state state;
+struct json *request_id;
+
/* Database locking. */
char *lock_name;/* Name of lock we need, NULL if none. */
bool has_lock; /* Has db server told us we have the lock? */
@@ -124,7 +137,9 @@ static struct vlog_rate_limit syntax_rl =
VLOG_RATE_LIMIT_INIT(1, 5);
static struct vlog_rate_limit semantic_rl = VLOG_RATE_LIMIT_INIT(1, 5);
static void ovsdb_idl_clear(struct ovsdb_idl *);
-static void ovsdb_idl_send_monitor_request(struct ovsdb_idl *);
+static void ovsdb_idl_send_schema_request(struct ovsdb_idl *);
+static void ovsdb_idl_send_monitor_request(struct ovsdb_idl *,
+ const struct json *schema_json);
static void ovsdb_idl_parse_update(struct ovsdb_idl *, const struct json *);
static struct ovsdb_error *ovsdb_idl_parse_update__(struct ovsdb_idl *,
const struct json *);
@@ -214,7 +229,10 @@ ovsdb_idl_create(const char *remote, const struct
ovsdb_idl_class *class,
hmap_init(&table->rows);
table->idl = idl;
}
-idl->last_monitor_request_seqno = UINT_MAX;
+
+idl->state_seqno = UINT_MAX;
+idl->request_id = NULL;
+
hmap_init(&idl->outstanding_txns);
return idl;
@@ -239,7 +257,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl)
}
shash_destroy(&idl->table_by_name);
free(idl->tables);
-json_destroy(idl->monitor_request_id);
+json_destroy(idl->request_id);
free(idl->lock_name);
json_destroy(idl->lock_request_id);
hmap_destroy(&idl->outstanding_txns);
@@ -298,14 +316,17 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
unsigned int seqno;
seqno = jsonrpc_session_get_seqno(idl->session);
-if (idl->last_monitor_request_seqno != seqno) {
-idl->last_monitor_request_seqno = seqno;
+if (idl->state_seqno != seqno) {
+idl->state_seqno = seqno;
+json_destroy(idl->request_id);
+idl->request_id = NULL;
ovsdb_idl_txn_abort_all(idl);
-ovsdb_idl_send_monitor_request(idl);
+
+ovsdb_idl_send_schema_request(idl);
+idl->state = IDL_S_SCHEMA_REQUESTED;
if (idl->lock_name) {
[ovs-dev] [PATCH 4/4] ovsdb-server: Correct malformed error replies to certain JSON-RPC requests.
I realized that this bug existed only a few weeks ago, but it has been
there since the earliest ovsdb-server code.
Signed-off-by: Ben Pfaff
---
ovsdb/jsonrpc-server.c | 26 ++
ovsdb/ovsdb-server.1.in | 29 -
tests/ovsdb-server.at | 2 +-
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 2ba5ddd..95b4995 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -78,8 +78,9 @@ static void ovsdb_jsonrpc_trigger_complete_done(
struct ovsdb_jsonrpc_session *);
/* Monitors. */
-static struct json *ovsdb_jsonrpc_monitor_create(
-struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
+static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_create(
+struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params,
+const struct json *request_id);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -674,7 +675,7 @@ ovsdb_jsonrpc_lookup_db(const struct ovsdb_jsonrpc_session
*s,
return db;
error:
-*replyp = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+*replyp = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return NULL;
}
@@ -752,7 +753,7 @@ ovsdb_jsonrpc_session_lock(struct ovsdb_jsonrpc_session *s,
return jsonrpc_create_reply(result, request->id);
error:
-reply = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+reply = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return reply;
}
@@ -812,7 +813,7 @@ ovsdb_jsonrpc_session_unlock(struct ovsdb_jsonrpc_session
*s,
return jsonrpc_create_reply(json_object_create(), request->id);
error:
-reply = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+reply = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return reply;
}
@@ -842,9 +843,8 @@ ovsdb_jsonrpc_session_got_request(struct
ovsdb_jsonrpc_session *s,
} else if (!strcmp(request->method, "monitor")) {
struct ovsdb *db = ovsdb_jsonrpc_lookup_db(s, request, &reply);
if (!reply) {
-reply = jsonrpc_create_reply(
-ovsdb_jsonrpc_monitor_create(s, db, request->params),
-request->id);
+reply = ovsdb_jsonrpc_monitor_create(s, db, request->params,
+ request->id);
}
} else if (!strcmp(request->method, "monitor_cancel")) {
reply = ovsdb_jsonrpc_monitor_cancel(s, json_array(request->params),
@@ -1221,9 +1221,10 @@ ovsdb_jsonrpc_parse_monitor_request(struct
ovsdb_jsonrpc_monitor_table *mt,
return NULL;
}
-static struct json *
+static struct jsonrpc_msg *
ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
- struct json *params)
+ struct json *params,
+ const struct json *request_id)
{
struct ovsdb_jsonrpc_monitor *m = NULL;
struct json *monitor_id, *monitor_requests;
@@ -1310,7 +1311,8 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
-return ovsdb_jsonrpc_monitor_get_initial(m);
+return jsonrpc_create_reply(ovsdb_jsonrpc_monitor_get_initial(m),
+request_id);
error:
if (m) {
@@ -1319,7 +1321,7 @@ error:
json = ovsdb_error_to_json(error);
ovsdb_error_destroy(error);
-return json;
+return jsonrpc_create_error(json, request_id);
}
static struct jsonrpc_msg *
diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 0fafc49..e33d718 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -251,7 +251,34 @@ vSwitch 2.4 and later extend to allow the use
of \fB<\fR,
of 0 or 1 integer'' and ``set of 0 or 1 real''. These conditions
evaluate to false when the column is empty, and otherwise as described
in RFC 7047 for integer and real types.
-
+.
+.SH "BUGS"
+.
+In Open vSwitch before version 2.4, when \fBovsdb\-server\fR sent
+JSON-RPC error responses to some requests, it incorrectly formulated
+them with the \fBresult\fR and \fBerror\fR swapped, so that the
+response appeared to indicate success (with a nonsensical result)
+rather than an error. The requests that suffered from this problem
+were:
+.
+.IP \fBtransact\fR
+.IQ \fBget_schema\fR
+Only if the request names a nonexistent database.
+.IP \fBmonitor\fR
+.IQ \fBlock\fR
+.IQ \fBunlock\fR
+In all error cases.
+.
+.PP
+Of these cases, the only error that a well-written application is
+likely to encounter in practice is \fBmonitor\fR of tables or columns
+that do not exist, in an situation where the application has been
+upgraded but the old database schema is s
