Re: [ovs-dev] OVN - Pluggable Distributed DB Infrastructure for OVSDB

2015-06-30 Thread Gal Sagie
Hi Justin,

The idea was not to choose one database implementation over the other, but
design things in such a way
that its pluggable, so in terms of OVN, switching between implementations
is transparent (as much as possible)

I think that different setups can have different scale criteria and might
need different DB solutions.

I think the IDL library (the python and C implementations) will be rather
easy to adjust as OVSDB have clear API

So doing this work now, we can easily change and experiments with different
DB solutions

Gal.

On Tue, Jun 30, 2015 at 3:48 AM, Justin Pettit  wrote:

>
> > On Jun 29, 2015, at 10:20 AM, Gal Sagie  wrote:
> >
> > I think there are many open source distributed DB alternatives that can
> be
> > plugged
> > and implement the OVSDB DB operations API and behaviour.
> >
> > Ben and Justin, i am interested to know whats your opinion on this
> > suggestion,
> > i think now is a good time to consider something like that.
> > I can say that personally i have heard many supporting opinions from
> people
> > regarding this
> > and would love to know what you guys think about this.
>
> Ben and I discussed this at some length when we were hashing out the
> design.  Our take was that we would start out with ovsdb-server, since we
> were familiar with the design and could rapidly make changes to improve
> shortcomings that arose.  If we eventually found that it wasn't scaling, we
> could then switch it out for a different one.  At that point, we'd have an
> understanding of where ovsdb-server worked and where it didn't, and then
> we'd have better criteria for choosing an alternative.  If we choose a
> different database now, it would almost certainly have its own issues.
> While the database is clearly a central part of the design, it's
> integration isn't that complicated, and we should be able to swap it out
> fairly easily later on.
>
> --Justin
>
>
>


-- 
Best Regards ,

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


Re: [ovs-dev] segmentation fault in openvswitch

2015-06-30 Thread Traynor, Kevin

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of
> ravali.bu...@wipro.com
> Sent: Tuesday, June 30, 2015 7:39 AM
> To: dev@openvswitch.org
> Subject: [ovs-dev] segmentation fault in openvswitch
> 
> Hi Team,
> From below link https://access.redhat.com/documentation/en-
> US/Red_Hat_Enterprise_Linux/6/html-
> single/Virtualization_Tuning_and_Optimization_Guide/index.html
> In section 4.5: Multi-queue virtio-net works well for incoming traffic, but
> can occasionally hurt performance for outgoing traffic. Enabling multi-queue
> virtio-net increases the total throughput, and in parallel increases CPU
> consumption.
> So for configuring multi-queue virtio-net for vhost-user I have applied the
> patch from https://lists.gnu.org/archive/html/qemu-devel/2015-
> 05/msg05657.html .
> 
> After applying the patch and running the QEMU command I am able to see the
> segmentation fault in openvswitch.
> 
> Can you just let us know how to overcome this.

Multiqueue vhost-user support is not added to DPDK or OVS yet. It should be
part of DPDK 2.2 and then we can add support to OVS.

> 
> Thanks & Regards,
> Ravali
> 
> The information contained in this electronic message and any attachments to
> this message are intended for the exclusive use of the addressee(s) and may
> contain proprietary, confidential or privileged information. If you are not
> the intended recipient, you should not disseminate, distribute or copy this
> e-mail. Please notify the sender immediately and destroy all copies of this
> message and any attachments. WARNING: Computer viruses can be transmitted via
> email. The recipient should check this email and any attachments for the
> presence of viruses. The company accepts no liability for any damage caused
> by any virus transmitted by this email. www.wipro.com
> ___
> 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 08/11] metaflow: Extend size of mf_value to 128 bytes.

2015-06-30 Thread Loftus, Ciara
> 
> On Wed, Jun 24, 2015 at 1:17 PM, Ben Pfaff  wrote:
> > On Fri, Jun 19, 2015 at 04:13:22PM -0700, Jesse Gross wrote:
> >> Tunnel metadata can be substantially larger than our existing fields
> >> (up to 124 bytes in a single Geneve option) so this extends the size
> >> of the data that we can handle with metaflow fields. This also
> >> breaks a few tests that assume that their max size is also the
> >> maximum that can be handled in a field.
> >>
> >> Signed-off-by: Jesse Gross 
> >
> > Did you look around at all to see whether this will unreasonably blow up
> > any data or algorithms?
> 
> I don't believe that it should have any significant effects.
> Generally, code does operations on the fields based on mf->n_bytes
> (with the exception of some memset()s here and there). I don't think
> that we really store these in a large number for any real period of
> time.

With this series of patches, in particular patch 10/11 "tunnel: Geneve TLV 
handling support for OpenFlow" I've measured a significant decrease in 
performance with the dpdkport type. For example, with a loopback test with 
64Byte packets I've seen a 25% decrease in throughput.
I suspect this is in relation to the size of the new tun_metadata struct. A 
quick perf analysis and I see we're spending significantly more time 
initialising packet metadata in the dp_netdev_process_rxq_port function.
Are there any plans to address this performance degradation?

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


[ovs-dev] [PATCH] Detecting dpdk vhost_cuse when building ovs

2015-06-30 Thread Mussar, Gary
Dpdk allows users to create a config that includes other config files and then 
override values.

Eg.
defconfig_x86_64-native_vhost_cuse-linuxapp-gcc:

#include "defconfig_x86_64-native-linuxapp-gcc"
CONFIG_RTE_BUILD_COMBINE_LIBS=y
CONFIG_RTE_BUILD_SHARED_LIB=n
CONFIG_RTE_LIBRTE_VHOST=y
CONFIG_RTE_LIBRTE_VHOST_USER=n

This allows you to have both a vhostuser and vhostcuse config in the same 
source tree without the need to replicate everything in those config files just 
to change a couple of settings. The resultant .config file has all of the 
settings from the included files with the updated settings at the end. The 
resultant rte_config.h contains multiple undefs and defines for the overridden 
settings.

Eg.
> grep RTE_LIBRTE_VHOST_USER 
> x86_64-native_vhost_cuse-linuxapp-gcc/include/rte_config.h
#undef RTE_LIBRTE_VHOST_USER
#define RTE_LIBRTE_VHOST_USER 1
#undef RTE_LIBRTE_VHOST_USER

The current mechanism to detect the RTE_LIBRTE_VHOST_USER setting merely greps 
the rte_config.h file for the string "define RTE_LIBRTE_VHOST_USER 1" rather 
than the final setting of RTE_LIBRTE_VHOST_USER. The following patch changes 
this test to detect the final setting of RTE_LIBRTE_VHOST_USER.

Gary

Author: Gary Mussar 
Date:   Fri Jun 26 16:49:03 2015 -0400

Fix detection of vhost_cuse in dpdk rte_config.h

Signed-off-by: Gary Mussar 
---
 acinclude.m4 | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 20391ec..ef6523a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -221,8 +221,13 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 AC_SUBST([DPDK_vswitchd_LDFLAGS])
 AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.])

-OVS_GREP_IFELSE([$RTE_SDK/include/rte_config.h], [define 
RTE_LIBRTE_VHOST_USER 1],
+AC_LANG_PUSH(C)
+AC_EGREP_CPP([int vhost = 1;], [
+#include <$RTE_SDK/include/rte_config.h>
+int vhost = RTE_LIBRTE_VHOST_USER;
+],
 [], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support 
enabled, vhost-user
+AC_LANG_POP()
   else
 RTE_SDK=
   fi
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/2] OVS_UNUSED is defined indirectly

2015-06-30 Thread Alin Serdean
Currently OVS_UNUSED is defined in compiler.h since syslog.h is a
standalone wrapper remove it from the parameters.

Signed-off-by: Alin Gabriel Serdean 
---
 include/windows/syslog.h | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/windows/syslog.h b/include/windows/syslog.h
index 242bfc4..d59cd3d 100644
--- a/include/windows/syslog.h
+++ b/include/windows/syslog.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 Cloudbase Solutions Srl
+ * Copyright 2013, 2015 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.You may obtain
@@ -50,13 +50,12 @@
 #define LOG_LOCAL7  (23<<3) /* reserved for local use */
 
 static inline void
-openlog(const char *ident OVS_UNUSED, int option OVS_UNUSED,
-int facility OVS_UNUSED)
+openlog(const char *ident, int option, int facility)
 {
 }
 
 static inline void
-syslog(int priority OVS_UNUSED, const char *format OVS_UNUSED, ...)
+syslog(int priority, const char *format, ...)
 {
 }
 
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/2] Undefined symbol make_unix_socket

2015-06-30 Thread Alin Serdean
syslog_direct_create defined in (lib/syslog-direct.c) uses make_unix_socket
which is currently undefined on the windows build.

We either can remove the new file from the chain but this patch proposes
to define a wrapper to make_unix_socket in which we return EINVAL.

This will avoid this kind of problems in the future.

Signed-off-by: Alin Gabriel Serdean 
---
 lib/socket-util.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/socket-util.h b/lib/socket-util.h
index 1178fb8..a0a5c36 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -95,6 +95,13 @@ int af_inet_ifreq_ioctl(const char *name, struct ifreq *,
 #endif
 
 #ifdef _WIN32
+static inline int make_unix_socket(int style, bool nonblock,
+   const char *bind_path,
+   const char *connect_path)
+{
+return EINVAL;
+}
+
 /* Windows defines the 'optval' argument as char * instead of void *. */
 #define setsockopt(sock, level, optname, optval, optlen) \
 rpl_setsockopt(sock, level, optname, optval, optlen)
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath-windows: OVS_PACKET_ATTR_PROBE

2015-06-30 Thread Alin Serdean
Since commit:
https://github.com/openvswitch/ovs/commit/2e460098bff351b9fddcb917447caa3b97a35d86
a new packet attribute was introduced.

This patch adds OVS_PACKET_ATTR_PROBE to nlPktExecPolicy in datapath-windows
and ignores it for the moment to maintain binary compatibility.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/User.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index 9f462cf..56f2f7c 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -332,7 +332,8 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 [OVS_PACKET_ATTR_ACTIONS] = {.type = NL_A_UNSPEC, .optional = FALSE},
 [OVS_PACKET_ATTR_USERDATA] = {.type = NL_A_UNSPEC, .optional = TRUE},
 [OVS_PACKET_ATTR_EGRESS_TUN_KEY] = {.type = NL_A_UNSPEC,
-.optional = TRUE}
+.optional = TRUE},
+[OVS_PACKET_ATTR_PROBE] = { .type = NL_A_FLAG, .optional = TRUE }
 };
 
 RtlZeroMemory(&execute, sizeof(OvsPacketExecute));
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Sabyasachi Sengupta


v2: fixed a bug in do_dump that was calling print_table incorrectly when
'table' option is specified

Added capability of displaying tables through an optional 'table' argument
to 'ovsdb-client dump'. When specified, ovsdb-client will iterate
through all tables in the chosen ovsdb, and create a transaction only
for that table instead of all, and then print the response for that
transaction.

Signed-off-by: Sabyasachi Sengupta 

---
 ovsdb/ovsdb-client.c |  105 +-
 1 files changed, 69 insertions(+), 36 deletions(-)

diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 2942953..84474d0 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -255,8 +255,8 @@ usage(void)
"\n  monitor [SERVER] [DATABASE] ALL\n"
"monitor all changes to all columns in all tables\n"
"in DATBASE on SERVER.\n"
-   "\n  dump [SERVER] [DATABASE]\n"
-   "dump contents of DATABASE on SERVER to stdout\n"
+   "\n  dump [SERVER] [DATABASE] [TABLE]\n"
+   "dump contents of table(s) in DATABASE on SERVER to stdout\n"
"\nThe default SERVER is unix:%s/db.sock.\n"
"The default DATABASE is Open_vSwitch.\n",
program_name, program_name, ovs_rundir());
@@ -1042,44 +1042,80 @@ dump_table(const struct ovsdb_table_schema *ts, struct 
json_array *rows)
 }

 static void
-do_dump(struct jsonrpc *rpc, const char *database,
-int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+create_table_transaction(struct json *transaction,
+ const struct shash_node *table)
+{
+const struct ovsdb_table_schema *ts = table->data;
+struct json *op, *columns;
+struct shash_node *node;
+
+columns = json_array_create_empty();
+SHASH_FOR_EACH (node, &ts->columns) {
+const struct ovsdb_column *column = node->data;
+
+if (strcmp(column->name, "_version")) {
+json_array_add(columns, json_string_create(column->name));
+}
+}
+
+op = json_object_create();
+json_object_put_string(op, "op", "select");
+json_object_put_string(op, "table", table->name);
+json_object_put(op, "where", json_array_create_empty());
+json_object_put(op, "columns", columns);
+json_array_add(transaction, op);
+}
+
+static void
+print_table(const struct json *op_result, const struct shash_node *table)
+{
+const struct ovsdb_table_schema *ts = table->data;
+struct json *rows;
+
+if (op_result->type != JSON_OBJECT
+|| !(rows = shash_find_data(json_object(op_result), "rows"))
+|| rows->type != JSON_ARRAY) {
+ovs_fatal(0, "%s table reply is not an object with a \"rows\" "
+  "member array: %s", ts->name, json_to_string(op_result, 0));
+}
+dump_table(ts, &rows->u.array);
+}
+
+static void
+do_dump(struct jsonrpc *rpc, const char *database, int argc, char *argv[])
 {
 struct jsonrpc_msg *request, *reply;
 struct ovsdb_schema *schema;
 struct json *transaction;

 const struct shash_node **tables;
-size_t n_tables;
+size_t n_tables, n_tables_out;
+char *table_name = (argc == 1) ? argv[0] : NULL;
+bool table_found = false;

 size_t i;

 schema = fetch_schema(rpc, database);
 tables = shash_sort(&schema->tables);
 n_tables = shash_count(&schema->tables);
+n_tables_out = table_name ? 1 : n_tables;

 /* Construct transaction to retrieve entire database. */
 transaction = json_array_create_1(json_string_create(database));
 for (i = 0; i < n_tables; i++) {
-const struct ovsdb_table_schema *ts = tables[i]->data;
-struct json *op, *columns;
-struct shash_node *node;
-
-columns = json_array_create_empty();
-SHASH_FOR_EACH (node, &ts->columns) {
-const struct ovsdb_column *column = node->data;
-
-if (strcmp(column->name, "_version")) {
-json_array_add(columns, json_string_create(column->name));
+if (table_name) {
+if (!strncmp(tables[i]->name, table_name,
+ MIN(strlen(tables[i]->name), strlen(table_name {
+create_table_transaction(transaction, tables[i]);
+table_found = true;
+break;
 }
+} else {
+create_table_transaction(transaction, tables[i]);
 }
-
-op = json_object_create();
-json_object_put_string(op, "op", "select");
-json_object_put_string(op, "table", tables[i]->name);
-json_object_put(op, "where", json_array_create_empty());
-json_object_put(op, "columns", columns);
-json_array_add(transaction, op);
+}
+if (table_name && !table_found) {
+ovs_fatal(0, "specified table '%s' does not exist", table_name);
 }

 /* Send request, get reply. */
@@ -1088,24 +1124,21 @@ do_dump(struct jsonrpc *rpc, const char *database,

 /* Print

Re: [ovs-dev] [RFC rhel-dkms] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Flavio Leitner
On Sun, Jun 28, 2015 at 09:38:13PM -0700, Alex Wang wrote:
> This commit adds a rpmbuild spec file for building ovs datapath
> in dkms similar to the one for debian.
> 
> Signed-off-by: Alex Wang 
> ---
>  rhel/.gitignore   |1 +
>  rhel/automake.mk  |6 +++
>  rhel/dkms.conf.in |   11 +
>  rhel/openvswitch-dkms.spec.in |   99 
> +
>  4 files changed, 117 insertions(+)
>  create mode 100644 rhel/dkms.conf.in
>  create mode 100644 rhel/openvswitch-dkms.spec.in
> 
> diff --git a/rhel/.gitignore b/rhel/.gitignore
> index fa2554f..164bb66 100644
> --- a/rhel/.gitignore
> +++ b/rhel/.gitignore
> @@ -1,3 +1,4 @@
> +openvswitch-dkms.spec
>  openvswitch-kmod-rhel5.spec
>  openvswitch-kmod-rhel6.spec
>  openvswitch-kmod-fedora.spec
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index f6272a3..e24c1a0 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -8,10 +8,13 @@
>  EXTRA_DIST += \
>   rhel/README.RHEL \
>   rhel/automake.mk \
> + rhel/dkms.conf.in \
>   rhel/etc_init.d_openvswitch \
>   rhel/etc_logrotate.d_openvswitch \
>   rhel/etc_sysconfig_network-scripts_ifdown-ovs \
>   rhel/etc_sysconfig_network-scripts_ifup-ovs \
> + rhel/openvswitch-dkms.spec \
> + rhel/openvswitch-dkms.spec.in \
>   rhel/openvswitch-kmod-rhel6.spec \
>   rhel/openvswitch-kmod-rhel6.spec.in \
>   rhel/openvswitch-kmod.files \
> @@ -33,6 +36,9 @@ update_rhel_spec = \
>  < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
>if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; 
> fi
>  
> +$(srcdir)/rhel/openvswitch-dkms.spec: rhel/openvswitch-dkms.spec.in 
> $(top_builddir)/config.status
> + $(update_rhel_spec)
> +
>  $(srcdir)/rhel/openvswitch-kmod-rhel6.spec: 
> rhel/openvswitch-kmod-rhel6.spec.in $(top_builddir)/config.status
>   $(update_rhel_spec)
>  
> diff --git a/rhel/dkms.conf.in b/rhel/dkms.conf.in
> new file mode 100644
> index 000..2c90b4d
> --- /dev/null
> +++ b/rhel/dkms.conf.in
> @@ -0,0 +1,11 @@

This file could be generated by the %install section so that
you don't have an extra file in OVS tree, macros are expanded
as usual and if we decide to have dkms support to specific
distros, then the config remains inside each spec.
It's just a suggestion to reduce the noise.


> +MODULES=( __MODULES__ )
> +
> +PACKAGE_NAME="openvswitch"
> +PACKAGE_VERSION="__VERSION__"
> +MAKE="./configure --with-linux='${kernel_source_dir}' && make -C 
> datapath/linux"

That's the instruction to build the module, see below.


> +for __idx in ${!MODULES[@]}; do
> +BUILT_MODULE_NAME[__idx]=${MODULES[__idx]}
> +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> +done
> +AUTOINSTALL=yes
> diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/openvswitch-dkms.spec.in
> new file mode 100644
> index 000..591418b
> --- /dev/null
> +++ b/rhel/openvswitch-dkms.spec.in
> @@ -0,0 +1,99 @@
> +# Spec file for Open vSwitch kernel modules using DKMS.
> +#
> +# Copyright (C) 2015 Nicira, Inc.
> +#
> +# Copying and distribution of this file, with or without modification,
> +# are permitted in any medium without royalty provided the copyright
> +# notice and this notice are preserved.  This file is offered as-is,
> +# without warranty of any kind.
> +
> +%define oname openvswitch
> +
> +Name: %{oname}-dkms
> +Version:  @VERSION@
> +Release:  1%{?dist}
> +Summary:  Open vSwitch kernel module
> +
> +Group:System/Kernel
> +License:  GPLv2
> +URL:  http://openvswitch.org/
> +Source:   %{oname}-%{version}.tar.gz
> +Requires: autoconf, gcc, make
> +Requires(post):   dkms
> +Requires(preun):  dkms
> +BuildRoot:%(mktemp -ud 
> %{_tmppath}/%{name}-%{version}-%{release}-XX)
> +
> +# Without this we get an empty openvswitch-debuginfo package (whose name
> +# conflicts with the openvswitch-debuginfo package for OVS userspace).
> +%undefine _enable_debug_packages
> +
> +
> +%description
> +Open vSwitch Linux kernel module.
> +
> +
> +%prep
> +%setup -n %{oname}-%{version}
> +
> +cat > %{oname}.conf << EOF
> +override %{oname} * extra/%{oname}
> +override %{oname} * weak-updates/%{oname}
> +EOF
> +
> +
> +%build
> +# for running the '%{__make} -C datapath print-build-modules' below.
> +./configure
> +
> +
> +%install
> +%{__rm} -rf %{buildroot}
> +
> +# Kernel module sources install for dkms
> +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> +
> +# check we can get kernel module names
> +%{__make} -C datapath print-build-modules
> +
> +# Prepare dkms.conf from the dkms.conf.in template
> +%{__sed} "s/__VERSION__/%{version}-%{release}/g; s/__MODULES__/`%{__make} -C 
> datapath print-build-modules | grep -v make`/" rhel/dkms.conf.in > 
> %{buildroot}%{_usrs

Re: [ovs-dev] OVN - Pluggable Distributed DB Infrastructure for OVSDB

2015-06-30 Thread Russell Bryant
On 06/30/2015 04:42 AM, Gal Sagie wrote:
> Hi Justin,
> 
> The idea was not to choose one database implementation over the other,
> but design things in such a way
> that its pluggable, so in terms of OVN, switching between
> implementations is transparent (as much as possible)
> 
> I think that different setups can have different scale criteria and
> might need different DB solutions.
> 
> I think the IDL library (the python and C implementations) will be
> rather easy to adjust as OVSDB have clear API
> 
> So doing this work now, we can easily change and experiments with
> different DB solutions

Ben and Justin have made a pretty good case that this is a premature
since we really haven't done much real testing to see how ovsdb-server
scales.  This could potentially be a pretty big distraction and I'm not
sure it's worth it.  I don't think adding this later will be
significantly more difficult than it is today if it's needed.

If someone wants to do some experimentation in the short term, it's
certainly possible.  There is an API generated from the ovsdb schema
that you could re-implement.

It might be better time spent to think about OVN testing and how we can
do good, reproducible scale tests.  Ben has done some awesome work here,
but it seems we may need to a multi-node environment since at some point
it sounds like the number of processes on a single host becomes a
problem in the simulated test env.

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


Re: [ovs-dev] OVN - Pluggable Distributed DB Infrastructure for OVSDB

2015-06-30 Thread Kyle Mestery
On Tue, Jun 30, 2015 at 2:35 PM, Russell Bryant  wrote:

> On 06/30/2015 04:42 AM, Gal Sagie wrote:
> > Hi Justin,
> >
> > The idea was not to choose one database implementation over the other,
> > but design things in such a way
> > that its pluggable, so in terms of OVN, switching between
> > implementations is transparent (as much as possible)
> >
> > I think that different setups can have different scale criteria and
> > might need different DB solutions.
> >
> > I think the IDL library (the python and C implementations) will be
> > rather easy to adjust as OVSDB have clear API
> >
> > So doing this work now, we can easily change and experiments with
> > different DB solutions
>
> Ben and Justin have made a pretty good case that this is a premature
> since we really haven't done much real testing to see how ovsdb-server
> scales.  This could potentially be a pretty big distraction and I'm not
> sure it's worth it.  I don't think adding this later will be
> significantly more difficult than it is today if it's needed.
>
> If someone wants to do some experimentation in the short term, it's
> certainly possible.  There is an API generated from the ovsdb schema
> that you could re-implement.
>
> It might be better time spent to think about OVN testing and how we can
> do good, reproducible scale tests.  Ben has done some awesome work here,
> but it seems we may need to a multi-node environment since at some point
> it sounds like the number of processes on a single host becomes a
> problem in the simulated test env.
>
>
+1

Thanks for taking the time to answer this one everyone, and I agree with
Russell that it makes more sense to do this later down the road rather than
distract things at the moment.

Thanks,
Kyle


> --
> 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] OVN - Pluggable Distributed DB Infrastructure for OVSDB

2015-06-30 Thread Gal Sagie
Thanks for the inputs, i cant say i agree because:

1) This is not just a scale issue, this is also a Single point of
failure/HA/Redundancy problem

2) As the code size increase it will be harder to do this abstraction and
it will be more error prone

3) I wasn't suggesting to actually replace the DB now, just make the API
pluggable now and plug it to ovsdb-server
current implementation, so in the future we would have an easier and
safer way to plug other solutions
because we gave this point some thought at this stage in time.

But since i am the only one thinking like that, i respect everyone else
opinion :)

On Tue, Jun 30, 2015 at 11:42 PM, Kyle Mestery  wrote:

> On Tue, Jun 30, 2015 at 2:35 PM, Russell Bryant 
> wrote:
>
>> On 06/30/2015 04:42 AM, Gal Sagie wrote:
>> > Hi Justin,
>> >
>> > The idea was not to choose one database implementation over the other,
>> > but design things in such a way
>> > that its pluggable, so in terms of OVN, switching between
>> > implementations is transparent (as much as possible)
>> >
>> > I think that different setups can have different scale criteria and
>> > might need different DB solutions.
>> >
>> > I think the IDL library (the python and C implementations) will be
>> > rather easy to adjust as OVSDB have clear API
>> >
>> > So doing this work now, we can easily change and experiments with
>> > different DB solutions
>>
>> Ben and Justin have made a pretty good case that this is a premature
>> since we really haven't done much real testing to see how ovsdb-server
>> scales.  This could potentially be a pretty big distraction and I'm not
>> sure it's worth it.  I don't think adding this later will be
>> significantly more difficult than it is today if it's needed.
>>
>> If someone wants to do some experimentation in the short term, it's
>> certainly possible.  There is an API generated from the ovsdb schema
>> that you could re-implement.
>>
>> It might be better time spent to think about OVN testing and how we can
>> do good, reproducible scale tests.  Ben has done some awesome work here,
>> but it seems we may need to a multi-node environment since at some point
>> it sounds like the number of processes on a single host becomes a
>> problem in the simulated test env.
>>
>>
> +1
>
> Thanks for taking the time to answer this one everyone, and I agree with
> Russell that it makes more sense to do this later down the road rather than
> distract things at the moment.
>
> Thanks,
> Kyle
>
>
>> --
>> Russell Bryant
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>


-- 
Best Regards ,

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


Re: [ovs-dev] [RFC rhel-dkms] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Alex Wang
Hey Flavio,

Thx a lot for the suggestions,

Please see my reply inline,

On Tue, Jun 30, 2015 at 10:41 AM, Flavio Leitner  wrote:

> >
> > diff --git a/rhel/dkms.conf.in b/rhel/dkms.conf.in
> > new file mode 100644
> > index 000..2c90b4d
> > --- /dev/null
> > +++ b/rhel/dkms.conf.in
> > @@ -0,0 +1,11 @@
>
> This file could be generated by the %install section so that
> you don't have an extra file in OVS tree, macros are expanded
> as usual and if we decide to have dkms support to specific
> distros, then the config remains inside each spec.
> It's just a suggestion to reduce the noise.
>
>

Sure, for all examples I saw, they define the dkms.conf file inside the spec
file.  I'll do that~




>
> > +MODULES=( __MODULES__ )
> > +
> > +PACKAGE_NAME="openvswitch"
> > +PACKAGE_VERSION="__VERSION__"
> > +MAKE="./configure --with-linux='${kernel_source_dir}' && make -C
> datapath/linux"
>
> That's the instruction to build the module, see below.
>
>
> > +for __idx in ${!MODULES[@]}; do
> > +BUILT_MODULE_NAME[__idx]=${MODULES[__idx]}
> > +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> > +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> > +done
> > +AUTOINSTALL=yes
> > diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/
> openvswitch-dkms.spec.in
> > new file mode 100644
> > index 000..591418b
> > --- /dev/null
> > +++ b/rhel/openvswitch-dkms.spec.in
> > @@ -0,0 +1,99 @@
> > +# Spec file for Open vSwitch kernel modules using DKMS.
> > +#
> > +# Copyright (C) 2015 Nicira, Inc.
> > +#
> > +# Copying and distribution of this file, with or without modification,
> > +# are permitted in any medium without royalty provided the copyright
> > +# notice and this notice are preserved.  This file is offered as-is,
> > +# without warranty of any kind.
> > +
> > +%define oname openvswitch
> > +
> > +Name: %{oname}-dkms
> > +Version:  @VERSION@
> > +Release:  1%{?dist}
> > +Summary:  Open vSwitch kernel module
> > +
> > +Group:System/Kernel
> > +License:  GPLv2
> > +URL:  http://openvswitch.org/
> > +Source:   %{oname}-%{version}.tar.gz
> > +Requires: autoconf, gcc, make
> > +Requires(post):   dkms
> > +Requires(preun):  dkms
> > +BuildRoot:%(mktemp -ud
> %{_tmppath}/%{name}-%{version}-%{release}-XX)
> > +
> > +# Without this we get an empty openvswitch-debuginfo package (whose name
> > +# conflicts with the openvswitch-debuginfo package for OVS userspace).
> > +%undefine _enable_debug_packages
> > +
> > +
> > +%description
> > +Open vSwitch Linux kernel module.
> > +
> > +
> > +%prep
> > +%setup -n %{oname}-%{version}
> > +
> > +cat > %{oname}.conf << EOF
> > +override %{oname} * extra/%{oname}
> > +override %{oname} * weak-updates/%{oname}
> > +EOF
> > +
> > +
> > +%build
> > +# for running the '%{__make} -C datapath print-build-modules' below.
> > +./configure
> > +
> > +
> > +%install
> > +%{__rm} -rf %{buildroot}
> > +
> > +# Kernel module sources install for dkms
> > +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > +
> > +# check we can get kernel module names
> > +%{__make} -C datapath print-build-modules
> > +
> > +# Prepare dkms.conf from the dkms.conf.in template
> > +%{__sed} "s/__VERSION__/%{version}-%{release}/g;
> s/__MODULES__/`%{__make} -C datapath print-build-modules | grep -v make`/"
> rhel/dkms.conf.in > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf
>
> Yeap, we could generate the entire file here.
>
> > +
> > +# We don't need the debian folder in there, just upstream sources.
> > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/debian
> > +# We don't need the rhel stuff in there either.
> > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/rhel
> > +# We don't need the xenserver stuff in there either.
> > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/xenserver
> > +# And we should also clean useless license files, which are already
> > +# descriped in our debian/copyright anyway.
> > +%{__rm} -f %{buildroot}%{_usrsrc}/%{oname}-%{version}/COPYING \
> > + %{buildroot}%{_usrsrc}/%{oname}-%{version}/LICENSE
>
> I am not really sure if the above is possible.
>
>

You mean the above will still be packaged?  or you mean it is not needed?



> > +
> > +install -d %{buildroot}%{_sysconfdir}/depmod.d/
> > +install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
> > +
> > +
> > +%post
> > +# Add to DKMS registry
> > +isadded=`dkms status -m "%{oname}" -v "%{version}"`
> > +if [ "x${isadded}" = "x" ] ; then
> > +dkms add -m "%{oname}" -v "%{version}" || :
> > +fi
> > +dkms build -m "%{oname}" -v "%{version}" || :
>
> Here it actually builds the module, see below.
>
>
> > +dkms install -m "%{oname}" -v "%{version}" --force || :
> > +
> > +
> > +%preun
> > +# Remove all versions from DKMS registry
> > +dkms remove -m "%{oname}" -v "%{version}" --all || :
> > +
> > +
> > +%clea

[ovs-dev] [PATCH] odp-util: Combine dpif_backer_support with odp_parms.

2015-06-30 Thread Joe Stringer
Both the ofproto layer and the odp-util layer have recently added
notions about fields supported by the datapath. This commit merges the
two into the same structure.

Signed-off-by: Joe Stringer 
---
 lib/dpif-netdev.c | 14 +++---
 lib/odp-util.c|  4 ++--
 lib/odp-util.h| 34 --
 lib/tnl-ports.c   |  4 ++--
 ofproto/bond.c|  2 +-
 ofproto/ofproto-dpif-upcall.c |  8 ++--
 ofproto/ofproto-dpif-xlate.c  |  8 
 ofproto/ofproto-dpif-xlate.h  |  2 +-
 ofproto/ofproto-dpif.c| 22 +-
 ofproto/ofproto-dpif.h| 30 ++
 tests/test-odp.c  |  4 +++-
 11 files changed, 65 insertions(+), 67 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5009c5f..10dc8b1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -87,6 +87,15 @@ static struct shash dp_netdevs 
OVS_GUARDED_BY(dp_netdev_mutex)
 
 static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
+static struct odp_support dp_netdev_support = {
+.variable_length_userdata = true,
+.max_mpls_depth = SIZE_MAX,
+.masked_set_action = true,
+.recirc = true,
+.tnl_push_pop = true,
+.ufid = true
+};
+
 /* Stores a miniflow with inline values */
 
 struct netdev_flow_key {
@@ -1825,8 +1834,7 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
*netdev_flow,
 struct odp_flow_key_parms odp_parms = {
 .flow = &netdev_flow->flow,
 .mask = &wc.masks,
-.recirc = true,
-.max_mpls_depth = SIZE_MAX,
+.support = dp_netdev_support,
 };
 
 miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
@@ -3024,7 +3032,7 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, struct 
dp_packet *packet_,
 .flow = flow,
 .mask = &wc->masks,
 .odp_in_port = flow->in_port.odp_port,
-.recirc = true,
+.support = dp_netdev_support,
 };
 
 ofpbuf_init(&key, 0);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 56979ac..2f4274c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3433,7 +3433,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 
 nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
 
-if (parms->recirc) {
+if (parms->support.recirc) {
 nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID, data->recirc_id);
 nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, data->dp_hash);
 }
@@ -3508,7 +3508,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 
 n = flow_count_mpls_labels(flow, NULL);
 if (export_mask) {
-n = MIN(n, parms->max_mpls_depth);
+n = MIN(n, parms->support.max_mpls_depth);
 }
 mpls_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_MPLS,
 n * sizeof *mpls_key);
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 59d29f3..6508fa8 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -158,6 +158,31 @@ int odp_flow_from_string(const char *s,
  const struct simap *port_names,
  struct ofpbuf *, struct ofpbuf *);
 
+/* Stores the various features which the datapath supports. */
+struct odp_support {
+/* True if the datapath supports variable-length
+ * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
+ * False if the datapath supports only 8-byte (or shorter) userdata. */
+bool variable_length_userdata;
+
+/* Maximum number of MPLS label stack entries that the datapath supports
+ * in a match */
+size_t max_mpls_depth;
+
+/* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
+ * actions. */
+bool masked_set_action;
+
+/* True if the datapath supports recirculation. */
+bool recirc;
+
+/* True if the datapath supports tnl_push and pop actions. */
+bool tnl_push_pop;
+
+/* True if the datapath supports OVS_FLOW_ATTR_UFID. */
+bool ufid;
+};
+
 struct odp_flow_key_parms {
 /* The flow and mask to be serialized. In the case of masks, 'flow'
  * is used as a template to determine how to interpret 'mask'.  For
@@ -173,12 +198,9 @@ struct odp_flow_key_parms {
 * port. */
 odp_port_t odp_in_port;
 
-/* Indicates support for recirculation fields. If this is true, then
- * these fields will always be serialised. */
-bool recirc;
-
-/* Only used for mask translation: */
-size_t max_mpls_depth;
+/* Indicates support for various fields. If the datapath supports a field,
+ * then it will always be serialised. */
+struct odp_support support;
 };
 
 void odp_flow_key_from_flow(const struct odp_flow_key_parms *, struct ofpbuf 
*);
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index a0a73c8..dd92359 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-po

Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Andy Zhou
On Tue, Jun 30, 2015 at 10:36 AM, Sabyasachi Sengupta
 wrote:
>
> v2: fixed a bug in do_dump that was calling print_table incorrectly when
> 'table' option is specified
>
> Added capability of displaying tables through an optional 'table' argument
> to 'ovsdb-client dump'. When specified, ovsdb-client will iterate
> through all tables in the chosen ovsdb, and create a transaction only
> for that table instead of all, and then print the response for that
> transaction.
>
> Signed-off-by: Sabyasachi Sengupta 

Is actually adding a new feature rather than fix a bug?
If so, you may also want to update the man pages with the new usage
and add unit tests for it.

If this patch also fixes bugs, then the bug fix should be in a separate patch.

What would be default argument for the new 'table' argument if it is
not specified?

Just to make sure we are not feature creeping, Can this be achieved via
shell scripts running on top of 'ovsdb-client dump db'?

>
> ---
>  ovsdb/ovsdb-client.c |  105
> +-
>  1 files changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> index 2942953..84474d0 100644
> --- a/ovsdb/ovsdb-client.c
> +++ b/ovsdb/ovsdb-client.c
> @@ -255,8 +255,8 @@ usage(void)
> "\n  monitor [SERVER] [DATABASE] ALL\n"
> "monitor all changes to all columns in all tables\n"
> "in DATBASE on SERVER.\n"
> -   "\n  dump [SERVER] [DATABASE]\n"
> -   "dump contents of DATABASE on SERVER to stdout\n"
> +   "\n  dump [SERVER] [DATABASE] [TABLE]\n"
> +   "dump contents of table(s) in DATABASE on SERVER to
> stdout\n"
> "\nThe default SERVER is unix:%s/db.sock.\n"
> "The default DATABASE is Open_vSwitch.\n",
> program_name, program_name, ovs_rundir());
> @@ -1042,44 +1042,80 @@ dump_table(const struct ovsdb_table_schema *ts,
> struct json_array *rows)
>  }
>
>  static void
> -do_dump(struct jsonrpc *rpc, const char *database,
> -int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> +create_table_transaction(struct json *transaction,
> + const struct shash_node *table)
> +{
> +const struct ovsdb_table_schema *ts = table->data;
> +struct json *op, *columns;
> +struct shash_node *node;
> +
> +columns = json_array_create_empty();
> +SHASH_FOR_EACH (node, &ts->columns) {
> +const struct ovsdb_column *column = node->data;
> +
> +if (strcmp(column->name, "_version")) {
> +json_array_add(columns, json_string_create(column->name));
> +}
> +}
> +
> +op = json_object_create();
> +json_object_put_string(op, "op", "select");
> +json_object_put_string(op, "table", table->name);
> +json_object_put(op, "where", json_array_create_empty());
> +json_object_put(op, "columns", columns);
> +json_array_add(transaction, op);
> +}
> +
> +static void
> +print_table(const struct json *op_result, const struct shash_node *table)
> +{
> +const struct ovsdb_table_schema *ts = table->data;
> +struct json *rows;
> +
> +if (op_result->type != JSON_OBJECT
> +|| !(rows = shash_find_data(json_object(op_result), "rows"))
> +|| rows->type != JSON_ARRAY) {
> +ovs_fatal(0, "%s table reply is not an object with a \"rows\" "
> +  "member array: %s", ts->name, json_to_string(op_result,
> 0));
> +}
> +dump_table(ts, &rows->u.array);
> +}
> +
> +static void
> +do_dump(struct jsonrpc *rpc, const char *database, int argc, char *argv[])
>  {
>  struct jsonrpc_msg *request, *reply;
>  struct ovsdb_schema *schema;
>  struct json *transaction;
>
>  const struct shash_node **tables;
> -size_t n_tables;
> +size_t n_tables, n_tables_out;
> +char *table_name = (argc == 1) ? argv[0] : NULL;
> +bool table_found = false;
>
>  size_t i;
>
>  schema = fetch_schema(rpc, database);
>  tables = shash_sort(&schema->tables);
>  n_tables = shash_count(&schema->tables);
> +n_tables_out = table_name ? 1 : n_tables;
>
>  /* Construct transaction to retrieve entire database. */
>  transaction = json_array_create_1(json_string_create(database));
>  for (i = 0; i < n_tables; i++) {
> -const struct ovsdb_table_schema *ts = tables[i]->data;
> -struct json *op, *columns;
> -struct shash_node *node;
> -
> -columns = json_array_create_empty();
> -SHASH_FOR_EACH (node, &ts->columns) {
> -const struct ovsdb_column *column = node->data;
> -
> -if (strcmp(column->name, "_version")) {
> -json_array_add(columns, json_string_create(column->name));
> +if (table_name) {
> +if (!strncmp(tables[i]->name, table_name,
> + MIN(strlen(tables[i]->name), strlen(table_name
> {
> +create_table_transaction(transaction, tables[i]);

Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Sabyasachi Sengupta



Is actually adding a new feature rather than fix a bug?


Actually v2 fixes a bug that was in the original patch (v1) that was sent 
out.



If so, you may also want to update the man pages with the new usage
and add unit tests for it.


I'll add the manpage extension and resend v3, but I'm not sure about the 
unit tests that needs to be added. Can you point me to what needs to be 
done to the framework?



If this patch also fixes bugs, then the bug fix should be in a separate patch.


No, it does not fix any bug. It just fixes a defect in my original 
implementation.



What would be default argument for the new 'table' argument if it is
not specified?


It dumps the whole db as indicated in the usage:

  dump [SERVER] [DATABASE] [TABLE]
dump contents of table(s) in DATABASE on SERVER to stdout

"TABLE" is optional.


Just to make sure we are not feature creeping, Can this be achieved via
shell scripts running on top of 'ovsdb-client dump db'?


Currently 'ovsdb-client dump' dumps the whole db, which can be lot of 
output. Cannot think of an easier way than writing a python script that goes 
over the entire output, then parses and prints the particular table in 
question. Bash scripting may not be so easy to write as tables can have 
varied number of rows. Its probably more practical and user friendly to have 
something in ovsdb-client itself that dumps a table rather than have users 
distribute/write custom scripts, especially if db is large in size..

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


[ovs-dev] [PATCHv2] odp-util: Combine dpif_backer_support with odp_parms.

2015-06-30 Thread Joe Stringer
Both the ofproto layer and the odp-util layer have recently added
notions about fields supported by the datapath. This commit merges the
two into the same structure.

Signed-off-by: Joe Stringer 
---
v2: Rebase against master.
---
 lib/dpif-netdev.c | 14 +++---
 lib/odp-util.c|  4 ++--
 lib/odp-util.h| 35 ---
 lib/tnl-ports.c   |  4 ++--
 ofproto/bond.c|  2 +-
 ofproto/ofproto-dpif-upcall.c |  7 ++-
 ofproto/ofproto-dpif-xlate.c  |  8 
 ofproto/ofproto-dpif-xlate.h  |  2 +-
 ofproto/ofproto-dpif.c| 22 +-
 ofproto/ofproto-dpif.h| 30 ++
 tests/test-odp.c  |  4 +++-
 11 files changed, 65 insertions(+), 67 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index dd6bb1f..653e8e5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -87,6 +87,15 @@ static struct shash dp_netdevs 
OVS_GUARDED_BY(dp_netdev_mutex)
 
 static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
+static struct odp_support dp_netdev_support = {
+.variable_length_userdata = true,
+.max_mpls_depth = SIZE_MAX,
+.masked_set_action = true,
+.recirc = true,
+.tnl_push_pop = true,
+.ufid = true
+};
+
 /* Stores a miniflow with inline values */
 
 struct netdev_flow_key {
@@ -1825,8 +1834,7 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
*netdev_flow,
 struct odp_flow_key_parms odp_parms = {
 .flow = &netdev_flow->flow,
 .mask = &wc.masks,
-.recirc = true,
-.max_mpls_depth = SIZE_MAX,
+.support = dp_netdev_support,
 };
 
 miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
@@ -3031,7 +3039,7 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, struct 
dp_packet *packet_,
 .flow = flow,
 .mask = &wc->masks,
 .odp_in_port = flow->in_port.odp_port,
-.recirc = true,
+.support = dp_netdev_support,
 };
 
 ofpbuf_init(&key, 0);
diff --git a/lib/odp-util.c b/lib/odp-util.c
index c70ee0f..ee763a9 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3466,7 +3466,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 
 nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
 
-if (parms->recirc) {
+if (parms->support.recirc) {
 nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID, data->recirc_id);
 nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, data->dp_hash);
 }
@@ -3541,7 +3541,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 
 n = flow_count_mpls_labels(flow, NULL);
 if (export_mask) {
-n = MIN(n, parms->max_mpls_depth);
+n = MIN(n, parms->support.max_mpls_depth);
 }
 mpls_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_MPLS,
 n * sizeof *mpls_key);
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 763e3f9..1a75e1a 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -158,6 +158,31 @@ int odp_flow_from_string(const char *s,
  const struct simap *port_names,
  struct ofpbuf *, struct ofpbuf *);
 
+/* Stores the various features which the datapath supports. */
+struct odp_support {
+/* True if the datapath supports variable-length
+ * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
+ * False if the datapath supports only 8-byte (or shorter) userdata. */
+bool variable_length_userdata;
+
+/* Maximum number of MPLS label stack entries that the datapath supports
+ * in a match */
+size_t max_mpls_depth;
+
+/* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
+ * actions. */
+bool masked_set_action;
+
+/* True if the datapath supports recirculation. */
+bool recirc;
+
+/* True if the datapath supports tnl_push and pop actions. */
+bool tnl_push_pop;
+
+/* True if the datapath supports OVS_FLOW_ATTR_UFID. */
+bool ufid;
+};
+
 struct odp_flow_key_parms {
 /* The flow and mask to be serialized. In the case of masks, 'flow'
  * is used as a template to determine how to interpret 'mask'.  For
@@ -173,13 +198,9 @@ struct odp_flow_key_parms {
 * port. */
 odp_port_t odp_in_port;
 
-/* Indicates support for recirculation fields. If this is true, then
- * these fields will always be serialised. */
-bool recirc;
-
-/* Only used for mask translation: */
-
-size_t max_mpls_depth;
+/* Indicates support for various fields. If the datapath supports a field,
+ * then it will always be serialised. */
+struct odp_support support;
 
 /* The netlink formatted version of the flow. It is used in cases where
  * the mask cannot be constructed from the OVS internal representation
diff --git a/lib

Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Andy Zhou
On Tue, Jun 30, 2015 at 4:43 PM, Sabyasachi Sengupta
 wrote:
>
>> Is actually adding a new feature rather than fix a bug?
>
>
> Actually v2 fixes a bug that was in the original patch (v1) that was sent
> out.
>
Ah, We usually write version delta comments in a separate section,
with a heading of "v1->v2: "
The general comment is only about the patch itself.
>> If so, you may also want to update the man pages with the new usage
>> and add unit tests for it.
>
>
> I'll add the manpage extension and resend v3, but I'm not sure about the
> unit tests that needs to be added. Can you point me to what needs to be done
> to the framework?
OVS uses autotest framework, The tests are in the "tests" directory.  Look into
ovsdb-tool.at for examples.
>
>> If this patch also fixes bugs, then the bug fix should be in a separate
>> patch.
>
>
> No, it does not fix any bug. It just fixes a defect in my original
> implementation.
>
>> What would be default argument for the new 'table' argument if it is
>> not specified?
>
>
> It dumps the whole db as indicated in the usage:
>
>   dump [SERVER] [DATABASE] [TABLE]
> dump contents of table(s) in DATABASE on SERVER to stdout
>
> "TABLE" is optional.
>
That's fine. The default is all tables. It should be documented.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Alex Wang
This commit adds a rpmbuild spec file for building ovs datapath
in dkms similar to the one for debian.

Signed-off-by: Alex Wang 

---
RFC->PATCH:
- generate the dkms.conf inside %install.
- remove rhel/dkms.conf.in.
---
 rhel/.gitignore   |1 +
 rhel/automake.mk  |5 +++
 rhel/openvswitch-dkms.spec.in |  100 +
 3 files changed, 106 insertions(+)
 create mode 100644 rhel/openvswitch-dkms.spec.in

diff --git a/rhel/.gitignore b/rhel/.gitignore
index fa2554f..164bb66 100644
--- a/rhel/.gitignore
+++ b/rhel/.gitignore
@@ -1,3 +1,4 @@
+openvswitch-dkms.spec
 openvswitch-kmod-rhel5.spec
 openvswitch-kmod-rhel6.spec
 openvswitch-kmod-fedora.spec
diff --git a/rhel/automake.mk b/rhel/automake.mk
index f6272a3..d263325 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -12,6 +12,8 @@ EXTRA_DIST += \
rhel/etc_logrotate.d_openvswitch \
rhel/etc_sysconfig_network-scripts_ifdown-ovs \
rhel/etc_sysconfig_network-scripts_ifup-ovs \
+   rhel/openvswitch-dkms.spec \
+   rhel/openvswitch-dkms.spec.in \
rhel/openvswitch-kmod-rhel6.spec \
rhel/openvswitch-kmod-rhel6.spec.in \
rhel/openvswitch-kmod.files \
@@ -33,6 +35,9 @@ update_rhel_spec = \
 < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
   if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi
 
+$(srcdir)/rhel/openvswitch-dkms.spec: rhel/openvswitch-dkms.spec.in 
$(top_builddir)/config.status
+   $(update_rhel_spec)
+
 $(srcdir)/rhel/openvswitch-kmod-rhel6.spec: 
rhel/openvswitch-kmod-rhel6.spec.in $(top_builddir)/config.status
$(update_rhel_spec)
 
diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/openvswitch-dkms.spec.in
new file mode 100644
index 000..6b6c30b
--- /dev/null
+++ b/rhel/openvswitch-dkms.spec.in
@@ -0,0 +1,100 @@
+# Spec file for Open vSwitch kernel modules using DKMS.
+#
+# Copyright (C) 2015 Nicira, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.  This file is offered as-is,
+# without warranty of any kind.
+
+%define oname openvswitch
+
+Name: %{oname}-dkms
+Version:  @VERSION@
+Release:  1%{?dist}
+Summary:  Open vSwitch kernel module
+
+Group:System/Kernel
+License:  GPLv2
+URL:  http://openvswitch.org/
+Source:   %{oname}-%{version}.tar.gz
+Requires: autoconf, gcc, make
+Requires(post):   dkms
+Requires(preun):  dkms
+BuildRoot:%(mktemp -ud 
%{_tmppath}/%{name}-%{version}-%{release}-XX)
+
+# Without this we get an empty openvswitch-debuginfo package (whose name
+# conflicts with the openvswitch-debuginfo package for OVS userspace).
+%undefine _enable_debug_packages
+
+
+%description
+Open vSwitch Linux kernel module.
+
+
+%prep
+%setup -n %{oname}-%{version}
+
+cat > %{oname}.conf << EOF
+override %{oname} * extra/%{oname}
+override %{oname} * weak-updates/%{oname}
+EOF
+
+
+%build
+# for running the '%{__make} -C datapath print-build-modules' below.
+./configure
+
+
+%install
+%{__rm} -rf %{buildroot}
+
+# Kernel module sources install for dkms
+%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
+%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
+
+# check we can get kernel module names
+%{__make} -C datapath print-build-modules
+
+# Prepare dkms.conf
+cat > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf << EOF
+MODULES=( `%{__make} -C datapath print-build-modules | grep -v make` )
+
+PACKAGE_NAME="openvswitch"
+PACKAGE_VERSION="%{version}-%{release}"
+MAKE="./configure --with-linux='\${kernel_source_dir}' && make -C 
datapath/linux"
+for __idx in \${!MODULES[@]}; do
+BUILT_MODULE_NAME[__idx]=\${MODULES[__idx]}
+BUILT_MODULE_LOCATION[__idx]=datapath/linux/
+DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
+done
+AUTOINSTALL=yes
+EOF
+
+install -d %{buildroot}%{_sysconfdir}/depmod.d/
+install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
+
+
+%post
+# Add to DKMS registry
+isadded=`dkms status -m "%{oname}" -v "%{version}"`
+if [ "x${isadded}" = "x" ] ; then
+dkms add -m "%{oname}" -v "%{version}" || :
+fi
+dkms build -m "%{oname}" -v "%{version}" || :
+dkms install -m "%{oname}" -v "%{version}" --force || :
+
+
+%preun
+# Remove all versions from DKMS registry
+dkms remove -m "%{oname}" -v "%{version}" --all || :
+
+
+%clean
+%{__rm} -rf %{buildroot}
+
+
+%files
+%defattr(755,root,root,755)
+%{_usrsrc}/%{oname}-%{version}/
+/etc/depmod.d/openvswitch.conf
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCHv2] odp-util: Combine dpif_backer_support with odp_parms.

2015-06-30 Thread Andy Zhou
On Tue, Jun 30, 2015 at 4:52 PM, Joe Stringer  wrote:
> Both the ofproto layer and the odp-util layer have recently added
> notions about fields supported by the datapath. This commit merges the
> two into the same structure.

while I see the benfits of combining the structure, I am not sure it
makes conceptual sense.

It seems we need a structure to store per dpif datpath features and
this strcture should be defined in ofproto-dpif.h.

odp-util also needs some of the same information, but not all of them,
right? Then why not define odp_support
as a seperate sturcture that contains only information needed for
serialization, and add a a function to fill
an odp_support struct  from dpif_backer_support when necessary?


>
> Signed-off-by: Joe Stringer 
> ---
> v2: Rebase against master.
> ---
>  lib/dpif-netdev.c | 14 +++---
>  lib/odp-util.c|  4 ++--
>  lib/odp-util.h| 35 ---
>  lib/tnl-ports.c   |  4 ++--
>  ofproto/bond.c|  2 +-
>  ofproto/ofproto-dpif-upcall.c |  7 ++-
>  ofproto/ofproto-dpif-xlate.c  |  8 
>  ofproto/ofproto-dpif-xlate.h  |  2 +-
>  ofproto/ofproto-dpif.c| 22 +-
>  ofproto/ofproto-dpif.h| 30 ++
>  tests/test-odp.c  |  4 +++-
>  11 files changed, 65 insertions(+), 67 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index dd6bb1f..653e8e5 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -87,6 +87,15 @@ static struct shash dp_netdevs 
> OVS_GUARDED_BY(dp_netdev_mutex)
>
>  static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
>
> +static struct odp_support dp_netdev_support = {
> +.variable_length_userdata = true,
> +.max_mpls_depth = SIZE_MAX,
> +.masked_set_action = true,
> +.recirc = true,
> +.tnl_push_pop = true,
> +.ufid = true
> +};
> +
>  /* Stores a miniflow with inline values */
>
>  struct netdev_flow_key {
> @@ -1825,8 +1834,7 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
> *netdev_flow,
>  struct odp_flow_key_parms odp_parms = {
>  .flow = &netdev_flow->flow,
>  .mask = &wc.masks,
> -.recirc = true,
> -.max_mpls_depth = SIZE_MAX,
> +.support = dp_netdev_support,
>  };
>
>  miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
> @@ -3031,7 +3039,7 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, 
> struct dp_packet *packet_,
>  .flow = flow,
>  .mask = &wc->masks,
>  .odp_in_port = flow->in_port.odp_port,
> -.recirc = true,
> +.support = dp_netdev_support,
>  };
>
>  ofpbuf_init(&key, 0);
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index c70ee0f..ee763a9 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -3466,7 +3466,7 @@ odp_flow_key_from_flow__(const struct 
> odp_flow_key_parms *parms,
>
>  nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
>
> -if (parms->recirc) {
> +if (parms->support.recirc) {
>  nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID, data->recirc_id);
>  nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, data->dp_hash);
>  }
> @@ -3541,7 +3541,7 @@ odp_flow_key_from_flow__(const struct 
> odp_flow_key_parms *parms,
>
>  n = flow_count_mpls_labels(flow, NULL);
>  if (export_mask) {
> -n = MIN(n, parms->max_mpls_depth);
> +n = MIN(n, parms->support.max_mpls_depth);
>  }
>  mpls_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_MPLS,
>  n * sizeof *mpls_key);
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 763e3f9..1a75e1a 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -158,6 +158,31 @@ int odp_flow_from_string(const char *s,
>   const struct simap *port_names,
>   struct ofpbuf *, struct ofpbuf *);
>
> +/* Stores the various features which the datapath supports. */
> +struct odp_support {
> +/* True if the datapath supports variable-length
> + * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions.
> + * False if the datapath supports only 8-byte (or shorter) userdata. */
> +bool variable_length_userdata;
> +
> +/* Maximum number of MPLS label stack entries that the datapath supports
> + * in a match */
> +size_t max_mpls_depth;
> +
> +/* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
> + * actions. */
> +bool masked_set_action;
> +
> +/* True if the datapath supports recirculation. */
> +bool recirc;
> +
> +/* True if the datapath supports tnl_push and pop actions. */
> +bool tnl_push_pop;
> +
> +/* True if the datapath supports OVS_FLOW_ATTR_UFID. */
> +bool ufid;
> +};
> +
>  struct odp_flow_key_parms {
>  /* The

Re: [ovs-dev] [RFC rhel-dkms] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Flavio Leitner
On Tue, Jun 30, 2015 at 03:56:44PM -0700, Alex Wang wrote:
> Hey Flavio,
> 
> Thx a lot for the suggestions,
> 
> Please see my reply inline,
> 
> On Tue, Jun 30, 2015 at 10:41 AM, Flavio Leitner  wrote:
> 
> > >
> > > diff --git a/rhel/dkms.conf.in b/rhel/dkms.conf.in
> > > new file mode 100644
> > > index 000..2c90b4d
> > > --- /dev/null
> > > +++ b/rhel/dkms.conf.in
> > > @@ -0,0 +1,11 @@
> >
> > This file could be generated by the %install section so that
> > you don't have an extra file in OVS tree, macros are expanded
> > as usual and if we decide to have dkms support to specific
> > distros, then the config remains inside each spec.
> > It's just a suggestion to reduce the noise.
> >
> >
> 
> Sure, for all examples I saw, they define the dkms.conf file inside the spec
> file.  I'll do that~
> 
> 
> 
> 
> >
> > > +MODULES=( __MODULES__ )
> > > +
> > > +PACKAGE_NAME="openvswitch"
> > > +PACKAGE_VERSION="__VERSION__"
> > > +MAKE="./configure --with-linux='${kernel_source_dir}' && make -C
> > datapath/linux"
> >
> > That's the instruction to build the module, see below.
> >
> >
> > > +for __idx in ${!MODULES[@]}; do
> > > +BUILT_MODULE_NAME[__idx]=${MODULES[__idx]}
> > > +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> > > +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> > > +done
> > > +AUTOINSTALL=yes
> > > diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/
> > openvswitch-dkms.spec.in
> > > new file mode 100644
> > > index 000..591418b
> > > --- /dev/null
> > > +++ b/rhel/openvswitch-dkms.spec.in
> > > @@ -0,0 +1,99 @@
> > > +# Spec file for Open vSwitch kernel modules using DKMS.
> > > +#
> > > +# Copyright (C) 2015 Nicira, Inc.
> > > +#
> > > +# Copying and distribution of this file, with or without modification,
> > > +# are permitted in any medium without royalty provided the copyright
> > > +# notice and this notice are preserved.  This file is offered as-is,
> > > +# without warranty of any kind.
> > > +
> > > +%define oname openvswitch
> > > +
> > > +Name: %{oname}-dkms
> > > +Version:  @VERSION@
> > > +Release:  1%{?dist}
> > > +Summary:  Open vSwitch kernel module
> > > +
> > > +Group:System/Kernel
> > > +License:  GPLv2
> > > +URL:  http://openvswitch.org/
> > > +Source:   %{oname}-%{version}.tar.gz
> > > +Requires: autoconf, gcc, make
> > > +Requires(post):   dkms
> > > +Requires(preun):  dkms
> > > +BuildRoot:%(mktemp -ud
> > %{_tmppath}/%{name}-%{version}-%{release}-XX)
> > > +
> > > +# Without this we get an empty openvswitch-debuginfo package (whose name
> > > +# conflicts with the openvswitch-debuginfo package for OVS userspace).
> > > +%undefine _enable_debug_packages
> > > +
> > > +
> > > +%description
> > > +Open vSwitch Linux kernel module.
> > > +
> > > +
> > > +%prep
> > > +%setup -n %{oname}-%{version}
> > > +
> > > +cat > %{oname}.conf << EOF
> > > +override %{oname} * extra/%{oname}
> > > +override %{oname} * weak-updates/%{oname}
> > > +EOF
> > > +
> > > +
> > > +%build
> > > +# for running the '%{__make} -C datapath print-build-modules' below.
> > > +./configure
> > > +
> > > +
> > > +%install
> > > +%{__rm} -rf %{buildroot}
> > > +
> > > +# Kernel module sources install for dkms
> > > +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > > +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > > +
> > > +# check we can get kernel module names
> > > +%{__make} -C datapath print-build-modules
> > > +
> > > +# Prepare dkms.conf from the dkms.conf.in template
> > > +%{__sed} "s/__VERSION__/%{version}-%{release}/g;
> > s/__MODULES__/`%{__make} -C datapath print-build-modules | grep -v make`/"
> > rhel/dkms.conf.in > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf
> >
> > Yeap, we could generate the entire file here.
> >
> > > +
> > > +# We don't need the debian folder in there, just upstream sources.
> > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/debian
> > > +# We don't need the rhel stuff in there either.
> > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/rhel
> > > +# We don't need the xenserver stuff in there either.
> > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/xenserver
> > > +# And we should also clean useless license files, which are already
> > > +# descriped in our debian/copyright anyway.
> > > +%{__rm} -f %{buildroot}%{_usrsrc}/%{oname}-%{version}/COPYING \
> > > + %{buildroot}%{_usrsrc}/%{oname}-%{version}/LICENSE
> >
> > I am not really sure if the above is possible.
> >
> >
> 
> You mean the above will still be packaged?  or you mean it is not needed?

I think it must be packaged.  If you look at the other rpms,
they always include the license/copying file.


> > > +
> > > +install -d %{buildroot}%{_sysconfdir}/depmod.d/
> > > +install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
> > > +
> > > +
> > > +%post
> > > +# Add to DKMS registry
> > > +isadded=`dkms

Re: [ovs-dev] [RFC rhel-dkms] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Alex Wang
On Tue, Jun 30, 2015 at 6:06 PM, Flavio Leitner  wrote:

> On Tue, Jun 30, 2015 at 03:56:44PM -0700, Alex Wang wrote:
> > Hey Flavio,
> >
> > Thx a lot for the suggestions,
> >
> > Please see my reply inline,
> >
> > On Tue, Jun 30, 2015 at 10:41 AM, Flavio Leitner 
> wrote:
> >
> > > >
> > > > diff --git a/rhel/dkms.conf.in b/rhel/dkms.conf.in
> > > > new file mode 100644
> > > > index 000..2c90b4d
> > > > --- /dev/null
> > > > +++ b/rhel/dkms.conf.in
> > > > @@ -0,0 +1,11 @@
> > >
> > > This file could be generated by the %install section so that
> > > you don't have an extra file in OVS tree, macros are expanded
> > > as usual and if we decide to have dkms support to specific
> > > distros, then the config remains inside each spec.
> > > It's just a suggestion to reduce the noise.
> > >
> > >
> >
> > Sure, for all examples I saw, they define the dkms.conf file inside the
> spec
> > file.  I'll do that~
> >
> >
> >
> >
> > >
> > > > +MODULES=( __MODULES__ )
> > > > +
> > > > +PACKAGE_NAME="openvswitch"
> > > > +PACKAGE_VERSION="__VERSION__"
> > > > +MAKE="./configure --with-linux='${kernel_source_dir}' && make -C
> > > datapath/linux"
> > >
> > > That's the instruction to build the module, see below.
> > >
> > >
> > > > +for __idx in ${!MODULES[@]}; do
> > > > +BUILT_MODULE_NAME[__idx]=${MODULES[__idx]}
> > > > +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> > > > +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> > > > +done
> > > > +AUTOINSTALL=yes
> > > > diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/
> > > openvswitch-dkms.spec.in
> > > > new file mode 100644
> > > > index 000..591418b
> > > > --- /dev/null
> > > > +++ b/rhel/openvswitch-dkms.spec.in
> > > > @@ -0,0 +1,99 @@
> > > > +# Spec file for Open vSwitch kernel modules using DKMS.
> > > > +#
> > > > +# Copyright (C) 2015 Nicira, Inc.
> > > > +#
> > > > +# Copying and distribution of this file, with or without
> modification,
> > > > +# are permitted in any medium without royalty provided the copyright
> > > > +# notice and this notice are preserved.  This file is offered as-is,
> > > > +# without warranty of any kind.
> > > > +
> > > > +%define oname openvswitch
> > > > +
> > > > +Name: %{oname}-dkms
> > > > +Version:  @VERSION@
> > > > +Release:  1%{?dist}
> > > > +Summary:  Open vSwitch kernel module
> > > > +
> > > > +Group:System/Kernel
> > > > +License:  GPLv2
> > > > +URL:  http://openvswitch.org/
> > > > +Source:   %{oname}-%{version}.tar.gz
> > > > +Requires: autoconf, gcc, make
> > > > +Requires(post):   dkms
> > > > +Requires(preun):  dkms
> > > > +BuildRoot:%(mktemp -ud
> > > %{_tmppath}/%{name}-%{version}-%{release}-XX)
> > > > +
> > > > +# Without this we get an empty openvswitch-debuginfo package (whose
> name
> > > > +# conflicts with the openvswitch-debuginfo package for OVS
> userspace).
> > > > +%undefine _enable_debug_packages
> > > > +
> > > > +
> > > > +%description
> > > > +Open vSwitch Linux kernel module.
> > > > +
> > > > +
> > > > +%prep
> > > > +%setup -n %{oname}-%{version}
> > > > +
> > > > +cat > %{oname}.conf << EOF
> > > > +override %{oname} * extra/%{oname}
> > > > +override %{oname} * weak-updates/%{oname}
> > > > +EOF
> > > > +
> > > > +
> > > > +%build
> > > > +# for running the '%{__make} -C datapath print-build-modules' below.
> > > > +./configure
> > > > +
> > > > +
> > > > +%install
> > > > +%{__rm} -rf %{buildroot}
> > > > +
> > > > +# Kernel module sources install for dkms
> > > > +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > > > +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > > > +
> > > > +# check we can get kernel module names
> > > > +%{__make} -C datapath print-build-modules
> > > > +
> > > > +# Prepare dkms.conf from the dkms.conf.in template
> > > > +%{__sed} "s/__VERSION__/%{version}-%{release}/g;
> > > s/__MODULES__/`%{__make} -C datapath print-build-modules | grep -v
> make`/"
> > > rhel/dkms.conf.in >
> %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf
> > >
> > > Yeap, we could generate the entire file here.
> > >
> > > > +
> > > > +# We don't need the debian folder in there, just upstream sources.
> > > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/debian
> > > > +# We don't need the rhel stuff in there either.
> > > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/rhel
> > > > +# We don't need the xenserver stuff in there either.
> > > > +%{__rm} -rf %{buildroot}%{_usrsrc}/%{oname}-%{version}/xenserver
> > > > +# And we should also clean useless license files, which are already
> > > > +# descriped in our debian/copyright anyway.
> > > > +%{__rm} -f %{buildroot}%{_usrsrc}/%{oname}-%{version}/COPYING \
> > > > + %{buildroot}%{_usrsrc}/%{oname}-%{version}/LICENSE
> > >
> > > I am not really sure if the above is possible.
> > >
> > >
> >
> > You mean the above will still be packaged?  or you mean it is 

Re: [ovs-dev] [PATCH] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Flavio Leitner
On Tue, Jun 30, 2015 at 05:26:00PM -0700, Alex Wang wrote:
> This commit adds a rpmbuild spec file for building ovs datapath
> in dkms similar to the one for debian.
> 
> Signed-off-by: Alex Wang 
> 
> ---
> RFC->PATCH:
> - generate the dkms.conf inside %install.
> - remove rhel/dkms.conf.in.
> ---
>  rhel/.gitignore   |1 +
>  rhel/automake.mk  |5 +++
>  rhel/openvswitch-dkms.spec.in |  100 
> +
>  3 files changed, 106 insertions(+)
>  create mode 100644 rhel/openvswitch-dkms.spec.in
> 
> diff --git a/rhel/.gitignore b/rhel/.gitignore
> index fa2554f..164bb66 100644
> --- a/rhel/.gitignore
> +++ b/rhel/.gitignore
> @@ -1,3 +1,4 @@
> +openvswitch-dkms.spec
>  openvswitch-kmod-rhel5.spec
>  openvswitch-kmod-rhel6.spec
>  openvswitch-kmod-fedora.spec
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index f6272a3..d263325 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -12,6 +12,8 @@ EXTRA_DIST += \
>   rhel/etc_logrotate.d_openvswitch \
>   rhel/etc_sysconfig_network-scripts_ifdown-ovs \
>   rhel/etc_sysconfig_network-scripts_ifup-ovs \
> + rhel/openvswitch-dkms.spec \
> + rhel/openvswitch-dkms.spec.in \
>   rhel/openvswitch-kmod-rhel6.spec \
>   rhel/openvswitch-kmod-rhel6.spec.in \
>   rhel/openvswitch-kmod.files \
> @@ -33,6 +35,9 @@ update_rhel_spec = \
>  < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
>if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; 
> fi
>  
> +$(srcdir)/rhel/openvswitch-dkms.spec: rhel/openvswitch-dkms.spec.in 
> $(top_builddir)/config.status
> + $(update_rhel_spec)
> +
>  $(srcdir)/rhel/openvswitch-kmod-rhel6.spec: 
> rhel/openvswitch-kmod-rhel6.spec.in $(top_builddir)/config.status
>   $(update_rhel_spec)
>  
> diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/openvswitch-dkms.spec.in
> new file mode 100644
> index 000..6b6c30b
> --- /dev/null
> +++ b/rhel/openvswitch-dkms.spec.in
> @@ -0,0 +1,100 @@
> +# Spec file for Open vSwitch kernel modules using DKMS.
> +#
> +# Copyright (C) 2015 Nicira, Inc.
> +#
> +# Copying and distribution of this file, with or without modification,
> +# are permitted in any medium without royalty provided the copyright
> +# notice and this notice are preserved.  This file is offered as-is,
> +# without warranty of any kind.
> +
> +%define oname openvswitch
> +
> +Name: %{oname}-dkms
> +Version:  @VERSION@
> +Release:  1%{?dist}
> +Summary:  Open vSwitch kernel module
> +
> +Group:System/Kernel
> +License:  GPLv2
> +URL:  http://openvswitch.org/
> +Source:   %{oname}-%{version}.tar.gz
> +Requires: autoconf, gcc, make
> +Requires(post):   dkms
> +Requires(preun):  dkms
> +BuildRoot:%(mktemp -ud 
> %{_tmppath}/%{name}-%{version}-%{release}-XX)
> +
> +# Without this we get an empty openvswitch-debuginfo package (whose name
> +# conflicts with the openvswitch-debuginfo package for OVS userspace).
> +%undefine _enable_debug_packages
> +
> +
> +%description
> +Open vSwitch Linux kernel module.
> +
> +
> +%prep
> +%setup -n %{oname}-%{version}
> +
> +cat > %{oname}.conf << EOF
> +override %{oname} * extra/%{oname}
> +override %{oname} * weak-updates/%{oname}
> +EOF
> +
> +
> +%build
> +# for running the '%{__make} -C datapath print-build-modules' below.
> +./configure
> +
> +
> +%install
> +%{__rm} -rf %{buildroot}
> +
> +# Kernel module sources install for dkms
> +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> +
> +# check we can get kernel module names
> +%{__make} -C datapath print-build-modules
> +
> +# Prepare dkms.conf
> +cat > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf << EOF
> +MODULES=( `%{__make} -C datapath print-build-modules | grep -v make` )
> +
> +PACKAGE_NAME="openvswitch"
> +PACKAGE_VERSION="%{version}-%{release}"
> +MAKE="./configure --with-linux='\${kernel_source_dir}' && make -C 
> datapath/linux"
> +for __idx in \${!MODULES[@]}; do
> +BUILT_MODULE_NAME[__idx]=\${MODULES[__idx]}
> +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> +done
> +AUTOINSTALL=yes
> +EOF
> +
> +install -d %{buildroot}%{_sysconfdir}/depmod.d/
> +install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
> +
> +
> +%post
> +# Add to DKMS registry
> +isadded=`dkms status -m "%{oname}" -v "%{version}"`
> +if [ "x${isadded}" = "x" ] ; then
> +dkms add -m "%{oname}" -v "%{version}" || :
> +fi
> +dkms build -m "%{oname}" -v "%{version}" || :
> +dkms install -m "%{oname}" -v "%{version}" --force || :
> +
> +
> +%preun
> +# Remove all versions from DKMS registry
> +dkms remove -m "%{oname}" -v "%{version}" --all || :
> +
> +
> +%clean
> +%{__rm} -rf %{buildroot}
> +
> +
> +%files
> +%defattr(755,root,root,755)
> +%{_usrsrc}/%{oname}-%{version}/

Now y

Re: [ovs-dev] [PATCH] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Alex Wang
On Tue, Jun 30, 2015 at 6:13 PM, Flavio Leitner  wrote:

> On Tue, Jun 30, 2015 at 05:26:00PM -0700, Alex Wang wrote:
> > This commit adds a rpmbuild spec file for building ovs datapath
> > in dkms similar to the one for debian.
> >
> > Signed-off-by: Alex Wang 
> >
> > ---
> > RFC->PATCH:
> > - generate the dkms.conf inside %install.
> > - remove rhel/dkms.conf.in.
> > ---
> >  rhel/.gitignore   |1 +
> >  rhel/automake.mk  |5 +++
> >  rhel/openvswitch-dkms.spec.in |  100
> +
> >  3 files changed, 106 insertions(+)
> >  create mode 100644 rhel/openvswitch-dkms.spec.in
> >
> > diff --git a/rhel/.gitignore b/rhel/.gitignore
> > index fa2554f..164bb66 100644
> > --- a/rhel/.gitignore
> > +++ b/rhel/.gitignore
> > @@ -1,3 +1,4 @@
> > +openvswitch-dkms.spec
> >  openvswitch-kmod-rhel5.spec
> >  openvswitch-kmod-rhel6.spec
> >  openvswitch-kmod-fedora.spec
> > diff --git a/rhel/automake.mk b/rhel/automake.mk
> > index f6272a3..d263325 100644
> > --- a/rhel/automake.mk
> > +++ b/rhel/automake.mk
> > @@ -12,6 +12,8 @@ EXTRA_DIST += \
> >   rhel/etc_logrotate.d_openvswitch \
> >   rhel/etc_sysconfig_network-scripts_ifdown-ovs \
> >   rhel/etc_sysconfig_network-scripts_ifup-ovs \
> > + rhel/openvswitch-dkms.spec \
> > + rhel/openvswitch-dkms.spec.in \
> >   rhel/openvswitch-kmod-rhel6.spec \
> >   rhel/openvswitch-kmod-rhel6.spec.in \
> >   rhel/openvswitch-kmod.files \
> > @@ -33,6 +35,9 @@ update_rhel_spec = \
> >  < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
> >if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv
> $(@F).tmp $@; fi
> >
> > +$(srcdir)/rhel/openvswitch-dkms.spec: rhel/openvswitch-dkms.spec.in
> $(top_builddir)/config.status
> > + $(update_rhel_spec)
> > +
> >  $(srcdir)/rhel/openvswitch-kmod-rhel6.spec: rhel/
> openvswitch-kmod-rhel6.spec.in $(top_builddir)/config.status
> >   $(update_rhel_spec)
> >
> > diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/
> openvswitch-dkms.spec.in
> > new file mode 100644
> > index 000..6b6c30b
> > --- /dev/null
> > +++ b/rhel/openvswitch-dkms.spec.in
> > @@ -0,0 +1,100 @@
> > +# Spec file for Open vSwitch kernel modules using DKMS.
> > +#
> > +# Copyright (C) 2015 Nicira, Inc.
> > +#
> > +# Copying and distribution of this file, with or without modification,
> > +# are permitted in any medium without royalty provided the copyright
> > +# notice and this notice are preserved.  This file is offered as-is,
> > +# without warranty of any kind.
> > +
> > +%define oname openvswitch
> > +
> > +Name: %{oname}-dkms
> > +Version:  @VERSION@
> > +Release:  1%{?dist}
> > +Summary:  Open vSwitch kernel module
> > +
> > +Group:System/Kernel
> > +License:  GPLv2
> > +URL:  http://openvswitch.org/
> > +Source:   %{oname}-%{version}.tar.gz
> > +Requires: autoconf, gcc, make
> > +Requires(post):   dkms
> > +Requires(preun):  dkms
> > +BuildRoot:%(mktemp -ud
> %{_tmppath}/%{name}-%{version}-%{release}-XX)
> > +
> > +# Without this we get an empty openvswitch-debuginfo package (whose name
> > +# conflicts with the openvswitch-debuginfo package for OVS userspace).
> > +%undefine _enable_debug_packages
> > +
> > +
> > +%description
> > +Open vSwitch Linux kernel module.
> > +
> > +
> > +%prep
> > +%setup -n %{oname}-%{version}
> > +
> > +cat > %{oname}.conf << EOF
> > +override %{oname} * extra/%{oname}
> > +override %{oname} * weak-updates/%{oname}
> > +EOF
> > +
> > +
> > +%build
> > +# for running the '%{__make} -C datapath print-build-modules' below.
> > +./configure
> > +
> > +
> > +%install
> > +%{__rm} -rf %{buildroot}
> > +
> > +# Kernel module sources install for dkms
> > +%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > +%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
> > +
> > +# check we can get kernel module names
> > +%{__make} -C datapath print-build-modules
> > +
> > +# Prepare dkms.conf
> > +cat > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf << EOF
> > +MODULES=( `%{__make} -C datapath print-build-modules | grep -v make` )
> > +
> > +PACKAGE_NAME="openvswitch"
> > +PACKAGE_VERSION="%{version}-%{release}"
> > +MAKE="./configure --with-linux='\${kernel_source_dir}' && make -C
> datapath/linux"
> > +for __idx in \${!MODULES[@]}; do
> > +BUILT_MODULE_NAME[__idx]=\${MODULES[__idx]}
> > +BUILT_MODULE_LOCATION[__idx]=datapath/linux/
> > +DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
> > +done
> > +AUTOINSTALL=yes
> > +EOF
> > +
> > +install -d %{buildroot}%{_sysconfdir}/depmod.d/
> > +install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
> > +
> > +
> > +%post
> > +# Add to DKMS registry
> > +isadded=`dkms status -m "%{oname}" -v "%{version}"`
> > +if [ "x${isadded}" = "x" ] ; then
> > +dkms add -m "%{oname}" -v "%{version}" || :
> > +fi
> > +dkms build -m "%{oname}" -v "%{ve

[ovs-dev] [PATCH V2] rhel: Add dkms support for ovs datapath build.

2015-06-30 Thread Alex Wang
This commit adds a rpmbuild spec file for building ovs datapath
in dkms similar to the one for debian.

Signed-off-by: Alex Wang 

---
PATCH->V2:
- adopt Flavio's suggestion and use file's default permission.

RFC->PATCH:
- generate the dkms.conf inside %install.
- remove rhel/dkms.conf.in.
---
 rhel/.gitignore   |1 +
 rhel/automake.mk  |5 +++
 rhel/openvswitch-dkms.spec.in |  100 +
 3 files changed, 106 insertions(+)
 create mode 100644 rhel/openvswitch-dkms.spec.in

diff --git a/rhel/.gitignore b/rhel/.gitignore
index fa2554f..164bb66 100644
--- a/rhel/.gitignore
+++ b/rhel/.gitignore
@@ -1,3 +1,4 @@
+openvswitch-dkms.spec
 openvswitch-kmod-rhel5.spec
 openvswitch-kmod-rhel6.spec
 openvswitch-kmod-fedora.spec
diff --git a/rhel/automake.mk b/rhel/automake.mk
index f6272a3..d263325 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -12,6 +12,8 @@ EXTRA_DIST += \
rhel/etc_logrotate.d_openvswitch \
rhel/etc_sysconfig_network-scripts_ifdown-ovs \
rhel/etc_sysconfig_network-scripts_ifup-ovs \
+   rhel/openvswitch-dkms.spec \
+   rhel/openvswitch-dkms.spec.in \
rhel/openvswitch-kmod-rhel6.spec \
rhel/openvswitch-kmod-rhel6.spec.in \
rhel/openvswitch-kmod.files \
@@ -33,6 +35,9 @@ update_rhel_spec = \
 < $(srcdir)/rhel/$(@F).in > $(@F).tmp || exit 1; \
   if cmp -s $(@F).tmp $@; then touch $@; rm $(@F).tmp; else mv $(@F).tmp $@; fi
 
+$(srcdir)/rhel/openvswitch-dkms.spec: rhel/openvswitch-dkms.spec.in 
$(top_builddir)/config.status
+   $(update_rhel_spec)
+
 $(srcdir)/rhel/openvswitch-kmod-rhel6.spec: 
rhel/openvswitch-kmod-rhel6.spec.in $(top_builddir)/config.status
$(update_rhel_spec)
 
diff --git a/rhel/openvswitch-dkms.spec.in b/rhel/openvswitch-dkms.spec.in
new file mode 100644
index 000..a47c038
--- /dev/null
+++ b/rhel/openvswitch-dkms.spec.in
@@ -0,0 +1,100 @@
+# Spec file for Open vSwitch kernel modules using DKMS.
+#
+# Copyright (C) 2015 Nicira, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.  This file is offered as-is,
+# without warranty of any kind.
+
+%define oname openvswitch
+
+Name: %{oname}-dkms
+Version:  @VERSION@
+Release:  1%{?dist}
+Summary:  Open vSwitch kernel module
+
+Group:System/Kernel
+License:  GPLv2
+URL:  http://openvswitch.org/
+Source:   %{oname}-%{version}.tar.gz
+Requires: autoconf, gcc, make
+Requires(post):   dkms
+Requires(preun):  dkms
+BuildRoot:%(mktemp -ud 
%{_tmppath}/%{name}-%{version}-%{release}-XX)
+
+# Without this we get an empty openvswitch-debuginfo package (whose name
+# conflicts with the openvswitch-debuginfo package for OVS userspace).
+%undefine _enable_debug_packages
+
+
+%description
+Open vSwitch Linux kernel module.
+
+
+%prep
+%setup -n %{oname}-%{version}
+
+cat > %{oname}.conf << EOF
+override %{oname} * extra/%{oname}
+override %{oname} * weak-updates/%{oname}
+EOF
+
+
+%build
+# for running the '%{__make} -C datapath print-build-modules' below.
+./configure
+
+
+%install
+%{__rm} -rf %{buildroot}
+
+# Kernel module sources install for dkms
+%{__mkdir_p} %{buildroot}%{_usrsrc}/%{oname}-%{version}/
+%{__cp} -r * %{buildroot}%{_usrsrc}/%{oname}-%{version}/
+
+# check we can get kernel module names
+%{__make} -C datapath print-build-modules
+
+# Prepare dkms.conf
+cat > %{buildroot}%{_usrsrc}/%{oname}-%{version}/dkms.conf << EOF
+MODULES=( `%{__make} -C datapath print-build-modules | grep -v make` )
+
+PACKAGE_NAME="openvswitch"
+PACKAGE_VERSION="%{version}-%{release}"
+MAKE="./configure --with-linux='\${kernel_source_dir}' && make -C 
datapath/linux"
+for __idx in \${!MODULES[@]}; do
+BUILT_MODULE_NAME[__idx]=\${MODULES[__idx]}
+BUILT_MODULE_LOCATION[__idx]=datapath/linux/
+DEST_MODULE_LOCATION[__idx]=/kernel/drivers/net/openvswitch/
+done
+AUTOINSTALL=yes
+EOF
+
+install -d %{buildroot}%{_sysconfdir}/depmod.d/
+install -m 644 %{oname}.conf %{buildroot}%{_sysconfdir}/depmod.d/
+
+
+%post
+# Add to DKMS registry
+isadded=`dkms status -m "%{oname}" -v "%{version}"`
+if [ "x${isadded}" = "x" ] ; then
+dkms add -m "%{oname}" -v "%{version}" || :
+fi
+dkms build -m "%{oname}" -v "%{version}" || :
+dkms install -m "%{oname}" -v "%{version}" --force || :
+
+
+%preun
+# Remove all versions from DKMS registry
+dkms remove -m "%{oname}" -v "%{version}" --all || :
+
+
+%clean
+%{__rm} -rf %{buildroot}
+
+
+%files
+%defattr(-,root,root)
+%{_usrsrc}/%{oname}-%{version}/
+/etc/depmod.d/openvswitch.conf
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Andy Zhou
>
> Currently 'ovsdb-client dump' dumps the whole db, which can be lot of
> output. Cannot think of an easier way than writing a python script that goes
> over the entire output, then parses and prints the particular table in
> question. Bash scripting may not be so easy to write as tables can have
> varied number of rows. Its probably more practical and user friendly to have
> something in ovsdb-client itself that dumps a table rather than have users
> distribute/write custom scripts, especially if db is large in size..

It seems we just need to extract the text between the table name and the first
blank line below it. this should pretty easy with either awk or sed.

As for efficiency, the db files I have access to are sufficiently
small that the script should work in practise,
especially used only for debugging.  Do you have a very large DB file
or a different use case?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCHv2] odp-util: Combine dpif_backer_support with odp_parms.

2015-06-30 Thread Joe Stringer
On 30 June 2015 at 17:42, Andy Zhou  wrote:
> On Tue, Jun 30, 2015 at 4:52 PM, Joe Stringer  wrote:
>> Both the ofproto layer and the odp-util layer have recently added
>> notions about fields supported by the datapath. This commit merges the
>> two into the same structure.
>
> while I see the benfits of combining the structure, I am not sure it
> makes conceptual sense.
>
> It seems we need a structure to store per dpif datpath features and
> this strcture should be defined in ofproto-dpif.h.

Right, dpif_backer_support {} has this at the moment, including flow
key support, action support, and ufid support.

> odp-util also needs some of the same information, but not all of them,
> right? Then why not define odp_support
> as a seperate sturcture that contains only information needed for
> serialization, and add a a function to fill
> an odp_support struct  from dpif_backer_support when necessary?

It's true that odp-util doesn't use all of the fields, it just seemed
simpler to represent it all using the same structure. I could perhaps
split it into OVS_KEY_ATTR_* fields (odp_support) vs. all other OVS_*
fields (dpif_backer_support, which includes a nested odp_support).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCHv2] odp-util: Combine dpif_backer_support with odp_parms.

2015-06-30 Thread Andy Zhou
On Tue, Jun 30, 2015 at 6:48 PM, Joe Stringer  wrote:
> On 30 June 2015 at 17:42, Andy Zhou  wrote:
>> On Tue, Jun 30, 2015 at 4:52 PM, Joe Stringer  wrote:
>>> Both the ofproto layer and the odp-util layer have recently added
>>> notions about fields supported by the datapath. This commit merges the
>>> two into the same structure.
>>
>> while I see the benfits of combining the structure, I am not sure it
>> makes conceptual sense.
>>
>> It seems we need a structure to store per dpif datpath features and
>> this strcture should be defined in ofproto-dpif.h.
>
> Right, dpif_backer_support {} has this at the moment, including flow
> key support, action support, and ufid support.
>
>> odp-util also needs some of the same information, but not all of them,
>> right? Then why not define odp_support
>> as a seperate sturcture that contains only information needed for
>> serialization, and add a a function to fill
>> an odp_support struct  from dpif_backer_support when necessary?
>
> It's true that odp-util doesn't use all of the fields, it just seemed
> simpler to represent it all using the same structure. I could perhaps
> split it into OVS_KEY_ATTR_* fields (odp_support) vs. all other OVS_*
> fields (dpif_backer_support, which includes a nested odp_support).
This sounds O.K.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Sabyasachi Sengupta



It seems we just need to extract the text between the table name and the first
blank line below it. this should pretty easy with either awk or sed.

As for efficiency, the db files I have access to are sufficiently
small that the script should work in practise,
especially used only for debugging.  Do you have a very large DB file
or a different use case?


Our deployments typically have large amount of data in tables in ovsdb and 
finding out content of one table has been hard out of such large outputs, 
especially from usability perspective. One can always go to awk/sed, but for 
convenience traditional unix/linux systems often provides different options 
of standard commands that helps narrow down output and present it to users 
in a more presentable manner. I suppose I don't need to mention about 
usefulness of '-d' option of ls, or '-type' of find here. Although it is 
possible to write wrappers over 'find' that does the job of '-type dir', but 
having it as part of 'find' helps in standardizing output and allows users 
to do 'other' stuff rather than each program or individual user do this 
separately..


While one can write their own scriptlets, the other important question is 
how one would distribute this script to deployable systems. One easier 
approach if there is something available as part of ovs package itself.


The motivation of contributing this actually came from your existing 
DATABASE option that appears to eliminate the need to have similar scripts, 
but extend it with a new convenience TABLE option.

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


Re: [ovs-dev] [PATCH v2]: ovsdb: add capability to dump table content in ovsdb-client

2015-06-30 Thread Andy Zhou
On Tue, Jun 30, 2015 at 7:47 PM, Sabyasachi Sengupta
 wrote:
>
>> It seems we just need to extract the text between the table name and the
>> first
>> blank line below it. this should pretty easy with either awk or sed.
>>
>> As for efficiency, the db files I have access to are sufficiently
>> small that the script should work in practise,
>> especially used only for debugging.  Do you have a very large DB file
>> or a different use case?
>
>
> Our deployments typically have large amount of data in tables in ovsdb and
> finding out content of one table has been hard out of such large outputs,
> especially from usability perspective. One can always go to awk/sed, but for
> convenience traditional unix/linux systems often provides different options
> of standard commands that helps narrow down output and present it to users
> in a more presentable manner. I suppose I don't need to mention about
> usefulness of '-d' option of ls, or '-type' of find here. Although it is
> possible to write wrappers over 'find' that does the job of '-type dir', but
> having it as part of 'find' helps in standardizing output and allows users
> to do 'other' stuff rather than each program or individual user do this
> separately..
O.K. I am sold on the table option.
>
> While one can write their own scriptlets, the other important question is
> how one would distribute this script to deployable systems. One easier
> approach if there is something available as part of ovs package itself.
>
OVS currently do not carry many helper scripts. A few of them
currently living in utilities.

> The motivation of contributing this actually came from your existing
> DATABASE option that appears to eliminate the need to have similar scripts,
> but extend it with a new convenience TABLE option.

Make sense. Thanks for working on it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] tunnels: Don't initialize unnecessary packet metadata.

2015-06-30 Thread Jesse Gross
The addition of Geneve options to packet metadata significantly
expanded its size. It was reported that this can decrease performance
for DPDK ports by up to 25% since we need to initialize the whole
structure on each packet receive.

It is not really necessary to zero out the entire structure because
miniflow_extract() only copies the tunnel metadata when particular
fields indicate that it is valid. Therefore, as long as we zero out
these fields when the metadata is initialized and ensure that the
rest of the structure is correctly set in the presence of a tunnel,
we can avoid touching the tunnel fields on packet reception.

Reported-by: Ciara Loftus 
Signed-off-by: Jesse Gross 
---
 lib/dp-packet.c   |  2 +-
 lib/dpif-netdev.c | 21 ++---
 lib/netdev-vport.c| 16 +---
 lib/netdev.c  |  2 +-
 lib/odp-util.c|  3 ++-
 lib/packets.h | 15 ---
 lib/tun-metadata.h|  2 +-
 ofproto/ofproto-dpif-upcall.c |  1 -
 utilities/ovs-ofctl.c |  4 ++--
 9 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 31fe9d3..098f816 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -29,7 +29,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum 
dp_packet_source so
 b->source = source;
 b->l2_pad_size = 0;
 b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX;
-b->md = PKT_METADATA_INITIALIZER(0);
+pkt_metadata_init(&b->md, 0);
 }
 
 static void
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index dd6bb1f..d146546 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -235,7 +235,7 @@ enum pmd_cycles_counter_type {
 
 /* A port in a netdev-based datapath. */
 struct dp_netdev_port {
-struct pkt_metadata md;
+odp_port_t port_no;
 struct netdev *netdev;
 struct cmap_node node;  /* Node in dp_netdev's 'ports'. */
 struct netdev_saved_flags *sf;
@@ -1085,7 +1085,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, 
const char *type,
 }
 }
 port = xzalloc(sizeof *port);
-port->md = PKT_METADATA_INITIALIZER(port_no);
+port->port_no = port_no;
 port->netdev = netdev;
 port->rxq = xmalloc(sizeof *port->rxq * netdev_n_rxq(netdev));
 port->type = xstrdup(type);
@@ -1190,7 +1190,7 @@ dp_netdev_lookup_port(const struct dp_netdev *dp, 
odp_port_t port_no)
 struct dp_netdev_port *port;
 
 CMAP_FOR_EACH_WITH_HASH (port, node, hash_port_no(port_no), &dp->ports) {
-if (port->md.in_port.odp_port == port_no) {
+if (port->port_no == port_no) {
 return port;
 }
 }
@@ -1300,8 +1300,7 @@ static void
 do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port)
 OVS_REQUIRES(dp->port_mutex)
 {
-cmap_remove(&dp->ports, &port->node,
-hash_odp_port(port->md.in_port.odp_port));
+cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no));
 seq_change(dp->port_seq);
 if (netdev_is_pmd(port->netdev)) {
 int numa_id = netdev_get_numa_id(port->netdev);
@@ -1323,7 +1322,7 @@ answer_port_query(const struct dp_netdev_port *port,
 {
 dpif_port->name = xstrdup(netdev_get_name(port->netdev));
 dpif_port->type = xstrdup(port->type);
-dpif_port->port_no = port->md.in_port.odp_port;
+dpif_port->port_no = port->port_no;
 }
 
 static int
@@ -1450,7 +1449,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void 
*state_,
 state->name = xstrdup(netdev_get_name(port->netdev));
 dpif_port->name = state->name;
 dpif_port->type = port->type;
-dpif_port->port_no = port->md.in_port.odp_port;
+dpif_port->port_no = port->port_no;
 
 retval = 0;
 } else {
@@ -2540,7 +2539,7 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
*pmd,
 
 /* XXX: initialize md in netdev implementation. */
 for (i = 0; i < cnt; i++) {
-packets[i]->md = port->md;
+pkt_metadata_init(&packets[i]->md, port->port_no);
 }
 cycles_count_start(pmd);
 dp_netdev_input(pmd, packets, cnt);
@@ -3652,12 +3651,12 @@ dpif_dummy_change_port_number(struct unixctl_conn 
*conn, int argc OVS_UNUSED,
 }
 
 /* Remove old port. */
-cmap_remove(&dp->ports, &old_port->node, 
hash_port_no(old_port->md.in_port.odp_port));
+cmap_remove(&dp->ports, &old_port->node, hash_port_no(old_port->port_no));
 ovsrcu_postpone(free, old_port);
 
 /* Insert new port (cmap semantics mean we cannot re-insert 'old_port'). */
 new_port = xmemdup(old_port, sizeof *old_port);
-new_port->md.in_port.odp_port = port_no;
+new_port->port_no = port_no;
 cmap_insert(&dp->ports, &new_port->node, hash_port_no(port_no));
 
 seq_change(dp->port_seq);
@@ -3688,7 +3687,7 @@ dpif_dummy_delete_port(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 ovs_mutex_lock(&dp->port_mutex);
 if (g

Re: [ovs-dev] [PATCH 08/11] metaflow: Extend size of mf_value to 128 bytes.

2015-06-30 Thread Jesse Gross
On Tue, Jun 30, 2015 at 7:56 AM, Loftus, Ciara  wrote:
>>
>> On Wed, Jun 24, 2015 at 1:17 PM, Ben Pfaff  wrote:
>> > On Fri, Jun 19, 2015 at 04:13:22PM -0700, Jesse Gross wrote:
>> >> Tunnel metadata can be substantially larger than our existing fields
>> >> (up to 124 bytes in a single Geneve option) so this extends the size
>> >> of the data that we can handle with metaflow fields. This also
>> >> breaks a few tests that assume that their max size is also the
>> >> maximum that can be handled in a field.
>> >>
>> >> Signed-off-by: Jesse Gross 
>> >
>> > Did you look around at all to see whether this will unreasonably blow up
>> > any data or algorithms?
>>
>> I don't believe that it should have any significant effects.
>> Generally, code does operations on the fields based on mf->n_bytes
>> (with the exception of some memset()s here and there). I don't think
>> that we really store these in a large number for any real period of
>> time.
>
> With this series of patches, in particular patch 10/11 "tunnel: Geneve TLV 
> handling support for OpenFlow" I've measured a significant decrease in 
> performance with the dpdkport type. For example, with a loopback test with 
> 64Byte packets I've seen a 25% decrease in throughput.
> I suspect this is in relation to the size of the new tun_metadata struct. A 
> quick perf analysis and I see we're spending significantly more time 
> initialising packet metadata in the dp_netdev_process_rxq_port function.
> Are there any plans to address this performance degradation?

Thanks for pointing that out. I just sent out a patch that should
hopefully avoid the problem of needing to initialize the newly
enlarged structure. I don't have a great way of doing performance
testing on it, would you mind seeing if it solves the problem you're
seeing?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev