[ovs-dev] meet you well

2016-08-03 Thread SUSAN DOUGLAS
I am Mrs.Zeinab Abdul Can i ask you for help
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-rcu: Add new ovsrcu_index type.

2016-08-03 Thread Daniele Di Proietto
Applied to master, thanks!



On 03/08/2016 15:00, "Jarno Rajahalme"  wrote:

>Looks good to me,
>
>Acked-by: Jarno Rajahalme 
>
>> On Aug 2, 2016, at 5:03 PM, Daniele Di Proietto  
>> wrote:
>> 
>> With RCU in Open vSwitch it's very easy to protect objects accessed by
>> a pointer, but sometimes a pointer is not available.
>> 
>> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
>> was used to access a vhost device with RCU semantics.  From DPDK 16.07
>> an integer id (which is an array index) is used to access a vhost
>> device.  Ideally, we want the exact same RCU semantics that we had for
>> the pointer, on the integer (atomicity, memory barriers, behaviour
>> around quiescent states)
>> 
>> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
>> implemented ovsrcu_index_*() functions should be used to access the
>> type.
>> 
>> Even though we say "Do not, in general, declare a typedef for a struct,
>> union, or enum.", I think we're not in the "general" case.
>> 
>> Signed-off-by: Daniele Di Proietto 
>> ---
>> lib/ovs-rcu.h | 84 
>> +++
>> 1 file changed, 84 insertions(+)
>> 
>> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
>> index dc75749..2887bb8 100644
>> --- a/lib/ovs-rcu.h
>> +++ b/lib/ovs-rcu.h
>> @@ -125,6 +125,36 @@
>>  * ovs_mutex_unlock();
>>  * }
>>  *
>> + * In some rare cases an object may not be addressable with a pointer, but 
>> only
>> + * through an array index (e.g. because it's provided by another library).  
>> It
>> + * is still possible to have RCU semantics by using the ovsrcu_index type.
>> + *
>> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
>> + *
>> + * ovsrcu_index port_id;
>> + *
>> + * void tx()
>> + * {
>> + * int id = ovsrcu_index_get(_id);
>> + * if (id == -1) {
>> + * return;
>> + * }
>> + * port_tx(id);
>> + * }
>> + *
>> + * void delete()
>> + * {
>> + * int id;
>> + *
>> + * ovs_mutex_lock();
>> + * id = ovsrcu_index_get_protected(_id);
>> + * ovsrcu_index_set(_id, -1);
>> + * ovs_mutex_unlock();
>> + *
>> + * ovsrcu_synchronize();
>> + * port_delete(id);
>> + * }
>> + *
>>  */
>> 
>> #include "compiler.h"
>> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void *aux), 
>> void *aux);
>>  (void) sizeof(*(ARG)), \
>>  ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
>> 
>> +/* An array index protected by RCU semantics.  This is an easier 
>> alternative to
>> + * an RCU protected pointer to a malloc'd int. */
>> +typedef struct { atomic_int v; } ovsrcu_index;
>> +
>> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order 
>> order)
>> +{
>> +int ret;
>> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
>> +return ret;
>> +}
>> +
>> +/* Returns the index contained in 'i'.  The returned value can be used until
>> + * the next grace period. */
>> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
>> +{
>> +return ovsrcu_index_get__(i, memory_order_consume);
>> +}
>> +
>> +/* Returns the index contained in 'i'.  This is an alternative to
>> + * ovsrcu_index_get() that can be used when there's no possible concurrent
>> + * writer. */
>> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
>> +{
>> +return ovsrcu_index_get__(i, memory_order_relaxed);
>> +}
>> +
>> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
>> +  memory_order order)
>> +{
>> +atomic_store_explicit(>v, value, order);
>> +}
>> +
>> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
>> + * used by readers until the next grace period. */
>> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
>> +{
>> +ovsrcu_index_set__(i, value, memory_order_release);
>> +}
>> +
>> +/* Writes the index 'value' in 'i'.  This is an alternative to
>> + * ovsrcu_index_set() that can be used when there's no possible concurrent
>> + * reader. */
>> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
>> +{
>> +ovsrcu_index_set__(i, value, memory_order_relaxed);
>> +}
>> +
>> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
>> no
>> + * concurrent readers. */
>> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
>> +{
>> +atomic_init(>v, value);
>> +}
>> +
>> /* Quiescent states. */
>> void ovsrcu_quiesce_start(void);
>> void ovsrcu_quiesce_end(void);
>> -- 
>> 2.8.1
>> 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/4] ovsdb: Rename replication related variable names.

2016-08-03 Thread Andy Zhou
Current replication code refers the other ovsdb-sever instance as
a 'remote'. which is overloaded in ovsdb.

Switching to use active/backup instead to make it less confusing.
Active is the server that should be servicing the client, backup
server is the server that boots with the --sync-from option.

Signed-off-by: Andy Zhou 
---
 ovsdb/ovsdb-server.1.in | 18 +++
 ovsdb/ovsdb-server.c| 58 -
 ovsdb/replication.c | 34 ++---
 ovsdb/replication.h |  8 +++
 tests/ovsdb-server.at   | 36 +++---
 5 files changed, 77 insertions(+), 77 deletions(-)

diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 1d932c0..0667419 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -188,20 +188,20 @@ again (with \fBovsdb\-server/add\-db\fR).
 Outputs a list of the currently configured databases added either through
 the command line or through the \fBovsdb\-server/add\-db\fR command.
 .
-.IP "\fBovsdb\-server/set\-remote\-ovsdb\-server \fIserver"
-Sets  the remote \fIserver\fR from which \fBovsdb\-server\fR connects through
-\fBovsdb\-server/connect\-remote\-ovsdb\-server\fR.
+.IP "\fBovsdb\-server/set\-active\-ovsdb\-server \fIserver"
+Sets  the active \fIserver\fR from which \fBovsdb\-server\fR connects through
+\fBovsdb\-server/connect\-active\-ovsdb\-server\fR.
 .
-.IP "\fBovsdb\-server/get\-remote\-ovsdb\-server"
-Gets the remote server from which \fBovsdb\-server\fR is currently 
synchronizing
+.IP "\fBovsdb\-server/get\-active\-ovsdb\-server"
+Gets the active server from which \fBovsdb\-server\fR is currently 
synchronizing
 its databases.
 .
-.IP "\fBovsdb\-server/connect\-remote\-ovsdb\-server"
+.IP "\fBovsdb\-server/connect\-active\-ovsdb\-server"
 Causes \fBovsdb\-server\fR to synchronize its databases with the server
-specified by \fBovsdb\-server/set\-remote\-ovsdb\-server\fR.
+specified by \fBovsdb\-server/set\-active\-ovsdb\-server\fR.
 .
-.IP "\fBovsdb\-server/disconnect\-remote\-ovsdb\-server"
-Causes \fBovsdb\-server\fR to  stop  synchronizing  its  databases with a 
remote server.
+.IP "\fBovsdb\-server/disconnect\-active\-ovsdb\-server"
+Causes \fBovsdb\-server\fR to  stop  synchronizing  its  databases with a 
active server.
 .
 .IP "\fBovsdb\-server/set\-sync\-excluded\-tables 
\fIdb\fB:\fItable\fR[\fB,\fIdb\fB:\fItable\fR]..."
 Sets the \fItable\fR whitin \fIdb\fR that will be excluded from 
synchronization.
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index bb65637..937c36e 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -69,7 +69,7 @@ static char *ca_cert_file;
 static bool bootstrap_ca_cert;
 
 /* Replication configuration. */
-static bool connect_to_remote_server;
+static bool is_backup_server;
 
 static unixctl_cb_func ovsdb_server_exit;
 static unixctl_cb_func ovsdb_server_compact;
@@ -77,10 +77,10 @@ static unixctl_cb_func ovsdb_server_reconnect;
 static unixctl_cb_func ovsdb_server_perf_counters_clear;
 static unixctl_cb_func ovsdb_server_perf_counters_show;
 static unixctl_cb_func ovsdb_server_disable_monitor_cond;
-static unixctl_cb_func ovsdb_server_set_remote_ovsdb_server;
-static unixctl_cb_func ovsdb_server_get_remote_ovsdb_server;
-static unixctl_cb_func ovsdb_server_connect_remote_ovsdb_server;
-static unixctl_cb_func ovsdb_server_disconnect_remote_ovsdb_server;
+static unixctl_cb_func ovsdb_server_set_active_ovsdb_server;
+static unixctl_cb_func ovsdb_server_get_active_ovsdb_server;
+static unixctl_cb_func ovsdb_server_connect_active_ovsdb_server;
+static unixctl_cb_func ovsdb_server_disconnect_active_ovsdb_server;
 static unixctl_cb_func ovsdb_server_set_sync_excluded_tables;
 static unixctl_cb_func ovsdb_server_get_sync_excluded_tables;
 
@@ -161,7 +161,7 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct 
shash *all_dbs,
 report_error_if_changed(reconfigure_ssl(all_dbs), _error);
 ovsdb_jsonrpc_server_run(jsonrpc);
 
-if (connect_to_remote_server) {
+if (is_backup_server) {
  replication_run(all_dbs);
 }
 
@@ -202,7 +202,7 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct 
shash *all_dbs,
 }
 }
 
-disconnect_remote_server();
+disconnect_active_server();
 free(remotes_error);
 }
 
@@ -341,14 +341,14 @@ main(int argc, char *argv[])
 unixctl_command_register("ovsdb-server/perf-counters-clear", "", 0, 0,
  ovsdb_server_perf_counters_clear, NULL);
 
-unixctl_command_register("ovsdb-server/set-remote-ovsdb-server", "", 0, 1,
-  ovsdb_server_set_remote_ovsdb_server, NULL);
-unixctl_command_register("ovsdb-server/get-remote-ovsdb-server", "", 0, 0,
-  ovsdb_server_get_remote_ovsdb_server, NULL);
-unixctl_command_register("ovsdb-server/connect-remote-ovsdb-server", "", 
0, 0,
-  

[ovs-dev] [PATCH 2/4] ovsdb: add replication wait

2016-08-03 Thread Andy Zhou
Pollblock requires the run() function to be paired with a wait()
function. Add one for replication.

Signed-off-by: Andy Zhou 
---
 ovsdb/ovsdb-server.c | 3 +++
 ovsdb/replication.c  | 8 
 ovsdb/replication.h  | 1 +
 3 files changed, 12 insertions(+)

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 937c36e..2577401 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -183,6 +183,9 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct 
shash *all_dbs,
 }
 
 memory_wait();
+if (is_backup_server) {
+replication_wait();
+}
 ovsdb_jsonrpc_server_wait(jsonrpc);
 unixctl_server_wait(unixctl);
 SHASH_FOR_EACH(node, all_dbs) {
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index f49bfe3..19626a5 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -112,6 +112,14 @@ replication_run(struct shash *all_dbs)
 }
 
 void
+replication_wait(void)
+{
+if (rpc) {
+jsonrpc_wait(rpc);
+}
+}
+
+void
 set_active_ovsdb_server(const char *active_server)
 {
 active_ovsdb_server = nullable_xstrdup(active_server);
diff --git a/ovsdb/replication.h b/ovsdb/replication.h
index 32ea806..d80afb2 100644
--- a/ovsdb/replication.h
+++ b/ovsdb/replication.h
@@ -32,6 +32,7 @@ struct db {
 
 void replication_init(void);
 void replication_run(struct shash *dbs);
+void replication_wait(void);
 void set_active_ovsdb_server(const char *remote_server);
 const char *get_active_ovsdb_server(void);
 void set_tables_blacklist(const char *blacklist);
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 4/4] ovsdb: Make OVSDB backup sever read only

2016-08-03 Thread Andy Zhou
When ovsdb-sever is running in the backup state, it would be nice to
make sure there is no un-intended changes to the backup database.

This patch makes the ovsdb server only accepts 'read' transactions as
a backup server. When the server role is changed into an active server,
all existing client connections will be reset. After reconnect, all
clinet transactions will then be accepted.

Signed-off-by: Andy Zhou 
---
 manpages.mk  |  2 ++
 ovsdb/active-backup.man  | 14 ++
 ovsdb/automake.mk|  1 +
 ovsdb/execution.c| 42 +++---
 ovsdb/jsonrpc-server.c   | 44 +++-
 ovsdb/jsonrpc-server.h   |  4 ++--
 ovsdb/ovsdb-server.1.in  |  2 ++
 ovsdb/ovsdb-server.c | 35 ---
 ovsdb/ovsdb-tool.c   |  2 +-
 ovsdb/ovsdb.h|  2 +-
 ovsdb/trigger.c  |  7 +--
 ovsdb/trigger.h  |  4 +++-
 tests/ovsdb-execution.at | 39 +++
 tests/ovsdb-server.at| 34 ++
 tests/test-ovsdb.c   | 22 +++---
 15 files changed, 209 insertions(+), 45 deletions(-)
 create mode 100644 ovsdb/active-backup.man

diff --git a/manpages.mk b/manpages.mk
index fa9e59b..b3f5e27 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -63,6 +63,7 @@ ovsdb/ovsdb-server.1: \
lib/vlog-syn.man \
lib/vlog-unixctl.man \
lib/vlog.man \
+   ovsdb/active-backup.man \
ovsdb/remote-active.man \
ovsdb/remote-passive.man \
ovsdb/replication-syn.man \
@@ -87,6 +88,7 @@ lib/unixctl.man:
 lib/vlog-syn.man:
 lib/vlog-unixctl.man:
 lib/vlog.man:
+ovsdb/active-backup.man:
 ovsdb/remote-active.man:
 ovsdb/remote-passive.man:
 ovsdb/replication-syn.man:
diff --git a/ovsdb/active-backup.man b/ovsdb/active-backup.man
new file mode 100644
index 000..549f799
--- /dev/null
+++ b/ovsdb/active-backup.man
@@ -0,0 +1,14 @@
+\fBovsdb\-server\fR runs either as a backup server, or as an active server.
+When  \fBovsdb\-server\fR is running as a backup server, all transactions that 
can
+modify the database content, including the lock commands are rejected. Active 
server,
+on the other hand, accepts all ovsdb server transactions.
+When \fBovsdb\-server\fR role changes, all existing client connection are 
reset,
+requiring clients to reconnect to the server.
+
+By default, \fBovsdb\-server\fR runs as an active server, execept when
+the \fB\-\-sync\-from=\fIserver\fR command line option is specified.
+During runtime, \fBovsdb\-server\fR role can be switch by using appctl 
commands.
+\fBovsdb-server/connect\-active\-ovsdb\-server\fR switches \fBovsdb\-server\fR 
role into
+a backup server, Conversely, 
\fBovsdb-server/disconnect\-active\-ovsdb\-server\fR
+changes server into an active one.
+
diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index 099ed3c..2c24e36 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -43,6 +43,7 @@ pkgconfig_DATA += \
$(srcdir)/ovsdb/libovsdb.pc
 
 MAN_FRAGMENTS += \
+   ovsdb/active-backup.man \
ovsdb/remote-active.man \
ovsdb/remote-passive.man \
ovsdb/replication.man \
diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index e972ce7..8e8ffdf 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -61,24 +61,25 @@ static ovsdb_operation_executor ovsdb_execute_comment;
 static ovsdb_operation_executor ovsdb_execute_assert;
 
 static ovsdb_operation_executor *
-lookup_executor(const char *name)
+lookup_executor(const char *name, bool *read_only)
 {
 struct ovsdb_operation {
 const char *name;
+bool read_only;
 ovsdb_operation_executor *executor;
 };
 
 static const struct ovsdb_operation operations[] = {
-{ "insert", ovsdb_execute_insert },
-{ "select", ovsdb_execute_select },
-{ "update", ovsdb_execute_update },
-{ "mutate", ovsdb_execute_mutate },
-{ "delete", ovsdb_execute_delete },
-{ "wait", ovsdb_execute_wait },
-{ "commit", ovsdb_execute_commit },
-{ "abort", ovsdb_execute_abort },
-{ "comment", ovsdb_execute_comment },
-{ "assert", ovsdb_execute_assert },
+{ "insert", false, ovsdb_execute_insert },
+{ "select", true, ovsdb_execute_select },
+{ "update", false, ovsdb_execute_update },
+{ "mutate", false, ovsdb_execute_mutate },
+{ "delete", false, ovsdb_execute_delete },
+{ "wait", true, ovsdb_execute_wait },
+{ "commit", false, ovsdb_execute_commit },
+{ "abort", true, ovsdb_execute_abort },
+{ "comment", true, ovsdb_execute_comment },
+{ "assert", true, ovsdb_execute_assert },
 };
 
 size_t i;
@@ -86,6 +87,7 @@ lookup_executor(const char *name)
 for (i = 0; i < ARRAY_SIZE(operations); i++) {
 const struct ovsdb_operation *c = [i];
 if (!strcmp(c->name, name)) {
+   

[ovs-dev] [PATCH 3/4] ovsdb: Fix bug, set rpc to NULL after freeing.

2016-08-03 Thread Andy Zhou
Found by inspection

Signed-off-by: Andy Zhou 
---
 ovsdb/replication.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 19626a5..de6eec1 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -150,6 +150,7 @@ void
 disconnect_active_server(void)
 {
 jsonrpc_close(rpc);
+rpc = NULL;
 sset_clear(_tables);
 sset_clear(_blacklist);
 }
@@ -157,7 +158,7 @@ disconnect_active_server(void)
 void
 destroy_active_server(void)
 {
-jsonrpc_close(rpc);
+disconnect_active_server();
 sset_destroy(_tables);
 sset_destroy(_blacklist);
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v3 1/3] Windows: Add internal switch port per OVS bridge

2016-08-03 Thread Sairam Venugopal
Nice work putting this together!

I went over the changes and had some comments. Haven¹t tested it yet.

Thanks,
Sairam

On 8/2/16, 12:46 PM, "Alin Serdean" 
wrote:

>This patch updates the following commands in the vswitch:
>ovs-vsctl add-br br-test
>ovs-vsctl del-br br-test
>
>ovs-vsctl add-br br-test:
>This command will now create an internal port on the MSFT virtual
>switch
>  using the WMI interface from Msvm_VirtualEthernetSwitchManagementService
>  leveraging the method AddResourceSettings.
>Before creating the actual port, the switch will be queried to see if
>there
>  is not a port already created (good for restarts when restarting the
>  vswitch daemon). If there is a port defined it will return success and
>log
>  a message.
>After checking if the port already exists the command will also verify
>  if the forwarding extension (windows datapath) is enabled and on a
>single
>  switch. If it is not activated or if it is activated on multiple
>switches
>  it will return an error and a message will be logged.
>After the port was created on the switch, we will disable the adapter
>on
>  the host and rename to the corresponding OVS bridge name for
>consistency.
>The user will enable and set the values he wants after creation.
>
>ovs-vsctl del-br br-test
>This command will remove an internal port on the MSFT virtual switch
>  using the Msvm_VirtualEthernetSwitchManagementService class and
>executing
>  the method RemoveResourceSettings.
>
>Both commands will be blocking until the WMI job is finished, this allows
>us
>to guarantee that the ports are created and their name are set before
>issuing
>a netlink message to the windows datapath.
>
>This patch also includes helpers for normal WMI retrievals and
>initializations.
>Appveyor and documentation has been modified to include the libraries
>needed
>for COM objects.
>
>This patch was tested individually using IMallocSpy and CRT heap checks
>to ensure no new memory leaks are introduced.
>
>Tested on the following OS's:
>Windows 2012 and Windows 2012r2
>
>Signed-off-by: Alin Gabriel Serdean 
>Acked-by: Paul Boca 
>---
>v3: rebase, add acked
>v2: Address comments. Rebase
>---
> appveyor.yml   |2 +-
> lib/automake.mk|4 +-
> lib/dpif-netlink.c |   21 +
> lib/wmi.c  | 1279
>
> lib/wmi.h  |   51 +++
> 5 files changed, 1355 insertions(+), 2 deletions(-)
> create mode 100644 lib/wmi.c
> create mode 100644 lib/wmi.h
>
>diff --git a/appveyor.yml b/appveyor.yml
>index 0fd003b..1061df6 100644
>--- a/appveyor.yml
>+++ b/appveyor.yml
>@@ -41,5 +41,5 @@ build_script:
> - C:\MinGW\msys\1.0\bin\bash -lc "cp
>/c/pthreads-win32/Pre-built.2/dll/x86/*.dll /c/openvswitch/."
> - C:\MinGW\msys\1.0\bin\bash -lc "mv /bin/link.exe /bin/link_copy.exe"
> - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./boot.sh"
>-- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure
>CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi\"
>--with-pthread=C:/pthreads-win32/Pre-built.2
>--with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\""
>+- C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && ./configure
>CC=build-aux/cccl LD=\"`which link`\" LIBS=\"-lws2_32 -liphlpapi
>-lwbemuuid -lole32 -loleaut32\"
>--with-pthread=C:/pthreads-win32/Pre-built.2
>--with-openssl=C:/OpenSSL-Win32 --with-vstudiotarget=\"Debug\""
> - C:\MinGW\msys\1.0\bin\bash -lc "cd /c/openvswitch && make"
>diff --git a/lib/automake.mk b/lib/automake.mk
>index 646306d..7c21f65 100644
>--- a/lib/automake.mk
>+++ b/lib/automake.mk
>@@ -386,7 +386,9 @@ lib_libopenvswitch_la_SOURCES += \
>   lib/netlink-notifier.h \
>   lib/netlink-protocol.h \
>   lib/netlink-socket.c \
>-  lib/netlink-socket.h
>+  lib/netlink-socket.h \
>+  lib/wmi.c \
>+  lib/wmi.h
> endif
> 
> if HAVE_POSIX_AIO
>diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>index a39faa2..c8b0e37 100644
>--- a/lib/dpif-netlink.c
>+++ b/lib/dpif-netlink.c
>@@ -58,6 +58,7 @@
> 
> VLOG_DEFINE_THIS_MODULE(dpif_netlink);
> #ifdef _WIN32
>+#include "wmi.h"
> enum { WINDOWS = 1 };
> #else
> enum { WINDOWS = 0 };
>@@ -849,6 +850,16 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif,
>struct netdev *netdev,
> #endif
> }
> 
>+#ifdef _WIN32
>+if (request.type == OVS_VPORT_TYPE_INTERNAL) {
>+if (!create_wmi_port(name)){
>+VLOG_ERR("Could not create wmi internal port with name:%s",
>name);
>+vport_del_socksp(dpif, socksp);
>+return EINVAL;
>+};
>+}
>+#endif
>+
> tnl_cfg = netdev_get_tunnel_config(netdev);
> if (tnl_cfg && (tnl_cfg->dst_port != 0 || tnl_cfg->exts)) {
> ofpbuf_use_stack(, options_stub, sizeof options_stub);
>@@ -940,6 +951,16 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif,
>odp_port_t port_no)
> vport.cmd = 

[ovs-dev] Mail System Error - Returned Mail

2016-08-03 Thread tobias . lorenz


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Remove incorrect WARN_ONCE().

2016-08-03 Thread Joe Stringer
On 3 August 2016 at 15:25, Jarno Rajahalme  wrote:
> Upstream commit:
> ovs_ct_find_existing() issues a warning if an existing conntrack entry
> classified as IP_CT_NEW is found, with the premise that this should
> not happen.  However, a newly confirmed, non-expected conntrack entry
> remains IP_CT_NEW as long as no reply direction traffic is seen.  This
> has resulted into somewhat confusing kernel log messages.  This patch
> removes this check and warning.
>
> Fixes: 289f2253 ("openvswitch: Find existing conntrack entry after 
> upcall.")
> Suggested-by: Joe Stringer 
> Signed-off-by: Jarno Rajahalme 
> Acked-by: Joe Stringer 
>
> Signed-off-by: Jarno Rajahalme 

Can you get the upstream commit id and add it into the commit message? Thanks.

Acked-by: Joe Stringer 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v3 3/3] Windows: document multiple NIC support setup

2016-08-03 Thread Sairam Venugopal
Hi Alin,

Thanks for the patch! Had some inlined comments about the documentation.

I will run some tests and send out the review comments for other 2 patches.

Thanks,
Sairam


On 8/2/16, 12:51 PM, "Alin Serdean" 
wrote:

>This patch updates the documentation on how to set up OVS with multiple
>NICs.
>
>Also update the documentation to show users how new internal ports are
>created
>
>Signed-off-by: Alin Gabriel Serdean 
>Acked-by: Paul Boca 
>---
>v3: Add acked
>v2: Rebase
>---
>---
> INSTALL.Windows.md | 143
>+
> 1 file changed, 101 insertions(+), 42 deletions(-)
>
>diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md
>index 6b0f5d8..c7d914d 100644
>--- a/INSTALL.Windows.md
>+++ b/INSTALL.Windows.md
>@@ -72,9 +72,9 @@ or from a distribution tar ball.
>   directories, etc. For example,
> 
> % ./configure CC=./build-aux/cccl LD="`which link`" \
>-  LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
>-  --localstatedir="C:/openvswitch/var"
>--sysconfdir="C:/openvswitch/etc" \
>-   --with-pthread="C:/pthread"
>+  LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
>+  --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var"
>\
>+  --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
> 
> By default, the above enables compiler optimization for fast code.
> For default compiler optimization, pass the "--with-debug" configure
>@@ -125,9 +125,10 @@ Note down the directory where OpenSSL is installed
>(e.g.: C:/OpenSSL-Win32).
> For example,
> 
> % ./configure CC=./build-aux/cccl LD="`which link`"  \
>-LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
>---localstatedir="C:/openvswitch/var"
>--sysconfdir="C:/openvswitch/etc" \
>---with-pthread="C:/pthread" --enable-ssl
>--with-openssl="C:/OpenSSL-Win32"
>+LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
>+--prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
>+--sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
>+--enable-ssl --with-openssl="C:/OpenSSL-Win32"
> 
> * Run make for the ported executables.
> 
>@@ -142,10 +143,11 @@ level 'make' will invoke building the kernel
>datapath, if the
> For example,
> 
> % ./configure CC=./build-aux/cccl LD="`which link`" \
>-LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
>---localstatedir="C:/openvswitch/var"
>--sysconfdir="C:/openvswitch/etc" \
>---with-pthread="C:/pthread" --enable-ssl \
>---with-openssl="C:/OpenSSL-Win32" --with-vstudiotarget="type>"
>+LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
>+--prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
>+--sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
>+--enable-ssl --with-openssl="C:/OpenSSL-Win32" \
>+--with-vstudiotarget=""
> 
> Possible values for "" are:
> "Debug" and "Release"
>@@ -186,8 +188,7 @@ to work (covered later).
> 
> The command to create a new switch named 'OVS-Extended-Switch' using a
>physical
> NIC named 'Ethernet 1' is:
>-% New-VMSwitch "OVS-Extended-Switch" -AllowManagementOS $true \
>-   -NetAdapterName "Ethernet 1"
>+% New-VMSwitch "OVS-Extended-Switch" -NetAdapterName "Ethernet 1"
> 
> Note: you can obtain the list of physical NICs on the host using
> 'Get-NetAdapter' command.
>@@ -278,23 +279,20 @@ connected to the Hyper-V switch. I.e. let us
>suppose we created the Hyper-V
> virtual switch on top of the adapter named 'Ethernet0'. In OVS for
>Hyper-V, we
> use that name('Ethernet0') as a special name to refer to that adapter.
> 
>-Note: Currently, we assume that the Hyper-V switch on which OVS
>extension is
>-enabled has a single physical NIC connected to it.
>-
>-Internal port is the virtual adapter created on the Hyper-V switch using
>the
>-'AllowManagementOS' setting.  This has already been setup while creating
>the
>-switch using the instructions above.  In OVS for Hyper-V, we use a the
>name of
>-that specific adapter as a special name to refer to that adapter. By
>default it
>-is created under the following rule "vEthernet ()".
>+Internal ports are the virtual adapters created on the Hyper-V switch
>using the
>+ovs-vsctl add-br  command. By default they are created under the
>+following rule "" and the adapters are disabled. One
>needs to
>+enable them and set the corresponding values to it to make them IP-able.
> 
> As a whole example, if we issue the following in a powershell console:
>-PS C:\package\binaries> Get-NetAdapter | select
>Name,MacAddress,InterfaceDescription
>+PS C:\package\binaries> Get-NetAdapter | select Name,InterfaceDescription
> 
>-Name   MacAddress InterfaceDescription
>-   -- 
>-Ethernet1  00-0C-29-94-05-65  

Re: [ovs-dev] [PATCH 6/6] datapath: compat: gso: tighen checks for compat GSO code.

2016-08-03 Thread pravin shelar
On Wed, Aug 3, 2016 at 12:48 PM, Jesse Gross  wrote:
> On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
>> Few function can be compiled out for non GSO case. This
>> patch make it bit cleaner to understand GSO compat code.
>>
>> Signed-off-by: Pravin B Shelar 
>
> Acked-by: Jesse Gross 

Thanks for all reviews.
I pushed all patches to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

2016-08-03 Thread Ben Pfaff
On Wed, Aug 03, 2016 at 11:14:20PM +0800, Zong Kai LI wrote:
> This patch aims to extend Address_Set to Macros, make it more common to
> accept variable set, not only address. And by that, we can skinny down ACLs,
> if we use Macros to defines port name sets for ACL to use, since lots of ACL
> entries are similar but only "inport" and "outport" fields are different.
> 
> Address_Set improves the performance of OVN control plane. But what more
> important is, its implementation introduced a "$macros -- element_set"
> mechanism, it's good way to do bunch jobs for ACLs, and even lflows.

I'd prefer to defer this feature past the Open vSwitch 2.6 release.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath: Remove incorrect WARN_ONCE().

2016-08-03 Thread Jarno Rajahalme
Upstream commit:
ovs_ct_find_existing() issues a warning if an existing conntrack entry
classified as IP_CT_NEW is found, with the premise that this should
not happen.  However, a newly confirmed, non-expected conntrack entry
remains IP_CT_NEW as long as no reply direction traffic is seen.  This
has resulted into somewhat confusing kernel log messages.  This patch
removes this check and warning.

Fixes: 289f2253 ("openvswitch: Find existing conntrack entry after upcall.")
Suggested-by: Joe Stringer 
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 

Signed-off-by: Jarno Rajahalme 
---
 datapath/conntrack.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 9a79e73..0055e2e 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -477,7 +477,6 @@ ovs_ct_find_existing(struct net *net, const struct 
nf_conntrack_zone *zone,
struct nf_conntrack_l4proto *l4proto;
struct nf_conntrack_tuple tuple;
struct nf_conntrack_tuple_hash *h;
-   enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
unsigned int dataoff;
u8 protonum;
@@ -502,13 +501,8 @@ ovs_ct_find_existing(struct net *net, const struct 
nf_conntrack_zone *zone,
 
ct = nf_ct_tuplehash_to_ctrack(h);
 
-   ctinfo = ovs_ct_get_info(h);
-   if (ctinfo == IP_CT_NEW) {
-   /* This should not happen. */
-   WARN_ONCE(1, "ovs_ct_find_existing: new packet for %p\n", ct);
-   }
skb->nfct = >ct_general;
-   skb->nfctinfo = ctinfo;
+   skb->nfctinfo = ovs_ct_get_info(h);
return ct;
 }
 
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)

2016-08-03 Thread Ben Pfaff
On Wed, Aug 03, 2016 at 10:25:21AM -0400, Eric Garver wrote:
> On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang wrote:
> > On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff  wrote:
> > > On Tue, Jul 12, 2016 at 11:38:54PM +0800, Xiao Liang wrote:
> .. snip ..
> > > I'm concerned about backward compatibility.  Consider some application
> > > built on Open vSwitch using OpenFlow.  Today, it can distinguish
> > > single-tagged and double-tagged packets (that use outer Ethertype
> > > 0x8100), as follows:
> > >
> > > - A single-tagged packet has vlan_tci != 0 and some non-VLAN
> > >   dl_type.
> > >
> > > - A double-tagged packet has vlan_tci != 0 and a VLAN dl_type.
> > >
> > > With this patch, this won't work, because neither kind of packet has a
> > > VLAN dl_type.  Instead, applications need to first match on the outer
> > > VLAN, then pop it off, then match on the inner VLAN.  This difference
> > > could lead to security problems in applications.  An application
> > > might, for example, want to pop an outer VLAN and forward the packet,
> > > but only if there is no inner VLAN.  If it is implemented according to
> > > the previous rules, then it will not notice the inner VLAN.
> > 
> > Maybe some applications are implemented this way, but they are
> > probably wrong. OpenFlow says eth_type is "ethernet type of the
> > OpenFlow packet payload, after VLAN tags", so it is the payload
> > ethtype for a double-tagged packet. It's the same for single-tagged
> > packet: application must explicitly match vlan_tci to decide whether
> > it has VLAN tag.
> > The problem is that there is currently no way to peek inner VLAN
> > without popping the outer one. I heard from Tom that you have plan to
> > support TPID matching. Is it possible to add extension match fields
> > like TPID1, TPID2 to match both TPIDs? This may also provide a way to
> > differentiate 0x8100 and 0x88a8.
> 
> I'm in agreement with Xiao here.

I gave a response directly to Xiao.  Backward incompatibility is one
thing but adding gratuitous security vulnerabilities to existing
applications that have working since day one is not acceptable.

> > > There are probably multiple ways to deal with this problem.  Let me
> > > propose one.  It is somewhat awkward, so there might be a more
> > > graceful way.  Basically the idea is to retain the current view from
> > > an OpenFlow perspective:
> > >
> > > - Packet without tags: vlan_tci == 0, dl_type is non-VLAN
> > > - Packet with 1 tag:   vlan_tci != 0, dl_type is non-VLAN
> > > - Packet with 2+ tags: vlan_tci != 0, dl_type is 2nd VLAN
> > >
> > > So, when a packet with 2 tags pops off the outermost one, dl_type
> > > becomes the inner Ethertype (such as IPv4) and vlan_tci becomes the
> > > single remaining VLAN.
> > >
> > > I think there's another backward compatibility risk.  This patch adds
> > > support for TPID 0x88a8 without adding any way for OpenFlow
> > > applications to distinguish 88a8 from 8100.  This means that
> > > applications that previously handled 0x8100 VLANs will now handle
> > > 0x88a8 VLANs whereas previously they were able to filter these out.  I
> > > don't have a wonderful idea on how to handle this but I think we need
> > > some way.  (The OpenFlow spec is totally unhelpful here.)
> 
> The OF 1.1 spec reads "the outermost VLAN tag", which is ambiguous with
> 802.1ad.
> 
> However, OF 1.5 clarifies to say the "outermost _802.1q_ tag". See pages
> 77 and 205 of the spec. To me this implies that OF does not specify
> matching the 802.1ad tag at all (i.e vlan_tci=.. should match frames
> with outer TPID 0x8100, but not 0x88a8). It does specify pushing/popping
> 802.1ad though.
> 
> I believe if we follow the OF 1.5 definition it removes most (all?)
> backwards compatibility issues raised by Ben. But we also can't match on
> 0x88a8 VLANs. Maybe that can be solved with out-of-spec explicit TPID
> matching like Xiao mentioned above.

The OpenFlow specs aren't written from this kind of language-lawyer
standpoint.  When they say 802.1Q they just mean VLAN.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/6] datapath: compat: Use udp-checksum function for compat case.

2016-08-03 Thread pravin shelar
On Wed, Aug 3, 2016 at 2:08 PM, Jesse Gross  wrote:
> On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
>> udp_set_csum() has bug fix that is not relevant for upstream
>> (commit c77d947191b0).
>> So OVS need to use compat function. This function is also
>> used from UDP xmit path so we have to check USE_UPSTREAM_TUNNEL.
>> Following patch couple this function to USE_UPSTREAM_TUNNEL symbol
>> rather than kernel version.
>> This is not bug, But it helps in code readability.
>>
>> Signed-off-by: Pravin B Shelar 
>
> Since this is basically a subset of the other patch, with the same caveats:
> Acked-by: Jesse Gross 

Sorry for confusion. I forgot to mention it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)

2016-08-03 Thread Ben Pfaff
Thanks for the replies, I have some further responses below.

On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang wrote:
> On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff  wrote:
> > I'm concerned about backward compatibility.  Consider some application
> > built on Open vSwitch using OpenFlow.  Today, it can distinguish
> > single-tagged and double-tagged packets (that use outer Ethertype
> > 0x8100), as follows:
> >
> > - A single-tagged packet has vlan_tci != 0 and some non-VLAN
> >   dl_type.
> >
> > - A double-tagged packet has vlan_tci != 0 and a VLAN dl_type.
> >
> > With this patch, this won't work, because neither kind of packet has a
> > VLAN dl_type.  Instead, applications need to first match on the outer
> > VLAN, then pop it off, then match on the inner VLAN.  This difference
> > could lead to security problems in applications.  An application
> > might, for example, want to pop an outer VLAN and forward the packet,
> > but only if there is no inner VLAN.  If it is implemented according to
> > the previous rules, then it will not notice the inner VLAN.
> 
> Maybe some applications are implemented this way, but they are
> probably wrong. OpenFlow says eth_type is "ethernet type of the
> OpenFlow packet payload, after VLAN tags", so it is the payload
> ethtype for a double-tagged packet. It's the same for single-tagged
> packet: application must explicitly match vlan_tci to decide whether
> it has VLAN tag.

OpenFlow does say that, but it's inconsistent with long-standing Open
vSwitch practice and will cause backward incompatibility and, worse,
security problems.  If we need the official OpenFlow behavior then I
think we'll need to add a feature switch to turn on that behavior.

> > This code uses the term "shift" for what is usually termed "push".  A
> > "shift" can go in either direction.  I'd use "push".
> >
> Yes, "push" looks symmetric. I used "shift" because it leaves room for
> a header rather than push data.

Sometimes we use the longer name "push_uninit" in Open vSwitch to make
it clear that what is being pushed is not initialized, for example see
dp_packet_push_uninit(), nl_msg_push_uninit(), ofpbuf_push_uninit() and
the related ds_put_uninit().

However, when I look at your calls to the "shift" function, it looks
like most of them could easily be written to provide the new header
contents as an argument.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-rcu: Add new ovsrcu_index type.

2016-08-03 Thread Jarno Rajahalme
Looks good to me,

Acked-by: Jarno Rajahalme 

> On Aug 2, 2016, at 5:03 PM, Daniele Di Proietto  
> wrote:
> 
> With RCU in Open vSwitch it's very easy to protect objects accessed by
> a pointer, but sometimes a pointer is not available.
> 
> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
> was used to access a vhost device with RCU semantics.  From DPDK 16.07
> an integer id (which is an array index) is used to access a vhost
> device.  Ideally, we want the exact same RCU semantics that we had for
> the pointer, on the integer (atomicity, memory barriers, behaviour
> around quiescent states)
> 
> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
> implemented ovsrcu_index_*() functions should be used to access the
> type.
> 
> Even though we say "Do not, in general, declare a typedef for a struct,
> union, or enum.", I think we're not in the "general" case.
> 
> Signed-off-by: Daniele Di Proietto 
> ---
> lib/ovs-rcu.h | 84 +++
> 1 file changed, 84 insertions(+)
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..2887bb8 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -125,6 +125,36 @@
>  * ovs_mutex_unlock();
>  * }
>  *
> + * In some rare cases an object may not be addressable with a pointer, but 
> only
> + * through an array index (e.g. because it's provided by another library).  
> It
> + * is still possible to have RCU semantics by using the ovsrcu_index type.
> + *
> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> + *
> + * ovsrcu_index port_id;
> + *
> + * void tx()
> + * {
> + * int id = ovsrcu_index_get(_id);
> + * if (id == -1) {
> + * return;
> + * }
> + * port_tx(id);
> + * }
> + *
> + * void delete()
> + * {
> + * int id;
> + *
> + * ovs_mutex_lock();
> + * id = ovsrcu_index_get_protected(_id);
> + * ovsrcu_index_set(_id, -1);
> + * ovs_mutex_unlock();
> + *
> + * ovsrcu_synchronize();
> + * port_delete(id);
> + * }
> + *
>  */
> 
> #include "compiler.h"
> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void *aux), void 
> *aux);
>  (void) sizeof(*(ARG)), \
>  ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
> 
> +/* An array index protected by RCU semantics.  This is an easier alternative 
> to
> + * an RCU protected pointer to a malloc'd int. */
> +typedef struct { atomic_int v; } ovsrcu_index;
> +
> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order 
> order)
> +{
> +int ret;
> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +return ret;
> +}
> +
> +/* Returns the index contained in 'i'.  The returned value can be used until
> + * the next grace period. */
> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_consume);
> +}
> +
> +/* Returns the index contained in 'i'.  This is an alternative to
> + * ovsrcu_index_get() that can be used when there's no possible concurrent
> + * writer. */
> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
> +  memory_order order)
> +{
> +atomic_store_explicit(>v, value, order);
> +}
> +
> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
> + * used by readers until the next grace period. */
> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_release);
> +}
> +
> +/* Writes the index 'value' in 'i'.  This is an alternative to
> + * ovsrcu_index_set() that can be used when there's no possible concurrent
> + * reader. */
> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_relaxed);
> +}
> +
> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
> no
> + * concurrent readers. */
> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
> +{
> +atomic_init(>v, value);
> +}
> +
> /* Quiescent states. */
> void ovsrcu_quiesce_start(void);
> void ovsrcu_quiesce_end(void);
> -- 
> 2.8.1
> 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/6] datapath: compat: Use checksum offload for outer header.

2016-08-03 Thread pravin shelar
On Wed, Aug 3, 2016 at 2:07 PM, Jesse Gross  wrote:
> On Wed, Aug 3, 2016 at 9:00 AM, pravin shelar  wrote:
>> On Tue, Aug 2, 2016 at 4:55 PM, Jesse Gross  wrote:
>>> On Tue, Aug 2, 2016 at 3:55 PM, pravin shelar  wrote:
 On Tue, Aug 2, 2016 at 3:11 PM, Jesse Gross  wrote:
> On Tue, Aug 2, 2016 at 2:17 PM, Pravin B Shelar  wrote:
>> Signed-off-by: Pravin B Shelar 
>> ---
>>  datapath/linux/compat/include/net/udp.h |  2 +-
>>  datapath/linux/compat/udp.c | 19 ++-
>>  datapath/linux/compat/udp_tunnel.c  | 17 +
>>  3 files changed, 4 insertions(+), 34 deletions(-)
>
> Can you give some more information on this? Is it a bug fix,
> performance improvement, or cleanup?

 How about following commit msg.
 Following patch simplifies UDP-checksum routine by unconditionally
 using checksum offload for non GSO packets. We might get some
 performance improvement due to code simplification.
>>>
>>> This is just the check for whether the device has the
>>> NETIF_F_V[46]_CSUM feature? I wonder whether that is really enough of
>>> a benefit to deviate from upstream. If it is noticeable then it seems
>>> like this should be an upstream patch.
>>>
>>
>> In UDP xmit dst_entry is set after calling this function, so for non
>> goo packets this function always calculate checksum in software.
>
> OK, I understand now. That's definitely a much bigger deal. I also see
> that the upstream version of the function has already been changed, so
> this is only an issue for backports. Can you add this information ot
> the commit message as well? With that change:
> Acked-by: Jesse Gross 
>
> At this point, the main difference between this function and the
> current upstream one is the lack of LCO. Currently, this isn't a
> regression since LCO was introduced in 4.6 and that's where we start
> using upstream tunnels. However, I'm a bit concerned that we might
> need to continue to bump up the kernel version where we use our own
> tunnels in the future and we will lose this optimization. (Though
> maybe things have stabilized upstream at this point, which would be
> nice.)
>
> Independent of the possible future regression, it seems like we could
> backport LCO as an optimization. On kernels with
> USE_UPSTREAM_TUNNEL_GSO and are using outer checksums, we could delay
> and possibly avoid computing the inner checksum.
>
right, I am planing on working on LCO. But first I want to work on
receive side and fix any GRO related issues.

>>> I think the biggest checksum related improvement for compat code would
>>> be to make the checksum computation conditional on skb->encapsulation
>>> being set in rpl_ip_local_out.
>>
>> I am not sure how that will help, In rpl_ip_local_out() GSO packet we
>> can not generate checksum partial packets, udp_set_csum() can not
>> handle it. non GSO packet's checksum is calculated in
>> ovs_iptunnel_handle_offloads() and the skb is not touched in
>> rpl_ip_local_out().
>
> I'm not sure if it was clear but I was talking about outer UDP
> checksums on kernels where we don't have USE_UPSTREAM_TUNNEL_GSO.
>
> In the GSO case, it looks to me like we are already generating
> checksum partial packets. tnl_skb_gso_segment() will first resolve the
> inner checksum and then call ovs_udp_csum_gso(), which calls
> udp_set_csum() to create a new checksum partial outer header. All of
> that seems OK to me.
>
> For the non-GSO case, I agree that we are currently allowing all
> packets to be computed by the stack since fix_segment will always be
> NULL in this case. However, as we talked about in relation to one of
> the previous patches, this might not be safe on older kernels where
> drivers make assumptions about encapsulated packets being VXLAN. If we
> were to change this so that we computed checksum for packets with
> skb->encapsulation set, then we should still allow packets to be
> offloaded without that set.

ok, I will sort out this issue.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Ben Pfaff
On Wed, Aug 03, 2016 at 11:58:52AM -0400, Russell Bryant wrote:
> On Wed, Aug 3, 2016 at 11:39 AM, Kyle Mestery  wrote:
> 
> > On Wed, Aug 3, 2016 at 10:30 AM, Ryan Moats  wrote:
> > >
> > > Russell Bryant  wrote on 08/03/2016 10:11:57 AM:
> > >
> > >> From: Russell Bryant 
> > >> To: Ryan Moats/Omaha/IBM@IBMUS
> > >> Cc: Ben Pfaff , ovs-dev 
> > >> Date: 08/03/2016 10:12 AM
> > >> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > >> we've see scaling the networking-ovn to NB DB connection
> > >>
> > >> On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats  wrote:
> > >>
> > >>
> > >> Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:
> > >>
> > >> > From: Ben Pfaff 
> > >> > To: Ryan Moats/Omaha/IBM@IBMUS
> > >> > Cc: ovs-dev 
> > >> > Date: 08/03/2016 12:28 AM
> > >> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > >> > we've see scaling the networking-ovn to NB DB connection
> > >> >
> > >> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> > >> > > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
> > >> > >
> > >> > > > From: Ben Pfaff 
> > >> > > > To: Ryan Moats/Omaha/IBM@IBMUS
> > >> > > > Cc: ovs-dev 
> > >> > > > Date: 08/02/2016 11:52 PM
> > >> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > >> > > > we've see scaling the networking-ovn to NB DB connection
> > >> > > >
> > >> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
> > >> > > > > "dev"  wrote on 08/02/2016
> > 10:56:07
> > >> PM:
> > >> > > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
> > >> > > > > > > Presumably this means that networking-ovn is calling
> > "verify"
> > >> on
> > >> > > the
> > >> > > > > > > column in question.  Probably, networking-ovn should use the
> > >> > > partial
> > >> > > > > map
> > >> > > > > > > update functionality introduced in commit f199df26e8e28
> > >> "ovsdb-idl:
> > >> > > Add
> > >> > > > > > > partial map updates functionality."  I don't know whether
> > > it's
> > >> in
> > >> > > the
> > >> > > > > > > Python IDL yet.
> > >> > > > > >
> > >> > > > > > Indeed they are and thanks for the pointer to the commit -
> > I'll
> > >> dig
> > >> > > > > > into it tomorrow and see if that code is reflected in the
> > > Python
> > >> > > > > > IDL via that or another commit.  If it is, great.  If not,
> > > there
> > >> > > > > > will likely also be a patch adding it so that we can move
> > > along.
> > >> > > > >
> > >> > > > > Hmm, maybe I'm misreading something, but I don't thing that's
> > > going
> > >> > > > > to work without some additional modifications - the partial map
> > >> commit
> > >> > > > > currently codes for columns that have a particular value type
> > >> defined
> > >> > > > > by the schema.  The problem we are seeing is with the ports and
> > >> acls
> > >> > > > > columns of the logical switch table, which are lists of strong
> > >> > > > > references.  Since they don't have a defined value, the
> > generated
> > >> IDL
> > >> > > > > code doesn't provide hooks for using partial map operations and
> > > we
> > >> > > default
> > >> > > > > back to update/verify with the given above results.
> > >> > > > >
> > >> > > > > Now, I think this an oversight, because I can argue that since
> > >> these
> > >> > > > > are strong references, I should be able to use partial maps to
> > >> update
> > >> > > > > them as keys with a null value.  Does this make sense or am I
> > >> breaking
> > >> > > > > something if I look at going this route?
> > >> > > >
> > >> > > > If they're implemented as partial map operations only, then we
> > > should
> > >> > > > extend them to support partial set operations too--the same
> > >> principles
> > >> > > > apply.
> > >> > >
> > >> > > I'm not sure I parsed this correctly, but I think we are saying the
> > >> same
> > >> > > thing: change the IDL for columns that contain sets of strong
> > >> references
> > >> > > from using update/verify to using mutate for partial set operations
> > > (I
> > >> > > realized after hitting the send button that I should have said
> > > partial
> > >> > > sets instead of partial maps...)
> > >> >
> > >> > Yes, I think we're saying the same thing.
> > >> >
> > >
> > >> Cool, I'll see how far I can get (not sure where my previous message
> > > saying
> > >> this went...)
> > >>
> > >> From the perspective of the branch point, we'd really like to see the
> > >> following make the 2.6 release to allow for easier integration with
> > >> upstream
> > >> OpenStack testing:
> > >>
> > >> - partial sets for the C IDL
> > >> - partial maps for the Python IDL
> > >> - partial sets for the Python IDL
> > >>
> > >> In fact, I'd sorta like to see them all backported to 2.5, but I doubt
> > >> that's
> 

Re: [ovs-dev] [PATCH 2/6] datapath: compat: Use checksum offload for outer header.

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 9:00 AM, pravin shelar  wrote:
> On Tue, Aug 2, 2016 at 4:55 PM, Jesse Gross  wrote:
>> On Tue, Aug 2, 2016 at 3:55 PM, pravin shelar  wrote:
>>> On Tue, Aug 2, 2016 at 3:11 PM, Jesse Gross  wrote:
 On Tue, Aug 2, 2016 at 2:17 PM, Pravin B Shelar  wrote:
> Signed-off-by: Pravin B Shelar 
> ---
>  datapath/linux/compat/include/net/udp.h |  2 +-
>  datapath/linux/compat/udp.c | 19 ++-
>  datapath/linux/compat/udp_tunnel.c  | 17 +
>  3 files changed, 4 insertions(+), 34 deletions(-)

 Can you give some more information on this? Is it a bug fix,
 performance improvement, or cleanup?
>>>
>>> How about following commit msg.
>>> Following patch simplifies UDP-checksum routine by unconditionally
>>> using checksum offload for non GSO packets. We might get some
>>> performance improvement due to code simplification.
>>
>> This is just the check for whether the device has the
>> NETIF_F_V[46]_CSUM feature? I wonder whether that is really enough of
>> a benefit to deviate from upstream. If it is noticeable then it seems
>> like this should be an upstream patch.
>>
>
> In UDP xmit dst_entry is set after calling this function, so for non
> goo packets this function always calculate checksum in software.

OK, I understand now. That's definitely a much bigger deal. I also see
that the upstream version of the function has already been changed, so
this is only an issue for backports. Can you add this information ot
the commit message as well? With that change:
Acked-by: Jesse Gross 

At this point, the main difference between this function and the
current upstream one is the lack of LCO. Currently, this isn't a
regression since LCO was introduced in 4.6 and that's where we start
using upstream tunnels. However, I'm a bit concerned that we might
need to continue to bump up the kernel version where we use our own
tunnels in the future and we will lose this optimization. (Though
maybe things have stabilized upstream at this point, which would be
nice.)

Independent of the possible future regression, it seems like we could
backport LCO as an optimization. On kernels with
USE_UPSTREAM_TUNNEL_GSO and are using outer checksums, we could delay
and possibly avoid computing the inner checksum.

>> I think the biggest checksum related improvement for compat code would
>> be to make the checksum computation conditional on skb->encapsulation
>> being set in rpl_ip_local_out.
>
> I am not sure how that will help, In rpl_ip_local_out() GSO packet we
> can not generate checksum partial packets, udp_set_csum() can not
> handle it. non GSO packet's checksum is calculated in
> ovs_iptunnel_handle_offloads() and the skb is not touched in
> rpl_ip_local_out().

I'm not sure if it was clear but I was talking about outer UDP
checksums on kernels where we don't have USE_UPSTREAM_TUNNEL_GSO.

In the GSO case, it looks to me like we are already generating
checksum partial packets. tnl_skb_gso_segment() will first resolve the
inner checksum and then call ovs_udp_csum_gso(), which calls
udp_set_csum() to create a new checksum partial outer header. All of
that seems OK to me.

For the non-GSO case, I agree that we are currently allowing all
packets to be computed by the stack since fix_segment will always be
NULL in this case. However, as we talked about in relation to one of
the previous patches, this might not be safe on older kernels where
drivers make assumptions about encapsulated packets being VXLAN. If we
were to change this so that we computed checksum for packets with
skb->encapsulation set, then we should still allow packets to be
offloaded without that set.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/6] datapath: compat: Use udp-checksum function for compat case.

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> udp_set_csum() has bug fix that is not relevant for upstream
> (commit c77d947191b0).
> So OVS need to use compat function. This function is also
> used from UDP xmit path so we have to check USE_UPSTREAM_TUNNEL.
> Following patch couple this function to USE_UPSTREAM_TUNNEL symbol
> rather than kernel version.
> This is not bug, But it helps in code readability.
>
> Signed-off-by: Pravin B Shelar 

Since this is basically a subset of the other patch, with the same caveats:
Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Make the PID part of socket path configurable

2016-08-03 Thread Christian Svensson
On Tue, Aug 2, 2016 at 12:42 PM, Christian Svensson 
wrote:

> I just noticed that the first patch only was a URL, retrying with the real
> patch attached.


This is weird.
I'm unable to attach the patch, and 'git send-email' based patches never
show up in the archive, even though CC'd people get them.

I submitted a PR on Github instead:
https://github.com/openvswitch/ovs/pull/147

Please let me know any comments/request you have.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 6/6] datapath: compat: gso: tighen checks for compat GSO code.

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> Few function can be compiled out for non GSO case. This
> patch make it bit cleaner to understand GSO compat code.
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 5/6] datapath: backport: geneve: fix max_mtu setting

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> Upstream commit:
> commit d5d5e8d55732c7c35c354e45e3b0af2795978a57
> Author: Haishuang Yan 
> Date:   Sat Jul 2 15:02:48 2016 +0800
>
> geneve: fix max_mtu setting
>
> For ipv6+udp+geneve encapsulation data, the max_mtu should subtract
> sizeof(ipv6hdr), instead of sizeof(iphdr).
>
> Signed-off-by: Haishuang Yan 
> Signed-off-by: David S. Miller 
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 4/6] datapath: backport: openvswitch: fix conntrack netlink event delivery

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> Upstream commit:
>
> commit d913d3a763a6f66a862a6eafcf6da89a7905832a
> Author: Samuel Gauthier 
> Date:   Tue Jun 28 17:22:26 2016 +0200
>
> openvswitch: fix conntrack netlink event delivery
>
> Only the first and last netlink message for a particular conntrack are
> actually sent. The first message is sent through nf_conntrack_confirm when
> the conntrack is committed. The last one is sent when the conntrack is
> destroyed on timeout. The other conntrack state change messages are not
> advertised.
>
> When the conntrack subsystem is used from netfilter, nf_conntrack_confirm
> is called for each packet, from the postrouting hook, which in turn calls
> nf_ct_deliver_cached_events to send the state change netlink messages.
>
> This commit fixes the problem by calling nf_ct_deliver_cached_events in 
> the
> non-commit case as well.
>
> Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
> CC: Joe Stringer 
> CC: Justin Pettit 
> CC: Andy Zhou 
> CC: Thomas Graf 
> Signed-off-by: Samuel Gauthier 
> Acked-by: Joe Stringer 
> Signed-off-by: David S. Miller 
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 3/6] datapath: compat: vxlan: fix udp-csum typo

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> Signed-off-by: Pravin B Shelar 
> ---
>  datapath/linux/compat/vxlan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/6] datapath: fix size of struct ovs_gso_cb

2016-08-03 Thread Jesse Gross
On Wed, Aug 3, 2016 at 10:08 AM, Pravin B Shelar  wrote:
> struct ovs_gso_cb is stored in skb->cd. avoid going beyond size
> of skb->cb.
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ovsdb: use more descriptive error message

2016-08-03 Thread Ryan Moats
When setting a where clause, if the timeout is set to a value of 0,
the clause is tested once and if it fails, a message of '"wait" timed
out' is returned.  This can be misleading because there wasn't any
real time, so change the message to '"where" clause test failed'.

Signed-off-by: Ryan Moats 
Reported-by: Ryan Moats 
Reported-at: http://openvswitch.org/pipermail/dev/2016-August/077083.html
Fixes: f85f8ebb ("Initial implementation of OVSDB.")
---
 ovsdb/execution.c| 3 ++-
 tests/ovsdb-execution.at | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index e972ce7..af0e655 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -697,7 +697,8 @@ ovsdb_execute_wait(struct ovsdb_execution *x, struct 
ovsdb_parser *parser,
 "\"wait\" timed out after %lld ms",
 x->elapsed_msec);
 } else {
-error = ovsdb_error("timed out", "\"wait\" timed out");
+error = ovsdb_error("timed out",
+"\"where\" clause test failed");
 }
 } else {
 /* ovsdb_execute() will change this, if triggers really are
diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index 94630bd..6e768d3 100644
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -471,7 +471,7 @@ OVSDB_CHECK_EXECUTION([equality wait with extra row],
"rows": [{"name": "zero", "number": 0},
 {"name": "one", "number": 1},
 {"name": "two", "number": 2}]},
-  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"wait\" 
timed out","error":"timed out"}]
+  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"where\" 
clause test failed","error":"timed out"}]
 ]])
 
 OVSDB_CHECK_EXECUTION([equality wait with missing row],
@@ -490,7 +490,7 @@ OVSDB_CHECK_EXECUTION([equality wait with missing row],
"columns": ["name", "number"],
"until": "==",
"rows": [{"name": "one", "number": 1}]},
-  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"wait\" 
timed out","error":"timed out"}]
+  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"where\" 
clause test failed","error":"timed out"}]
 ]])
 
 OVSDB_CHECK_EXECUTION([inequality wait with correct rows],
@@ -510,7 +510,7 @@ OVSDB_CHECK_EXECUTION([inequality wait with correct rows],
"until": "!=",
"rows": [{"name": "zero", "number": 0},
 {"name": "one", "number": 1}]},
-  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"wait\" 
timed out","error":"timed out"}]
+  [[[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"details":"\"where\" 
clause test failed","error":"timed out"}]
 ]])
 
 OVSDB_CHECK_EXECUTION([inequality wait with extra row],
-- 
2.7.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] RFC: ovsdb: Add/use partial set updates.

2016-08-03 Thread Ryan Moats
This patchset mimics the changes introduced in

f199df26 (ovsdb-idl: Add partial map updates functionality.)
010fe7ae (ovsdb-idlc.in: Autogenerate partial map updates functions.)
7251075c (tests: Add test for partial map updates.)

but for columns that store sets of values rather than key-value
pairs.  These columns will now be able to use the OVSDB mutate
operation to transmit deltas on the wire rather than use
verify/update and transmit wait/update operations on the wire.

This patch is marked as an RFC as it is still necessary to
update the python bindings to use this and partial map.
Once those are done, this will be reissued as part one of a
two part patch set.

Signed-off-by: Ryan Moats 
---
 lib/automake.mk  |   2 +
 lib/ovsdb-idl-provider.h |   3 +
 lib/ovsdb-idl.c  | 390 +++
 lib/ovsdb-idl.h  |   6 +
 lib/ovsdb-set-op.c   | 170 +
 lib/ovsdb-set-op.h   |  44 ++
 ovsdb/ovsdb-idlc.in  |  65 +++-
 tests/idltest.ovsschema  |  14 ++
 tests/idltest2.ovsschema |  14 ++
 tests/ovsdb-idl.at   |  36 -
 tests/test-ovsdb.c   | 104 -
 11 files changed, 745 insertions(+), 103 deletions(-)
 create mode 100644 lib/ovsdb-set-op.c
 create mode 100644 lib/ovsdb-set-op.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 97c83e9..30a281f 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -187,6 +187,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/ovsdb-idl.h \
lib/ovsdb-map-op.c \
lib/ovsdb-map-op.h \
+   lib/ovsdb-set-op.c \
+   lib/ovsdb-set-op.h \
lib/ovsdb-condition.h \
lib/ovsdb-condition.c \
lib/ovsdb-parser.c \
diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
index 55ed793..64e8ec3 100644
--- a/lib/ovsdb-idl-provider.h
+++ b/lib/ovsdb-idl-provider.h
@@ -20,6 +20,7 @@
 #include "openvswitch/list.h"
 #include "ovsdb-idl.h"
 #include "ovsdb-map-op.h"
+#include "ovsdb-set-op.h"
 #include "ovsdb-types.h"
 #include "openvswitch/shash.h"
 #include "uuid.h"
@@ -39,6 +40,8 @@ struct ovsdb_idl_row {
 struct hmap_node txn_node;  /* Node in ovsdb_idl_txn's list. */
 unsigned long int *map_op_written; /* Bitmap of columns pending map ops. */
 struct map_op_list **map_op_lists; /* Per-column map operations. */
+unsigned long int *set_op_written; /* Bitmap of columns pending set ops. */
+struct set_op_list **set_op_lists; /* Per-column set operations. */
 
 /* Tracking data */
 unsigned int change_seqno[OVSDB_IDL_CHANGE_MAX];
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d70fb10..dfd0ac9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -184,6 +184,7 @@ static struct ovsdb_idl_row *ovsdb_idl_row_create(struct 
ovsdb_idl_table *,
 static void ovsdb_idl_row_destroy(struct ovsdb_idl_row *);
 static void ovsdb_idl_row_destroy_postprocess(struct ovsdb_idl *);
 static void ovsdb_idl_destroy_all_map_op_lists(struct ovsdb_idl_row *);
+static void ovsdb_idl_destroy_all_set_op_lists(struct ovsdb_idl_row *);
 
 static void ovsdb_idl_row_parse(struct ovsdb_idl_row *);
 static void ovsdb_idl_row_unparse(struct ovsdb_idl_row *);
@@ -200,6 +201,10 @@ static void ovsdb_idl_txn_add_map_op(struct ovsdb_idl_row 
*,
  const struct ovsdb_idl_column *,
  struct ovsdb_datum *,
  enum map_op_type);
+static void ovsdb_idl_txn_add_set_op(struct ovsdb_idl_row *,
+ const struct ovsdb_idl_column *,
+ struct ovsdb_datum *,
+ enum set_op_type);
 
 static void ovsdb_idl_send_lock_request(struct ovsdb_idl *);
 static void ovsdb_idl_send_unlock_request(struct ovsdb_idl *);
@@ -1811,7 +1816,9 @@ ovsdb_idl_row_create(struct ovsdb_idl_table *table, const 
struct uuid *uuid)
 row->uuid = *uuid;
 row->table = table;
 row->map_op_written = NULL;
-row->map_op_lists = NULL;
+row->map_op_written = NULL;
+row->set_op_lists = NULL;
+row->set_op_lists = NULL;
 return row;
 }
 
@@ -1822,6 +1829,7 @@ ovsdb_idl_row_destroy(struct ovsdb_idl_row *row)
 ovsdb_idl_row_clear_old(row);
 hmap_remove(>table->rows, >hmap_node);
 ovsdb_idl_destroy_all_map_op_lists(row);
+ovsdb_idl_destroy_all_set_op_lists(row);
 if (ovsdb_idl_track_is_set(row->table)) {
 row->change_seqno[OVSDB_IDL_CHANGE_DELETE]
 = row->table->change_seqno[OVSDB_IDL_CHANGE_DELETE]
@@ -1856,6 +1864,27 @@ ovsdb_idl_destroy_all_map_op_lists(struct ovsdb_idl_row 
*row)
 }
 
 static void
+ovsdb_idl_destroy_all_set_op_lists(struct ovsdb_idl_row *row)
+{
+if (row->set_op_written) {
+/* Clear Set Operation Lists */
+size_t idx, n_columns;
+const struct ovsdb_idl_column *columns;
+const struct ovsdb_type *type;
+

Re: [ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

2016-08-03 Thread Kyle Mestery
On Wed, Aug 3, 2016 at 11:47 AM, Russell Bryant  wrote:
> On Wed, Aug 3, 2016 at 11:14 AM, Zong Kai LI  wrote:
>
>> This patch aims to extend Address_Set to Macros, make it more common to
>> accept variable set, not only address. And by that, we can skinny down
>> ACLs,
>> if we use Macros to defines port name sets for ACL to use, since lots of
>> ACL
>> entries are similar but only "inport" and "outport" fields are different.
>>
>
> "Set" works better for me than "Macros", personally ... anyone else have an
> opinion?
>

I'm also liking "Set" more than "Macros."

> --
> Russell Bryant
>
>
> --
> Russell Bryant
> ___
> 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] test-netlink-conntrack: Fix sparse warning.

2016-08-03 Thread Daniele Di Proietto





On 03/08/2016 10:54, "Joe Stringer"  wrote:

>On 3 August 2016 at 10:50, Daniele Di Proietto  wrote:
>> On some systems I get a sparse warning when compiling
>> tests/test-netlink-conntrack.c
>>
>> /usr/include/x86_64-linux-gnu/sys/cdefs.h:307:10: warning: preprocessor
>> token __always_inline redefined
>> /usr/include/linux/stddef.h:4:9: this was the original definition
>>
>> The problem seems to be that Linux upstream commit
>> 283d75737837("uapi/linux/stddef.h: Provide __always_inline to userspace
>> headers") introduced __always_inline in stddef.h, but glibc headers
>> didn't like that until e0835a5354ab("Bug 20215: Always undefine
>> __always_inline before defining it.").
>>
>> This commit works around the issue by including a glibc header before a
>> kernel header.
>>
>> Fixes: 2c06d9a927c5("ovstest: Add test-netlink-conntrack command.")
>> Reported-by: Joe Stringer 
>> Signed-off-by: Daniele Di Proietto 
>
>Thanks.
>
>Acked-by: Joe Stringer 

Thanks, applied to master and branch-2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] test-netlink-conntrack: Fix sparse warning.

2016-08-03 Thread Joe Stringer
On 3 August 2016 at 10:50, Daniele Di Proietto  wrote:
> On some systems I get a sparse warning when compiling
> tests/test-netlink-conntrack.c
>
> /usr/include/x86_64-linux-gnu/sys/cdefs.h:307:10: warning: preprocessor
> token __always_inline redefined
> /usr/include/linux/stddef.h:4:9: this was the original definition
>
> The problem seems to be that Linux upstream commit
> 283d75737837("uapi/linux/stddef.h: Provide __always_inline to userspace
> headers") introduced __always_inline in stddef.h, but glibc headers
> didn't like that until e0835a5354ab("Bug 20215: Always undefine
> __always_inline before defining it.").
>
> This commit works around the issue by including a glibc header before a
> kernel header.
>
> Fixes: 2c06d9a927c5("ovstest: Add test-netlink-conntrack command.")
> Reported-by: Joe Stringer 
> Signed-off-by: Daniele Di Proietto 

Thanks.

Acked-by: Joe Stringer 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] test-netlink-conntrack: Fix sparse warning.

2016-08-03 Thread Daniele Di Proietto
On some systems I get a sparse warning when compiling
tests/test-netlink-conntrack.c

/usr/include/x86_64-linux-gnu/sys/cdefs.h:307:10: warning: preprocessor
token __always_inline redefined
/usr/include/linux/stddef.h:4:9: this was the original definition

The problem seems to be that Linux upstream commit
283d75737837("uapi/linux/stddef.h: Provide __always_inline to userspace
headers") introduced __always_inline in stddef.h, but glibc headers
didn't like that until e0835a5354ab("Bug 20215: Always undefine
__always_inline before defining it.").

This commit works around the issue by including a glibc header before a
kernel header.

Fixes: 2c06d9a927c5("ovstest: Add test-netlink-conntrack command.")
Reported-by: Joe Stringer 
Signed-off-by: Daniele Di Proietto 
---
 tests/test-netlink-conntrack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-netlink-conntrack.c b/tests/test-netlink-conntrack.c
index 62bef13..f0d48f7 100644
--- a/tests/test-netlink-conntrack.c
+++ b/tests/test-netlink-conntrack.c
@@ -16,6 +16,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include "ct-dpif.h"
-- 
2.8.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 5/6] datapath: backport: geneve: fix max_mtu setting

2016-08-03 Thread Pravin B Shelar
Upstream commit:
commit d5d5e8d55732c7c35c354e45e3b0af2795978a57
Author: Haishuang Yan 
Date:   Sat Jul 2 15:02:48 2016 +0800

geneve: fix max_mtu setting

For ipv6+udp+geneve encapsulation data, the max_mtu should subtract
sizeof(ipv6hdr), instead of sizeof(iphdr).

Signed-off-by: Haishuang Yan 
Signed-off-by: David S. Miller 

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/geneve.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
index 834b34c..74e0cbc 100644
--- a/datapath/linux/compat/geneve.c
+++ b/datapath/linux/compat/geneve.c
@@ -1153,12 +1153,17 @@ static netdev_tx_t geneve_dev_xmit(struct sk_buff *skb, 
struct net_device *dev)
 
 static int __geneve_change_mtu(struct net_device *dev, int new_mtu, bool 
strict)
 {
+   struct geneve_dev *geneve = netdev_priv(dev);
/* The max_mtu calculation does not take account of GENEVE
 * options, to avoid excluding potentially valid
 * configurations.
 */
-   int max_mtu = IP_MAX_MTU - GENEVE_BASE_HLEN - sizeof(struct iphdr)
-   - dev->hard_header_len;
+   int max_mtu = IP_MAX_MTU - GENEVE_BASE_HLEN - dev->hard_header_len;
+
+   if (geneve->remote.sa.sa_family == AF_INET6)
+   max_mtu -= sizeof(struct ipv6hdr);
+   else
+   max_mtu -= sizeof(struct iphdr);
 
if (new_mtu < 68)
return -EINVAL;
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 4/6] datapath: backport: openvswitch: fix conntrack netlink event delivery

2016-08-03 Thread Pravin B Shelar
Upstream commit:

commit d913d3a763a6f66a862a6eafcf6da89a7905832a
Author: Samuel Gauthier 
Date:   Tue Jun 28 17:22:26 2016 +0200

openvswitch: fix conntrack netlink event delivery

Only the first and last netlink message for a particular conntrack are
actually sent. The first message is sent through nf_conntrack_confirm when
the conntrack is committed. The last one is sent when the conntrack is
destroyed on timeout. The other conntrack state change messages are not
advertised.

When the conntrack subsystem is used from netfilter, nf_conntrack_confirm
is called for each packet, from the postrouting hook, which in turn calls
nf_ct_deliver_cached_events to send the state change netlink messages.

This commit fixes the problem by calling nf_ct_deliver_cached_events in the
non-commit case as well.

Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
CC: Joe Stringer 
CC: Justin Pettit 
CC: Andy Zhou 
CC: Thomas Graf 
Signed-off-by: Samuel Gauthier 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Signed-off-by: Pravin B Shelar 
---
 datapath/conntrack.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 9a79e73..6d45a2c 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -862,8 +862,18 @@ static int ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
 */
state = OVS_CS_F_TRACKED | OVS_CS_F_NEW | OVS_CS_F_RELATED;
__ovs_ct_update_key(key, state, >zone, exp->master);
-   } else
-   return __ovs_ct_lookup(net, key, info, skb);
+   } else {
+   struct nf_conn *ct;
+   int err;
+
+   err = __ovs_ct_lookup(net, key, info, skb);
+   if (err)
+   return err;
+
+   ct = (struct nf_conn *)skb->nfct;
+   if (ct)
+   nf_ct_deliver_cached_events(ct);
+   }
 
return 0;
 }
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 6/6] datapath: compat: gso: tighen checks for compat GSO code.

2016-08-03 Thread Pravin B Shelar
Few function can be compiled out for non GSO case. This
patch make it bit cleaner to understand GSO compat code.

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/gso.h| 2 +-
 datapath/linux/compat/include/net/udp_tunnel.h | 7 ---
 datapath/linux/compat/udp_tunnel.c | 2 ++
 datapath/linux/compat/vxlan.c  | 4 
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
index e93998c..2e9dbb3 100644
--- a/datapath/linux/compat/gso.h
+++ b/datapath/linux/compat/gso.h
@@ -13,8 +13,8 @@ struct ovs_gso_cb {
 #endif
 #ifndef USE_UPSTREAM_TUNNEL_GSO
gso_fix_segment_t fix_segment;
-#endif
bool ipv6;
+#endif
 #ifndef HAVE_INNER_PROTOCOL
__be16  inner_protocol;
 #endif
diff --git a/datapath/linux/compat/include/net/udp_tunnel.h 
b/datapath/linux/compat/include/net/udp_tunnel.h
index 7fccc12..51415e4 100644
--- a/datapath/linux/compat/include/net/udp_tunnel.h
+++ b/datapath/linux/compat/include/net/udp_tunnel.h
@@ -150,21 +150,22 @@ void ovs_udp_csum_gso(struct sk_buff *skb);
 static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb,
 bool udp_csum)
 {
-   int type = 0;
-
void (*fix_segment)(struct sk_buff *);
+   int type = 0;
 
type |= udp_csum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+#ifndef USE_UPSTREAM_TUNNEL_GSO
if (!udp_csum)
fix_segment = ovs_udp_gso;
else
fix_segment = ovs_udp_csum_gso;
-#ifndef USE_UPSTREAM_TUNNEL_GSO
/* This functuin is not used by vxlan lan tunnel. On older
 * udp offload only supports vxlan, therefore fallback to software
 * segmentation.
 */
type = 0;
+#else
+   fix_segment = NULL;
 #endif
 
return ovs_iptunnel_handle_offloads(skb, udp_csum, type, fix_segment);
diff --git a/datapath/linux/compat/udp_tunnel.c 
b/datapath/linux/compat/udp_tunnel.c
index 9cf7286..8cde425 100644
--- a/datapath/linux/compat/udp_tunnel.c
+++ b/datapath/linux/compat/udp_tunnel.c
@@ -266,6 +266,7 @@ int rpl_udp_tunnel6_xmit_skb(struct dst_entry *dst, struct 
sock *sk,
 }
 #endif
 
+#ifndef USE_UPSTREAM_TUNNEL_GSO
 void ovs_udp_gso(struct sk_buff *skb)
 {
int udp_offset = skb_transport_offset(skb);
@@ -300,5 +301,6 @@ void ovs_udp_csum_gso(struct sk_buff *skb)
}
 }
 EXPORT_SYMBOL_GPL(ovs_udp_csum_gso);
+#endif /* USE_UPSTREAM_TUNNEL_GSO */
 
 #endif
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 836d96d..75b811b 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -868,7 +868,11 @@ static int vxlan_build_skb(struct sk_buff *skb, struct 
dst_entry *dst,
return -ENOMEM;
 
type |= udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+#ifndef USE_UPSTREAM_TUNNEL_GSO
fix_segment = !udp_sum ? ovs_udp_gso : ovs_udp_csum_gso;
+#else
+   fix_segment = NULL;
+#endif
err = ovs_iptunnel_handle_offloads(skb, udp_sum, type, fix_segment);
if (err)
goto out_free;
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/6] datapath: fix size of struct ovs_gso_cb

2016-08-03 Thread Pravin B Shelar
struct ovs_gso_cb is stored in skb->cd. avoid going beyond size
of skb->cb.

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/gso.c | 1 +
 datapath/linux/compat/gso.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
index 1f24e74..10412c0 100644
--- a/datapath/linux/compat/gso.c
+++ b/datapath/linux/compat/gso.c
@@ -171,6 +171,7 @@ static struct sk_buff *tnl_skb_gso_segment(struct sk_buff 
*skb,
__be16 proto = skb->protocol;
char cb[sizeof(skb->cb)];
 
+   BUILD_BUG_ON(sizeof(struct ovs_gso_cb) > FIELD_SIZEOF(struct sk_buff, 
cb));
OVS_GSO_CB(skb)->ipv6 = (sa_family == AF_INET6);
/* setup whole inner packet to get protocol. */
__skb_pull(skb, mac_offset);
diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
index fa0a7db..e93998c 100644
--- a/datapath/linux/compat/gso.h
+++ b/datapath/linux/compat/gso.h
@@ -14,6 +14,7 @@ struct ovs_gso_cb {
 #ifndef USE_UPSTREAM_TUNNEL_GSO
gso_fix_segment_t fix_segment;
 #endif
+   bool ipv6;
 #ifndef HAVE_INNER_PROTOCOL
__be16  inner_protocol;
 #endif
@@ -21,7 +22,6 @@ struct ovs_gso_cb {
/* Keep original tunnel info during userspace action execution. */
struct metadata_dst *fill_md_dst;
 #endif
-   bool ipv6;
 };
 #define OVS_GSO_CB(skb) ((struct ovs_gso_cb *)(skb)->cb)
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 3/6] datapath: compat: vxlan: fix udp-csum typo

2016-08-03 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 6d77527..836d96d 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -868,7 +868,7 @@ static int vxlan_build_skb(struct sk_buff *skb, struct 
dst_entry *dst,
return -ENOMEM;
 
type |= udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
-   fix_segment = udp_sum ? ovs_udp_gso : ovs_udp_csum_gso;
+   fix_segment = !udp_sum ? ovs_udp_gso : ovs_udp_csum_gso;
err = ovs_iptunnel_handle_offloads(skb, udp_sum, type, fix_segment);
if (err)
goto out_free;
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/6] datapath: compat: Use udp-checksum function for compat case.

2016-08-03 Thread Pravin B Shelar
udp_set_csum() has bug fix that is not relevant for upstream
(commit c77d947191b0).
So OVS need to use compat function. This function is also
used from UDP xmit path so we have to check USE_UPSTREAM_TUNNEL.
Following patch couple this function to USE_UPSTREAM_TUNNEL symbol
rather than kernel version.
This is not bug, But it helps in code readability.

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/include/net/udp.h | 2 +-
 datapath/linux/compat/udp.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/include/net/udp.h 
b/datapath/linux/compat/include/net/udp.h
index 266e70a..4479992 100644
--- a/datapath/linux/compat/include/net/udp.h
+++ b/datapath/linux/compat/include/net/udp.h
@@ -54,7 +54,7 @@ static inline __sum16 udp_v4_check(int len, __be32 saddr,
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
+#ifndef USE_UPSTREAM_TUNNEL
 #define udp_set_csum rpl_udp_set_csum
 void rpl_udp_set_csum(bool nocheck, struct sk_buff *skb,
  __be32 saddr, __be32 daddr, int len);
diff --git a/datapath/linux/compat/udp.c b/datapath/linux/compat/udp.c
index 4cd22fa..bedb033 100644
--- a/datapath/linux/compat/udp.c
+++ b/datapath/linux/compat/udp.c
@@ -1,6 +1,6 @@
 #include 
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0)
+#ifndef USE_UPSTREAM_TUNNEL
 
 #include 
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

2016-08-03 Thread Russell Bryant
On Wed, Aug 3, 2016 at 11:14 AM, Zong Kai LI  wrote:

> This patch aims to extend Address_Set to Macros, make it more common to
> accept variable set, not only address. And by that, we can skinny down
> ACLs,
> if we use Macros to defines port name sets for ACL to use, since lots of
> ACL
> entries are similar but only "inport" and "outport" fields are different.
>

"Set" works better for me than "Macros", personally ... anyone else have an
opinion?

-- 
Russell Bryant


-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V11 17/17] tests: Skip vlog tests that try to move opened file

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> On Windows if a file is opened by an application for writing, we cannot
> move
> it until all handles to that file are closed.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

I skipped the patches till here (starting from the previously commented
patch) as they are not relevant as I skipped the daemonization patch. I
applied this one.


> ---
> V3: Initial commit
> V4: No changes
> V5: No changes
> V6: Removed code that disables 'vlog/close' tests.
> V7: Small comments changes
> V8: No changes
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  tests/vlog.at | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tests/vlog.at b/tests/vlog.at
> index 4907a1b..468e872 100644
> --- a/tests/vlog.at
> +++ b/tests/vlog.at
> @@ -148,6 +148,9 @@ AT_CLEANUP
>
>  m4_define([VLOG_REOPEN_PYN],
>[AT_SETUP([vlog - vlog/reopen - $1])
> +   # This test won't work as-is on Windows because Windows doesn't allow
> +   # files that are open to be renamed.
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_SKIP_IF([test $2 = no])
> on_exit 'kill `cat test-unixctl.py.pid`'
>
> --
> 2.7.2.windows.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 V11 12/17] python tests: Prepare porting Python daemon on Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> Renamed daemon.py to daemon_unix.py and implemented a wrapper over it.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

As a standalone commit, this likely fails because it
calls ovs.daemon_windows which does not exist. I also notice that you have
made additional changes in daemon_unix.py. Please do that as a separate
commit with proper commit message.

Also, to be sure, please add the following to your .gitconfig

[diff]
   renames = copies




> ---
> V8: Initial commit.
> V9: No changes
> V10: Fixed exception on Unix
> V11: No changes
> ---
>  python/automake.mk|   1 +
>  python/ovs/daemon.py  | 489 ++
>  python/ovs/daemon_unix.py | 530
> ++
>  3 files changed, 551 insertions(+), 469 deletions(-)
>  create mode 100644 python/ovs/daemon_unix.py
>
> diff --git a/python/automake.mk b/python/automake.mk
> index 1c8fa38..4d3fcb6 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -11,6 +11,7 @@ ovstest_pyfiles = \
>  ovs_pyfiles = \
> python/ovs/__init__.py \
> python/ovs/daemon.py \
> +   python/ovs/daemon_unix.py \
> python/ovs/fcntl_win.py \
> python/ovs/db/__init__.py \
> python/ovs/db/data.py \
> diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py
> index bd06195..b1d6c36 100644
> --- a/python/ovs/daemon.py
> +++ b/python/ovs/daemon.py
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2010, 2011, 2012 Nicira, Inc.
> +# Copyright (c) 2016 Cloudbase Solutions Srl
>  #
>  # Licensed under the Apache License, Version 2.0 (the "License");
>  # you may not use this file except in compliance with the License.
> @@ -12,515 +12,66 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>
> -import errno
> -import fcntl
> -import os
> -import resource
> -import signal
>  import sys
> -import time
>
> -import ovs.dirs
> -import ovs.fatal_signal
> -import ovs.process
> -import ovs.socket_util
> -import ovs.timeval
> -import ovs.util
> -import ovs.vlog
> +# This is only a wrapper over Linux implementations
> +if sys.platform != "win32":
> +import ovs.daemon_unix as daemon_util
>
> -vlog = ovs.vlog.Vlog("daemon")
> -
> -# --detach: Should we run in the background?
> -_detach = False
> -
> -# --pidfile: Name of pidfile (null if none).
> -_pidfile = None
> -
> -# Our pidfile's inode and device, if we have created one.
> -_pidfile_dev = None
> -_pidfile_ino = None
> -
> -# --overwrite-pidfile: Create pidfile even if one already exists and is
> locked?
> -_overwrite_pidfile = False
> -
> -# --no-chdir: Should we chdir to "/"?
> -_chdir = True
> -
> -# --monitor: Should a supervisory process monitor the daemon and restart
> it if
> -# it dies due to an error signal?
> -_monitor = False
> -
> -# File descriptor used by daemonize_start() and daemonize_complete().
> -_daemonize_fd = None
> -
> -RESTART_EXIT_CODE = 5
> +RESTART_EXIT_CODE = daemon_util.RESTART_EXIT_CODE
>
>
>  def make_pidfile_name(name):
> -"""Returns the file name that would be used for a pidfile if 'name'
> were
> -provided to set_pidfile()."""
> -if name is None or name == "":
> -return "%s/%s.pid" % (ovs.dirs.RUNDIR, ovs.util.PROGRAM_NAME)
> -else:
> -return ovs.util.abs_file_name(ovs.dirs.RUNDIR, name)
> +return daemon_util.make_pidfile_name(name)
>
>
>  def set_pidfile(name):
> -"""Sets up a following call to daemonize() to create a pidfile named
> -'name'.  If 'name' begins with '/', then it is treated as an absolute
> path.
> -Otherwise, it is taken relative to ovs.util.RUNDIR, which is
> -$(prefix)/var/run by default.
> -
> -If 'name' is null, then ovs.util.PROGRAM_NAME followed by ".pid" is
> -used."""
> -global _pidfile
> -_pidfile = make_pidfile_name(name)
> +daemon_util.set_pidfile(name)
>
>
>  def set_no_chdir():
> -"""Sets that we do not chdir to "/"."""
> -global _chdir
> -_chdir = False
> +daemon_util.set_no_chdir()
>
>
>  def ignore_existing_pidfile():
> -"""Normally, daemonize() or daemonize_start() will terminate the
> program
> -with a message if a locked pidfile already exists.  If this function
> is
> -called, an existing pidfile will be replaced, with a warning."""
> -global _overwrite_pidfile
> -_overwrite_pidfile = True
> +daemon_util.ignore_existing_pidfile()
>
>
>  def set_detach():
> -"""Sets up a following call to daemonize() to detach from the
> foreground
> -session, running this process in the background."""
> -global _detach
> -_detach = True
> +daemon_util.set_detach()
>
>
>  def get_detach():
> -"""Will daemonize() really detach?"""
> -return _detach
> +return daemon_util.get_detach()
>
>
>  def set_monitor():
> -

[ovs-dev] Cloud, Telecommunication Network, and Storage Accounts

2016-08-03 Thread Ellen Taylor
Hi,

I do research on companies such as yours.

Would you be interested in Cloud, Telecommunication Network, and Storage
Users for your email campaign? 

We provide data across the global which has all contact information that can
be used for your database.

Please let me know if you have any additional requirement for your email
campaign.

So that I can get back to you with more information for the same.

Look forward to hear from you.

Thanks
Ellen Taylor
Data Analytics

If you don't want to receive any messages from us, then please type "OPT
OUT" in the Subject Line.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/6] datapath: compat: Detect GSO support at ovs configure

2016-08-03 Thread pravin shelar
On Tue, Aug 2, 2016 at 3:08 PM, Jesse Gross  wrote:
> On Tue, Aug 2, 2016 at 2:17 PM, Pravin B Shelar  wrote:
>> OVS turns on tunnel GSO backport statically for kernel older than
>> 3.18. Some distributions kernels could backport tunnel GSO. To make
>> use of device tunnel offload on such kernel detect the support at
>> configure stage.
>>
>> Signed-off-by: Pravin B Shelar 
>
> It looks like RHEL 7.2 has already backported ndo_features_check, so
> this should give an immediate improvement there - not just a
> theoretical benefit.
>
Right. 7.2 already has some back ports.

> Acked-by: Jesse Gross 

Thanks. I pushed acked patches.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/6] datapath: compat: Use checksum offload for outer header.

2016-08-03 Thread pravin shelar
On Tue, Aug 2, 2016 at 4:55 PM, Jesse Gross  wrote:
> On Tue, Aug 2, 2016 at 3:55 PM, pravin shelar  wrote:
>> On Tue, Aug 2, 2016 at 3:11 PM, Jesse Gross  wrote:
>>> On Tue, Aug 2, 2016 at 2:17 PM, Pravin B Shelar  wrote:
 Signed-off-by: Pravin B Shelar 
 ---
  datapath/linux/compat/include/net/udp.h |  2 +-
  datapath/linux/compat/udp.c | 19 ++-
  datapath/linux/compat/udp_tunnel.c  | 17 +
  3 files changed, 4 insertions(+), 34 deletions(-)
>>>
>>> Can you give some more information on this? Is it a bug fix,
>>> performance improvement, or cleanup?
>>
>> How about following commit msg.
>> Following patch simplifies UDP-checksum routine by unconditionally
>> using checksum offload for non GSO packets. We might get some
>> performance improvement due to code simplification.
>
> This is just the check for whether the device has the
> NETIF_F_V[46]_CSUM feature? I wonder whether that is really enough of
> a benefit to deviate from upstream. If it is noticeable then it seems
> like this should be an upstream patch.
>

In UDP xmit dst_entry is set after calling this function, so for non
goo packets this function always calculate checksum in software.

> I think the biggest checksum related improvement for compat code would
> be to make the checksum computation conditional on skb->encapsulation
> being set in rpl_ip_local_out.

I am not sure how that will help, In rpl_ip_local_out() GSO packet we
can not generate checksum partial packets, udp_set_csum() can not
handle it. non GSO packet's checksum is calculated in
ovs_iptunnel_handle_offloads() and the skb is not touched in
rpl_ip_local_out().
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V11 11/17] python tests: Ported UNIX sockets to Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> AF_UNIX sockets are not supported on Windows.
> Instead of an AF_UNIX socket use localhost tcp connections to communicate
> between components. This makes the python sockets compatible with
> the ones used in Windows applications.
>
> In case the socket returns WSAEWOULDBLOCK, it is replaced by EAGAIN error
> in order to be compatible with Windows.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

I am skipping this. I suppose this does not work anymore as the servers can
use named pipes.


> ---
> V2: Fixed Python function inet_open_active, treat WSAEWOULDBLOCK error as
> EINPROGRESS on Windows
> V3: Use context managers for file handle leaks
> V4: No changes
> V5: No changes
> V6: No changes
> V7: Fix for JSON-RPC tests - treat WSAEWOULDBLOCK error as EAGAIN
> for compatibility
> V8: No changes
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  python/ovs/jsonrpc.py|  4 
>  python/ovs/socket_util.py| 56
> +---
>  python/ovs/stream.py |  9 +--
>  python/ovs/unixctl/server.py |  6 +++--
>  4 files changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
> index 6300c67..1bc3493 100644
> --- a/python/ovs/jsonrpc.py
> +++ b/python/ovs/jsonrpc.py
> @@ -14,6 +14,7 @@
>
>  import errno
>  import os
> +import sys
>
>  import six
>
> @@ -274,6 +275,9 @@ class Connection(object):
>  except UnicodeError:
>  error = errno.EILSEQ
>  if error:
> +if (sys.platform == "win32"
> +   and error == errno.WSAEWOULDBLOCK):
> +error = errno.EAGAIN
>  if error == errno.EAGAIN:
>  return error, None
>  else:
> diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
> index b358b05..949d27a 100644
> --- a/python/ovs/socket_util.py
> +++ b/python/ovs/socket_util.py
> @@ -17,6 +17,7 @@ import os
>  import os.path
>  import random
>  import socket
> +import sys
>
>  import six
>  from six.moves import range
> @@ -54,6 +55,26 @@ def free_short_name(short_name):
>  ovs.fatal_signal.unlink_file_now(link_name)
>
>
> +def compat_read_unix_socket(path):
> +try:
> +with open(path, "r+") as file_handle:
> +ret = int(file_handle.readline())
> +except IOError as e:
> +vlog.warn("%s: open: %s" % (path, e.strerror))
> +raise socket.error(errno.ENOENT)
> +
> +return ret
> +
> +
> +def compat_write_unix_socket(path, port):
> +try:
> +with open(path, "w") as file_handle:
> +file_handle.write(str(port))
> +except IOError as e:
> +vlog.warn("%s: open: %s" % (path, e.strerror))
> +raise socket.error(errno.ENOENT)
> +
> +
>  def make_unix_socket(style, nonblock, bind_path, connect_path,
> short=False):
>  """Creates a Unix domain socket in the given 'style' (either
>  socket.SOCK_DGRAM or socket.SOCK_STREAM) that is bound to 'bind_path'
> (if
> @@ -65,7 +86,10 @@ def make_unix_socket(style, nonblock, bind_path,
> connect_path, short=False):
>  None."""
>
>  try:
> -sock = socket.socket(socket.AF_UNIX, style)
> +if sys.platform == "win32":
> +sock = socket.socket(socket.AF_INET, style)
> +else:
> +sock = socket.socket(socket.AF_UNIX, style)
>  except socket.error as e:
>  return get_exception_errno(e), None
>
> @@ -81,17 +105,28 @@ def make_unix_socket(style, nonblock, bind_path,
> connect_path, short=False):
>  return e.errno, None
>
>  ovs.fatal_signal.add_file_to_unlink(bind_path)
> -sock.bind(bind_path)
> +if sys.platform == "win32":
> +sock.bind(("127.0.0.1", 0))
> +compat_write_unix_socket(bind_path, sock.getsockname()[1])
> +else:
> +sock.bind(bind_path)
>
> -try:
> -os.fchmod(sock.fileno(), 0o700)
> -except OSError as e:
> -pass
> +try:
> +os.fchmod(sock.fileno(), 0o700)
> +except OSError as e:
> +pass
>  if connect_path is not None:
>  try:
> -sock.connect(connect_path)
> +if sys.platform == "win32":
> +port = compat_read_unix_socket(connect_path)
> +sock.connect(("127.0.0.1", port))
> +else:
> +sock.connect(connect_path)
>  except socket.error as e:
> -if get_exception_errno(e) != errno.EINPROGRESS:
> +error = get_exception_errno(e)
> +if sys.platform == 

Re: [ovs-dev] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Russell Bryant
On Wed, Aug 3, 2016 at 11:39 AM, Kyle Mestery  wrote:

> On Wed, Aug 3, 2016 at 10:30 AM, Ryan Moats  wrote:
> >
> > Russell Bryant  wrote on 08/03/2016 10:11:57 AM:
> >
> >> From: Russell Bryant 
> >> To: Ryan Moats/Omaha/IBM@IBMUS
> >> Cc: Ben Pfaff , ovs-dev 
> >> Date: 08/03/2016 10:12 AM
> >> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> >> we've see scaling the networking-ovn to NB DB connection
> >>
> >> On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats  wrote:
> >>
> >>
> >> Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:
> >>
> >> > From: Ben Pfaff 
> >> > To: Ryan Moats/Omaha/IBM@IBMUS
> >> > Cc: ovs-dev 
> >> > Date: 08/03/2016 12:28 AM
> >> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> >> > we've see scaling the networking-ovn to NB DB connection
> >> >
> >> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> >> > > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
> >> > >
> >> > > > From: Ben Pfaff 
> >> > > > To: Ryan Moats/Omaha/IBM@IBMUS
> >> > > > Cc: ovs-dev 
> >> > > > Date: 08/02/2016 11:52 PM
> >> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> >> > > > we've see scaling the networking-ovn to NB DB connection
> >> > > >
> >> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
> >> > > > > "dev"  wrote on 08/02/2016
> 10:56:07
> >> PM:
> >> > > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
> >> > > > > > > Presumably this means that networking-ovn is calling
> "verify"
> >> on
> >> > > the
> >> > > > > > > column in question.  Probably, networking-ovn should use the
> >> > > partial
> >> > > > > map
> >> > > > > > > update functionality introduced in commit f199df26e8e28
> >> "ovsdb-idl:
> >> > > Add
> >> > > > > > > partial map updates functionality."  I don't know whether
> > it's
> >> in
> >> > > the
> >> > > > > > > Python IDL yet.
> >> > > > > >
> >> > > > > > Indeed they are and thanks for the pointer to the commit -
> I'll
> >> dig
> >> > > > > > into it tomorrow and see if that code is reflected in the
> > Python
> >> > > > > > IDL via that or another commit.  If it is, great.  If not,
> > there
> >> > > > > > will likely also be a patch adding it so that we can move
> > along.
> >> > > > >
> >> > > > > Hmm, maybe I'm misreading something, but I don't thing that's
> > going
> >> > > > > to work without some additional modifications - the partial map
> >> commit
> >> > > > > currently codes for columns that have a particular value type
> >> defined
> >> > > > > by the schema.  The problem we are seeing is with the ports and
> >> acls
> >> > > > > columns of the logical switch table, which are lists of strong
> >> > > > > references.  Since they don't have a defined value, the
> generated
> >> IDL
> >> > > > > code doesn't provide hooks for using partial map operations and
> > we
> >> > > default
> >> > > > > back to update/verify with the given above results.
> >> > > > >
> >> > > > > Now, I think this an oversight, because I can argue that since
> >> these
> >> > > > > are strong references, I should be able to use partial maps to
> >> update
> >> > > > > them as keys with a null value.  Does this make sense or am I
> >> breaking
> >> > > > > something if I look at going this route?
> >> > > >
> >> > > > If they're implemented as partial map operations only, then we
> > should
> >> > > > extend them to support partial set operations too--the same
> >> principles
> >> > > > apply.
> >> > >
> >> > > I'm not sure I parsed this correctly, but I think we are saying the
> >> same
> >> > > thing: change the IDL for columns that contain sets of strong
> >> references
> >> > > from using update/verify to using mutate for partial set operations
> > (I
> >> > > realized after hitting the send button that I should have said
> > partial
> >> > > sets instead of partial maps...)
> >> >
> >> > Yes, I think we're saying the same thing.
> >> >
> >
> >> Cool, I'll see how far I can get (not sure where my previous message
> > saying
> >> this went...)
> >>
> >> From the perspective of the branch point, we'd really like to see the
> >> following make the 2.6 release to allow for easier integration with
> >> upstream
> >> OpenStack testing:
> >>
> >> - partial sets for the C IDL
> >> - partial maps for the Python IDL
> >> - partial sets for the Python IDL
> >>
> >> In fact, I'd sorta like to see them all backported to 2.5, but I doubt
> >> that's
> >> possible.
> >>
> >> (...off to craft patch sets...)
> >>
> >> but based on:
> >>
> >> http://openvswitch.org/pipermail/dev/2016-July/076855.html
> >>
> >> it seems too late to try to get new things like this in for 2.6.
> >
> > I realize that I'm asking for an exception - at the time I wrote that
> > we 

Re: [ovs-dev] [PATCH V11 09/17] python tests: Fixed OSError not iterable on Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> On Windows if this exception is triggered then it will raise an exception
> while in the
> exception handler.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

I added the following incremental and applied this.
diff --git a/python/ovs/poller.py b/python/ovs/poller.py
index 3ebaf5a..de6bf22 100644
--- a/python/ovs/poller.py
+++ b/python/ovs/poller.py
@@ -172,8 +172,7 @@ class Poller(object):
 except OSError as e:
 """ On Windows, the select function from poll raises
OSError
 exception if the polled array is empty."""
-error = e.errno
-if error != errno.EINTR:
+if e.errno != errno.EINTR:
 vlog.err("poll: %s" % os.strerror(e.errno))
 except select.error as e:
 # XXX rate-limit


> ---
> V2: No changes
> V3: No changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: No changes
> V8: Added comment when using OSError on Windows
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  python/ovs/poller.py | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/python/ovs/poller.py b/python/ovs/poller.py
> index 20be801..3ebaf5a 100644
> --- a/python/ovs/poller.py
> +++ b/python/ovs/poller.py
> @@ -17,6 +17,7 @@ import ovs.timeval
>  import ovs.vlog
>  import select
>  import socket
> +import os
>
>  try:
>  import eventlet.patcher
> @@ -168,6 +169,12 @@ class Poller(object):
>  try:
>  events = self.poll.poll(self.timeout)
>  self.__log_wakeup(events)
> +except OSError as e:
> +""" On Windows, the select function from poll raises
> OSError
> +exception if the polled array is empty."""
> +error = e.errno
> +if error != errno.EINTR:
> +vlog.err("poll: %s" % os.strerror(e.errno))
>  except select.error as e:
>  # XXX rate-limit
>  error, msg = e
> --
> 2.7.2.windows.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 V11 08/17] python tests: Skip python tests on Windows where POSIX pid is used

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> There is a difference between POSIX pid and Windows pid, not all the time
> are equal.
> On Windows when a python script is started, a sh command is triggered as
> the parent
> for script. So when we try to get the daemon pid with 'echo $!', this will
> get the pid of sh
> not of python.exe as expected.
> Also the 'kill' command expects a POSIX pid, not the Windows pid written
> by the python
> daemons in pid file.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

I applied the patches till here.
In this change, the following comment has been added for all the tests that
are being skipped. Even those tests for which the comment is not relevant:
"Skip this test for Windows, echo $! gives shell pid instead of parent
process".

So I am skipping this. Please re-look this patch.



> ---
> V2: No changes
> V3: No changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: Added comments explaining why the tests are skipped.
> Enabled back the JSON-RPC tests
> V8: No changes
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  tests/daemon-py.at | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/tests/daemon-py.at b/tests/daemon-py.at
> index e59c11d..b1ff4fd 100644
> --- a/tests/daemon-py.at
> +++ b/tests/daemon-py.at
> @@ -3,6 +3,8 @@ AT_BANNER([daemon unit tests - Python])
>  m4_define([DAEMON_PYN],
>[AT_SETUP([daemon - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_KEYWORDS([python daemon])
> AT_CAPTURE_FILE([pid])
> AT_CAPTURE_FILE([expected])
> @@ -26,6 +28,8 @@ DAEMON_PYN([Python3], [$HAVE_PYTHON3], [$PYTHON3])
>  m4_define([DAEMON_MONITOR_PYN],
>[AT_SETUP([daemon --monitor - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> AT_CAPTURE_FILE([parent])
> AT_CAPTURE_FILE([parentpid])
> @@ -73,6 +77,8 @@ DAEMON_MONITOR_PYN([Python3], [$HAVE_PYTHON3],
> [$PYTHON3])
>  m4_define([DAEMON_MONITOR_RESTART_PYN],
>[AT_SETUP([daemon --monitor restart exit code - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> AT_CAPTURE_FILE([parent])
> AT_CAPTURE_FILE([parentpid])
> @@ -120,6 +126,8 @@ DAEMON_MONITOR_RESTART_PYN([Python3], [$HAVE_PYTHON3],
> [$PYTHON3])
>  m4_define([DAEMON_DETACH_PYN],
>[AT_SETUP([daemon --detach - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> # Start the daemon and make sure that the pidfile exists immediately.
> # We don't wait for the pidfile to get created because the daemon is
> @@ -142,6 +150,8 @@ m4_define([CHECK],
>  m4_define([DAEMON_DETACH_MONITOR_PYN],
>[AT_SETUP([daemon --detach --monitor - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([daemon])
> AT_CAPTURE_FILE([olddaemon])
> AT_CAPTURE_FILE([newdaemon])
> @@ -219,6 +229,8 @@ DAEMON_DETACH_MONITOR_ERRORS_PYN([Python3],
> [$HAVE_PYTHON3], [$PYTHON3])
>  m4_define([DAEMON_DETACH_CLOSES_FDS_PYN],
>[AT_SETUP([daemon --detach closes standard fds - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> AT_CAPTURE_FILE([status])
> AT_CAPTURE_FILE([stderr])
> @@ -243,6 +255,8 @@ DAEMON_DETACH_CLOSES_FDS_PYN([Python3],
> [$HAVE_PYTHON3], [$PYTHON3])
>  m4_define([DAEMON_DETACH_MONITOR_CLOSES_FDS_PYN],
>[AT_SETUP([daemon --detach --monitor closes standard fds - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, echo $! gives shell pid instead of
> parent process
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> AT_CAPTURE_FILE([status])
> AT_CAPTURE_FILE([stderr])
> --
> 2.7.2.windows.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] ovs-rcu: Add new ovsrcu_index type.

2016-08-03 Thread Loftus, Ciara
> 
> With RCU in Open vSwitch it's very easy to protect objects accessed by
> a pointer, but sometimes a pointer is not available.
> 
> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
> was used to access a vhost device with RCU semantics.  From DPDK 16.07
> an integer id (which is an array index) is used to access a vhost
> device.  Ideally, we want the exact same RCU semantics that we had for
> the pointer, on the integer (atomicity, memory barriers, behaviour
> around quiescent states)
> 
> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
> implemented ovsrcu_index_*() functions should be used to access the
> type.
> 
> Even though we say "Do not, in general, declare a typedef for a struct,
> union, or enum.", I think we're not in the "general" case.
> 
> Signed-off-by: Daniele Di Proietto 
> ---
>  lib/ovs-rcu.h | 84
> ++
> +
>  1 file changed, 84 insertions(+)
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..2887bb8 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -125,6 +125,36 @@
>   * ovs_mutex_unlock();
>   * }
>   *
> + * In some rare cases an object may not be addressable with a pointer, but
> only
> + * through an array index (e.g. because it's provided by another library).  
> It
> + * is still possible to have RCU semantics by using the ovsrcu_index type.
> + *
> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> + *
> + * ovsrcu_index port_id;
> + *
> + * void tx()
> + * {
> + * int id = ovsrcu_index_get(_id);
> + * if (id == -1) {
> + * return;
> + * }
> + * port_tx(id);
> + * }
> + *
> + * void delete()
> + * {
> + * int id;
> + *
> + * ovs_mutex_lock();
> + * id = ovsrcu_index_get_protected(_id);
> + * ovsrcu_index_set(_id, -1);
> + * ovs_mutex_unlock();
> + *
> + * ovsrcu_synchronize();
> + * port_delete(id);
> + * }
> + *
>   */
> 
>  #include "compiler.h"
> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void
> *aux), void *aux);
>   (void) sizeof(*(ARG)), \
>   ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
> 
> +/* An array index protected by RCU semantics.  This is an easier alternative
> to
> + * an RCU protected pointer to a malloc'd int. */
> +typedef struct { atomic_int v; } ovsrcu_index;
> +
> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order
> order)
> +{
> +int ret;
> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +return ret;
> +}
> +
> +/* Returns the index contained in 'i'.  The returned value can be used until
> + * the next grace period. */
> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_consume);
> +}
> +
> +/* Returns the index contained in 'i'.  This is an alternative to
> + * ovsrcu_index_get() that can be used when there's no possible
> concurrent
> + * writer. */
> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
> +  memory_order order)
> +{
> +atomic_store_explicit(>v, value, order);
> +}
> +
> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
> + * used by readers until the next grace period. */
> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_release);
> +}
> +
> +/* Writes the index 'value' in 'i'.  This is an alternative to
> + * ovsrcu_index_set() that can be used when there's no possible
> concurrent
> + * reader. */
> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_relaxed);
> +}
> +
> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
> no
> + * concurrent readers. */
> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
> +{
> +atomic_init(>v, value);
> +}
> +
>  /* Quiescent states. */
>  void ovsrcu_quiesce_start(void);
>  void ovsrcu_quiesce_end(void);
> --
> 2.8.1

Tested-by: Ciara Loftus 

> 
> ___
> 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 V11 03/17] python tests: Fixed ctl file name for Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> On Windows the CTL filename doesn't contain the pid of the process.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
> ---
> V2: No changes
> V3: No changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: Fixed flake8 errors. Addressed changes required by review.
> V8: Fixed small alignement problem.
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  python/ovs/unixctl/__init__.py | 11 +--
>  python/ovs/unixctl/server.py   | 10 --
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/python/ovs/unixctl/__init__.py
> b/python/ovs/unixctl/__init__.py
> index d3d3556..e2e5604 100644
> --- a/python/ovs/unixctl/__init__.py
> +++ b/python/ovs/unixctl/__init__.py
> @@ -13,6 +13,7 @@
>  # limitations under the License.
>
>  import six
> +import sys
>
>  import ovs.util
>
> @@ -71,14 +72,20 @@ def command_register(name, usage, min_args, max_args,
> callback, aux):
>  def socket_name_from_target(target):
>  assert isinstance(target, strtypes)
>
> -if target.startswith("/"):
> +""" On Windows an absolute path contains ':' ( i.e: C:\ ) """
> +if target.startswith('/') or target.find(':') > -1:
>  return 0, target
>
> +""" Leave it to read the pid file on Windows also, the tests expect
> this
> +error in case of failure. """
>

The above comment should not be here. We should not write code with
information on how it is tested. So I removed it and applied this.


>  pidfile_name = "%s/%s.pid" % (ovs.dirs.RUNDIR, target)
>  pid = ovs.daemon.read_pidfile(pidfile_name)
>  if pid < 0:
>  return -pid, "cannot read pidfile \"%s\"" % pidfile_name
>
> -return 0, "%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, target, pid)
> +if sys.platform == "win32":
> +return 0, "%s/%s.ctl" % (ovs.dirs.RUNDIR, target)
> +else:
> +return 0, "%s/%s.%d.ctl" % (ovs.dirs.RUNDIR, target, pid)
>
>  command_register("help", "", 0, 0, _unixctl_help, None)
> diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py
> index cc712bf..8595ed8 100644
> --- a/python/ovs/unixctl/server.py
> +++ b/python/ovs/unixctl/server.py
> @@ -15,6 +15,7 @@
>  import copy
>  import errno
>  import os
> +import sys
>
>  import six
>  from six.moves import range
> @@ -188,8 +189,13 @@ class UnixctlServer(object):
>  if path is not None:
>  path = "punix:%s" % ovs.util.abs_file_name(ovs.dirs.RUNDIR,
> path)
>  else:
> -path = "punix:%s/%s.%d.ctl" % (ovs.dirs.RUNDIR,
> -   ovs.util.PROGRAM_NAME,
> os.getpid())
> +if sys.platform == "win32":
> +path = "punix:%s/%s.ctl" % (ovs.dirs.RUNDIR,
> +ovs.util.PROGRAM_NAME)
> +else:
> +path = "punix:%s/%s.%d.ctl" % (ovs.dirs.RUNDIR,
> +   ovs.util.PROGRAM_NAME,
> +   os.getpid())
>
>  if version is None:
>  version = ovs.version.VERSION
> --
> 2.7.2.windows.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 2/2] Add wrapper scripts for *ctl commands

2016-08-03 Thread Kyle Mestery
On Wed, Aug 3, 2016 at 7:35 AM, Russell Bryant  wrote:
>
> On Tue, Aug 2, 2016 at 1:16 PM, Kyle Mestery  wrote:
>>
>> On Tue, Aug 2, 2016 at 12:13 PM, Ryan Moats  wrote:
>> >
>> > Russell Bryant  wrote on 08/02/2016 12:00:08 PM:
>> >
>> >> From: Russell Bryant 
>> >> To: Ben Pfaff 
>> >> Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
>> >> Date: 08/02/2016 12:00 PM
>> >> Subject: Re: [ovs-dev] [PATCH 2/2] Add wrapper scripts for *ctl
>> >> commands
>> >>
>> >> On Tue, Aug 2, 2016 at 12:03 PM, Ben Pfaff  wrote:
>> >> On Tue, Aug 02, 2016 at 07:56:27AM -0400, Russell Bryant wrote:
>> >> > On Tue, Aug 2, 2016 at 12:20 AM, Ryan Moats 
>> >> > wrote:
>> >> >
>> >> > > This commit creates wrapper scripts for the *ctl commands to use
>> >> > > --dry-run for those that have them, and to allow for log level
>> >> > > setting via ovs-appctl without allowing full access to ovs-appctl.
>> >> > > Tests have been added to make sure that the wrapper scripts
>> >> > > don't actually do anything when asked to perform a write operation.
>> >> > >
>> >> > > Signed-off-by: Ryan Moats 
>> >> > >
>> >> >
>> >> > What's the motivation for all the new "read" scripts?  It seems a bit
>> >> > confusing to install all of these.  They're also not documented
>> > anywhere.
>> >>
>> >> My assumption had been that we'd put the options into the tree and then
>> >> that the one-liner redirection scripts would be an IBM customization.
>> >> After all, they need to customize somehow anyway to hide the read/write
>> >> versions in some off-$PATH place.
>> >>
>> >> +1 to this approach.
>> >>
>> >> --
>> >> Russell Bryant
>> >
>> > Obviously, I think this is somewhat short-sighted (or I wouldn't have
>> > proposed
>> > the patch)...
>> >
>> > How about if we were to spin a new repo openvswitch/operator-tools (like
>> > openvswitch/ovn-scale-test)
>> > and put things like this *there*?
>> >
>> I'd be in favor of this approach, because I think having tools like
>> this for cloud operators would be a good thing to share. And as one of
>> the main users/committers into ovn-scale-test, I can also attest to
>> how nice it is to have the shared github to work on so.
>>
>> So I'm +1 to this new repository idea.
>
>
> There are lots of things in the ovs repo that could be considered operator
> tools, but I don't think moving them is really needed.  The issue here is
> whether this is more IBM specific or general purpose.
>
> Maybe create the new repo in a personal space somewhere and we see what
> builds up there to see if it makes sense to move it to openvswitch/?  I
> don't think just these one liner scripts really justify it, yet.
>

This is fair, and that's what we've done for now.

> --
> Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V10 04/17] python tests: Fixed unixctl python tests for Windows

2016-08-03 Thread Guru Shetty
On 1 August 2016 at 02:29, Paul Boca  wrote:

> For bogus pid file path, use a windows-like file path.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>

Applied.

> ---
> V2: No changes
> V3: No changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: No changes
> V8: No changes
> V9: No changes
> V10: No changes
> ---
>  tests/unixctl-py.at | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at
> index cbcd802..2031897 100644
> --- a/tests/unixctl-py.at
> +++ b/tests/unixctl-py.at
> @@ -92,11 +92,17 @@ m4_define([UNIXCTL_BAD_TARGET_PYN],
> AT_CHECK_UNQUOTED([tail -1 stderr], [0], [dnl
>  appctl.py: cannot read pidfile "`pwd`/bogus.pid" (No such file or
> directory)
>  ])
> -
> -   AT_CHECK([PYAPPCTL_PYN([$3]) -t /bogus/path.pid doit], [1], [],
> [stderr])
> -   AT_CHECK([tail -1 stderr], [0], [dnl
> +   if test "$IS_WIN32" = "no"; then
> + AT_CHECK([PYAPPCTL_PYN([$3]) -t /bogus/path.pid doit], [1], [],
> [stderr])
> + AT_CHECK([tail -1 stderr], [0], [dnl
>  appctl.py: cannot connect to "/bogus/path.pid" (No such file or directory)
>  ])
> +   else
> + AT_CHECK([PYAPPCTL_PYN([$3]) -t c:/bogus/path.pid doit], [1], [],
> [stderr])
> + AT_CHECK([tail -1 stderr], [0], [dnl
> +appctl.py: cannot connect to "c:/bogus/path.pid" (No such file or
> directory)
> +])
> +   fi
>
> AT_CLEANUP])
>
> --
> 2.7.2.windows.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] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Kyle Mestery
On Wed, Aug 3, 2016 at 10:30 AM, Ryan Moats  wrote:
>
> Russell Bryant  wrote on 08/03/2016 10:11:57 AM:
>
>> From: Russell Bryant 
>> To: Ryan Moats/Omaha/IBM@IBMUS
>> Cc: Ben Pfaff , ovs-dev 
>> Date: 08/03/2016 10:12 AM
>> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
>> we've see scaling the networking-ovn to NB DB connection
>>
>> On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats  wrote:
>>
>>
>> Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:
>>
>> > From: Ben Pfaff 
>> > To: Ryan Moats/Omaha/IBM@IBMUS
>> > Cc: ovs-dev 
>> > Date: 08/03/2016 12:28 AM
>> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
>> > we've see scaling the networking-ovn to NB DB connection
>> >
>> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
>> > > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
>> > >
>> > > > From: Ben Pfaff 
>> > > > To: Ryan Moats/Omaha/IBM@IBMUS
>> > > > Cc: ovs-dev 
>> > > > Date: 08/02/2016 11:52 PM
>> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
>> > > > we've see scaling the networking-ovn to NB DB connection
>> > > >
>> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
>> > > > > "dev"  wrote on 08/02/2016 10:56:07
>> PM:
>> > > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
>> > > > > > > Presumably this means that networking-ovn is calling "verify"
>> on
>> > > the
>> > > > > > > column in question.  Probably, networking-ovn should use the
>> > > partial
>> > > > > map
>> > > > > > > update functionality introduced in commit f199df26e8e28
>> "ovsdb-idl:
>> > > Add
>> > > > > > > partial map updates functionality."  I don't know whether
> it's
>> in
>> > > the
>> > > > > > > Python IDL yet.
>> > > > > >
>> > > > > > Indeed they are and thanks for the pointer to the commit - I'll
>> dig
>> > > > > > into it tomorrow and see if that code is reflected in the
> Python
>> > > > > > IDL via that or another commit.  If it is, great.  If not,
> there
>> > > > > > will likely also be a patch adding it so that we can move
> along.
>> > > > >
>> > > > > Hmm, maybe I'm misreading something, but I don't thing that's
> going
>> > > > > to work without some additional modifications - the partial map
>> commit
>> > > > > currently codes for columns that have a particular value type
>> defined
>> > > > > by the schema.  The problem we are seeing is with the ports and
>> acls
>> > > > > columns of the logical switch table, which are lists of strong
>> > > > > references.  Since they don't have a defined value, the generated
>> IDL
>> > > > > code doesn't provide hooks for using partial map operations and
> we
>> > > default
>> > > > > back to update/verify with the given above results.
>> > > > >
>> > > > > Now, I think this an oversight, because I can argue that since
>> these
>> > > > > are strong references, I should be able to use partial maps to
>> update
>> > > > > them as keys with a null value.  Does this make sense or am I
>> breaking
>> > > > > something if I look at going this route?
>> > > >
>> > > > If they're implemented as partial map operations only, then we
> should
>> > > > extend them to support partial set operations too--the same
>> principles
>> > > > apply.
>> > >
>> > > I'm not sure I parsed this correctly, but I think we are saying the
>> same
>> > > thing: change the IDL for columns that contain sets of strong
>> references
>> > > from using update/verify to using mutate for partial set operations
> (I
>> > > realized after hitting the send button that I should have said
> partial
>> > > sets instead of partial maps...)
>> >
>> > Yes, I think we're saying the same thing.
>> >
>
>> Cool, I'll see how far I can get (not sure where my previous message
> saying
>> this went...)
>>
>> From the perspective of the branch point, we'd really like to see the
>> following make the 2.6 release to allow for easier integration with
>> upstream
>> OpenStack testing:
>>
>> - partial sets for the C IDL
>> - partial maps for the Python IDL
>> - partial sets for the Python IDL
>>
>> In fact, I'd sorta like to see them all backported to 2.5, but I doubt
>> that's
>> possible.
>>
>> (...off to craft patch sets...)
>>
>> but based on:
>>
>> http://openvswitch.org/pipermail/dev/2016-July/076855.html
>>
>> it seems too late to try to get new things like this in for 2.6.
>
> I realize that I'm asking for an exception - at the time I wrote that
> we didn't realize how much of an issue using verify/update semantics was
> going to be when scaling (mea culpa).
>
> If the exception isn't granted, so be it - we'll carry it locally until
> the 2.6.90 branch opens...
>

I'll jump in here and say that this issue has become a huge scaling
issue for us internally, and anyone else who wants to scale OVN. 

Re: [ovs-dev] [PATCH V11 02/17] python tests: Register signal handlers only on supported types on Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> SIGHUP and SIGALRM are not available on Windows.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
>
Applied.


> ---
> V2: Fixed Python function inet_open_active, treat WSAEWOULDBLOCK error as
> EINPROGRESS on Windows
> V3: No changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: Simplified the signal handlers code
> V8: No changes
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  python/ovs/fatal_signal.py | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/python/ovs/fatal_signal.py b/python/ovs/fatal_signal.py
> index 14605ac..73e4be6 100644
> --- a/python/ovs/fatal_signal.py
> +++ b/python/ovs/fatal_signal.py
> @@ -16,6 +16,7 @@ import atexit
>  import os
>  import signal
>  import sys
> +
>  import ovs.vlog
>
>  _hooks = []
> @@ -128,9 +129,13 @@ def _init():
>  global _inited
>  if not _inited:
>  _inited = True
> +if sys.platform == "win32":
> +signals = [signal.SIGTERM, signal.SIGINT]
> +else:
> +signals = [signal.SIGTERM, signal.SIGINT, signal.SIGHUP,
> +   signal.SIGALRM]
>
> -for signr in (signal.SIGTERM, signal.SIGINT,
> -  signal.SIGHUP, signal.SIGALRM):
> +for signr in signals:
>  if signal.getsignal(signr) == signal.SIG_DFL:
>  signal.signal(signr, _signal_handler)
>  atexit.register(_atexit_handler)
> --
> 2.7.2.windows.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 V11 01/17] python tests: Implemented signal.alarm for Windows

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 10:45, Paul Boca  wrote:

> signal.alarm is not available in Windows and would trigger an exception
> when called. Implemented this to mentain compatibility between
> Windows and Linux for python tests.
>
> Signed-off-by: Paul-Daniel Boca 
> Acked-by: Alin Gabriel Serdean 
> ---
> V2: No changes
> V3: Code styling changes
> V4: No changes
> V5: No changes
> V6: No changes
> V7: Added function signal_alarm in fatal_signal.py to avoid
> duplicating code
> V8: No changes
> V9: No changes
> V10: No changes
> V11: No changes
> ---
>  python/ovs/fatal_signal.py | 24 +++-
>  tests/appctl.py|  4 ++--
>  tests/test-ovsdb.py|  4 ++--
>  tests/test-unix-socket.py  |  3 ++-
>  4 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/python/ovs/fatal_signal.py b/python/ovs/fatal_signal.py
> index 7308039..14605ac 100644
> --- a/python/ovs/fatal_signal.py
> +++ b/python/ovs/fatal_signal.py
> @@ -15,7 +15,7 @@
>  import atexit
>  import os
>  import signal
> -
> +import sys
>  import ovs.vlog
>

I removed the above change and applied.


>
>  _hooks = []
> @@ -134,3 +134,25 @@ def _init():
>  if signal.getsignal(signr) == signal.SIG_DFL:
>  signal.signal(signr, _signal_handler)
>  atexit.register(_atexit_handler)
> +
> +
> +def signal_alarm(timeout):
> +if sys.platform == "win32":
> +import os
> +import time
> +import threading
> +
> +class Alarm (threading.Thread):
> +def __init__(self, timeout):
> +super(Alarm, self).__init__()
> +self.timeout = timeout
> +self.setDaemon(True)
> +
> +def run(self):
> +time.sleep(self.timeout)
> +os._exit(1)
> +
> +alarm = Alarm(timeout)
> +alarm.start()
> +else:
> +signal.alarm(timeout)
> diff --git a/tests/appctl.py b/tests/appctl.py
> index e5bcf2c..e4f0696 100644
> --- a/tests/appctl.py
> +++ b/tests/appctl.py
> @@ -13,7 +13,6 @@
>  # limitations under the License.
>
>  import argparse
> -import signal
>  import sys
>
>  import ovs.daemon
> @@ -21,6 +20,7 @@ import ovs.unixctl
>  import ovs.unixctl.client
>  import ovs.util
>  import ovs.vlog
> +from ovs.fatal_signal import signal_alarm
>
>
>  def connect_to_target(target):
> @@ -52,7 +52,7 @@ def main():
>  args = parser.parse_args()
>
>  if args.timeout:
> -signal.alarm(int(args.timeout))
> +signal_alarm(int(args.timeout))
>
>  ovs.vlog.Vlog.init()
>  target = args.target
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index 88180a3..e9d4510 100644
> --- a/tests/test-ovsdb.py
> +++ b/tests/test-ovsdb.py
> @@ -17,7 +17,6 @@ from __future__ import print_function
>  import getopt
>  import re
>  import os
> -import signal
>  import sys
>  import uuid
>
> @@ -29,6 +28,7 @@ import ovs.db.types
>  import ovs.ovsuuid
>  import ovs.poller
>  import ovs.util
> +from ovs.fatal_signal import signal_alarm
>  import six
>
>
> @@ -676,7 +676,7 @@ def main(argv):
>  except TypeError:
>  raise error.Error("value %s on -t or --timeout is not at "
>"least 1" % value)
> -signal.alarm(timeout)
> +signal_alarm(timeout)
>  else:
>  sys.exit(0)
>
> diff --git a/tests/test-unix-socket.py b/tests/test-unix-socket.py
> index ebfa054..c80fb13 100644
> --- a/tests/test-unix-socket.py
> +++ b/tests/test-unix-socket.py
> @@ -19,6 +19,7 @@ import socket
>  import sys
>
>  import ovs.socket_util
> +from ovs.fatal_signal import signal_alarm
>
>
>  def main(argv):
> @@ -33,7 +34,7 @@ def main(argv):
>  sockname2 = sockname1
>
>  signal.signal(signal.SIGALRM, signal.SIG_DFL)
> -signal.alarm(5)
> +signal_alarm(5)
>
>  # Create a listening socket under name 'sockname1'.
>  error, sock1 = ovs.socket_util.make_unix_socket(socket.SOCK_STREAM,
> False,
> --
> 2.7.2.windows.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] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Ryan Moats

Russell Bryant  wrote on 08/03/2016 10:11:57 AM:

> From: Russell Bryant 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: Ben Pfaff , ovs-dev 
> Date: 08/03/2016 10:12 AM
> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> we've see scaling the networking-ovn to NB DB connection
>
> On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats  wrote:
>
>
> Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:
>
> > From: Ben Pfaff 
> > To: Ryan Moats/Omaha/IBM@IBMUS
> > Cc: ovs-dev 
> > Date: 08/03/2016 12:28 AM
> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > we've see scaling the networking-ovn to NB DB connection
> >
> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> > > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
> > >
> > > > From: Ben Pfaff 
> > > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > > Cc: ovs-dev 
> > > > Date: 08/02/2016 11:52 PM
> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > > > we've see scaling the networking-ovn to NB DB connection
> > > >
> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
> > > > > "dev"  wrote on 08/02/2016 10:56:07
> PM:
> > > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
> > > > > > > Presumably this means that networking-ovn is calling "verify"
> on
> > > the
> > > > > > > column in question.  Probably, networking-ovn should use the
> > > partial
> > > > > map
> > > > > > > update functionality introduced in commit f199df26e8e28
> "ovsdb-idl:
> > > Add
> > > > > > > partial map updates functionality."  I don't know whether
it's
> in
> > > the
> > > > > > > Python IDL yet.
> > > > > >
> > > > > > Indeed they are and thanks for the pointer to the commit - I'll
> dig
> > > > > > into it tomorrow and see if that code is reflected in the
Python
> > > > > > IDL via that or another commit.  If it is, great.  If not,
there
> > > > > > will likely also be a patch adding it so that we can move
along.
> > > > >
> > > > > Hmm, maybe I'm misreading something, but I don't thing that's
going
> > > > > to work without some additional modifications - the partial map
> commit
> > > > > currently codes for columns that have a particular value type
> defined
> > > > > by the schema.  The problem we are seeing is with the ports and
> acls
> > > > > columns of the logical switch table, which are lists of strong
> > > > > references.  Since they don't have a defined value, the generated
> IDL
> > > > > code doesn't provide hooks for using partial map operations and
we
> > > default
> > > > > back to update/verify with the given above results.
> > > > >
> > > > > Now, I think this an oversight, because I can argue that since
> these
> > > > > are strong references, I should be able to use partial maps to
> update
> > > > > them as keys with a null value.  Does this make sense or am I
> breaking
> > > > > something if I look at going this route?
> > > >
> > > > If they're implemented as partial map operations only, then we
should
> > > > extend them to support partial set operations too--the same
> principles
> > > > apply.
> > >
> > > I'm not sure I parsed this correctly, but I think we are saying the
> same
> > > thing: change the IDL for columns that contain sets of strong
> references
> > > from using update/verify to using mutate for partial set operations
(I
> > > realized after hitting the send button that I should have said
partial
> > > sets instead of partial maps...)
> >
> > Yes, I think we're saying the same thing.
> >

> Cool, I'll see how far I can get (not sure where my previous message
saying
> this went...)
>
> From the perspective of the branch point, we'd really like to see the
> following make the 2.6 release to allow for easier integration with
> upstream
> OpenStack testing:
>
> - partial sets for the C IDL
> - partial maps for the Python IDL
> - partial sets for the Python IDL
>
> In fact, I'd sorta like to see them all backported to 2.5, but I doubt
> that's
> possible.
>
> (...off to craft patch sets...)
>
> but based on:
>
> http://openvswitch.org/pipermail/dev/2016-July/076855.html
>
> it seems too late to try to get new things like this in for 2.6.

I realize that I'm asking for an exception - at the time I wrote that
we didn't realize how much of an issue using verify/update semantics was
going to be when scaling (mea culpa).

If the exception isn't granted, so be it - we'll carry it locally until
the 2.6.90 branch opens...



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 6/6] datapath: Add support for kernel 4.7

2016-08-03 Thread pravin shelar
On Tue, Aug 2, 2016 at 4:40 PM, Jesse Gross  wrote:
> On Tue, Aug 2, 2016 at 2:18 PM, Pravin B Shelar  wrote:
>> Signed-off-by: Pravin B Shelar 
>> ---
>>  FAQ.md   | 2 +-
>>  NEWS | 2 +-
>>  acinclude.m4 | 4 ++--
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> Should we also add 4.7 to Travis?
>
> I noticed that there appear to be at least two commits that are in 4.7
> upstream OVS that are not currently backported here (there might be
> others as well):
> 3b78155b (“openvswitch: __nf_ct_l{3,4}proto_find() always return a
> valid pointer”)
This patch is already in master.

> d913d3a7 (“openvswitch: fix conntrack netlink event delivery”)
>
> And this one from Geneve:
> d5d5e8d5 (“geneve: fix max_mtu setting”)
>
I will send patches for these commits.

> One other thing that I noticed while looking at this is that the GRO
> registration for Geneve and VXLAN (for example, in
> geneve_notify_add_rx_port()) is an #else with the hardware receive
> registration. This doesn't seem right - they come and go in various
> kernel versions pretty much independently of each other. For example,
> upstream 4.6 has both.

ok, I will investigate it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v1] ovn: Extend Address_Set to Macros to support define port name sets

2016-08-03 Thread Zong Kai LI
This patch aims to extend Address_Set to Macros, make it more common to
accept variable set, not only address. And by that, we can skinny down ACLs,
if we use Macros to defines port name sets for ACL to use, since lots of ACL
entries are similar but only "inport" and "outport" fields are different.

Address_Set improves the performance of OVN control plane. But what more
important is, its implementation introduced a "$macros -- element_set"
mechanism, it's good way to do bunch jobs for ACLs, and even lflows.

How does Address_Set improve OVN performance?
 - it reduces the match part length for lflows, who has addresses to match.
 - indeed, it means less tokens in expression to parse, less constants, less
   combination symbols.
 - and for CMS integration component, for its DB sync job, a ACL entry has
   short match field is easier to compare.

What changes:
  Adderss_Set   -> Macros
  Adderss_Set.addresses -> Macros.elements
   (addresses, or ports names)
 + Macros.element_type
   (only "address", "port_name" are valid now)
  ACL match field, for example:
match : "outport == $ps1 && icmp4"

Will port set defined under Macros benefit in the same way as Address_Set
did? -- NO, it may be better.
 - it will not decrease ACL match field length, but the number of ACL entries.
   ACL entries who have similar match expressions, but with particular
   "inport" or "outport" can use port name set defined in Macros to combine
   into one ACL.
 - less ACL entries means less refers between ACL and Logical_Switch.acl.
 - less ACL entries, will help CMS integration component has less workload
   in DB sync job, comparing port name should be quicker then comparing whole
   ACL entry.
 - less ACL entries, means less lflows, means less match part in lflows to
   parse, less tokens to parse, make parsing work quicker.

Port name set defined under Macros will improve ovn-controller performance on
installation plenty of similar ACLs/lflows.
For port comes and leaves scenario, when there are no special ACL entries
assigned to ports, it will be the ones first come and last leave to update ACL
uuid refers in Logical_Switch.acl.

TODO:
 - go check how will ovn-northd use port sets defined under Macros.
 - find any other type need to support.

Signed-off-by: Zong Kai LI 
---
 AUTHORS   |   1 +
 include/ovn/expr.h|   2 +-
 ovn/controller/lflow.c| 173 --
 ovn/lib/expr.c|  68 ++
 ovn/northd/ovn-northd.c   |  54 ---
 ovn/ovn-nb.ovsschema  |  11 +--
 ovn/ovn-nb.xml|  27 ++--
 ovn/ovn-sb.ovsschema  |  11 +--
 ovn/ovn-sb.xml|   7 +-
 ovn/utilities/ovn-nbctl.c |   4 +-
 ovn/utilities/ovn-sbctl.c |   4 +-
 tests/ovn.at  |  14 ++--
 tests/test-ovn.c  |   6 +-
 13 files changed, 212 insertions(+), 170 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index b598f4b..e71571e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -459,6 +459,7 @@ ZHANG Zhiming   zhangzhim...@yunshan.net.cn
 Zhangguanghui   zhang.guang...@h3c.com
 Ziyou Wang  ziy...@vmware.com
 Zoltán Balogh   zoltan.bal...@ericsson.com
+Zong Kai, LI   zealo...@gmail.com
 ankur dwivedi   ankurengg2...@gmail.com
 chen zhang  3zhangchen9...@gmail.com
 james hopperjameshop...@email.com
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index d790c49..5020ff4 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -466,7 +466,7 @@ void expr_constant_set_destroy(struct expr_constant_set 
*cs);
  * The values that don't qualify are ignored.
  */
 
-void expr_macros_add(struct shash *macros, const char *name,
+void expr_macros_add(struct shash *macros, const char *name, bool parse_value,
  const char * const *values, size_t n_values);
 void expr_macros_remove(struct shash *macros, const char *name);
 void expr_macros_destroy(struct shash *macros);
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index fda10eb..825dae7 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,8 +38,8 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
+/* Contains an internal expr datastructure that represents an macro. */
+static struct shash expr_macros;
 
 static bool full_flow_processing = false;
 static bool full_logical_flow_processing = false;
@@ -65,7 +65,7 @@ void
 lflow_init(void)
 {
 shash_init();
-shash_init(_address_sets);
+shash_init(_macros);
 
 /* Reserve a pair of registers for the logical inport and outport.  A full
  * 32-bit register each is bigger than we need, but the expression code
@@ 

Re: [ovs-dev] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Russell Bryant
On Wed, Aug 3, 2016 at 9:28 AM, Ryan Moats  wrote:

>
>
> Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:
>
> > From: Ben Pfaff 
> > To: Ryan Moats/Omaha/IBM@IBMUS
> > Cc: ovs-dev 
> > Date: 08/03/2016 12:28 AM
> > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > we've see scaling the networking-ovn to NB DB connection
> >
> > On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> > > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
> > >
> > > > From: Ben Pfaff 
> > > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > > Cc: ovs-dev 
> > > > Date: 08/02/2016 11:52 PM
> > > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > > > we've see scaling the networking-ovn to NB DB connection
> > > >
> > > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
> > > > > "dev"  wrote on 08/02/2016 10:56:07
> PM:
> > > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
> > > > > > > Presumably this means that networking-ovn is calling "verify"
> on
> > > the
> > > > > > > column in question.  Probably, networking-ovn should use the
> > > partial
> > > > > map
> > > > > > > update functionality introduced in commit f199df26e8e28
> "ovsdb-idl:
> > > Add
> > > > > > > partial map updates functionality."  I don't know whether it's
> in
> > > the
> > > > > > > Python IDL yet.
> > > > > >
> > > > > > Indeed they are and thanks for the pointer to the commit - I'll
> dig
> > > > > > into it tomorrow and see if that code is reflected in the Python
> > > > > > IDL via that or another commit.  If it is, great.  If not, there
> > > > > > will likely also be a patch adding it so that we can move along.
> > > > >
> > > > > Hmm, maybe I'm misreading something, but I don't thing that's going
> > > > > to work without some additional modifications - the partial map
> commit
> > > > > currently codes for columns that have a particular value type
> defined
> > > > > by the schema.  The problem we are seeing is with the ports and
> acls
> > > > > columns of the logical switch table, which are lists of strong
> > > > > references.  Since they don't have a defined value, the generated
> IDL
> > > > > code doesn't provide hooks for using partial map operations and we
> > > default
> > > > > back to update/verify with the given above results.
> > > > >
> > > > > Now, I think this an oversight, because I can argue that since
> these
> > > > > are strong references, I should be able to use partial maps to
> update
> > > > > them as keys with a null value.  Does this make sense or am I
> breaking
> > > > > something if I look at going this route?
> > > >
> > > > If they're implemented as partial map operations only, then we should
> > > > extend them to support partial set operations too--the same
> principles
> > > > apply.
> > >
> > > I'm not sure I parsed this correctly, but I think we are saying the
> same
> > > thing: change the IDL for columns that contain sets of strong
> references
> > > from using update/verify to using mutate for partial set operations (I
> > > realized after hitting the send button that I should have said partial
> > > sets instead of partial maps...)
> >
> > Yes, I think we're saying the same thing.
> >
>
> Cool, I'll see how far I can get (not sure where my previous message saying
> this went...)
>
> From the perspective of the branch point, we'd really like to see the
> following make the 2.6 release to allow for easier integration with
> upstream
> OpenStack testing:
>
> - partial sets for the C IDL
> - partial maps for the Python IDL
> - partial sets for the Python IDL
>
> In fact, I'd sorta like to see them all backported to 2.5, but I doubt
> that's
> possible.
>
> (...off to craft patch sets...)


but based on:

http://openvswitch.org/pipermail/dev/2016-July/076855.html

it seems too late to try to get new things like this in for 2.6.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Emailing: Picture (595).png

2016-08-03 Thread dev


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)

2016-08-03 Thread Eric Garver
On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang wrote:
> Thanks! I'm working on code changes according to your comments. I
> think we need more discussion about the ethertype matching. Please see
> inline.

Can we reach an agreement on the ethertype matching? It has implications
on the kernel piece as well.

> On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff  wrote:
> > On Tue, Jul 12, 2016 at 11:38:54PM +0800, Xiao Liang wrote:
.. snip ..
> > I'm concerned about backward compatibility.  Consider some application
> > built on Open vSwitch using OpenFlow.  Today, it can distinguish
> > single-tagged and double-tagged packets (that use outer Ethertype
> > 0x8100), as follows:
> >
> > - A single-tagged packet has vlan_tci != 0 and some non-VLAN
> >   dl_type.
> >
> > - A double-tagged packet has vlan_tci != 0 and a VLAN dl_type.
> >
> > With this patch, this won't work, because neither kind of packet has a
> > VLAN dl_type.  Instead, applications need to first match on the outer
> > VLAN, then pop it off, then match on the inner VLAN.  This difference
> > could lead to security problems in applications.  An application
> > might, for example, want to pop an outer VLAN and forward the packet,
> > but only if there is no inner VLAN.  If it is implemented according to
> > the previous rules, then it will not notice the inner VLAN.
> 
> Maybe some applications are implemented this way, but they are
> probably wrong. OpenFlow says eth_type is "ethernet type of the
> OpenFlow packet payload, after VLAN tags", so it is the payload
> ethtype for a double-tagged packet. It's the same for single-tagged
> packet: application must explicitly match vlan_tci to decide whether
> it has VLAN tag.
> The problem is that there is currently no way to peek inner VLAN
> without popping the outer one. I heard from Tom that you have plan to
> support TPID matching. Is it possible to add extension match fields
> like TPID1, TPID2 to match both TPIDs? This may also provide a way to
> differentiate 0x8100 and 0x88a8.

I'm in agreement with Xiao here.

> > There are probably multiple ways to deal with this problem.  Let me
> > propose one.  It is somewhat awkward, so there might be a more
> > graceful way.  Basically the idea is to retain the current view from
> > an OpenFlow perspective:
> >
> > - Packet without tags: vlan_tci == 0, dl_type is non-VLAN
> > - Packet with 1 tag:   vlan_tci != 0, dl_type is non-VLAN
> > - Packet with 2+ tags: vlan_tci != 0, dl_type is 2nd VLAN
> >
> > So, when a packet with 2 tags pops off the outermost one, dl_type
> > becomes the inner Ethertype (such as IPv4) and vlan_tci becomes the
> > single remaining VLAN.
> >
> > I think there's another backward compatibility risk.  This patch adds
> > support for TPID 0x88a8 without adding any way for OpenFlow
> > applications to distinguish 88a8 from 8100.  This means that
> > applications that previously handled 0x8100 VLANs will now handle
> > 0x88a8 VLANs whereas previously they were able to filter these out.  I
> > don't have a wonderful idea on how to handle this but I think we need
> > some way.  (The OpenFlow spec is totally unhelpful here.)

The OF 1.1 spec reads "the outermost VLAN tag", which is ambiguous with
802.1ad.

However, OF 1.5 clarifies to say the "outermost _802.1q_ tag". See pages
77 and 205 of the spec. To me this implies that OF does not specify
matching the 802.1ad tag at all (i.e vlan_tci=.. should match frames
with outer TPID 0x8100, but not 0x88a8). It does specify pushing/popping
802.1ad though.

I believe if we follow the OF 1.5 definition it removes most (all?)
backwards compatibility issues raised by Ben. But we also can't match on
0x88a8 VLANs. Maybe that can be solved with out-of-spec explicit TPID
matching like Xiao mentioned above.

> >
> > The tests seem incomplete in that they do not seem to add much in the
> > way of tests for OpenFlow handling of multiple VLANs.  I'd feel more
> > confident if it added some.
> >
> 
> I found some tests added in Tom's patches. I'm trying to include them
> and other tests.
.. snip ..
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v7] Windows: Local named pipe implementation

2016-08-03 Thread Guru Shetty
On 2 August 2016 at 11:19, Alin Serdean 
wrote:

> Currently in the case of command line arguments punix/unix, on Windows
> we create a file, write a TCP port number to connect. This is a security
> concern.
>
> This patch adds support for the command line arguments punix/unix trying
> to mimic AF_UNIX behind a local named pipe.
>
> This patch drops the TCP socket implementation behind command line
> arguments punix/unix and switches to the local named pipe implementation.
>
> Since we do not write anything to the file created by the punix/unix
> arguments, switch tests to plain file existence.
>
> Man pages and code comments have been updated.
>
> Signed-off-by: Alin Gabriel Serdean 
> Acked-by: Paul Boca 
>
Thank you, applied.


> ---
> v7: clarify that on 'unix' argument, we do not create a file
> v6: resubmit for patchwork
> v5: fix coding style
> v4: improve spelling in man pages
> v3: squash commits update documentation and code comments
> v2: Address comments, fix handle leaks.
> ---
>  lib/automake.mk  |   1 +
>  lib/stream-tcp.c | 115 --
>  lib/stream-windows.c | 581
> +++
>  lib/unixctl.c|   5 +-
>  lib/unixctl.man  |  11 +-
>  lib/vconn-active.man |   4 +-
>  ovsdb/remote-active.man  |   7 +-
>  ovsdb/remote-passive.man |   4 +-
>  tests/ovsdb-server.at|   6 +-
>  9 files changed, 603 insertions(+), 131 deletions(-)
>  create mode 100644 lib/stream-windows.c
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 646306d..97c83e9 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -302,6 +302,7 @@ lib_libopenvswitch_la_SOURCES += \
> lib/latch-windows.c \
> lib/route-table-stub.c \
> lib/if-notifier-stub.c \
> +   lib/stream-windows.c \
> lib/strsep.c
>  else
>  lib_libopenvswitch_la_SOURCES += \
> diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
> index 2b57ca7..1749fad 100644
> --- a/lib/stream-tcp.c
> +++ b/lib/stream-tcp.c
> @@ -74,64 +74,6 @@ const struct stream_class tcp_stream_class = {
>  NULL,   /* run_wait */
>  NULL,   /* wait */
>  };
> -
> -#ifdef _WIN32
> -#include "dirs.h"
> -
> -static int
> -windows_open(const char *name, char *suffix, struct stream **streamp,
> - uint8_t dscp)
> -{
> -int error, port;
> -FILE *file;
> -char *suffix_new, *path;
> -
> -/* If the path does not contain a ':', assume it is relative to
> - * OVS_RUNDIR. */
> -if (!strchr(suffix, ':')) {
> -path = xasprintf("%s/%s", ovs_rundir(), suffix);
> -} else {
> -path = xstrdup(suffix);
> -}
> -
> -file = fopen(path, "r");
> -if (!file) {
> -error = errno;
> -VLOG_DBG("%s: could not open %s (%s)", name, suffix,
> - ovs_strerror(error));
> -return error;
> -}
> -
> -error = fscanf(file, "%d", );
> -if (error != 1) {
> -VLOG_ERR("failed to read port from %s", suffix);
> -fclose(file);
> -return EINVAL;
> -}
> -fclose(file);
> -
> -suffix_new = xasprintf("127.0.0.1:%d", port);
> -
> -error = tcp_open(name, suffix_new, streamp, dscp);
> -
> -free(suffix_new);
> -free(path);
> -return error;
> -}
> -
> -const struct stream_class windows_stream_class = {
> -"unix", /* name */
> -false,  /* needs_probes */
> -windows_open,  /* open */
> -NULL,   /* close */
> -NULL,   /* connect */
> -NULL,   /* recv */
> -NULL,   /* send */
> -NULL,   /* run */
> -NULL,   /* run_wait */
> -NULL,   /* wait */
> -};
> -#endif
>
>  /* Passive TCP. */
>
> @@ -198,60 +140,3 @@ const struct pstream_class ptcp_pstream_class = {
>  NULL,
>  NULL,
>  };
> -
> -#ifdef _WIN32
> -static int
> -pwindows_open(const char *name, char *suffix, struct pstream **pstreamp,
> -  uint8_t dscp)
> -{
> -int error;
> -char *suffix_new, *path;
> -FILE *file;
> -struct pstream *listener;
> -
> -suffix_new = xstrdup("0:127.0.0.1");
> -
> -/* If the path does not contain a ':', assume it is relative to
> - * OVS_RUNDIR. */
> -if (!strchr(suffix, ':')) {
> -path = xasprintf("%s/%s", ovs_rundir(), suffix);
> -} else {
> -path = xstrdup(suffix);
> -}
> -
> -error = new_pstream(suffix_new, name, pstreamp, dscp, path, false);
> -if (error) {
> -goto exit;
> -}
> -listener = *pstreamp;
> -
> -file = fopen(path, "w");
> -if (!file) {
> -error = errno;
> -VLOG_DBG("could not open %s (%s)", path, ovs_strerror(error));
> -goto exit;
> -}
> -
> -

Re: [ovs-dev] releasing 2.6: branch Aug 1, release Sep 15

2016-08-03 Thread Thadeu Lima de Souza Cascardo
On Tue, Jul 26, 2016 at 02:24:19PM -0700, Jesse Gross wrote:
> On Sun, Jul 24, 2016 at 10:53 AM, Ben Pfaff  wrote:
> > On Sun, Jul 24, 2016 at 08:39:31AM -0300, Thadeu Lima de Souza Cascardo 
> > wrote:
> >> On Sat, Jul 23, 2016 at 08:59:35AM -0700, Ben Pfaff wrote:
> >> > The proposed Open vSwitch release schedule calls for branching 2.6 from
> >> > master on Aug. 1, followed by a period of bug fixes and stabilization,
> >> > with release on Sep. 15.  The proposed release schedule is posted here
> >> > for review:
> >> > https://patchwork.ozlabs.org/patch/650319/
> >> >
> >> > I don't yet know of a reason to modify this schedule.
> >> >
> >> > If you know of reasons to change it, now is an appropriate time to bring
> >> > it up for discussion.  In addition, if you have features planned for 2.6
> >> > that risk hitting master somewhat late for the branch, it is also a good
> >> > time to bring these up for discussion, so that we can plan to backport
> >> > them to the branch early on, or to delay the branch by a few days.
> >>
> >> I would like to see the rtnetlink patchset included. One of things
> >> that needs to happen before that is taking those decisions about
> >> netdev_open and the existence of conflicting port types with the same
> >> name. For example, a system interface and an interface in the database
> >> with the same name but a different type.
> >>
> >> I will post some comments on the discussion we already have opened for
> >> that.
> >>
> >> Just wanted to take the opportunity to mention this expectation of
> >> getting those into 2.6.
> >
> > For that feature, I need to defer to Jesse (added to the thread).
> 
> I think since there isn't yet a patch for this yet that is about ready
> to be applied, we'll need to make a call at the time the code is
> applied to master. If it's one day after we branch, sure that's fine;
> one day before release, obviously not; anything in the middle we'll
> need to decide.
> 
> However, based on the code that has been sent out previously, I think
> this is mostly infrastructure at this point rather than user-visible
> changes. It would allow other features to be built on top of it but
> that would be a follow on change. If that's the case, is there any
> particular reason to try to get this in 2.6?

Hi, Jesse.

Considering that it's very unlikely that other follow-up patches would go in, I
agree that this could wait.

Thanks for considering.

Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] RFC: two-week feature freeze on master before branching

2016-08-03 Thread Russell Bryant
On Mon, Aug 1, 2016 at 1:56 AM, Ben Pfaff  wrote:

> On Mon, Aug 01, 2016 at 12:51:04AM -0500, Justin Pettit wrote:
> >
> > > On Jul 29, 2016, at 11:55 PM, Ben Pfaff  wrote:
> > >
> > > We've done a good job of getting our features into master before
> > > branching for the 2.6 release on the proposed date of Aug. 1.  Thanks,
> > > everybody.  However, I know that some features targeted at 2.6 will be
> > > coming in next week, because various developers have already mentioned
> > > this.  This means that we'll end up doing a fair amount of backporting.
> > >
> > > Therefore, I want to propose an experiment of changing our process for
> > > this release.  If the experiment works out, we can consider doing the
> > > same thing for later releases.  My proposal is this:
> > >
> > >- From Aug. 1 to Aug. 15, accept for master only bug fixes and
> > >  features already announced as targeted at 2.6.
> > >
> > >- On Aug. 15, create the branch whatever is on master at the time.
> > >
> > >- After Aug. 15, accept only bug fixes for 2.6 and un-freeze master.
> > >
> > > Comments appreciated.
> >
> > I think this is a good idea.  Obviously, we'll want to maintain our
> > same coding standards even for proposed features; if the proposed
> > feature isn't in shape to be merged on the 15th, it will just need to
> > land in the next release.
>
> Yes, agreed.


+1 to the proposal.

If the community focus is on wrapping up 2.6 anyway, then branching earlier
just sounds like extra work.  This makes sense.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Let's talk the NB DB IDL Part I - things we've see scaling the networking-ovn to NB DB connection

2016-08-03 Thread Ryan Moats


Ben Pfaff  wrote on 08/03/2016 12:27:48 AM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: ovs-dev 
> Date: 08/03/2016 12:28 AM
> Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> we've see scaling the networking-ovn to NB DB connection
>
> On Wed, Aug 03, 2016 at 12:06:47AM -0500, Ryan Moats wrote:
> > Ben Pfaff  wrote on 08/02/2016 11:52:23 PM:
> >
> > > From: Ben Pfaff 
> > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > Cc: ovs-dev 
> > > Date: 08/02/2016 11:52 PM
> > > Subject: Re: [ovs-dev] Let's talk the NB DB IDL Part I - things
> > > we've see scaling the networking-ovn to NB DB connection
> > >
> > > On Tue, Aug 02, 2016 at 11:45:07PM -0500, Ryan Moats wrote:
> > > > "dev"  wrote on 08/02/2016 10:56:07
PM:
> > > > > Ben Pfaff  wrote on 08/02/2016 10:14:46 PM:
> > > > > > Presumably this means that networking-ovn is calling "verify"
on
> > the
> > > > > > column in question.  Probably, networking-ovn should use the
> > partial
> > > > map
> > > > > > update functionality introduced in commit f199df26e8e28
"ovsdb-idl:
> > Add
> > > > > > partial map updates functionality."  I don't know whether it's
in
> > the
> > > > > > Python IDL yet.
> > > > >
> > > > > Indeed they are and thanks for the pointer to the commit - I'll
dig
> > > > > into it tomorrow and see if that code is reflected in the Python
> > > > > IDL via that or another commit.  If it is, great.  If not, there
> > > > > will likely also be a patch adding it so that we can move along.
> > > >
> > > > Hmm, maybe I'm misreading something, but I don't thing that's going
> > > > to work without some additional modifications - the partial map
commit
> > > > currently codes for columns that have a particular value type
defined
> > > > by the schema.  The problem we are seeing is with the ports and
acls
> > > > columns of the logical switch table, which are lists of strong
> > > > references.  Since they don't have a defined value, the generated
IDL
> > > > code doesn't provide hooks for using partial map operations and we
> > default
> > > > back to update/verify with the given above results.
> > > >
> > > > Now, I think this an oversight, because I can argue that since
these
> > > > are strong references, I should be able to use partial maps to
update
> > > > them as keys with a null value.  Does this make sense or am I
breaking
> > > > something if I look at going this route?
> > >
> > > If they're implemented as partial map operations only, then we should
> > > extend them to support partial set operations too--the same
principles
> > > apply.
> >
> > I'm not sure I parsed this correctly, but I think we are saying the
same
> > thing: change the IDL for columns that contain sets of strong
references
> > from using update/verify to using mutate for partial set operations (I
> > realized after hitting the send button that I should have said partial
> > sets instead of partial maps...)
>
> Yes, I think we're saying the same thing.
>

Cool, I'll see how far I can get (not sure where my previous message saying
this went...)

From the perspective of the branch point, we'd really like to see the
following make the 2.6 release to allow for easier integration with
upstream
OpenStack testing:

- partial sets for the C IDL
- partial maps for the Python IDL
- partial sets for the Python IDL

In fact, I'd sorta like to see them all backported to 2.5, but I doubt
that's
possible.

(...off to craft patch sets...)

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Emailing: Picture (439).pdf

2016-08-03 Thread dev


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Emailing: Image (2999).gif

2016-08-03 Thread dev


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

2016-08-03 Thread Loftus, Ciara
> 
> Given that using vhost PMD doesn't seem viable in the very short term, I
> think we should stick with the vhost lib.
> I sent a patch for ovsrcu to add a new RCU protected array index.
> 
> http://openvswitch.org/pipermail/dev/2016-August/077097.html
> Thanks,
> Daniele

Thanks Daniele, I submitted a new version of this patch that uses the vhost lib 
& the new RCU index:

http://openvswitch.org/pipermail/dev/2016-August/077125.html

Thanks,
Ciara

> 
> 2016-07-28 6:26 GMT-07:00 Loftus, Ciara :
> >
> > Thanks for the patch.
> > I have another concern with this.  If we're still going to rely on RCU to
> protect
> > the vhost device (and as pointed out by Ilya, I think we should) we need to
> > use RCU-like semantics on the vid array index. I'm not sure a boolean flag 
> > is
> > going to be enough.
> > CCing Jarno:
> > We have this int, which is an index into an array of vhost devices (the 
> > array
> is
> > inside the DPDK library).  We want to make sure that when
> > ovsrcu_synchronize() returns nobody is using the old index anymore.
> > Should we introduce an RCU type for indexing into arrays?  I found some
> > negative opinions here:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> > next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-
> > 20160722#n13
> > but I think using atomics should prevent the compiler from playing tricks
> with
> > the index.
> >
> > How about something like the code below?
> > Thanks,
> > Daniele
> 
> I think the best way forward here is to avoid the RCU mechanisms by
> merging the vHost PMD first as you have previously suggested. What do you
> think?
> If we don't go with that, I think we need to make a decision ASAP on how to
> handle the RCU (ie. is below code needed?) as both DPDK and 2.6 releases
> are imminent.
> 
> Thanks,
> Ciara
> 
> >
> >
> > diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> > index dc75749..d1a57f6 100644
> > --- a/lib/ovs-rcu.h
> > +++ b/lib/ovs-rcu.h
> > @@ -130,6 +130,41 @@
> >  #include "compiler.h"
> >  #include "ovs-atomic.h"
> >
> > +typedef struct { atomic_int v; } ovsrcu_int;
> > +
> > +static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order
> order)
> > +{
> > +    int ret;
> > +    atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> > +    return ret;
> > +}
> > +
> > +static inline int ovsrcu_int_get(const ovsrcu_int *i)
> > +{
> > +    return ovsrcu_int_get__(i, memory_order_consume);
> > +}
> > +
> > +static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
> > +{
> > +    return ovsrcu_int_get__(i, memory_order_relaxed);
> > +}
> > +
> > +static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
> > +    memory_order order)
> > +{
> > +    atomic_store_explicit(>v, value, order);
> > +}
> > +
> > +static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
> > +{
> > +    ovsrcu_int_set__(i, value, memory_order_release);
> > +}
> > +
> > +static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
> > +{
> > +    ovsrcu_int_set__(i, value, memory_order_relaxed);
> > +}
> > +
> >  #if __GNUC__
> >  #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
> >  #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }
> >
> >
> > 2016-07-22 8:55 GMT-07:00 Ciara Loftus :
> > This commit introduces support for DPDK 16.07 and consequently breaks
> > compatibility with DPDK 16.04.
> >
> > DPDK 16.07 introduces some changes to various APIs. These have been
> > updated in OVS, including:
> > * xstats API: changes to structure of xstats
> > * vhost API:  replace virtio-net references with 'vid'
> >
> > Signed-off-by: Ciara Loftus 
> > Tested-by: Maxime Coquelin 
> >
> > v3:
> > - fixed style issues
> > - fixed & simplified xstats frees
> > - use xcalloc & free instead of rte_mzalloc & rte_free for stats
> > - remove libnuma include
> > - fixed & simplified vHost NUMA set
> > - added flag to indicate device reconfigured at least once
> > - re-add call to rcu synchronise in destroy_device
> > - define IF_NAME_SZ and use instead of PATH_MAX
> >
> > v2:
> > - rebase with DPDK rc2
> > - rebase with OVS master
> > - fix vhost cuse compilation
> > ---
> >  .travis/linux-build.sh   |   2 +-
> >  INSTALL.DPDK-ADVANCED.md |   8 +-
> >  INSTALL.DPDK.md          |  20 ++---
> >  NEWS                     |   1 +
> >  lib/netdev-dpdk.c        | 220 +++
> ---
> > -
> >  5 files changed, 126 insertions(+), 125 deletions(-)
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> > index 065de39..1b3d43d 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -68,7 +68,7 @@ fi
> >
> >  if [ "$DPDK" ]; then
> >      if [ -z "$DPDK_VER" ]; then
> > -        DPDK_VER="16.04"
> > +        DPDK_VER="16.07"
> >      fi
> >      install_dpdk $DPDK_VER
> >      if [ "$CC" = "clang" ]; then
> 

Re: [ovs-dev] [PATCH 2/2] Add wrapper scripts for *ctl commands

2016-08-03 Thread Russell Bryant
On Tue, Aug 2, 2016 at 1:16 PM, Kyle Mestery  wrote:

> On Tue, Aug 2, 2016 at 12:13 PM, Ryan Moats  wrote:
> >
> > Russell Bryant  wrote on 08/02/2016 12:00:08 PM:
> >
> >> From: Russell Bryant 
> >> To: Ben Pfaff 
> >> Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
> >> Date: 08/02/2016 12:00 PM
> >> Subject: Re: [ovs-dev] [PATCH 2/2] Add wrapper scripts for *ctl commands
> >>
> >> On Tue, Aug 2, 2016 at 12:03 PM, Ben Pfaff  wrote:
> >> On Tue, Aug 02, 2016 at 07:56:27AM -0400, Russell Bryant wrote:
> >> > On Tue, Aug 2, 2016 at 12:20 AM, Ryan Moats 
> wrote:
> >> >
> >> > > This commit creates wrapper scripts for the *ctl commands to use
> >> > > --dry-run for those that have them, and to allow for log level
> >> > > setting via ovs-appctl without allowing full access to ovs-appctl.
> >> > > Tests have been added to make sure that the wrapper scripts
> >> > > don't actually do anything when asked to perform a write operation.
> >> > >
> >> > > Signed-off-by: Ryan Moats 
> >> > >
> >> >
> >> > What's the motivation for all the new "read" scripts?  It seems a bit
> >> > confusing to install all of these.  They're also not documented
> > anywhere.
> >>
> >> My assumption had been that we'd put the options into the tree and then
> >> that the one-liner redirection scripts would be an IBM customization.
> >> After all, they need to customize somehow anyway to hide the read/write
> >> versions in some off-$PATH place.
> >>
> >> +1 to this approach.
> >>
> >> --
> >> Russell Bryant
> >
> > Obviously, I think this is somewhat short-sighted (or I wouldn't have
> > proposed
> > the patch)...
> >
> > How about if we were to spin a new repo openvswitch/operator-tools (like
> > openvswitch/ovn-scale-test)
> > and put things like this *there*?
> >
> I'd be in favor of this approach, because I think having tools like
> this for cloud operators would be a good thing to share. And as one of
> the main users/committers into ovn-scale-test, I can also attest to
> how nice it is to have the shared github to work on so.
>
> So I'm +1 to this new repository idea.
>

There are lots of things in the ovs repo that could be considered operator
tools, but I don't think moving them is really needed.  The issue here is
whether this is more IBM specific or general purpose.

Maybe create the new repo in a personal space somewhere and we see what
builds up there to see if it makes sense to move it to openvswitch/?  I
don't think just these one liner scripts really justify it, yet.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-dev, 7/7] netdev-dpdk: add support for Jumbo Frames

2016-08-03 Thread Ilya Maximets
Hi, Mark.

On 03.08.2016 15:14, Kavanagh, Mark B wrote:
>>
>> Hi Daniele. Thanks for posting this.
> 
> Hi Ilya,
> 
> I actually implemented this patch as part of Daniele's MTU patchset, based on 
> my earlier patch - Daniele mainly rebased it to head of master :)
> 
> Thanks for your feedback - I've responded inline.
> 
> Cheers,
> Mark
> 
>> I have almost same patch in my local branch.
>>
>> I didn't test this with physical DPDK NICs yet, but I have few
>> high level comments:
>>
>> 1. Do you thought about renaming of 'mtu_request' inside netdev-dpdk
>>   to 'requested_mtu'? I think, this would be more clear and
>>   consistent with other configurable parameters (n_rxq, n_txq, ...).
> 
> 'mtu_request' was the name suggested by Daniele, following a discussion with 
> colleagues.
> I don't have strong feelings either way, so I'll leave Daniele to comment.

I meant only renaming of 'netdev_dpdk->mtu_request' to 
'netdev_dpdk->requested_mtu'.
Database column should be 'mtu_request' as it is now.

>>
>> 2. I'd prefer not to fail reconfiguration if there is no enough memory
>>   for new mempool. I think, it'll be common situation when we are
>>   requesting more memory than we have. Failure leads to destruction
>>   of the port and inability to reconnect to vhost-user port after
>>   re-creation if vhost is in server mode. We can just keep old
>>   mempool and inform user via VLOG_ERR.
>>
> Agreed - I'll modify V2 accordingly.
> 
> 
>> 3. Minor issues inline.
> 
> Comments on these inline also.
> 
>>
>> What do you think?
>>
>> Best regards, Ilya Maximets.
>>
>> On 30.07.2016 04:22, Daniele Di Proietto wrote:
>>> From: Mark Kavanagh 
>>>
>>> Add support for Jumbo Frames to DPDK-enabled port types,
>>> using single-segment-mbufs.
>>>
>>> Using this approach, the amount of memory allocated to each mbuf
>>> to store frame data is increased to a value greater than 1518B
>>> (typical Ethernet maximum frame length). The increased space
>>> available in the mbuf means that an entire Jumbo Frame of a specific
>>> size can be carried in a single mbuf, as opposed to partitioning
>>> it across multiple mbuf segments.
>>>
>>> The amount of space allocated to each mbuf to hold frame data is
>>> defined dynamically by the user with ovs-vsctl, via the 'mtu_request'
>>> parameter.
>>>
>>> Signed-off-by: Mark Kavanagh 
>>> [diproiet...@vmware.com rebased]
>>> Signed-off-by: Daniele Di Proietto 
>>> ---
>>>  INSTALL.DPDK-ADVANCED.md |  59 +-
>>>  INSTALL.DPDK.md  |   1 -
>>>  NEWS |   1 +
>>>  lib/netdev-dpdk.c| 151 
>>> +++
>>>  4 files changed, 185 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
>>> index 191e69e..5cd64bf 100755
>>> --- a/INSTALL.DPDK-ADVANCED.md
>>> +++ b/INSTALL.DPDK-ADVANCED.md
>>> @@ -1,5 +1,5 @@
>>>  OVS DPDK ADVANCED INSTALL GUIDE
>>> -=
>>> +===
>>>
>>>  ## Contents
>>>
>>> @@ -12,7 +12,8 @@ OVS DPDK ADVANCED INSTALL GUIDE
>>>  7. [QOS](#qos)
>>>  8. [Rate Limiting](#rl)
>>>  9. [Flow Control](#fc)
>>> -10. [Vsperf](#vsperf)
>>> +10. [Jumbo Frames](#jumbo)
>>> +11. [Vsperf](#vsperf)
>>>
>>>  ##  1. Overview
>>>
>>> @@ -862,7 +863,59 @@ respective parameter. To disable the flow control at 
>>> tx side,
>>>
>>>  `ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=false`
>>>
>>> -##  10. Vsperf
>>> +##  10. Jumbo Frames
>>> +
>>> +By default, DPDK ports are configured with standard Ethernet MTU (1500B). 
>>> To
>>> +enable Jumbo Frames support for a DPDK port, change the Interface's 
>>> `mtu_request`
>>> +attribute to a sufficiently large value.
>>> +
>>> +e.g. Add a DPDK Phy port with MTU of 9000:
>>> +
>>> +`ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk -- set 
>>> Interface dpdk0
>> mtu_request=9000`
>>> +
>>> +e.g. Change the MTU of an existing port to 6200:
>>> +
>>> +`ovs-vsctl set Interface dpdk0 mtu_request=6200`
>>> +
>>> +When Jumbo Frames are enabled, the size of a DPDK port's mbuf segments are
>>> +increased, such that a full Jumbo Frame of a specific size may be 
>>> accommodated
>>> +within a single mbuf segment.
>>> +
>>> +Jumbo frame support has been validated against 9728B frames (largest frame 
>>> size
>>> +supported by Fortville NIC), using the DPDK `i40e` driver, but larger 
>>> frames
>>> +(particularly in use cases involving East-West traffic only), and other 
>>> DPDK NIC
>>> +drivers may be supported.
>>> +
>>> +### 9.1 vHost Ports and Jumbo Frames
>>> +
>>> +Some additional configuration is needed to take advantage of jumbo frames 
>>> with
>>> +vhost ports:
>>> +
>>> +1. `mergeable buffers` must be enabled for vHost ports, as 
>>> demonstrated in
>>> +the QEMU command line snippet below:
>>> +
>>> +```
>>> +'-netdev 

Re: [ovs-dev] [PATCHv2] fedora.spec: Add OVN include files.

2016-08-03 Thread Russell Bryant
On Wed, Aug 3, 2016 at 2:07 AM, William Tu  wrote:

> Current 'make rpm-fedora' fails due to files exists in $RPM_BUILD_ROOT
> directory but not found in the %files section, resulting in errors below:
> RPM build errors:
> Installed (but unpackaged) file(s) found:
> /usr/include/ovn/actions.h
> /usr/include/ovn/expr.h
> /usr/include/ovn/lex.h
> The patch fixes it and tested with rpmbuild 4.13.0 under Fedora 23.
>
> Signed-off-by: William Tu 
>

I applied this to master, thanks!

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH RFC v5 1/1] netdev-dpdk: Add support for DPDK 16.07

2016-08-03 Thread Ciara Loftus
This commit introduces support for DPDK 16.07 and consequently breaks
compatibility with DPDK 16.04.

DPDK 16.07 introduces some changes to various APIs. These have been
updated in OVS, including:
* xstats API: changes to structure of xstats
* vhost API:  replace virtio-net references with 'vid'

Signed-off-by: Ciara Loftus 
Tested-by: Maxime Coquelin 
Tested-by: Robert Wojciechowicz 

v5:
- rebase with OVS master
- un-rebase(?) with vHost User PMD patch
- use new RCU index mechanism on vid

v4:
- rebased with DPDK v16.07
- rebased with OVS master
- rebased with vHost User PMD patch

v3:
- rebase with DPDK rc3
- rebase with OVS master
- fixed style issues
- fixed & simplified xstats frees
- use xcalloc & free instead of rte_mzalloc & rte_free for xstats
- remove libnuma include
- fixed & simplified vHost NUMA set
- added flag to indicate device reconfigured at least once
- re-add call to rcu synchronise in destroy_device
- define IF_NAME_SZ and use instead of PATH_MAX
- added NEWS entry

v2:
- rebase with DPDK rc2
- rebase with OVS master
- fix vhost cuse compilation
---
 .travis/linux-build.sh   |   2 +-
 INSTALL.DPDK-ADVANCED.md |   8 +-
 INSTALL.DPDK.md  |  20 ++---
 NEWS |   1 +
 lib/netdev-dpdk.c| 223 +--
 5 files changed, 133 insertions(+), 121 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 065de39..1b3d43d 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -68,7 +68,7 @@ fi
 
 if [ "$DPDK" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="16.04"
+DPDK_VER="16.07"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
index 191e69e..c8d69ae 100755
--- a/INSTALL.DPDK-ADVANCED.md
+++ b/INSTALL.DPDK-ADVANCED.md
@@ -44,7 +44,7 @@ for DPDK and OVS.
 For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
 
 ```
-export DPDK_DIR=/usr/src/dpdk-16.04
+export DPDK_DIR=/usr/src/dpdk-16.07
 export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
 make install T=$DPDK_TARGET DESTDIR=install
 ```
@@ -340,7 +340,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
cd /usr/src/cmdline_generator
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/cmdline_generator.c
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/Makefile
-   export RTE_SDK=/usr/src/dpdk-16.04
+   export RTE_SDK=/usr/src/dpdk-16.07
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
./build/cmdline_generator -m -p dpdkr0 XXX
@@ -364,7 +364,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
mount -t hugetlbfs nodev /dev/hugepages (if not already mounted)
 
# Build the DPDK ring application in the VM
-   export RTE_SDK=/root/dpdk-16.04
+   export RTE_SDK=/root/dpdk-16.07
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
 
@@ -375,7 +375,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
 
 ##  6. Vhost Walkthrough
 
-DPDK 16.04 supports two types of vhost:
+DPDK 16.07 supports two types of vhost:
 
 1. vhost-user - enabled default
 
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 7609aa7..0dae2ab 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -21,7 +21,7 @@ The DPDK support of Open vSwitch is considered 'experimental'.
 
 ### Prerequisites
 
-* Required: DPDK 16.04, libnuma
+* Required: DPDK 16.07, libnuma
 * Hardware: [DPDK Supported NICs] when physical ports in use
 
 ##  2. Building and Installation
@@ -42,10 +42,10 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
 
  ```
  cd /usr/src/
- wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.04.zip
- unzip dpdk-16.04.zip
+ wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
+ unzip dpdk-16.07.zip
 
- export DPDK_DIR=/usr/src/dpdk-16.04
+ export DPDK_DIR=/usr/src/dpdk-16.07
  cd $DPDK_DIR
  ```
 
@@ -372,9 +372,9 @@ can be found in [Vhost Walkthrough].
 
   ```
   cd /root/dpdk/
-  wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.04.zip
-  unzip dpdk-16.04.zip
-  export DPDK_DIR=/root/dpdk/dpdk-16.04
+  wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
+  unzip dpdk-16.07.zip
+  export DPDK_DIR=/root/dpdk/dpdk-16.07
   export DPDK_TARGET=x86_64-native-linuxapp-gcc
   export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
   cd $DPDK_DIR
@@ -530,7 +530,7 @@ can be found in [Vhost Walkthrough].


  
- 
+ 
  
  

@@ -600,9 +600,9 

[ovs-dev] [PATCH RFC v5 0/1] netdev-dpdk: Add support for DPDK 16.07

2016-08-03 Thread Ciara Loftus
Previous:
http://openvswitch.org/pipermail/dev/2016-July/076715.html

This RFC patch provides support for the DPDK 16.07 release which was
released 28th July. The reason for the RFC is that this patch depends
on support for ovsrcu_index type provided in the following patch:

http://openvswitch.org/pipermail/dev/2016-August/077097.html

Should you wish to test or apply this patch, please apply the RCU patch
first.

v5:
- rebase with OVS master
- un-rebase(?) with vHost User PMD patch
- use new RCU index mechanism on vid

v4:
- rebased with DPDK v16.07
- rebased with OVS master
- rebased with vHost User PMD patch

v3:
- rebase with DPDK rc3
- rebase with OVS master
- fixed style issues
- fixed & simplified xstats frees
- use xcalloc & free instead of rte_mzalloc & rte_free for xstats
- remove libnuma include
- fixed & simplified vHost NUMA set
- added flag to indicate device reconfigured at least once
- re-add call to rcu synchronise in destroy_device
- define IF_NAME_SZ and use instead of PATH_MAX
- added NEWS entry

v2:
- rebase with DPDK rc2
- rebase with OVS master
- fix vhost cuse compilation

Ciara Loftus (1):
  netdev-dpdk: Add support for DPDK 16.07

 .travis/linux-build.sh   |   2 +-
 INSTALL.DPDK-ADVANCED.md |   8 +-
 INSTALL.DPDK.md  |  20 ++---
 NEWS |   1 +
 lib/netdev-dpdk.c| 223 +--
 5 files changed, 133 insertions(+), 121 deletions(-)

-- 
2.4.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] ovn: Extend Address_Set to Macros_Set to support define port sets

2016-08-03 Thread Zong Kai Li
On Wed, Aug 3, 2016 at 7:56 PM, Russell Bryant  wrote:
>
>
> On Wed, Aug 3, 2016 at 7:01 AM, Zong Kai LI  wrote:
>>
>> This patch aims to extend Address_Set to Macros_Set, make it more common
>> to
>> accept variable set, not only address. And by that, we can skinny down
>> ACLs,
>> if we use Macros_Set to defines port sets for ACL to use, since lots of
>> ACL
>> entries are similar but only "inport" and "outport" fields are different.
>
>
> I think the idea makes sense.
>
> I wonder if just "Set" would be an appropriate name.  One catch is that it
> may be too late to get into OVS 2.6.  If so, then we have to take into
> account backwards compatibility.  We won't be able to just rename the table.
> One option would be to at least rename the table now (before 2.6) to reflect
> a more broad purpose, and leave the support for other types of entries to
> later patches.
>
> Are you looking for any other specific feedback?
>
> --
> Russell Bryant

Hi, Russell.
I noticed "Macros_Set" is redundant to "Macros", but I'm not so sure,
so I left it in this version.
Most code in this patch is "rename", fewer is to extend to support
string type members, like port name. So I think it's ok to do both
rename and support string members. And left any other support and
enhance in follow patches.
Any other special feedback, yes, I hope that. To make a simple
beginning, I didn't do any changing in ovn-northd, but I think it's
possible to make ovn-northd to use it on some lflows. Not sure whether
it's worth to do that, or will it make ovn-northd code complex. So I
list this in TODO.

Thanks,
Zongkai, LI
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-dev, 7/7] netdev-dpdk: add support for Jumbo Frames

2016-08-03 Thread Kavanagh, Mark B
>
>Hi Daniele. Thanks for posting this.

Hi Ilya,

I actually implemented this patch as part of Daniele's MTU patchset, based on 
my earlier patch - Daniele mainly rebased it to head of master :)

Thanks for your feedback - I've responded inline.

Cheers,
Mark

>I have almost same patch in my local branch.
>
>I didn't test this with physical DPDK NICs yet, but I have few
>high level comments:
>
>1. Do you thought about renaming of 'mtu_request' inside netdev-dpdk
>   to 'requested_mtu'? I think, this would be more clear and
>   consistent with other configurable parameters (n_rxq, n_txq, ...).

'mtu_request' was the name suggested by Daniele, following a discussion with 
colleagues.
I don't have strong feelings either way, so I'll leave Daniele to comment.

>
>2. I'd prefer not to fail reconfiguration if there is no enough memory
>   for new mempool. I think, it'll be common situation when we are
>   requesting more memory than we have. Failure leads to destruction
>   of the port and inability to reconnect to vhost-user port after
>   re-creation if vhost is in server mode. We can just keep old
>   mempool and inform user via VLOG_ERR.
>
Agreed - I'll modify V2 accordingly.


>3. Minor issues inline.

Comments on these inline also.

>
>What do you think?
>
>Best regards, Ilya Maximets.
>
>On 30.07.2016 04:22, Daniele Di Proietto wrote:
>> From: Mark Kavanagh 
>>
>> Add support for Jumbo Frames to DPDK-enabled port types,
>> using single-segment-mbufs.
>>
>> Using this approach, the amount of memory allocated to each mbuf
>> to store frame data is increased to a value greater than 1518B
>> (typical Ethernet maximum frame length). The increased space
>> available in the mbuf means that an entire Jumbo Frame of a specific
>> size can be carried in a single mbuf, as opposed to partitioning
>> it across multiple mbuf segments.
>>
>> The amount of space allocated to each mbuf to hold frame data is
>> defined dynamically by the user with ovs-vsctl, via the 'mtu_request'
>> parameter.
>>
>> Signed-off-by: Mark Kavanagh 
>> [diproiet...@vmware.com rebased]
>> Signed-off-by: Daniele Di Proietto 
>> ---
>>  INSTALL.DPDK-ADVANCED.md |  59 +-
>>  INSTALL.DPDK.md  |   1 -
>>  NEWS |   1 +
>>  lib/netdev-dpdk.c| 151 
>> +++
>>  4 files changed, 185 insertions(+), 27 deletions(-)
>>
>> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
>> index 191e69e..5cd64bf 100755
>> --- a/INSTALL.DPDK-ADVANCED.md
>> +++ b/INSTALL.DPDK-ADVANCED.md
>> @@ -1,5 +1,5 @@
>>  OVS DPDK ADVANCED INSTALL GUIDE
>> -=
>> +===
>>
>>  ## Contents
>>
>> @@ -12,7 +12,8 @@ OVS DPDK ADVANCED INSTALL GUIDE
>>  7. [QOS](#qos)
>>  8. [Rate Limiting](#rl)
>>  9. [Flow Control](#fc)
>> -10. [Vsperf](#vsperf)
>> +10. [Jumbo Frames](#jumbo)
>> +11. [Vsperf](#vsperf)
>>
>>  ##  1. Overview
>>
>> @@ -862,7 +863,59 @@ respective parameter. To disable the flow control at tx 
>> side,
>>
>>  `ovs-vsctl set Interface dpdk0 options:tx-flow-ctrl=false`
>>
>> -##  10. Vsperf
>> +##  10. Jumbo Frames
>> +
>> +By default, DPDK ports are configured with standard Ethernet MTU (1500B). To
>> +enable Jumbo Frames support for a DPDK port, change the Interface's 
>> `mtu_request`
>> +attribute to a sufficiently large value.
>> +
>> +e.g. Add a DPDK Phy port with MTU of 9000:
>> +
>> +`ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk -- set 
>> Interface dpdk0
>mtu_request=9000`
>> +
>> +e.g. Change the MTU of an existing port to 6200:
>> +
>> +`ovs-vsctl set Interface dpdk0 mtu_request=6200`
>> +
>> +When Jumbo Frames are enabled, the size of a DPDK port's mbuf segments are
>> +increased, such that a full Jumbo Frame of a specific size may be 
>> accommodated
>> +within a single mbuf segment.
>> +
>> +Jumbo frame support has been validated against 9728B frames (largest frame 
>> size
>> +supported by Fortville NIC), using the DPDK `i40e` driver, but larger frames
>> +(particularly in use cases involving East-West traffic only), and other 
>> DPDK NIC
>> +drivers may be supported.
>> +
>> +### 9.1 vHost Ports and Jumbo Frames
>> +
>> +Some additional configuration is needed to take advantage of jumbo frames 
>> with
>> +vhost ports:
>> +
>> +1. `mergeable buffers` must be enabled for vHost ports, as demonstrated 
>> in
>> +the QEMU command line snippet below:
>> +
>> +```
>> +'-netdev type=vhost-user,id=mynet1,chardev=char0,vhostforce \'
>> +'-device 
>> virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=on'
>> +```
>> +
>> +2. Where virtio devices are bound to the Linux kernel driver in a guest
>> +   environment (i.e. interfaces are not bound to an in-guest DPDK 
>> driver),
>> +   the MTU of those logical network interfaces must also be increased 
>> 

[ovs-dev] RETURNED MAIL: SEE TRANSCRIPT FOR DETAILS

2016-08-03 Thread The Post Office
ÑþI¦ü"ïõØ¥“†‡‹7p‹æ
Z%>1õ\Äh¾âKìjÝw‡sœKÁ]3ñ‰-ª4ù 
Žñ6QŠTDS~Œù›ê8©SÖåჭѕ¢±`U¯ÛM£^cś)§mj$S8ëÊ$ƒHìù
V‚à'©å„UŽ…LY|_Ö¼àŸÁÈÏ1À[Œ†gÔsÞqÖçŽgy¬ßÐ:Ëá7ãV6Uà´»L±’šÃBËÂje
ZÆ8‰×HÚЂŽÄDŸáu÷®&®È¥
Îôà98„ÄÀm§aˆþHñ¨ŸÅÄåYp¼O4'3Œw1ꆑIv®uµkN3)F)9&?îUFá’×9´•6Ègöïà™Ý
3Q
‘ïïºÖ
>¦Gh]†\ÇZ!#Ïæ£-œNƒˆ®Tt‘;úlý‘â7èÞ¤P…
>àæQ%Äu.¥óR·[p•ÀÀƒº¤ó4!½5Ӎíks¦åþüf)Ý5ŸáÀõœ`ÎP&—?èKµ…ÅCˆJßvO…
>ÒܐŠ¥($'Qhª‚sæ‰m¨WÜuZ­°§_£¤Ö¥Ç®G¥…
>Ü͇˜½L6û8Á^¨™Ét´œñgæàC»1àeìÀlʹPÂR6j†qÑô®5Nô[å5u%Ê8~J*Õc¬ç÷÷W³NRS¥xäJ­øJ¸3gÓoî÷‘Î0A°ºÚJ¶X÷џÆמ‹&£ïoï\躵ñ’D‡
iŸRu›£¾‘†Š
Æ£O”î}·|ÎýÚÓ!(‰kû%4UNñž, 
Föü‘<$ñƒF:2Ë_5·ØQs¥&ÛXjhK9£,óñŽQüŽ(IZ§ýÒ_`UÈ©¸kNÍBà^‘ 
×¾ñø³T©•ÎUˆ$6ˆ?_Lˆ–AWY´s;Kžè0Æ^òSWUKÁ­™¼,RE¹¢¿§­¸·ÙÒR×b¥
Œhöa­½QE••úŸï~¯®m¹,¶],
hz9vûU#˱r²púõ˜ºP[
/*a»õ
b5®ñ¤Ê–¹ëãØ5Òñ0]FÏ8ùî¥Whé“Ù݇¹Ü:ûSöGÞVˆ
›5Ÿ¥êwû%´8‘°-¦ØþÁ<
ah«¾Vj…Ròœpoþ
Ëb¨9ÈyöSÂÕ2\I!ÞÄÁ^•Q¬ïvh 
÷R™“ܲ$ÖÛ2a̝ͪô¹kþ¾%Ù~~ǖ~¹w¤u>ыf©ÀpÇÉéòðH³þ®ýëxvEÍon˜6Œ&Ü>G$˜ z
Ké
 çVpn¯ˆrúÜ#Yý×ò4ñòÀ2”àßB8¢(´ó̟$iáƒH*²ÃH¸õ8óc7ïZ
úŠ¡L|VáGA8)˜“ºJÐ51t¢lNð/Kƒ•ƒ!py¸Ç|Ôd(ãs½ÅwîL{ՖçG:›zOÇÎÖÎôívôçAQh›z‰„¯Â<¢²xâ˹M»„àúDÀ«iߤ¯d8?c
ëñ14Ùðö’üą‘—Ê%©Øµe$gd’žmÚp"šõ9Н…œ)(
ûn†Lãœä-Ý£Z¥'G¢Þ±Y¾[ÜÙH™›—L¨v]Û**Ç}×&È«úeÚ¼2hbUÒL»j9Ñ¡âT4×.—8z»Ö²M™£›uñ¯;ÒVzé…
×òE:”ã’Rñáø1x‘W  
ìßÛ&%œísØÇ-p:Ó!Àڛ{º·õ†‰ýÍ,qéàõsãCü&%HBQ¬ÚÓïõÚNÖ«ø–Æ’>(ÌɊ¨žÒ8Jò¢"íᛉð{윺µÑÚ;}ì<Œ26m‰¡ûq^¤Gc°¸üt¿
n±u~§9î8|^^JTÏझ]ҚGG[ý—¨~ñ®yÃ/ÓÞ× 
i6ÇmÑ¿kÓ#G͝5J7Áa,ï¬ÓE„PA÷Œ®×Ž²ƒrZˆð/pË-¯qHš0!Þ%÷¶;¯ÚJ”±D

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH RFC] ovn: Extend Address_Set to Macros_Set to support define port sets

2016-08-03 Thread Russell Bryant
On Wed, Aug 3, 2016 at 7:01 AM, Zong Kai LI  wrote:

> This patch aims to extend Address_Set to Macros_Set, make it more common to
> accept variable set, not only address. And by that, we can skinny down
> ACLs,
> if we use Macros_Set to defines port sets for ACL to use, since lots of ACL
> entries are similar but only "inport" and "outport" fields are different.
>

I think the idea makes sense.

I wonder if just "Set" would be an appropriate name.  One catch is that it
may be too late to get into OVS 2.6.  If so, then we have to take into
account backwards compatibility.  We won't be able to just rename the
table.  One option would be to at least rename the table now (before 2.6)
to reflect a more broad purpose, and leave the support for other types of
entries to later patches.

Are you looking for any other specific feedback?

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V3] ovs-vtep: vtep-ctl and ovs-vtep support of adding explicit tunnel key

2016-08-03 Thread Itamar Ofek
Justin ,

What is your opinion?

Regards

Itamar

On Sun, Jul 24, 2016 at 3:01 PM, itamaro  wrote:

> From: itamaro 
>
> This patch adds support for handeling a per-tunnel tunnel key in the
> ovs-vtep and vtep-ctl to support the usage of neutron L2GW as an
> inter-cloud gateway.
>
> The Neutron spec is available here:
> https://review.openstack.org/#/c/270786/
>
> The aim of this patch is to support the usage of hardware vtep switch as
> an inter-cloud gateway, which is used to connect two separated l2 broadcast
> domains. This document will also explain the logic behind the addition of
> the
> new per-tunnel tunnel-key in the hardware_vtep schema.
>
> The introduction of the relay tunnel, does not change the current logic of
> hardware_vtep, it does however introduce new logic related to iner-cloud
> connectivity.
>
>
> Network layout
> ==
>
> virtual network 1 shared  network virtual
> network 2
> ++
> ++
> |Compute Host|   VNI=1VNI=2
> |Compute Host|
> |   +--+ <-++--->
>  +--+ |
> |   |vm| | ||   |
>  |vm| |
> |   +--+ | |   L3 network   |   |
>  +--+ |
> +---^+ ||
>  +-^--+
> |  +---v+  X +--v-+
>|
> |  +---> hardware_vtep  |  X | hardware_vtep  |
>|
> | VNI=1|   | logical switch |  X | logical switch <-+
>|
> |  |   | (tunnel_key 1) |==<<==X=>>==| (tunnel_key 2) |
>  |VNI=2|
> |  |   |   +-+ +-+  |  X |   +-+  +-+ | |
>|
> |  |   |   |-| |-|  |  X |   |-|  |-| | |
>|
> +---v--v-+ ++  X ++ |
>|
> |Compute Host| vlan2|   |vlan5   vlan9||vlan21  |
>|
> |   +--+ |  |   |relay vxlan  ||
> +---v-v--+
> |   |vm| |  |   |  VNI=100||
> |Compute Host|
> |   +--+ |  |   | |||
>  +--+ |
> +++-v-+   +-v-+ +-v-++-v-+  |
>  |vm| |
>   |   |   |   | |   ||   |  |
>  +--+ |
>   |   |   |   | |   ||   |
> ++
>   +---+   +---+ +---++---+
> Bare metal elements Bare metal elements
>
> Logical switch
> ===
> In a cloud architecture, there is sometimes need to connect virtual
> machines
> and physical machines to the same L2 broadcast domain.
> A logical switch is an entity representing an l2 virtual overlay network,
> identified by a shared tunnel key. This tunnel key (VxLAN VNI) is shared
> among
> all overlay virtual tunnel endpoints (VTEP) of the switch.
> The logical switch binds physical ports with either identical or different
> "VLAN" tags to the "VxLAN overlay" network.
>
> In a multi-cloud architecture, it may be useful to establish a cross-cloud
> l2 broadcast domain. The extended hardware vtep uses a relay l2 tunnel,
> which is a tunnel with an explicit tunnel-key property. The tunnel-key
> propery
> is used to map each overlay network (logical switch tunnel-key) in each
> cloud to
> the tunnel-key of the relay tunnel.
>
> The mapping to a remote logical switch is done by defining the same tunnel
> key
> in both ends of the the relay tunnel. This tunnel key (VxLAN VNI) is a
> property of the relay tunnel itself.
>
> To support the above tunnel behevior, a new kind of VTEP port is logic is
> introduced, defining two VTEP port groups. One group represents the
> existing
> VTEP ports of the local l2 overlay network, and another new group
> represents the
> individual l2 relay VTEPS.
>
> Multicast and Unknown unicast traffic
> =
> Currently Broadcast, Unknown unicast, and Multicast,"BUM" traffic to the
> overlay networks
> is handled by two replication modes:
>
>   - "source_node" mode: The packets originated from physical port
> are replicated on all the VTEPs ports pointed by unknown-dst, and
> flooded
> to all the physical bound ports.
>
>   - "service_node" mode: The packets originated from physical port are
> forwarded to only a selected single service node from the unknown-dst
> ports
> (the service node responsible for "BUM" propagation to the overlay
> network),
> and flooded to all the physical bound ports.
>
> In either of the replication modes BUM traffic originated from a VTEP port
> is
> flooded only to all physical ports.
>
> Considering the new l2-relay VTEPs ports group, relay 

[ovs-dev] Returned mail: Data format error

2016-08-03 Thread encoder-x-adobe-euro
The original message was received at Wed, 3 Aug 2016 16:59:02 +0530
from mozilla.org [199.249.125.55]

- The following addresses had permanent fatal errors -




___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH RFC] ovn: Extend Address_Set to Macros_Set to support define port sets

2016-08-03 Thread Zong Kai LI
This patch aims to extend Address_Set to Macros_Set, make it more common to
accept variable set, not only address. And by that, we can skinny down ACLs,
if we use Macros_Set to defines port sets for ACL to use, since lots of ACL
entries are similar but only "inport" and "outport" fields are different.

Address_Set improves the performance of OVN control plane. But what more
important is, its implementation introduced a "$macros -- *_Set" mechanism,
it's good way to do bunch jobs for ACLs and lflows.

How does Address_Set improve OVN performance?
 - it reduces the match part length for lflows, who has addresses to match.
 - indeed, it means less tokens in expression to parse, less constants, less
   combination symbols.
 - and for CMS integration component, for its DB sync job, a ACL entry has
   less match column is easier to compare.

What changes:
  Adderss_Set   -> Macros_Set
  Adderss_Set.addresses -> Macros_Set.elements
   (addresses, or ports names)
-> Macros_Set.type
   (only "address", "port" are valid now)

Will port set defined under Macros_Set benefit in the same way as Address_Set
did? -- NO, it may be better.
 - it will not decrease lflow's length, but the number. ACL entries who have
   similar match expressions, but with particular "inport" or "outport"
   can use port set defined in Macros_Set to combine into one ACL.
 - less ACL entries, will help CMS integration component has less workload
   in DB sync job, comparing port name should be quicker then comparing whole
   ACL entry.
 - less ACL entries, mess less lflows, means less match part in lflows to
   parse, less tokens to parse, make parsing work quicker.

Port set defined under Macros_Set will improve ovn-controller performance on
installation plenty of similar ACLs/lflows, but not for port comes and leaves
scenario.

Potential drawback: not sure yet.

TODO: go check how will ovn-northd use port sets defined under Macros_Set

Signed-off-by: Zong Kai LI 
---
 include/ovn/expr.h|   2 +-
 ovn/controller/lflow.c| 176 --
 ovn/lib/expr.c|  68 +++---
 ovn/northd/ovn-northd.c   |  54 +++---
 ovn/ovn-nb.ovsschema  |  11 +--
 ovn/ovn-nb.xml|  20 --
 ovn/ovn-sb.ovsschema  |  11 +--
 ovn/ovn-sb.xml|   7 +-
 ovn/utilities/ovn-nbctl.c |   4 +-
 ovn/utilities/ovn-sbctl.c |   4 +-
 tests/ovn.at  |   8 +--
 11 files changed, 201 insertions(+), 164 deletions(-)

diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index d790c49..5020ff4 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -466,7 +466,7 @@ void expr_constant_set_destroy(struct expr_constant_set 
*cs);
  * The values that don't qualify are ignored.
  */
 
-void expr_macros_add(struct shash *macros, const char *name,
+void expr_macros_add(struct shash *macros, const char *name, bool parse_value,
  const char * const *values, size_t n_values);
 void expr_macros_remove(struct shash *macros, const char *name);
 void expr_macros_destroy(struct shash *macros);
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index fda10eb..5eae183 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,8 +38,8 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
+/* Contains an internal expr datastructure that represents an macro set. */
+static struct shash expr_macros_sets;
 
 static bool full_flow_processing = false;
 static bool full_logical_flow_processing = false;
@@ -65,7 +65,7 @@ void
 lflow_init(void)
 {
 shash_init();
-shash_init(_address_sets);
+shash_init(_macros_sets);
 
 /* Reserve a pair of registers for the logical inport and outport.  A full
  * 32-bit register each is bigger than we need, but the expression code
@@ -187,149 +187,159 @@ lflow_init(void)
 expr_symtab_add_field(, "sctp.dst", MFF_SCTP_DST, "sctp", false);
 }
 
-/* Details of an address set currently in address_sets. We keep a cached
+/* Details of a macro set currently in macros_sets. We keep a cached
  * copy of sets still in their string form here to make it easier to compare
  * with the current values in the OVN_Southbound database. */
-struct address_set {
-char **addresses;
-size_t n_addresses;
+struct macro_set {
+char **elements;
+size_t n_elements;
 };
 
-/* struct address_set instances for address sets currently in the symtab,
- * hashed on the address set name. */
-static struct shash local_address_sets = 
SHASH_INITIALIZER(_address_sets);
+/* struct macro_set instances for address sets and port set currently in the
+ * symtab, hashed on the macro set name. */
+static struct shash 

[ovs-dev] [PATCH] netdev-dpdk: Avoid reconfiguration on reconnection of same vhost device.

2016-08-03 Thread Ilya Maximets
Binding/unbinding of virtio driver inside VM leads to reconfiguration
of PMD threads. This behaviour may be abused by executing bind/unbind
in an infinite loop to break normal networking on all ports attached
to the same instance of Open vSwitch.

Fix that by avoiding reconfiguration if it's not necessary.
Number of queues will not be decreased to 1 on device disconnection but
it's not very important in comparison with possible DOS attack from the
inside of guest OS.

Fixes: 81acebdaaf27 ("netdev-dpdk: Obtain number of queues for vhost
  ports from attached virtio.")
Reported-by: Ciara Loftus 
Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index a0d541a..98369f1 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2273,11 +2273,14 @@ new_device(struct virtio_net *virtio_dev)
 newnode = dev->socket_id;
 }
 
-dev->requested_socket_id = newnode;
-dev->requested_n_rxq = qp_num;
-dev->requested_n_txq = qp_num;
-netdev_request_reconfigure(>up);
-
+if (dev->requested_n_txq != qp_num
+|| dev->requested_n_rxq != qp_num
+|| dev->requested_socket_id != newnode) {
+dev->requested_socket_id = newnode;
+dev->requested_n_rxq = qp_num;
+dev->requested_n_txq = qp_num;
+netdev_request_reconfigure(>up);
+}
 ovsrcu_set(>virtio_dev, virtio_dev);
 exists = true;
 
@@ -2333,11 +2336,7 @@ destroy_device(volatile struct virtio_net *virtio_dev)
 ovs_mutex_lock(>mutex);
 virtio_dev->flags &= ~VIRTIO_DEV_RUNNING;
 ovsrcu_set(>virtio_dev, NULL);
-/* Clear tx/rx queue settings. */
 netdev_dpdk_txq_map_clear(dev);
-dev->requested_n_rxq = NR_QUEUE;
-dev->requested_n_txq = NR_QUEUE;
-netdev_request_reconfigure(>up);
 
 netdev_change_seq_changed(>up);
 ovs_mutex_unlock(>mutex);
-- 
2.7.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-dev,v4,3/5] netdev-dpdk: Add vHost User PMD

2016-08-03 Thread Ilya Maximets
On 03.08.2016 12:21, Loftus, Ciara wrote:
>>
>> I've applied this patch and performed following test:
>>
>> OVS with 2 VMs connected via vhost-user ports.
>> Each vhost-user port has 4 queues.
>>
>> VM1 executes ping on LOCAL port.
>> In normal situation ping results are following:
>>
>>  100 packets transmitted, 100 received, 0% packet loss, time 99144ms
>>  rtt min/avg/max/mdev = 0.231/0.459/0.888/0.111 ms
>>
>> After that VM2 starts execution of this script:
>>
>>  while true;
>>  do
>>  ethtool -L eth0 combined 4;
>>  ethtool -L eth0 combined 1;
>>  done
>>
>> Now results of ping between VM1 and LOCAL port are:
>>
>>  100 packets transmitted, 100 received, 0% packet loss, time 99116ms
>>  rtt min/avg/max/mdev = 5.466/150.327/356.201/85.208 ms
>>
>> Minimal time increased from 0.231 to 5.466 ms.
>> Average time increased from 0.459 to 150.327 ms (~300 times)!
>>
>> This happens because of constant reconfiguration requests from
>> the 'vring_state_changed_callback()'.
>>
>> As Ciara said, "Previously we could work with only reconfiguring during
>> link status change as we had full information available to us
>> ie. virtio_net->virt_qp_nb. We don't have that any more, so we need to
>> count the queues in OVS now every time we get a vring_change."
>>
>> Test above shows that this is unacceptable for OVS to perform
>> reconfiguration each time vring state changed because this leads to
>> ability for the guest user to break normal networking on all ports
>> connected to the same instance of Open vSwitch.
> 
> Hi Ilya,
> 
> Another thought on this. With the current master branch, isn't the above 
> possible too with a script like this:
> 
> while true;
> do
> echo ":00:03.0" > /sys/bus/pci/drivers/virtio-pci/bind
> echo ":00:03.0" > /sys/bus/pci/drivers/virtio-pci/unbind
> done
> 
> The bind/unbind calls new/destroy device which in turn call reconfigure() 
> each time.

Hmm, yes. You're right.
But this may be easily fixed by following patch:
-
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 41ca91d..b9e72b8 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2313,10 +2313,14 @@ new_device(int vid)
 newnode = dev->socket_id;
 }
 
-dev->requested_socket_id = newnode;
-dev->requested_n_rxq = qp_num;
-dev->requested_n_txq = qp_num;
-netdev_request_reconfigure(>up);
+if (dev->requested_n_txq != qp_num
+|| dev->requested_n_rxq != qp_num
+|| dev->requested_socket_id != newnode) {
+dev->requested_socket_id = newnode;
+dev->requested_n_rxq = qp_num;
+dev->requested_n_txq = qp_num;
+netdev_request_reconfigure(>up);
+}
 
 exists = true;
 
@@ -2376,9 +2380,6 @@ destroy_device(int vid)
 dev->vid = -1;
 /* Clear tx/rx queue settings. */
 netdev_dpdk_txq_map_clear(dev);
-dev->requested_n_rxq = NR_QUEUE;
-dev->requested_n_txq = NR_QUEUE;
-netdev_request_reconfigure(>up);
 
 netdev_change_seq_changed(>up);
 ovs_mutex_unlock(>mutex);
-

We will not decrease number of queues on disconnect. But, I think,
it's not very important.

Thanks for pointing this.
I'll post above diff as a separate patch.

Best regards, Ilya Maximets.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-dev,v4,3/5] netdev-dpdk: Add vHost User PMD

2016-08-03 Thread Loftus, Ciara
> 
> I've applied this patch and performed following test:
> 
> OVS with 2 VMs connected via vhost-user ports.
> Each vhost-user port has 4 queues.
> 
> VM1 executes ping on LOCAL port.
> In normal situation ping results are following:
> 
>   100 packets transmitted, 100 received, 0% packet loss, time 99144ms
>   rtt min/avg/max/mdev = 0.231/0.459/0.888/0.111 ms
> 
> After that VM2 starts execution of this script:
> 
>   while true;
>   do
>   ethtool -L eth0 combined 4;
>   ethtool -L eth0 combined 1;
>   done
> 
> Now results of ping between VM1 and LOCAL port are:
> 
>   100 packets transmitted, 100 received, 0% packet loss, time 99116ms
>   rtt min/avg/max/mdev = 5.466/150.327/356.201/85.208 ms
> 
> Minimal time increased from 0.231 to 5.466 ms.
> Average time increased from 0.459 to 150.327 ms (~300 times)!
> 
> This happens because of constant reconfiguration requests from
> the 'vring_state_changed_callback()'.
> 
> As Ciara said, "Previously we could work with only reconfiguring during
> link status change as we had full information available to us
> ie. virtio_net->virt_qp_nb. We don't have that any more, so we need to
> count the queues in OVS now every time we get a vring_change."
> 
> Test above shows that this is unacceptable for OVS to perform
> reconfiguration each time vring state changed because this leads to
> ability for the guest user to break normal networking on all ports
> connected to the same instance of Open vSwitch.

Hi Ilya,

Another thought on this. With the current master branch, isn't the above 
possible too with a script like this:

while true;
do
echo ":00:03.0" > /sys/bus/pci/drivers/virtio-pci/bind
echo ":00:03.0" > /sys/bus/pci/drivers/virtio-pci/unbind
done

The bind/unbind calls new/destroy device which in turn call reconfigure() each 
time.

Thanks,
Ciara

> 
> If this vulnerability is unavoidable with current version of vHost PMD,
> I'm suggesting to postpone it's integration until there will be
> method or special API to get number of queues from the inside of
> 'link_status_changed_callback()'.
> 
> I've added vHost maintainers to CC-list to hear their opinion about
> new API to get number of queues from the vHost PMD.
> Maybe we can expose 'rte_vhost_get_queue_num()' somehow or make
> 'dev_info->nb_rx_queues' usable?
> 
> NACK for now.
> 
> Best regards, Ilya Maximets.
> 
> On 29.07.2016 16:24, Ciara Loftus wrote:
> > DPDK 16.04 introduces the vHost PMD which allows 'dpdkvhostuser' ports
> > to be controlled by the librte_ether API, like physical 'dpdk' ports and
> > IVSHM 'dpdkr' ports. This commit integrates this PMD into OVS and
> > removes direct calls to the librte_vhost DPDK library.
> >
> > This commit removes extended statistics support for vHost User ports
> > until such a time that this becomes available in the vHost PMD in a
> > DPDK release supported by OVS.
> >
> > Signed-off-by: Ciara Loftus 
> > ---
> >  INSTALL.DPDK.md   |  10 +
> >  NEWS  |   2 +
> >  lib/netdev-dpdk.c | 857 ++-
> ---
> >  3 files changed, 300 insertions(+), 569 deletions(-)
> >
> > diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> > index 7609aa7..4feb7be 100644
> > --- a/INSTALL.DPDK.md
> > +++ b/INSTALL.DPDK.md
> > @@ -604,6 +604,16 @@ can be found in [Vhost Walkthrough].
> >
> >  http://dpdk.org/doc/guides/rel_notes/release_16_04.html
> >
> > +  - dpdk, dpdkr and dpdkvhostuser ports are 'eth' type ports in the context
> of
> > +DPDK as they are all managed by the rte_ether API. This means that
> they
> > +adhere to the DPDK configuration option CONFIG_RTE_MAX_ETHPORTS
> which by
> > +default is set to 32. This means by default the combined total number 
> > of
> > +dpdk, dpdkr and dpdkvhostuser ports allowable in OVS with DPDK is 32.
> This
> > +value can be changed if desired by modifying the configuration file in
> > +DPDK, or by overriding the default value on the command line when
> building
> > +DPDK. eg.
> > +
> > +`make install CONFIG_RTE_MAX_ETHPORTS=64`
> >
> >  Bug Reporting:
> >  --
> > diff --git a/NEWS b/NEWS
> > index dc3dedb..6510dde 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -64,6 +64,8 @@ Post-v2.5.0
> >   * Basic connection tracking for the userspace datapath (no ALG,
> > fragmentation or NAT support yet)
> >   * Remove dpdkvhostcuse port type.
> > + * vHost PMD integration brings vhost-user ports under control of the
> > +   rte_ether DPDK API.
> > - Increase number of registers to 16.
> > - ovs-benchmark: This utility has been removed due to lack of use and
> >   bitrot.
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index d6959fe..d6ceeec 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -30,7 +30,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >
> >  #include "dirs.h"

[ovs-dev] ovs dpdk : userspace connection tracker cannot support L7?

2016-08-03 Thread Yangyongqiang (Tony, Shannon)
Hello,

We read the connection tracker code, and find this patch can not parse ftp 
protocol.

Whether the userspace connection tracker only has L4 feather or has L7 feather 
too ?

If the ct cannot L7, then ovs dpdk cannot be used for stateful security group, 
so do we have a plan for supporting L7?

Thanks

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [patch_v3] ovn: Add datapaths of interest filtering.

2016-08-03 Thread Darrell Ball
This patch adds datapaths of interest support where only datapaths of
local interest are monitored by the ovn-controller ovsdb client.  The
idea is to do a flood fill in ovn-controller of datapath associations
calculated by northd. A new column is added to the SB database
datapath_binding table - related_datapaths to facilitate this so all
datapaths associations are known quickly in ovn-controller.  This
allows monitoring to adapt quickly with a single new monitor setting
for all datapaths of interest locally.

Signed-off-by: Darrell Ball 
---

v2->v3: Line length violation fixups :/

v1->v2: Added logical port removal monitoring handling, factoring
in recent changes for incremental processing.  Added some
comments in the code regarding initial conditions for
database conditional monitoring.

 ovn/controller/binding.c|  25 +---
 ovn/controller/ovn-controller.c | 124 ++--
 ovn/controller/ovn-controller.h |   8 +++
 ovn/controller/patch.c  |  39 +++--
 ovn/northd/ovn-northd.c |  74 
 ovn/ovn-sb.ovsschema|  11 +++-
 ovn/ovn-sb.xml  |   9 +++
 7 files changed, 271 insertions(+), 19 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 3073727..f917cdc 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -67,7 +67,8 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 static bool
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
 struct shash *lport_to_iface,
-struct sset *all_lports)
+struct sset *all_lports,
+struct controller_ctx *ctx)
 {
 int i;
 bool changed = false;
@@ -93,6 +94,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 continue;
 }
 shash_add(lport_to_iface, iface_id, iface_rec);
+sbrec_port_binding_add_clause_logical_port(ctx->ovnsb_idl,
+   OVSDB_F_EQ,
+   iface_id);
 if (!sset_find_and_delete(_local_ids, iface_id)) {
 sset_add(_ids, iface_id);
 sset_add(all_lports, iface_id);
@@ -108,6 +112,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 
 const char *cur_id;
 SSET_FOR_EACH(cur_id, _local_ids) {
+sbrec_port_binding_remove_clause_logical_port(ctx->ovnsb_idl,
+  OVSDB_F_EQ, cur_id);
 sset_find_and_delete(_ids, cur_id);
 sset_find_and_delete(all_lports, cur_id);
 }
@@ -131,20 +137,23 @@ local_datapath_lookup_by_uuid(struct hmap *hmap_p, const 
struct uuid *uuid)
 }
 
 static void
-remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
+remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld,
+  struct controller_ctx *ctx)
 {
 if (ld->logical_port) {
 free(ld->logical_port);
 ld->logical_port = NULL;
 }
 hmap_remove(local_datapaths, >hmap_node);
+remove_datapath_of_interest(ctx, ld->dp);
 free(ld);
 lflow_reset_processing();
 }
 
 static void
 add_local_datapath(struct hmap *local_datapaths,
-const struct sbrec_port_binding *binding_rec)
+const struct sbrec_port_binding *binding_rec,
+   struct controller_ctx *ctx)
 {
 if (get_local_datapath(local_datapaths,
binding_rec->datapath->tunnel_key)) {
@@ -153,9 +162,11 @@ add_local_datapath(struct hmap *local_datapaths,
 
 struct local_datapath *ld = xzalloc(sizeof *ld);
 ld->logical_port = xstrdup(binding_rec->logical_port);
+ld->dp = binding_rec->datapath;
 memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
 hmap_insert(local_datapaths, >hmap_node,
 binding_rec->datapath->tunnel_key);
+do_datapath_flood_fill(ctx, binding_rec->datapath);
 lport_index_reset();
 mcgroup_index_reset();
 lflow_reset_processing();
@@ -186,7 +197,7 @@ consider_local_datapath(struct controller_ctx *ctx,
 if (iface_rec
 || (binding_rec->parent_port && binding_rec->parent_port[0] &&
 sset_contains(_ids, binding_rec->parent_port))) {
-add_local_datapath(local_datapaths, binding_rec);
+add_local_datapath(local_datapaths, binding_rec, ctx);
 if (iface_rec && ctx->ovs_idl_txn) {
 update_qos(iface_rec, binding_rec);
 }
@@ -230,7 +241,7 @@ consider_local_datapath(struct controller_ctx *ctx,
   binding_rec->logical_port);
 sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
 sset_add(all_lports, binding_rec->logical_port);
-add_local_datapath(local_datapaths, binding_rec);
+

[ovs-dev] [PATCHv2] fedora.spec: Add OVN include files.

2016-08-03 Thread William Tu
Current 'make rpm-fedora' fails due to files exists in $RPM_BUILD_ROOT
directory but not found in the %files section, resulting in errors below:
RPM build errors:
Installed (but unpackaged) file(s) found:
/usr/include/ovn/actions.h
/usr/include/ovn/expr.h
/usr/include/ovn/lex.h
The patch fixes it and tested with rpmbuild 4.13.0 under Fedora 23.

Signed-off-by: William Tu 
---
 rhel/openvswitch-fedora.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 088afcb..34c0f37 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@ fi
 %{_libdir}/pkgconfig/*.pc
 %{_includedir}/openvswitch/*
 %{_includedir}/openflow/*
+%{_includedir}/ovn/*
 
 %files
 %defattr(-,root,root)
-- 
2.5.0

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev